From 43079b6d51a8866b688712cf130e9024de14884b Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 23 Feb 2026 15:36:33 +0000 Subject: [PATCH 01/15] CurvesPrimitive : Avoid repeating variable size logic --- src/IECoreScene/CurvesPrimitive.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/IECoreScene/CurvesPrimitive.cpp b/src/IECoreScene/CurvesPrimitive.cpp index 6ac82db2be..f5a73d5100 100644 --- a/src/IECoreScene/CurvesPrimitive.cpp +++ b/src/IECoreScene/CurvesPrimitive.cpp @@ -200,15 +200,7 @@ void CurvesPrimitive::setTopology( ConstIntVectorDataPtr verticesPerCurve, const const std::vector &v = m_vertsPerCurve->readable(); for( size_t i=0; i Date: Wed, 4 Mar 2026 11:54:49 +0000 Subject: [PATCH 02/15] CurvesPrimitive : Prefer anonymous namespace to `static` And make globals const. --- src/IECoreScene/CurvesPrimitive.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/IECoreScene/CurvesPrimitive.cpp b/src/IECoreScene/CurvesPrimitive.cpp index f5a73d5100..b59586c4f6 100644 --- a/src/IECoreScene/CurvesPrimitive.cpp +++ b/src/IECoreScene/CurvesPrimitive.cpp @@ -40,12 +40,17 @@ using namespace IECore; using namespace IECoreScene; using namespace Imath; -static IndexedIO::EntryID g_basisMatrixEntry("basisMatrix"); -static IndexedIO::EntryID g_basisStepEntry("basisStep"); -static IndexedIO::EntryID g_periodicEntry("periodic"); -static IndexedIO::EntryID g_verticesPerCurveEntry("verticesPerCurve"); -static IndexedIO::EntryID g_numVertsEntry("numVerts"); -static IndexedIO::EntryID g_numFaceVaryingEntry("numFaceVarying"); +namespace +{ + +const IndexedIO::EntryID g_basisMatrixEntry( "basisMatrix" ); +const IndexedIO::EntryID g_basisStepEntry( "basisStep" ); +const IndexedIO::EntryID g_periodicEntry( "periodic" ); +const IndexedIO::EntryID g_verticesPerCurveEntry( "verticesPerCurve" ); +const IndexedIO::EntryID g_numVertsEntry( "numVerts" ); +const IndexedIO::EntryID g_numFaceVaryingEntry( "numFaceVarying" ); + +} // namespace const unsigned int CurvesPrimitive::m_ioVersion = 0; IE_CORE_DEFINEOBJECTTYPEDESCRIPTION( CurvesPrimitive ); From 65c722045c3c3155af5dde80d40d9bec047c111d Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 4 Mar 2026 11:58:05 +0000 Subject: [PATCH 03/15] CurvesPrimitive : Make protected members private --- Changes | 3 + include/IECoreScene/CurvesPrimitive.h | 7 +- src/IECoreScene/CurvesPrimitive.cpp | 105 +++++++++++++------------- 3 files changed, 57 insertions(+), 58 deletions(-) diff --git a/Changes b/Changes index e2e67f3e01..72e108195b 100644 --- a/Changes +++ b/Changes @@ -1,7 +1,10 @@ 10.7.x.x (relative to 10.7.0.0a7) ======== +Breaking Changes +---------------- +- CurvesPrimitive : Made protected members private. 10.7.0.0a7 (relative to 10.7.0.0a6) ========== diff --git a/include/IECoreScene/CurvesPrimitive.h b/include/IECoreScene/CurvesPrimitive.h index 835028c740..e2abb463f7 100644 --- a/include/IECoreScene/CurvesPrimitive.h +++ b/include/IECoreScene/CurvesPrimitive.h @@ -79,10 +79,7 @@ class IECORESCENE_API CurvesPrimitive : public Primitive void topologyHash( IECore::MurmurHash &h ) const override; - protected : - - /// Throws an exception if numVerts is an inappropriate number for the current basis. - static unsigned int numSegments( bool linear, int step, bool periodic, int numVerts ); + private : IECore::CubicBasisf m_basis; bool m_linear; @@ -91,8 +88,6 @@ class IECORESCENE_API CurvesPrimitive : public Primitive unsigned m_numVerts; unsigned m_numFaceVarying; - private : - static const unsigned int m_ioVersion; }; diff --git a/src/IECoreScene/CurvesPrimitive.cpp b/src/IECoreScene/CurvesPrimitive.cpp index b59586c4f6..42603e6104 100644 --- a/src/IECoreScene/CurvesPrimitive.cpp +++ b/src/IECoreScene/CurvesPrimitive.cpp @@ -50,6 +50,57 @@ const IndexedIO::EntryID g_verticesPerCurveEntry( "verticesPerCurve" ); const IndexedIO::EntryID g_numVertsEntry( "numVerts" ); const IndexedIO::EntryID g_numFaceVaryingEntry( "numFaceVarying" ); +/// Throws an exception if numVerts is an inappropriate number for the specified basis. +unsigned int numSegmentsInternal( bool linear, int step, bool periodic, int numVerts ) +{ + if( linear ) + { + if( periodic ) + { + if( numVerts < 3 ) + { + throw Exception( "Linear periodic curve with less than 3 vertices." ); + } + return numVerts; + } + else + { + if( numVerts < 2 ) + { + throw Exception( "Linear non-periodic curve with less than 2 vertices." ); + } + return numVerts - 1; + } + } + else + { + if( periodic ) + { + if( numVerts < 3 ) + { + throw Exception( "Cubic periodic curve with less than 3 vertices." ); + } + if( (numVerts - 1) % step ) + { + throw Exception( "Cubic curve with extra vertices." ); + } + return numVerts / step; + } + else + { + if( numVerts < 4 ) + { + throw Exception( "Cubic nonperiodic curve with less than 4 vertices." ); + } + if( (numVerts - 4) % step ) + { + throw Exception( "Cubic curve with extra vertices." ); + } + return (numVerts - 4 )/ step + 1; + } + } +} + } // namespace const unsigned int CurvesPrimitive::m_ioVersion = 0; @@ -264,62 +315,12 @@ unsigned CurvesPrimitive::numSegments( unsigned curveIndex ) const { throw Exception( "Curve index out of range." ); } - return numSegments( m_linear, m_basis.step, m_periodic, m_vertsPerCurve->readable()[curveIndex] ); + return numSegmentsInternal( m_linear, m_basis.step, m_periodic, m_vertsPerCurve->readable()[curveIndex] ); } unsigned CurvesPrimitive::numSegments( const CubicBasisf &basis, bool periodic, unsigned numVerts ) { - return numSegments( basis==CubicBasisf::linear(), basis.step, periodic, numVerts ); -} - -unsigned int CurvesPrimitive::numSegments( bool linear, int step, bool periodic, int numVerts ) -{ - if( linear ) - { - if( periodic ) - { - if( numVerts < 3 ) - { - throw Exception( "Linear periodic curve with less than 3 vertices." ); - } - return numVerts; - } - else - { - if( numVerts < 2 ) - { - throw Exception( "Linear non-periodic curve with less than 2 vertices." ); - } - return numVerts - 1; - } - } - else - { - if( periodic ) - { - if( numVerts < 3 ) - { - throw Exception( "Cubic periodic curve with less than 3 vertices." ); - } - if( (numVerts - 1) % step ) - { - throw Exception( "Cubic curve with extra vertices." ); - } - return numVerts / step; - } - else - { - if( numVerts < 4 ) - { - throw Exception( "Cubic nonperiodic curve with less than 4 vertices." ); - } - if( (numVerts - 4) % step ) - { - throw Exception( "Cubic curve with extra vertices." ); - } - return (numVerts - 4 )/ step + 1; - } - } + return numSegmentsInternal( basis==CubicBasisf::linear(), basis.step, periodic, numVerts ); } CurvesPrimitivePtr CurvesPrimitive::createBox( const Imath::Box3f &b ) From d79d56e2d95056bb8ebcaeb34f788daef463799c Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 4 Mar 2026 15:04:29 +0000 Subject: [PATCH 04/15] CurvesPrimitiveBinding : Add keyword arguments --- Changes | 5 +++++ src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index 72e108195b..70a06266c2 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,11 @@ 10.7.x.x (relative to 10.7.0.0a7) ======== +Improvements +------------ + +- CurvesPrimitive : Added argument names to `variableSize()` and `numSegments()` Python bindings, so they can be passed as keywords. + Breaking Changes ---------------- diff --git a/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp b/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp index 1cafd0ba2e..a516b5bf36 100644 --- a/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp +++ b/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp @@ -69,8 +69,8 @@ void bindCurvesPrimitive() .def( "periodic", &CurvesPrimitive::periodic ) .def( "setTopology", &CurvesPrimitive::setTopology ) .def( "variableSize", (size_t (CurvesPrimitive::*)( PrimitiveVariable::Interpolation )const)&CurvesPrimitive::variableSize ) - .def( "variableSize", (size_t (CurvesPrimitive::*)( PrimitiveVariable::Interpolation, unsigned )const)&CurvesPrimitive::variableSize ) - .def( "numSegments", (unsigned (CurvesPrimitive::*)( unsigned )const)&CurvesPrimitive::numSegments ) + .def( "variableSize", (size_t (CurvesPrimitive::*)( PrimitiveVariable::Interpolation, unsigned )const)&CurvesPrimitive::variableSize, arg( "curveIndex" ) ) + .def( "numSegments", (unsigned (CurvesPrimitive::*)( unsigned )const)&CurvesPrimitive::numSegments, arg( "curveIndex" ) ) .def( "createBox", &CurvesPrimitive::createBox ).staticmethod( "createBox" ) ; } From 7b1a7f4ce236c7d2a048d6b85176e6913400303c Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 23 Mar 2026 16:03:08 +0000 Subject: [PATCH 05/15] CurvesAlgo : Fix handling of periodic curves in `deleteCurves()` --- Changes | 5 +++ .../private/PrimitiveVariableAlgos.h | 3 +- test/IECoreScene/CurvesAlgoTest.py | 32 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index 70a06266c2..4871c42241 100644 --- a/Changes +++ b/Changes @@ -6,6 +6,11 @@ Improvements - CurvesPrimitive : Added argument names to `variableSize()` and `numSegments()` Python bindings, so they can be passed as keywords. +Fixes +----- + +- CurvesAlgo : Fixed handling of periodic curves in `deleteCurves()`. + Breaking Changes ---------------- diff --git a/include/IECoreScene/private/PrimitiveVariableAlgos.h b/include/IECoreScene/private/PrimitiveVariableAlgos.h index fa7162ae64..936deefabd 100644 --- a/include/IECoreScene/private/PrimitiveVariableAlgos.h +++ b/include/IECoreScene/private/PrimitiveVariableAlgos.h @@ -291,8 +291,7 @@ class DeleteFlaggedVaryingFunctor : public DeleteFlagged size_t offset = 0; for( size_t c = 0; c < m_curvesPrimitive->numCurves(); ++c ) { - int numVarying = m_curvesPrimitive->numSegments( c ) + 1; - + const int numVarying = m_curvesPrimitive->variableSize( PrimitiveVariable::Varying, c ); if( this->shouldKeepPrimitive( c ) ) { for( int v = 0; v < numVarying; ++v ) diff --git a/test/IECoreScene/CurvesAlgoTest.py b/test/IECoreScene/CurvesAlgoTest.py index 938fdb9881..0d96f92a70 100644 --- a/test/IECoreScene/CurvesAlgoTest.py +++ b/test/IECoreScene/CurvesAlgoTest.py @@ -1794,6 +1794,38 @@ def testDeleteInvalidPrimVars( self ): curves = self.curvesBad() self.assertRaises( RuntimeError, IECoreScene.CurvesAlgo.deleteCurves, curves, deletePrimVar ) + def testDeletePeriodicWithVarying( self ) : + + sourceCurves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 3, 3 ], ), IECore.CubicBasisf.linear(), True, IECore.V3fVectorData( [ imath.V3f( i ) for i in range( 0, 6 ) ] ) + ) + sourceCurves["varying"] = IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Varying, + IECore.FloatVectorData( range( 0, 6 ) ) + ) + self.assertTrue( sourceCurves.arePrimitiveVariablesValid() ) + + curves = IECoreScene.CurvesAlgo.deleteCurves( + sourceCurves, IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Uniform, IECore.IntVectorData( [ 1, 0 ] ) ) + ) + self.assertTrue( curves.arePrimitiveVariablesValid() ) + self.assertEqual( + curves["P"], + IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Vertex, + IECore.V3fVectorData( + [ imath.V3f( i ) for i in range( 3, 6 ) ], + IECore.GeometricData.Interpretation.Point + ) + ) + ) + self.assertEqual( + curves["varying"], + IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Varying, + IECore.FloatVectorData( range( 3, 6 ) ), + ) + ) class CurvesAlgoUpdateEndpointMultiplicityTest( unittest.TestCase ): From 5a6e4413fbb839e91f75d5b70fed72d296f1613c Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 23 Mar 2026 15:43:55 +0000 Subject: [PATCH 06/15] CurvesPrimitive : Add WrapMode enum This generalises the periodic/nonperiodic concept and adds a Pinned value to line up with USD's pinned wrap mode. --- Changes | 4 +- include/IECoreScene/CurvesPrimitive.h | 23 ++- src/IECoreScene/CurvesPrimitive.cpp | 189 +++++++++++------- .../bindings/CurvesPrimitiveBinding.cpp | 20 +- test/IECoreScene/CurvesPrimitiveTest.py | 124 +++++++++++- .../IECoreScene/data/curvesV0-nonPeriodic.cob | Bin 0 -> 1884 bytes test/IECoreScene/data/curvesV0-periodic.cob | Bin 0 -> 1876 bytes 7 files changed, 277 insertions(+), 83 deletions(-) create mode 100644 test/IECoreScene/data/curvesV0-nonPeriodic.cob create mode 100644 test/IECoreScene/data/curvesV0-periodic.cob diff --git a/Changes b/Changes index 4871c42241..9cd9ce06c0 100644 --- a/Changes +++ b/Changes @@ -4,7 +4,9 @@ Improvements ------------ -- CurvesPrimitive : Added argument names to `variableSize()` and `numSegments()` Python bindings, so they can be passed as keywords. +- CurvesPrimitive : + - Added `Wrap` enum, with `Periodic`, `NonPeriodic` and `Pinned`. This generalises the previous `periodic` boolean property. + - Added argument names to `variableSize()` and `numSegments()` Python bindings, so they can be passed as keywords. Fixes ----- diff --git a/include/IECoreScene/CurvesPrimitive.h b/include/IECoreScene/CurvesPrimitive.h index e2abb463f7..f9fccc4c94 100644 --- a/include/IECoreScene/CurvesPrimitive.h +++ b/include/IECoreScene/CurvesPrimitive.h @@ -51,8 +51,17 @@ class IECORESCENE_API CurvesPrimitive : public Primitive { public : + enum class Wrap + { + NonPeriodic, + Periodic, + Pinned + }; + CurvesPrimitive(); - /// Copies of vertsPerCurve and p are taken. + /// Copies of `vertsPerCurve` and `p` are taken. + CurvesPrimitive( const IECore::IntVectorData *vertsPerCurve, const IECore::CubicBasisf &basis=IECore::CubicBasisf::linear(), Wrap wrap = Wrap::NonPeriodic, const IECore::V3fVectorData *p = nullptr ); + /// \deprecated Use the version taking `wrap` argument. CurvesPrimitive( IECore::ConstIntVectorDataPtr vertsPerCurve, const IECore::CubicBasisf &basis=IECore::CubicBasisf::linear(), bool periodic = false, IECore::ConstV3fVectorDataPtr p = nullptr ); ~CurvesPrimitive() override; @@ -61,7 +70,12 @@ class IECORESCENE_API CurvesPrimitive : public Primitive size_t numCurves() const; const IECore::IntVectorData *verticesPerCurve() const; const IECore::CubicBasisf &basis() const; + Wrap wrap() const; + /// \deprecated Use `wrap() == Wrap::Periodic`. bool periodic() const; + /// A copy of `verticesPerCurve` is taken. + void setTopology( const IECore::IntVectorData *verticesPerCurve, const IECore::CubicBasisf &basis, Wrap wrap ); + /// \deprecated Use the version taking `wrap` argument. void setTopology( IECore::ConstIntVectorDataPtr verticesPerCurve, const IECore::CubicBasisf &basis, bool periodic ); /// Follows the RenderMan specification for variable sizes. @@ -72,6 +86,8 @@ class IECORESCENE_API CurvesPrimitive : public Primitive /// Returns the number of segments in a given curve. unsigned numSegments( unsigned curveIndex ) const; /// Returns the number of segments of a curve with the given topology. + static unsigned numSegments( const IECore::CubicBasisf &basis, Wrap wrap, unsigned numVerts ); + /// \deprecated Use the version taking `wrap` argument. static unsigned numSegments( const IECore::CubicBasisf &basis, bool periodic, unsigned numVerts ); /// Creates a wireframe box of the specified size. @@ -82,8 +98,9 @@ class IECORESCENE_API CurvesPrimitive : public Primitive private : IECore::CubicBasisf m_basis; - bool m_linear; - bool m_periodic; + // Cached from `m_basis.standardBasis()`. + IECore::StandardCubicBasis m_standardBasis; + Wrap m_wrap; IECore::IntVectorDataPtr m_vertsPerCurve; unsigned m_numVerts; unsigned m_numFaceVarying; diff --git a/src/IECoreScene/CurvesPrimitive.cpp b/src/IECoreScene/CurvesPrimitive.cpp index 42603e6104..fa990ab3b8 100644 --- a/src/IECoreScene/CurvesPrimitive.cpp +++ b/src/IECoreScene/CurvesPrimitive.cpp @@ -46,81 +46,95 @@ namespace const IndexedIO::EntryID g_basisMatrixEntry( "basisMatrix" ); const IndexedIO::EntryID g_basisStepEntry( "basisStep" ); const IndexedIO::EntryID g_periodicEntry( "periodic" ); +const IndexedIO::EntryID g_wrapEntry( "wrap" ); const IndexedIO::EntryID g_verticesPerCurveEntry( "verticesPerCurve" ); const IndexedIO::EntryID g_numVertsEntry( "numVerts" ); const IndexedIO::EntryID g_numFaceVaryingEntry( "numFaceVarying" ); /// Throws an exception if numVerts is an inappropriate number for the specified basis. -unsigned int numSegmentsInternal( bool linear, int step, bool periodic, int numVerts ) +unsigned int numSegmentsInternal( StandardCubicBasis basis, int step, CurvesPrimitive::Wrap wrap, int numVerts ) { - if( linear ) + if( basis == StandardCubicBasis::Linear ) { - if( periodic ) + switch( wrap ) { - if( numVerts < 3 ) - { - throw Exception( "Linear periodic curve with less than 3 vertices." ); - } - return numVerts; - } - else - { - if( numVerts < 2 ) - { - throw Exception( "Linear non-periodic curve with less than 2 vertices." ); - } - return numVerts - 1; + case CurvesPrimitive::Wrap::Periodic : + if( numVerts < 3 ) + { + throw Exception( "Linear periodic curve with less than 3 vertices." ); + } + return numVerts; + case CurvesPrimitive::Wrap::NonPeriodic : + case CurvesPrimitive::Wrap::Pinned : + if( numVerts < 2 ) + { + throw Exception( "Linear non-periodic curve with less than 2 vertices." ); + } + return numVerts - 1; + default : + return 0; } } else { - if( periodic ) + switch( wrap ) { - if( numVerts < 3 ) - { - throw Exception( "Cubic periodic curve with less than 3 vertices." ); - } - if( (numVerts - 1) % step ) - { - throw Exception( "Cubic curve with extra vertices." ); - } - return numVerts / step; - } - else - { - if( numVerts < 4 ) - { - throw Exception( "Cubic nonperiodic curve with less than 4 vertices." ); - } - if( (numVerts - 4) % step ) - { - throw Exception( "Cubic curve with extra vertices." ); - } - return (numVerts - 4 )/ step + 1; + case CurvesPrimitive::Wrap::Periodic : + if( numVerts < 3 ) + { + throw Exception( "Cubic periodic curve with less than 3 vertices." ); + } + if( (numVerts - 1) % step ) + { + throw Exception( "Cubic curve with extra vertices." ); + } + return numVerts / step; + case CurvesPrimitive::Wrap::Pinned : + if( basis == StandardCubicBasis::BSpline || basis == StandardCubicBasis::CatmullRom ) + { + if( numVerts < 2 ) + { + throw Exception( "Cubic pinned curve with less than 2 vertices." ); + } + return numVerts - 1; + } + [[fallthrough]]; // Treat as NonPeriodic. + case CurvesPrimitive::Wrap::NonPeriodic : + if( numVerts < 4 ) + { + throw Exception( "Cubic non-periodic curve with less than 4 vertices." ); + } + if( (numVerts - 4) % step ) + { + throw Exception( "Cubic curve with extra vertices." ); + } + return (numVerts - 4 ) / step + 1; + default : + return 0; } } } } // namespace -const unsigned int CurvesPrimitive::m_ioVersion = 0; +const unsigned int CurvesPrimitive::m_ioVersion = 1; IE_CORE_DEFINEOBJECTTYPEDESCRIPTION( CurvesPrimitive ); CurvesPrimitive::CurvesPrimitive() : m_basis( CubicBasisf::linear() ), - m_linear( true ), - m_periodic( false ), + m_standardBasis( StandardCubicBasis::Linear ), + m_wrap( Wrap::NonPeriodic ), m_vertsPerCurve( new IntVectorData() ), m_numVerts( 0 ), m_numFaceVarying( 0 ) { } -CurvesPrimitive::CurvesPrimitive( ConstIntVectorDataPtr vertsPerCurve, const CubicBasisf &basis, bool periodic, ConstV3fVectorDataPtr p ) - : m_basis( CubicBasisf::linear() ) +CurvesPrimitive::CurvesPrimitive( const IECore::IntVectorData *vertsPerCurve, const IECore::CubicBasisf &basis, Wrap wrap, const IECore::V3fVectorData *p ) + : m_basis( CubicBasisf::linear() ) { - setTopology( vertsPerCurve, basis, periodic ); + setTopology( vertsPerCurve, basis, wrap ); if( p ) { @@ -130,6 +144,11 @@ CurvesPrimitive::CurvesPrimitive( ConstIntVectorDataPtr vertsPerCurve, const Cub } } +CurvesPrimitive::CurvesPrimitive( ConstIntVectorDataPtr vertsPerCurve, const CubicBasisf &basis, bool periodic, ConstV3fVectorDataPtr p ) + : CurvesPrimitive( vertsPerCurve.get(), basis, periodic ? Wrap::Periodic : Wrap::NonPeriodic, p.get() ) +{ +} + CurvesPrimitive::~CurvesPrimitive() { } @@ -145,7 +164,7 @@ bool CurvesPrimitive::isEqualTo( const Object *other ) const { return false; } - if( m_periodic!=tOther->m_periodic ) + if( m_wrap!=tOther->m_wrap ) { return false; } @@ -162,8 +181,8 @@ void CurvesPrimitive::copyFrom( const Object *other, CopyContext *context ) Primitive::copyFrom( other, context ); const CurvesPrimitive *tOther = static_cast( other ); m_basis = tOther->m_basis; - m_linear = tOther->m_linear; - m_periodic = tOther->m_periodic; + m_standardBasis = tOther->m_standardBasis; + m_wrap = tOther->m_wrap; m_vertsPerCurve = tOther->m_vertsPerCurve->copy(); // don't need to use context as we don't share this data with anyone m_numVerts = tOther->m_numVerts; m_numFaceVarying = tOther->m_numFaceVarying; @@ -176,8 +195,17 @@ void CurvesPrimitive::save( IECore::Object::SaveContext *context ) const IndexedIOPtr container = context->container( staticTypeName(), m_ioVersion ); container->write( g_basisMatrixEntry, m_basis.matrix.getValue(), 16 ); container->write( g_basisStepEntry, m_basis.step ); - int p = m_periodic; - container->write( g_periodicEntry, p ); + int wrap = static_cast( m_wrap ); + container->write( g_wrapEntry, wrap ); + if( m_wrap != Wrap::Pinned ) + { + // Although it is redundant, we write the `periodic` entry for the benefit + // of old versions of the library, so that loading won't fail. We don't do + // this for pinned curves though, since they aren't supported in old versions. + /// \todo Remove + int p = periodic(); + container->write( g_periodicEntry, p ); + } context->save( m_vertsPerCurve.get(), container.get(), g_verticesPerCurveEntry ); // we could recompute these on loading, but it'd take a while and the overhead // of storing them isn't great. @@ -194,10 +222,20 @@ void CurvesPrimitive::load( IECore::Object::LoadContextPtr context ) float *f = m_basis.matrix.getValue(); container->read( g_basisMatrixEntry, f, 16 ); container->read( g_basisStepEntry, m_basis.step ); - m_linear = m_basis==CubicBasisf::linear(); - int p = 0; - container->read( g_periodicEntry, p ); - m_periodic = p; + m_standardBasis = m_basis.standardBasis(); + + if( v >= 1 ) + { + int wrap = 0; + container->read( g_wrapEntry, wrap ); + m_wrap = static_cast( wrap ); + } + else + { + int p = 0; + container->read( g_periodicEntry, p ); + m_wrap = p ? Wrap::Periodic : Wrap::NonPeriodic; + } m_vertsPerCurve = context->load( container.get(), g_verticesPerCurveEntry ); container->read( g_numVertsEntry, m_numVerts ); container->read( g_numFaceVaryingEntry, m_numFaceVarying ); @@ -219,8 +257,7 @@ void CurvesPrimitive::topologyHash( MurmurHash &h ) const { h.append( m_basis.matrix ); h.append( m_basis.step ); - h.append( m_linear ); - h.append( m_periodic ); + h.append( m_wrap ); m_vertsPerCurve->hash( h ); } @@ -239,16 +276,21 @@ const CubicBasisf &CurvesPrimitive::basis() const return m_basis; } +CurvesPrimitive::Wrap CurvesPrimitive::wrap() const +{ + return m_wrap; +} + bool CurvesPrimitive::periodic() const { - return m_periodic; + return m_wrap == Wrap::Periodic; } -void CurvesPrimitive::setTopology( ConstIntVectorDataPtr verticesPerCurve, const CubicBasisf &basis, bool periodic ) +void CurvesPrimitive::setTopology( const IECore::IntVectorData *verticesPerCurve, const IECore::CubicBasisf &basis, Wrap wrap ) { m_basis = basis; - m_linear = m_basis==CubicBasisf::linear(); - m_periodic = periodic; + m_standardBasis = m_basis.standardBasis(); + m_wrap = wrap; m_vertsPerCurve = verticesPerCurve->copy(); m_numVerts = 0; @@ -261,6 +303,11 @@ void CurvesPrimitive::setTopology( ConstIntVectorDataPtr verticesPerCurve, const } } +void CurvesPrimitive::setTopology( ConstIntVectorDataPtr verticesPerCurve, const CubicBasisf &basis, bool periodic ) +{ + setTopology( verticesPerCurve.get(), basis, periodic ? Wrap::Periodic : Wrap::NonPeriodic ); +} + size_t CurvesPrimitive::variableSize( PrimitiveVariable::Interpolation interpolation ) const { switch( interpolation ) @@ -294,13 +341,16 @@ size_t CurvesPrimitive::variableSize( PrimitiveVariable::Interpolation interpola return 1; case PrimitiveVariable::Varying : case PrimitiveVariable::FaceVarying : - if( m_periodic ) + switch( m_wrap ) { - return numSegments( curveIndex ); - } - else - { - return 1 + numSegments( curveIndex ); + case Wrap::Periodic : + return numSegments( curveIndex ); + case Wrap::NonPeriodic : + case Wrap::Pinned : + return 1 + numSegments( curveIndex ); + default : + + return 0; } case PrimitiveVariable::Vertex : return m_vertsPerCurve->readable()[curveIndex]; @@ -315,12 +365,17 @@ unsigned CurvesPrimitive::numSegments( unsigned curveIndex ) const { throw Exception( "Curve index out of range." ); } - return numSegmentsInternal( m_linear, m_basis.step, m_periodic, m_vertsPerCurve->readable()[curveIndex] ); + return numSegmentsInternal( m_standardBasis, m_basis.step, m_wrap, m_vertsPerCurve->readable()[curveIndex] ); +} + +unsigned CurvesPrimitive::numSegments( const IECore::CubicBasisf &basis, Wrap wrap, unsigned numVerts ) +{ + return numSegmentsInternal( basis.standardBasis(), basis.step, wrap, numVerts ); } unsigned CurvesPrimitive::numSegments( const CubicBasisf &basis, bool periodic, unsigned numVerts ) { - return numSegmentsInternal( basis==CubicBasisf::linear(), basis.step, periodic, numVerts ); + return numSegmentsInternal( basis.standardBasis(), basis.step, periodic ? Wrap::Periodic : Wrap::NonPeriodic, numVerts ); } CurvesPrimitivePtr CurvesPrimitive::createBox( const Imath::Box3f &b ) diff --git a/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp b/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp index a516b5bf36..0c07159ac6 100644 --- a/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp +++ b/src/IECoreScene/bindings/CurvesPrimitiveBinding.cpp @@ -56,18 +56,34 @@ static IntVectorDataPtr verticesPerFace( const CurvesPrimitive &p ) void bindCurvesPrimitive() { - RunTimeTypedClass() + RunTimeTypedClass curvesClass; + + scope s = curvesClass; + enum_( "Wrap" ) + .value( "NonPeriodic", CurvesPrimitive::Wrap::NonPeriodic ) + .value( "Periodic", CurvesPrimitive::Wrap::Periodic ) + .value( "Pinned", CurvesPrimitive::Wrap::Pinned ) + ; + + curvesClass .def( init<>() ) .def( init( ( arg( "verticesPerCurve" ), arg( "basis" ) = IECore::CubicBasisf::linear(), arg( "periodic" ) = false, arg( "p" ) = object() ) ) ) + .def( + init( + ( arg( "verticesPerCurve" ), arg( "basis" ) = IECore::CubicBasisf::linear(), arg( "wrap" ) = CurvesPrimitive::Wrap::NonPeriodic, arg( "p" ) = object() ) + ) + ) .def( "numCurves", &CurvesPrimitive::numCurves ) .def( "verticesPerCurve", &verticesPerFace, "A copy of the list of vertices per curve." ) .def( "basis", &CurvesPrimitive::basis, return_value_policy() ) + .def( "wrap", &CurvesPrimitive::wrap ) .def( "periodic", &CurvesPrimitive::periodic ) - .def( "setTopology", &CurvesPrimitive::setTopology ) + .def( "setTopology", (void (CurvesPrimitive::*)( ConstIntVectorDataPtr, const CubicBasisf &, bool ))&CurvesPrimitive::setTopology ) + .def( "setTopology", (void (CurvesPrimitive::*)( const IntVectorData *, const CubicBasisf &, CurvesPrimitive::Wrap ))&CurvesPrimitive::setTopology ) .def( "variableSize", (size_t (CurvesPrimitive::*)( PrimitiveVariable::Interpolation )const)&CurvesPrimitive::variableSize ) .def( "variableSize", (size_t (CurvesPrimitive::*)( PrimitiveVariable::Interpolation, unsigned )const)&CurvesPrimitive::variableSize, arg( "curveIndex" ) ) .def( "numSegments", (unsigned (CurvesPrimitive::*)( unsigned )const)&CurvesPrimitive::numSegments, arg( "curveIndex" ) ) diff --git a/test/IECoreScene/CurvesPrimitiveTest.py b/test/IECoreScene/CurvesPrimitiveTest.py index f7f7e3d186..793e5e93de 100644 --- a/test/IECoreScene/CurvesPrimitiveTest.py +++ b/test/IECoreScene/CurvesPrimitiveTest.py @@ -34,6 +34,7 @@ import os import os.path +import pathlib import math import unittest import imath @@ -47,6 +48,7 @@ def testConstructors( self ) : c = IECoreScene.CurvesPrimitive() self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData() ) self.assertEqual( c.basis(), IECore.CubicBasisf.linear() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) self.assertEqual( c.periodic(), False ) self.assertEqual( c.keys(), [] ) self.assertEqual( c.numCurves(), 0 ) @@ -54,6 +56,7 @@ def testConstructors( self ) : c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 2 ] ) ) self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData( [ 2 ] ) ) self.assertEqual( c.basis(), IECore.CubicBasisf.linear() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) self.assertEqual( c.periodic(), False ) self.assertEqual( c.keys(), [] ) self.assertEqual( c.numCurves(), 1 ) @@ -61,22 +64,25 @@ def testConstructors( self ) : c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline() ) self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData( [ 4 ] ) ) self.assertEqual( c.basis(), IECore.CubicBasisf.bSpline() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) self.assertEqual( c.periodic(), False ) self.assertEqual( c.keys(), [] ) self.assertEqual( c.numCurves(), 1 ) - c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), True ) + c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData( [ 4 ] ) ) self.assertEqual( c.basis(), IECore.CubicBasisf.bSpline() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) self.assertEqual( c.periodic(), True ) self.assertEqual( c.keys(), [] ) self.assertEqual( c.numCurves(), 1 ) i = IECore.IntVectorData( [ 4 ] ) p = IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ), imath.V3f( 2 ), imath.V3f( 3 ) ] ) - c = IECoreScene.CurvesPrimitive( i, IECore.CubicBasisf.bSpline(), True, p ) + c = IECoreScene.CurvesPrimitive( i, IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic, p ) self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData( [ 4 ] ) ) self.assertEqual( c.basis(), IECore.CubicBasisf.bSpline() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) self.assertEqual( c.periodic(), True ) self.assertEqual( c.keys(), [ "P" ] ) self.assertNotEqual( c["P"].data, p ) @@ -88,6 +94,15 @@ def testConstructors( self ) : self.assertFalse( c["P"].data.isSame( pp ) ) self.assertFalse( c.verticesPerCurve().isSame( i ) ) + def testLegacyConstructors( self ) : + + for periodic in ( True, False ) : + curves = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), periodic ) + self.assertEqual( curves.verticesPerCurve(), IECore.IntVectorData( [ 4 ] ) ) + self.assertEqual( curves.basis(), IECore.CubicBasisf.bSpline() ) + self.assertEqual( curves.wrap(), IECoreScene.CurvesPrimitive.Wrap.Periodic if periodic else IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + self.assertEqual( curves.periodic(), periodic ) + def testConstructorValidation( self ) : self.assertRaises( Exception, IECoreScene.CurvesPrimitive, IECore.IntVectorData( [ 1 ] ) ) @@ -96,6 +111,20 @@ def testConstructorValidation( self ) : def testConstructorKeywords( self ) : + c = IECoreScene.CurvesPrimitive( + verticesPerCurve = IECore.IntVectorData( [ 3 ] ), + wrap = IECoreScene.CurvesPrimitive.Wrap.Periodic, + p = IECore.V3fVectorData( [ imath.V3f( x ) for x in range( 0, 3 ) ] ) + ) + + self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData( [ 3 ] ) ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) + self.assertEqual( c.periodic(), True ) + self.assertEqual( c.basis(), IECore.CubicBasisf.linear() ) + self.assertEqual( c["P"].data, IECore.V3fVectorData( [ imath.V3f( x ) for x in range( 0, 3 ) ], IECore.GeometricData.Interpretation.Point ) ) + + def testLegacyConstructorKeywords( self ) : + c = IECoreScene.CurvesPrimitive( verticesPerCurve = IECore.IntVectorData( [ 3 ] ), periodic = True, @@ -111,7 +140,7 @@ def testCopy( self ) : i = IECore.IntVectorData( [ 4 ] ) p = IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ), imath.V3f( 2 ), imath.V3f( 3 ) ] ) - c = IECoreScene.CurvesPrimitive( i, IECore.CubicBasisf.bSpline(), True, p ) + c = IECoreScene.CurvesPrimitive( i, IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic, p ) cc = c.copy() self.assertEqual( c, cc ) @@ -120,7 +149,7 @@ def testIO( self ) : i = IECore.IntVectorData( [ 4 ] ) p = IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ), imath.V3f( 2 ), imath.V3f( 3 ) ] ) - c = IECoreScene.CurvesPrimitive( i, IECore.CubicBasisf.bSpline(), True, p ) + c = IECoreScene.CurvesPrimitive( i, IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic, p ) IECore.Writer.create( c, os.path.join( "test", "IECore", "data", "curves.cob" ) ).write() @@ -132,7 +161,7 @@ def testIO( self ) : def testVariableSize( self ) : - c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), True ) + c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Constant ), 1 ) self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Uniform ), 1 ) @@ -151,7 +180,7 @@ def testVariableSize( self ) : self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.FaceVarying, 0 ), 4 ) self.assertEqual( c.numSegments( 0 ), 4 ) - c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), False ) + c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Constant ), 1 ) self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Uniform ), 1 ) @@ -167,6 +196,34 @@ def testVariableSize( self ) : def testSetTopology( self ) : + c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) + + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Constant ), 1 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Uniform ), 1 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex ), 4 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Varying ), 4 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.FaceVarying ), 4 ) + + newVertsPerCurve = IECore.IntVectorData( [ 4, 4 ] ) + c.setTopology( newVertsPerCurve, IECore.CubicBasisf.bezier(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + + self.assertEqual( c.verticesPerCurve(), newVertsPerCurve ) + self.assertEqual( c.basis(), IECore.CubicBasisf.bezier() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + self.assertEqual( c.periodic(), False ) + self.assertEqual( c.numCurves(), 2 ) + + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Constant ), 1 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Uniform ), 2 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex ), 8 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Varying ), 4 ) + self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.FaceVarying ), 4 ) + + newVertsPerCurve.append( 10 ) + self.assertEqual( c.verticesPerCurve(), IECore.IntVectorData( [ 4, 4 ] ) ) + + def testLegacySetTopology( self ) : + c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), True ) self.assertEqual( c.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Constant ), 1 ) @@ -180,6 +237,7 @@ def testSetTopology( self ) : self.assertEqual( c.verticesPerCurve(), newVertsPerCurve ) self.assertEqual( c.basis(), IECore.CubicBasisf.bezier() ) + self.assertEqual( c.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) self.assertEqual( c.periodic(), False ) self.assertEqual( c.numCurves(), 2 ) @@ -194,7 +252,7 @@ def testSetTopology( self ) : def testHash( self ) : - c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), True ) + c = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) h = c.hash() t = c.topologyHash() @@ -202,19 +260,19 @@ def testHash( self ) : self.assertEqual( c2.hash(), h ) self.assertEqual( c2.topologyHash(), t ) - c.setTopology( IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.bSpline(), True ) + c.setTopology( IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) self.assertNotEqual( c.hash(), h ) self.assertNotEqual( c.topologyHash(), h ) h = c.hash() t = c.topologyHash() - c.setTopology( IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.catmullRom(), True ) + c.setTopology( IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.catmullRom(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) self.assertNotEqual( c.hash(), h ) self.assertNotEqual( c.topologyHash(), h ) h = c.hash() t = c.topologyHash() - c.setTopology( IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.catmullRom(), False ) + c.setTopology( IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.catmullRom(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) self.assertNotEqual( c.hash(), h ) self.assertNotEqual( c.topologyHash(), h ) h = c.hash() @@ -224,6 +282,52 @@ def testHash( self ) : self.assertNotEqual( c.hash(), h ) self.assertEqual( c.topologyHash(), t ) + def testIOVersion0( self ) : + + dataDir = pathlib.Path( "test" ) / "IECoreScene" / "data" + + curves = IECore.ObjectReader( str( dataDir / "curvesV0-periodic.cob" ) ).read() + self.assertEqual( curves.wrap(), IECoreScene.CurvesPrimitive.Wrap.Periodic ) + self.assertTrue( curves.periodic() ) + + curves = IECore.ObjectReader( str( dataDir / "curvesV0-nonPeriodic.cob" ) ).read() + self.assertEqual( curves.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + self.assertFalse( curves.periodic() ) + + def testPinnedWrap( self ) : + + for basis in ( IECore.CubicBasisf.catmullRom(), IECore.CubicBasisf.bSpline() ) : + with self.subTest( basis = basis ) : + + curves = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 2 ] ), basis, IECoreScene.CurvesPrimitive.Wrap.Pinned ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex ), 2 ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Varying ), 2 ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex, curveIndex = 0 ), 2 ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Varying, curveIndex = 0 ), 2 ) + self.assertEqual( curves.numSegments( 0 ), 1 ) + + curves = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 3 ] ), basis, IECoreScene.CurvesPrimitive.Wrap.Pinned ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex ), 3 ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Varying ), 3 ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex, curveIndex = 0 ), 3 ) + self.assertEqual( curves.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Varying, curveIndex = 0 ), 3 ) + self.assertEqual( curves.numSegments( 0 ), 2 ) + + def testPinnedBezier( self ) : + + # Pinning doesn't apply to beziers, so we can't make a 2-vertex curve. + with self.assertRaises( RuntimeError ) : + IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 2 ] ), IECore.CubicBasisf.bezier(), IECoreScene.CurvesPrimitive.Wrap.Pinned ) + + # But we should be able to make a 4-vertex curve and have it be interpreted + # as a normal non-periodic curve. + nonPeriodic = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bezier(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + pinned = IECoreScene.CurvesPrimitive( IECore.IntVectorData( [ 4 ] ), IECore.CubicBasisf.bezier(), IECoreScene.CurvesPrimitive.Wrap.Pinned ) + + for interpolation in IECoreScene.PrimitiveVariable.Interpolation.values.values() : + self.assertEqual( nonPeriodic.variableSize( interpolation ), pinned.variableSize( interpolation ) ) + self.assertEqual( nonPeriodic.numSegments( 0 ), pinned.numSegments( 0 ) ) + def tearDown( self ) : if os.path.isfile( os.path.join( "test", "IECore", "data", "curves.cob" ) ) : diff --git a/test/IECoreScene/data/curvesV0-nonPeriodic.cob b/test/IECoreScene/data/curvesV0-nonPeriodic.cob new file mode 100644 index 0000000000000000000000000000000000000000..5bcd006e9960d575faaa022e8ae27cb02aabec73 GIT binary patch literal 1884 zcma)6OK%%h7(FwN$9A0YBQHM_kEz78QfutQanf`Vw@#u48e^KoDoD^wl4(6vJY#0Y zROy_BzN~HR>}m_ug$B-C?>RjiyF*jkG!L|ex>misy124RkKoWF36Q1o*ta3zYnoVR zN&aJwZ*i>45+vwhs92969Lix6V~}(#g%HbO>mp*%kyH^{P9v@(tM-tIwK1-vl+-a* zO6yQNA|2SJ?SXCtTv8C9;3~@C2|DQ5qdP)*WCciAMT60FG#IOm=n%_S5N?Br%G&#= z>qyUi)y8w&4~Uy0W|br&ybT&E##A&#s2NBymWxQ`ag|U~M(js*Zj!(>WFDS?tOu0E z1==Vj1nU&L+@#J3F`~amn`DG*gb0lzu3}TLCFVH!pA(qX7-A8GWX!5$9T3nF(y_b* zCI8|YI@X$)Bl)$@5Ydt9;2Y#fGiWk6(YW9v%-RYB;b20nx9?SMW z+Jj$!2Wu}6zB&iCZ2uJof|sx{TK`Nf8Ge|iiM3G69ga6Ro~dP;YPrhPQ!Tdw*F4g( zDNzchGn*?SpD&_TMEJabzV;guxCwa$Pv)7`pz)+o-@oND8_0$AyEB06}&XxkgZ*wN}L5a!KM_N`fDXDPzd7T7Q$a$1yijSx>kBS23 z${3U)lq9ND={d2nC*yZSiP-r~S=^O_y*Q5#+iBLo-x*xOgOTg!$htEDtv^!mG~~N}mg#9Z z>(|1(*n#KH;#tDjDbR7?h|G*)kF7g z0#X6#10C`f%D%G-`647<`}jNt60t1Tjzb!dsAyC8i~aYseHsRRg0#9;1XX4@g0#uC zZ+a}aS72EC0;DN^jWeZ23C*F`A4NsIPhEeS8M9p9NubuZ{R*m{aX*Q)wTHCZL(Mha zC3^XJe9|{hO3Z1mj1TWkkeh8ri9t&~|1N=bSr@<7| z9Mg&58NBv{C|X+$Byn88q)$7%fRxX_k(*=g`V%O3m}G)zmZhL+4-7ks+Th5@^5+6g`q-*^VQB<-?zeWjce}nbz2evu;dqX!HNyr_8)Mbnhu?=xHDJpJ?Xx4&%5_LwVlDx z8QDi0hHpUFuJ2m*-C)@sxntAoy4Jw*t+9!Et=gs9mk^6i!C{nh5}W&`Ky+U`zQ>)zbl-0AM!-uSR{b!WS+?e$06yN0bTUe;>M zYm1Gw#)`JNyQeMI7MnQRde7(lu zx}4Ubc11a`N52JbBjB=v_!L)BCQopKj$7Q0Pysms5?0YL5D8$`yn=AfmGV zF&aA3^WS#x68970rifW1i3m@Friuv_Es<&#l8luSQUzQkl#~_UqdGs5zzk#tPeIlL z%HjYWloNt=ijRCoXNDNjKcZ)t;Tj=AlZdO>5^RY%N&e>qW-W$T1R)u7Dp>~vbcA%Q zEJG>0I);w*7UoHQ{RtvEQeAwHJZT0d^C_mir8_QD!Ie)2FEj%%upp)S#ql6196;* z0@AT*(E^q+moK4ED4||L_`E3DwLg)-O~|Wwy1;4%#z>*MH)S#x$b@~nGld>%C?VQJ zFNH7BD&hS{RAguq;~*uZCVs&)A<_U*^zLgqHNf&pXtsnn+Q=fDV;)>bqJW{;nnOG* zxMtO+2%jmIAkH(-+;5+?SPArXDx%8)IrtFA2 z`-p%tu1r8FK}n)UeVz;SDdAa(QWKHA`GLfWUM=}lsilm`iyK?H4S$diCJAbM}GPE3ocDJZQtevXuxv literal 0 HcmV?d00001 From 0084dad2bf48e5c6ae4f6324f6dd3e247684e912 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 4 Mar 2026 12:15:10 +0000 Subject: [PATCH 07/15] IECoreUSD CurvesAlgo : Support wrap modes fully --- Changes | 1 + .../IECoreUSD/src/IECoreUSD/CurvesAlgo.cpp | 44 ++++++++++++------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/Changes b/Changes index 9cd9ce06c0..b4f819662a 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,7 @@ Improvements - CurvesPrimitive : - Added `Wrap` enum, with `Periodic`, `NonPeriodic` and `Pinned`. This generalises the previous `periodic` boolean property. - Added argument names to `variableSize()` and `numSegments()` Python bindings, so they can be passed as keywords. +- USDScene : Added support for pinned curves. Fixes ----- diff --git a/contrib/IECoreUSD/src/IECoreUSD/CurvesAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/CurvesAlgo.cpp index 779e09e674..076b77ccda 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/CurvesAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/CurvesAlgo.cpp @@ -56,7 +56,7 @@ using namespace IECoreUSD; namespace { -IECore::ObjectPtr readCurves( pxr::UsdGeomCurves &curves, pxr::UsdTimeCode time, const IECore::CubicBasisf &basis, bool periodic, const Canceller *canceller ) +IECore::ObjectPtr readCurves( pxr::UsdGeomCurves &curves, pxr::UsdTimeCode time, const IECore::CubicBasisf &basis, CurvesPrimitive::Wrap wrap, const Canceller *canceller ) { Canceller::check( canceller ); pxr::VtIntArray vertexCountsArray; @@ -64,7 +64,7 @@ IECore::ObjectPtr readCurves( pxr::UsdGeomCurves &curves, pxr::UsdTimeCode time, IECore::IntVectorDataPtr countData = DataAlgo::fromUSD( vertexCountsArray ); Canceller::check( canceller ); - IECoreScene::CurvesPrimitivePtr newCurves = new IECoreScene::CurvesPrimitive( countData, basis, periodic ); + IECoreScene::CurvesPrimitivePtr newCurves = new IECoreScene::CurvesPrimitive( countData.get(), basis, wrap ); PrimitiveAlgo::readPrimitiveVariables( curves, time, newCurves.get(), canceller ); Canceller::check( canceller ); @@ -116,19 +116,23 @@ IECore::ObjectPtr readBasisCurves( pxr::UsdGeomBasisCurves &curves, pxr::UsdTime // Wrap - bool periodic = false; - pxr::TfToken wrap; - curves.GetWrapAttr().Get( &wrap, time ); - if( wrap == pxr::UsdGeomTokens->periodic ) + CurvesPrimitive::Wrap wrap = IECoreScene::CurvesPrimitive::Wrap::NonPeriodic; + pxr::TfToken usdWrap; + curves.GetWrapAttr().Get( &usdWrap, time ); + if( usdWrap == pxr::UsdGeomTokens->periodic ) + { + wrap = IECoreScene::CurvesPrimitive::Wrap::Periodic; + } + else if( usdWrap == pxr::UsdGeomTokens->pinned ) { - periodic = true; + wrap = IECoreScene::CurvesPrimitive::Wrap::Pinned; } - else if( wrap != pxr::UsdGeomTokens->nonperiodic ) + else if( usdWrap != pxr::UsdGeomTokens->nonperiodic ) { - IECore::msg( IECore::Msg::Warning, "USDScene", "Unsupported wrap \"{}\" reading \"{}\"", wrap.GetString(), curves.GetPath().GetAsString() ); + IECore::msg( IECore::Msg::Warning, "USDScene", "Unsupported wrap \"{}\" reading \"{}\"", usdWrap.GetString(), curves.GetPath().GetAsString() ); } - return readCurves( curves, time, basis, periodic, canceller ); + return readCurves( curves, time, basis, wrap, canceller ); } bool basisCurvesMightBeTimeVarying( pxr::UsdGeomBasisCurves &curves ) @@ -153,7 +157,7 @@ IECore::ObjectPtr readNurbsCurves( pxr::UsdGeomNurbsCurves &curves, pxr::UsdTime basis = CubicBasisf::bSpline(); } - return readCurves( curves, time, basis, false, canceller ); + return readCurves( curves, time, basis, CurvesPrimitive::Wrap::NonPeriodic, canceller ); } bool nurbsCurvesMightBeTimeVarying( pxr::UsdGeomNurbsCurves &curves ) @@ -185,10 +189,20 @@ bool writeCurves( const IECoreScene::CurvesPrimitive *curves, const pxr::UsdStag usdCurves.CreateCurveVertexCountsAttr().Set( DataAlgo::toUSD( curves->verticesPerCurve() ), time ); - usdCurves.CreateWrapAttr().Set( - curves->periodic() ? pxr::UsdGeomTokens->periodic : pxr::UsdGeomTokens->nonperiodic, - time - ); + pxr::TfToken wrap; + switch( curves->wrap() ) + { + case CurvesPrimitive::Wrap::NonPeriodic : + wrap = pxr::UsdGeomTokens->nonperiodic; + break; + case CurvesPrimitive::Wrap::Periodic : + wrap = pxr::UsdGeomTokens->periodic; + break; + case CurvesPrimitive::Wrap::Pinned : + wrap = pxr::UsdGeomTokens->pinned; + break; + } + usdCurves.CreateWrapAttr().Set( wrap, time ); pxr::TfToken basis; if( curves->basis() == CubicBasisf::bezier() ) From cab67a2b2f92160c61f58eb77c3f3bad9197172d Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 18 Mar 2026 12:32:07 +0000 Subject: [PATCH 08/15] IECoreGL : Use raw string literals for GLSL source I don't know how I ever coped editing them the way they were formatted before. --- include/IECoreGL/CurvesPrimitive.h | 4 - include/IECoreGL/PointsPrimitive.h | 2 - src/IECoreGL/CurvesPrimitive.cpp | 229 ++++++++++++------------- src/IECoreGL/PointsPrimitive.cpp | 110 ++++++------ src/IECoreGL/Shader.cpp | 264 +++++++++++++++-------------- 5 files changed, 307 insertions(+), 302 deletions(-) diff --git a/include/IECoreGL/CurvesPrimitive.h b/include/IECoreGL/CurvesPrimitive.h index 0f64690b26..f1e21fba3e 100644 --- a/include/IECoreGL/CurvesPrimitive.h +++ b/include/IECoreGL/CurvesPrimitive.h @@ -89,10 +89,6 @@ class IECOREGL_API CurvesPrimitive : public Primitive void renderMode( const State *state, bool &linear, bool &ribbons ) const; - static const std::string &cubicLinesGeometrySource(); - static const std::string &cubicRibbonsGeometrySource(); - static const std::string &linearRibbonsGeometrySource(); - void ensureVertIds() const; void ensureAdjacencyVertIds() const; void ensureLinearAdjacencyVertIds() const; diff --git a/include/IECoreGL/PointsPrimitive.h b/include/IECoreGL/PointsPrimitive.h index 8d86046bf2..23cb4ef328 100644 --- a/include/IECoreGL/PointsPrimitive.h +++ b/include/IECoreGL/PointsPrimitive.h @@ -95,8 +95,6 @@ class IECOREGL_API PointsPrimitive : public Primitive Type effectiveType( const State *state ) const; - static std::string &instancingVertexSource(); - IE_CORE_FORWARDDECLARE( MemberData ); MemberDataPtr m_memberData; diff --git a/src/IECoreGL/CurvesPrimitive.cpp b/src/IECoreGL/CurvesPrimitive.cpp index 4dff45653d..01decf69ce 100644 --- a/src/IECoreGL/CurvesPrimitive.cpp +++ b/src/IECoreGL/CurvesPrimitive.cpp @@ -53,6 +53,116 @@ using namespace IECoreGL; using namespace Imath; using namespace std; +////////////////////////////////////////////////////////////////////////// +// GLSL source +////////////////////////////////////////////////////////////////////////// + +namespace +{ + +const string g_cubicLinesGeometrySource = R"( + +#version 150 compatibility +#include "IECoreGL/CurvesPrimitive.h" + +IECOREGL_CURVESPRIMITIVE_DECLARE_CUBIC_LINES_PARAMETERS + +void main() +{ + + for( int i = 0; i < 10; i++ ) + { + float t = float( i ) / 9.0; + vec4 coeffs = IECOREGL_CURVESPRIMITIVE_COEFFICIENTS( t ); + IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_CUBIC( coeffs ); + gl_Position = IECOREGL_CURVESPRIMITIVE_POSITION( coeffs ); + EmitVertex(); + } +} + +)"; + +const string g_cubicRibbonsGeometrySource = R"( + +#version 150 compatibility +#include "IECoreGL/CurvesPrimitive.h" + +IECOREGL_CURVESPRIMITIVE_DECLARE_CUBIC_RIBBONS_PARAMETERS + +out vec3 fragmentI; +out vec3 fragmentN; +out vec2 fragmentuv; + +void main() +{ + + for( int i = 0; i < 10; i++ ) + { + + float t = float( i ) / 9.0; + vec4 coeffs = IECOREGL_CURVESPRIMITIVE_COEFFICIENTS( t ); + vec4 derivCoeffs = IECOREGL_CURVESPRIMITIVE_DERIVATIVE_COEFFICIENTS( t ); + vec4 p, n, uTangent, vTangent; + IECOREGL_CURVESPRIMITIVE_CUBICFRAME( coeffs, derivCoeffs, p, n, uTangent, vTangent ); + + IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_CUBIC( coeffs ) + fragmentN = n.xyz; + fragmentI = -n.xyz; + fragmentuv = vec2( 0, t ); + gl_Position = p + width * uTangent; + EmitVertex(); + + IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_CUBIC( coeffs ) + fragmentN = n.xyz; + fragmentI = -n.xyz; + fragmentuv = vec2( 1, t ); + gl_Position = p - width * uTangent; + EmitVertex(); + + } +} + +)"; + +const string g_linearRibbonsGeometrySource = R"( + +#version 150 compatibility + +#include "IECoreGL/CurvesPrimitive.h" + +IECOREGL_CURVESPRIMITIVE_DECLARE_LINEAR_RIBBONS_PARAMETERS + +out vec3 fragmentI; +out vec3 fragmentN; + +void main() +{ + + for( int i = 0; i < 2; i++ ) + { + + vec4 p, n, uTangent, vTangent; + IECOREGL_CURVESPRIMITIVE_LINEARFRAME( i, p, n, uTangent, vTangent ); + + IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_LINEAR( i ) + fragmentN = n.xyz; + fragmentI = -n.xyz; + gl_Position = p + width * uTangent; + EmitVertex(); + + IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_LINEAR( i ) + fragmentN = n.xyz; + fragmentI = -n.xyz; + gl_Position = p - width * uTangent; + EmitVertex(); + + } +} + +)"; + +} // namespace + ////////////////////////////////////////////////////////////////////////// // StateComponents ////////////////////////////////////////////////////////////////////////// @@ -184,11 +294,11 @@ const Shader::Setup *CurvesPrimitive::shaderSetup( const Shader *shader, State * { if( linear ) { - geometryShader = shaderLoader->create( geometryShader->vertexSource(), linearRibbonsGeometrySource(), geometryShader->fragmentSource() ); + geometryShader = shaderLoader->create( geometryShader->vertexSource(), g_linearRibbonsGeometrySource, geometryShader->fragmentSource() ); } else { - geometryShader = shaderLoader->create( geometryShader->vertexSource(), cubicRibbonsGeometrySource(), geometryShader->fragmentSource() ); + geometryShader = shaderLoader->create( geometryShader->vertexSource(), g_cubicRibbonsGeometrySource, geometryShader->fragmentSource() ); } } else @@ -196,7 +306,7 @@ const Shader::Setup *CurvesPrimitive::shaderSetup( const Shader *shader, State * // When drawing as lines, default to the constant fragment source, because using a facing ratio // shader doesn't work geometryShader = shaderLoader->create( - geometryShader->vertexSource(), cubicLinesGeometrySource(), + geometryShader->vertexSource(), g_cubicLinesGeometrySource, geometryShader->fragmentSource() != "" ? geometryShader->fragmentSource() : Shader::constantFragmentSource() ); } @@ -300,119 +410,6 @@ bool CurvesPrimitive::renderUsesGLLines( const State *state ) const return glslVersion() < 150; } -const std::string &CurvesPrimitive::cubicLinesGeometrySource() -{ - static std::string s = - - "#version 150 compatibility\n" - "" - "#include \"IECoreGL/CurvesPrimitive.h\"\n" - "" - "IECOREGL_CURVESPRIMITIVE_DECLARE_CUBIC_LINES_PARAMETERS\n" - "" - "void main()" - "{" - "" - " for( int i = 0; i < 10; i++ )" - " {" - " float t = float( i ) / 9.0;" - " vec4 coeffs = IECOREGL_CURVESPRIMITIVE_COEFFICIENTS( t );" - " IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_CUBIC( coeffs );" - " gl_Position = IECOREGL_CURVESPRIMITIVE_POSITION( coeffs );" - " EmitVertex();" - "" - " }" - "}"; - - return s; -} - -const std::string &CurvesPrimitive::cubicRibbonsGeometrySource() -{ - static std::string s = - - "#version 150 compatibility\n" - "" - "#include \"IECoreGL/CurvesPrimitive.h\"\n" - "" - "IECOREGL_CURVESPRIMITIVE_DECLARE_CUBIC_RIBBONS_PARAMETERS\n" - "" - "out vec3 fragmentI;" - "out vec3 fragmentN;" - "out vec2 fragmentuv;" - "" - "void main()" - "{" - "" - " for( int i = 0; i < 10; i++ )" - " {" - "" - " float t = float( i ) / 9.0;" - " vec4 coeffs = IECOREGL_CURVESPRIMITIVE_COEFFICIENTS( t );" - " vec4 derivCoeffs = IECOREGL_CURVESPRIMITIVE_DERIVATIVE_COEFFICIENTS( t );" - " vec4 p, n, uTangent, vTangent;" - " IECOREGL_CURVESPRIMITIVE_CUBICFRAME( coeffs, derivCoeffs, p, n, uTangent, vTangent );" - "" - " IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_CUBIC( coeffs )" - " fragmentN = n.xyz;" - " fragmentI = -n.xyz;" - " fragmentuv = vec2( 0, t );" - " gl_Position = p + width * uTangent;" - " EmitVertex();" - "" - " IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_CUBIC( coeffs )" - " fragmentN = n.xyz;" - " fragmentI = -n.xyz;" - " fragmentuv = vec2( 1, t );" - " gl_Position = p - width * uTangent;" - " EmitVertex();" - "" - " }" - "}"; - - return s; -} - -const std::string &CurvesPrimitive::linearRibbonsGeometrySource() -{ - static std::string s = - - "#version 150 compatibility\n" - "" - "#include \"IECoreGL/CurvesPrimitive.h\"\n" - "" - "IECOREGL_CURVESPRIMITIVE_DECLARE_LINEAR_RIBBONS_PARAMETERS\n" - "" - "out vec3 fragmentI;" - "out vec3 fragmentN;" - "" - "void main()" - "{" - - " for( int i = 0; i < 2; i++ )" - " {" - - " vec4 p, n, uTangent, vTangent;" - " IECOREGL_CURVESPRIMITIVE_LINEARFRAME( i, p, n, uTangent, vTangent );" - - " IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_LINEAR( i )" - " fragmentN = n.xyz;" - " fragmentI = -n.xyz;" - " gl_Position = p + width * uTangent;" - " EmitVertex();" - - " IECOREGL_ASSIGN_VERTEX_PASS_THROUGH_LINEAR( i )" - " fragmentN = n.xyz;" - " fragmentI = -n.xyz;" - " gl_Position = p - width * uTangent;" - " EmitVertex();" - "" - " }" - "}"; - - return s; -} - void CurvesPrimitive::ensureVertIds() const { if( m_memberData->vertIdsBuffer ) diff --git a/src/IECoreGL/PointsPrimitive.cpp b/src/IECoreGL/PointsPrimitive.cpp index 948e528bf5..267d890ab3 100644 --- a/src/IECoreGL/PointsPrimitive.cpp +++ b/src/IECoreGL/PointsPrimitive.cpp @@ -69,6 +69,63 @@ IECOREGL_TYPEDSTATECOMPONENT_SPECIALISEANDINSTANTIATE( PointsPrimitive::GLPointW } // namespace IECoreGL +////////////////////////////////////////////////////////////////////////// +// GLSL Source +////////////////////////////////////////////////////////////////////////// + +namespace +{ + +const std::string g_instancingVertexSource = R"( + +#version 120 + +#include "IECoreGL/PointsPrimitive.h" +#include "IECoreGL/VertexShader.h" + +IECOREGL_POINTSPRIMITIVE_DECLAREVERTEXPARAMETERS + +IECOREGL_VERTEXSHADER_IN vec3 vertexCs; +uniform bool vertexCsActive = false; + +uniform vec3 Cs = vec3( 1, 1, 1 ); + +IECOREGL_VERTEXSHADER_IN vec3 instanceP; +IECOREGL_VERTEXSHADER_IN vec3 instanceN; +IECOREGL_VERTEXSHADER_IN vec2 instanceuv; + +IECOREGL_VERTEXSHADER_OUT vec3 fragmentI; +IECOREGL_VERTEXSHADER_OUT vec3 fragmentN; +IECOREGL_VERTEXSHADER_OUT vec2 fragmentuv; +IECOREGL_VERTEXSHADER_OUT vec3 fragmentCs; + +void main() +{ + mat4 instanceMatrix = IECOREGL_POINTSPRIMITIVE_INSTANCEMATRIX; + + vec4 pCam = instanceMatrix * vec4( instanceP, 1 ); + gl_Position = gl_ProjectionMatrix * pCam; + + fragmentN = normalize( instanceMatrix * vec4( instanceN, 0.0 ) ).xyz; + + if( gl_ProjectionMatrix[2][3] != 0.0 ) + { + fragmentI = normalize( -pCam.xyz ); + } + else + { + fragmentI = vec3( 0.0, 0.0, -1.0 ); + } + + fragmentCs = mix( Cs, vertexCs, float( vertexCsActive ) ); + fragmentuv = instanceuv; + +}; + +)"; + +} // namespace + ////////////////////////////////////////////////////////////////////////// // MemberData ////////////////////////////////////////////////////////////////////////// @@ -223,7 +280,7 @@ const Shader::Setup *PointsPrimitive::shaderSetup( const Shader *shader, State * // a shader capable of performing the instancing, but if not then we substitute in our own // instancing vertex shader. ShaderLoader *shaderLoader = shaderStateComponent->shaderLoader(); - instancingShader = shaderLoader->create( instancingVertexSource(), "", shader->fragmentSource() ); + instancingShader = shaderLoader->create( g_instancingVertexSource, "", shader->fragmentSource() ); } Shader::SetupPtr instancingShaderSetup = new Shader::Setup( instancingShader ); @@ -362,54 +419,3 @@ PointsPrimitive::Type PointsPrimitive::effectiveType( const State *state ) const } return result; } - -std::string &PointsPrimitive::instancingVertexSource() -{ - static std::string s = - - "#version 120\n" - "" - "#include \"IECoreGL/PointsPrimitive.h\"\n" - "#include \"IECoreGL/VertexShader.h\"\n" - "" - "IECOREGL_POINTSPRIMITIVE_DECLAREVERTEXPARAMETERS\n" - "" - "IECOREGL_VERTEXSHADER_IN vec3 vertexCs;" - "uniform bool vertexCsActive = false;" - "" - "uniform vec3 Cs = vec3( 1, 1, 1 );" - "" - "IECOREGL_VERTEXSHADER_IN vec3 instanceP;" - "IECOREGL_VERTEXSHADER_IN vec3 instanceN;" - "IECOREGL_VERTEXSHADER_IN vec2 instanceuv;" - "" - "IECOREGL_VERTEXSHADER_OUT vec3 fragmentI;" - "IECOREGL_VERTEXSHADER_OUT vec3 fragmentN;" - "IECOREGL_VERTEXSHADER_OUT vec2 fragmentuv;" - "IECOREGL_VERTEXSHADER_OUT vec3 fragmentCs;" - "" - "void main()" - "{" - " mat4 instanceMatrix = IECOREGL_POINTSPRIMITIVE_INSTANCEMATRIX;" - "" - " vec4 pCam = instanceMatrix * vec4( instanceP, 1 );" - " gl_Position = gl_ProjectionMatrix * pCam;" - "" - " fragmentN = normalize( instanceMatrix * vec4( instanceN, 0.0 ) ).xyz;" - "" - " if( gl_ProjectionMatrix[2][3] != 0.0 )" - " {" - " fragmentI = normalize( -pCam.xyz );" - " }" - " else" - " {" - " fragmentI = vec3( 0.0, 0.0, -1.0 );" - " }" - "" - " fragmentCs = mix( Cs, vertexCs, float( vertexCsActive ) );" - " fragmentuv = instanceuv;" - "" - "}"; - - return s; -} diff --git a/src/IECoreGL/Shader.cpp b/src/IECoreGL/Shader.cpp index 880a485449..d4f8e097ba 100644 --- a/src/IECoreGL/Shader.cpp +++ b/src/IECoreGL/Shader.cpp @@ -968,150 +968,158 @@ Shader::Setup::ScopedBinding::~ScopedBinding() const std::string &Shader::defaultVertexSource() { - static string s = - - "#version 120\n" - "" - "#if __VERSION__ <= 120\n" - "#define in attribute\n" - "#define out varying\n" - "#endif\n" - "" - "uniform vec3 Cs = vec3( 1, 1, 1 );" - "uniform bool vertexCsActive = false;" - "uniform bool vertexNActive = false;" - "" - "in vec3 vertexP;" - "in vec3 vertexN;" - "in vec2 vertexuv;" - "in vec3 vertexCs;" - "" - "out vec3 geometryI;" - "out vec3 geometryP;" - "out vec3 geometryN;" - "out vec2 geometryuv;" - "out vec3 geometryCs;" - "" - "out vec3 fragmentI;" - "out vec3 fragmentP;" - "out vec3 fragmentN;" - "out vec2 fragmentuv;" - "out vec3 fragmentCs;" - "" - "void main()" - "{" - " vec4 pCam = gl_ModelViewMatrix * vec4( vertexP, 1 );" - " gl_Position = gl_ProjectionMatrix * pCam;" - " geometryP = pCam.xyz;" - "" - " if( vertexNActive )" - " {" - " geometryN = normalize( gl_NormalMatrix * vertexN );" - " }" - " else" - " {" - " geometryN = vec3( 0.0, 0.0, 1.0 );" - " }" - "" - " if( gl_ProjectionMatrix[2][3] != 0.0 )" - " {" - " geometryI = normalize( -pCam.xyz );" - " }" - " else" - " {" - " geometryI = vec3( 0, 0, -1 );" - " }" - "" - " geometryuv = vertexuv;" - " geometryCs = mix( Cs, vertexCs, float( vertexCsActive ) );" - "" - " fragmentI = geometryI;" - " fragmentP = geometryP;" - " fragmentN = geometryN;" - " fragmentuv = geometryuv;" - " fragmentCs = geometryCs;" - "}"; - - return s; + static const string g_s = R"( + + #version 150 compatibility + + #if __VERSION__ <= 120 + #define in attribute + #define out varying + #endif + + uniform vec3 Cs = vec3( 1, 1, 1 ); + uniform bool vertexCsActive = false; + uniform bool vertexNActive = false; + + in vec3 vertexP; + in vec3 vertexN; + in vec2 vertexuv; + in vec3 vertexCs; + + out vec3 geometryI; + out vec3 geometryP; + out vec3 geometryN; + out vec2 geometryuv; + out vec3 geometryCs; + + out vec3 fragmentI; + out vec3 fragmentP; + out vec3 fragmentN; + out vec2 fragmentuv; + out vec3 fragmentCs; + + void main() + { + vec4 pCam = gl_ModelViewMatrix * vec4( vertexP, 1 ); + gl_Position = gl_ProjectionMatrix * pCam; + geometryP = pCam.xyz; + + if( vertexNActive ) + { + geometryN = normalize( gl_NormalMatrix * vertexN ); + } + else + { + geometryN = vec3( 0.0, 0.0, 1.0 ); + } + + if( gl_ProjectionMatrix[2][3] != 0.0 ) + { + geometryI = normalize( -pCam.xyz ); + } + else + { + geometryI = vec3( 0, 0, -1 ); + } + + geometryuv = vertexuv; + geometryCs = mix( Cs, vertexCs, float( vertexCsActive ) ); + + fragmentI = geometryI; + fragmentP = geometryP; + fragmentN = geometryN; + fragmentuv = geometryuv; + fragmentCs = geometryCs; + } + + )"; + + return g_s; } const std::string &Shader::defaultGeometrySource() { - static string s = ""; - return s; + static const string g_s = ""; + return g_s; } const std::string &Shader::defaultFragmentSource() { - static string s = - - "#if __VERSION__ <= 120\n" - "#define in varying\n" - "#endif\n" - "" - "in vec3 fragmentI;" - "in vec3 fragmentN;" - "in vec3 fragmentCs;" - "" - "void main()" - "{" - " vec3 Nf = faceforward( fragmentN, -fragmentI, fragmentN );" - " float f = dot( normalize( fragmentI ), normalize(Nf) );" - " gl_FragColor = vec4( f * fragmentCs, 1 );" - "}"; - - return s; + static const string g_s = R"( + + #if __VERSION__ <= 120 + #define in varying + #endif + + in vec3 fragmentI; + in vec3 fragmentN; + in vec3 fragmentCs; + + void main() + { + vec3 Nf = faceforward( fragmentN, -fragmentI, fragmentN ); + float f = dot( normalize( fragmentI ), normalize(Nf) ); + gl_FragColor = vec4( f * fragmentCs, 1 ); + }; + + )"; + + return g_s; } const std::string &Shader::constantFragmentSource() { - static string s = - - "#if __VERSION__ <= 120\n" - "#define in varying\n" - "#endif\n" - "" - "in vec3 fragmentCs;" - "" - "void main()" - "{" - " gl_FragColor = vec4( fragmentCs, 1 );" - "}"; - - return s; + static const string g_s = R"( + + #if __VERSION__ <= 120 + #define in varying + #endif + + in vec3 fragmentCs; + + void main() + { + gl_FragColor = vec4( fragmentCs, 1 ); + }; + + )"; + + return g_s; } const std::string &Shader::lambertFragmentSource() { - static string s = - - "#if __VERSION__ <= 120\n" - "#define in varying\n" - "#endif\n" - "" - "#include \"IECoreGL/Lights.h\"\n" - "#include \"IECoreGL/ColorAlgo.h\"\n" - "#include \"IECoreGL/Diffuse.h\"\n" - - "in vec3 fragmentP;" - "in vec3 fragmentN;" - "in vec3 fragmentCs;" - "" - "void main()" - "{" - " vec3 n = normalize( fragmentN );" - "" - " vec3 L[ gl_MaxLights ];" - " vec3 Cl[ gl_MaxLights ];" - "" - " lights( fragmentP, Cl, L, gl_MaxLights );" - "" - " vec3 Cdiffuse = ieDiffuse( fragmentP, n, Cl, L, gl_MaxLights );" - "" - " gl_FragColor = vec4( Cdiffuse, 1.0 );" - "}"; - - return s; + static const string g_s = R"( + + #if __VERSION__ <= 120 + #define in varying + #endif + + #include "IECoreGL/Lights.h" + #include "IECoreGL/ColorAlgo.h" + #include "IECoreGL/Diffuse.h" + + in vec3 fragmentP; + in vec3 fragmentN; + in vec3 fragmentCs; + + void main() + { + vec3 n = normalize( fragmentN ); + + vec3 L[ gl_MaxLights ]; + vec3 Cl[ gl_MaxLights ]; + + lights( fragmentP, Cl, L, gl_MaxLights ); + + vec3 Cdiffuse = ieDiffuse( fragmentP, n, Cl, L, gl_MaxLights ); + + gl_FragColor = vec4( Cdiffuse, 1.0 ); + }; + + )"; + + return g_s; } /////////////////////////////////////////////////////////////////////////////// From 619f2ae7befb48cc42da6e9c35843a8124444d54 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 16 Mar 2026 17:57:57 +0000 Subject: [PATCH 09/15] IECoreGL : Support pinned curves --- glsl/IECoreGL/CurvesPrimitive.h | 31 ++++- include/IECoreGL/CurvesPrimitive.h | 4 + src/IECoreGL/CurvesPrimitive.cpp | 108 ++++++++++++++++-- src/IECoreGL/Shader.cpp | 3 + src/IECoreGL/ToGLCurvesConverter.cpp | 2 +- .../bindings/CurvesPrimitiveBinding.cpp | 13 +++ 6 files changed, 148 insertions(+), 13 deletions(-) diff --git a/glsl/IECoreGL/CurvesPrimitive.h b/glsl/IECoreGL/CurvesPrimitive.h index ea65877be1..6cfc6ffc44 100644 --- a/glsl/IECoreGL/CurvesPrimitive.h +++ b/glsl/IECoreGL/CurvesPrimitive.h @@ -47,7 +47,12 @@ layout( lines_adjacency ) in;\ layout( line_strip, max_vertices = 10 ) out;\ \ + in int geometryIsCurveEndPoint[];\ + \ uniform mat4x4 basis;\ + uniform mat4x4 phantomBasis0;\ + uniform mat4x4 phantomBasis1;\ + uniform mat4x4 phantomBasis2;\ \ IECOREGL_CURVESPRIMITIVE_DECLARE_VERTEX_PASS_THROUGH_PARAMETERS @@ -65,16 +70,36 @@ layout( lines_adjacency ) in;\ layout( triangle_strip, max_vertices = 20 ) out;\ \ + in int geometryIsCurveEndPoint[];\ + \ uniform mat4x4 basis;\ + uniform mat4x4 phantomBasis0;\ + uniform mat4x4 phantomBasis1;\ + uniform mat4x4 phantomBasis2;\ uniform float width;\ \ IECOREGL_CURVESPRIMITIVE_DECLARE_VERTEX_PASS_THROUGH_PARAMETERS +#define IECOREGL_CURVESPRIMITIVE_SELECT_BASIS \ + mat4x4 selectedBasis;\ + if( bool( geometryIsCurveEndPoint[1] ) )\ + {\ + selectedBasis = bool( geometryIsCurveEndPoint[2] ) ? phantomBasis2 : phantomBasis0;\ + }\ + else if( bool( geometryIsCurveEndPoint[2] ) )\ + {\ + selectedBasis = phantomBasis1;\ + }\ + else\ + {\ + selectedBasis = basis;\ + } + #define IECOREGL_CURVESPRIMITIVE_COEFFICIENTS( t ) \ - ieCurvesPrimitiveCoefficients( basis, t ) + ieCurvesPrimitiveCoefficients( selectedBasis, t ) #define IECOREGL_CURVESPRIMITIVE_DERIVATIVE_COEFFICIENTS( t ) \ - ieCurvesPrimitiveDerivativeCoefficients( basis, t ) + ieCurvesPrimitiveDerivativeCoefficients( selectedBasis, t ) #define IECOREGL_CURVESPRIMITIVE_POSITION( coeffs )\ ieCurvesPrimitivePosition( coeffs ) @@ -96,7 +121,7 @@ vec4 ieCurvesPrimitiveCoefficients( in mat4x4 basis, in float t ) float t2 = t * t; float t3 = t2 * t; - return vec4( + return vec4( basis[0][0] * t3 + basis[1][0] * t2 + basis[2][0] * t + basis[3][0], basis[0][1] * t3 + basis[1][1] * t2 + basis[2][1] * t + basis[3][1], basis[0][2] * t3 + basis[1][2] * t2 + basis[2][2] * t + basis[3][2], diff --git a/include/IECoreGL/CurvesPrimitive.h b/include/IECoreGL/CurvesPrimitive.h index f1e21fba3e..3272b4fb21 100644 --- a/include/IECoreGL/CurvesPrimitive.h +++ b/include/IECoreGL/CurvesPrimitive.h @@ -38,6 +38,8 @@ #include "IECoreGL/Export.h" #include "IECoreGL/Primitive.h" +#include "IECoreScene/CurvesPrimitive.h" + #include "IECore/CubicBasis.h" #include "IECore/VectorTypedData.h" @@ -52,6 +54,8 @@ class IECOREGL_API CurvesPrimitive : public Primitive IE_CORE_DECLARERUNTIMETYPEDEXTENSION( IECoreGL::CurvesPrimitive, CurvesPrimitiveTypeId, Primitive ); + CurvesPrimitive( const IECore::CubicBasisf &basis, IECoreScene::CurvesPrimitive::Wrap wrap, IECore::ConstIntVectorDataPtr vertsPerCurve, float width=1.0f ); + /// \deprecated Use the version taking `wrap` argument instead. CurvesPrimitive( const IECore::CubicBasisf &basis, bool periodic, IECore::ConstIntVectorDataPtr vertsPerCurve, float width=1.0f ); ~CurvesPrimitive() override; diff --git a/src/IECoreGL/CurvesPrimitive.cpp b/src/IECoreGL/CurvesPrimitive.cpp index 01decf69ce..0e374b61bf 100644 --- a/src/IECoreGL/CurvesPrimitive.cpp +++ b/src/IECoreGL/CurvesPrimitive.cpp @@ -69,6 +69,7 @@ IECOREGL_CURVESPRIMITIVE_DECLARE_CUBIC_LINES_PARAMETERS void main() { + IECOREGL_CURVESPRIMITIVE_SELECT_BASIS; for( int i = 0; i < 10; i++ ) { @@ -95,6 +96,7 @@ out vec2 fragmentuv; void main() { + IECOREGL_CURVESPRIMITIVE_SELECT_BASIS; for( int i = 0; i < 10; i++ ) { @@ -185,14 +187,22 @@ class CurvesPrimitive::MemberData : public IECore::RefCounted public : - MemberData( const IECore::CubicBasisf &b, bool p, IECore::ConstIntVectorDataPtr v, float w ) - : basis( b ), periodic( p ), vertsPerCurve( v->copy() ), width( w ) + MemberData( const IECore::CubicBasisf &b, IECoreScene::CurvesPrimitive::Wrap wrap, IECore::ConstIntVectorDataPtr v, float w ) + : basis( b ), wrap( wrap ), vertsPerCurve( v->copy() ), width( w ) { + if( wrap == IECoreScene::CurvesPrimitive::Wrap::Pinned ) + { + if( basis != IECore::CubicBasisf::catmullRom() && basis != IECore::CubicBasisf::bSpline() ) + { + // Pinning doesn't apply to this basis. + wrap = IECoreScene::CurvesPrimitive::Wrap::NonPeriodic; + } + } } Imath::Box3f bound; IECore::CubicBasisf basis; - bool periodic; + IECoreScene::CurvesPrimitive::Wrap wrap; IECore::IntVectorDataPtr vertsPerCurve; float width; IECore::V3fVectorData::ConstPtr points; @@ -229,8 +239,13 @@ class CurvesPrimitive::MemberData : public IECore::RefCounted IE_CORE_DEFINERUNTIMETYPED( CurvesPrimitive ); +CurvesPrimitive::CurvesPrimitive( const IECore::CubicBasisf &basis, IECoreScene::CurvesPrimitive::Wrap wrap, IECore::ConstIntVectorDataPtr vertsPerCurve, float width ) + : m_memberData( new MemberData( basis, wrap, vertsPerCurve, width ) ) +{ +} + CurvesPrimitive::CurvesPrimitive( const IECore::CubicBasisf &basis, bool periodic, IECore::ConstIntVectorDataPtr vertsPerCurve, float width ) - : m_memberData( new MemberData( basis, periodic, vertsPerCurve, width ) ) + : CurvesPrimitive( basis, periodic ? IECoreScene::CurvesPrimitive::Wrap::Periodic : IECoreScene::CurvesPrimitive::Wrap::NonPeriodic, vertsPerCurve, width ) { } @@ -318,6 +333,52 @@ const Shader::Setup *CurvesPrimitive::shaderSetup( const Shader *shader, State * geometryShaderSetup->addUniformParameter( "basis", new IECore::M44fData( m_memberData->basis.matrix ) ); geometryShaderSetup->addUniformParameter( "width", new IECore::M44fData( m_memberData->width ) ); + if( m_memberData->wrap == IECoreScene::CurvesPrimitive::Wrap::Pinned && !linear ) + { + // To handle "phantom vertices" in the geometry shader, we need to know which + // vertices correspond to the original endpoints of the curve. We do that using + // the `vertexIsCurveEndPoint` attribute. + IECore::IntVectorDataPtr isEndPointData = new IECore::IntVectorData(); + vector &isEndPoint = isEndPointData->writable(); + isEndPoint.resize( m_memberData->points->readable().size(), 0 ); + + int i = 0; + for( auto c : m_memberData->vertsPerCurve->readable() ) + { + isEndPoint[i] = 1; + isEndPoint[i+c-1] = 1; + i += c; + } + geometryShaderSetup->addVertexAttribute( "vertexIsCurveEndPoint", isEndPointData ); + + // And we use specially adjusted basis matrices to give the effect of the + // phantom vertices being in the required position. + M44f phantomBasis0, phantomBasis1, phantomBasis2; + for( int x = 0; x < 4; ++x ) + { + // Basis for first segment. The phantom vertex `p[0]` is required to be at `2 * p[1] - p[2]`, + // so we distribute its weights onto `p[1]` and `p[2]` appropriately. + phantomBasis0[x][0] = 0.0f; + phantomBasis0[x][1] = m_memberData->basis.matrix[x][1] + 2 * m_memberData->basis.matrix[x][0]; + phantomBasis0[x][2] = m_memberData->basis.matrix[x][2] - m_memberData->basis.matrix[x][0]; + phantomBasis0[x][3] = m_memberData->basis.matrix[x][3]; + // Basis for last segment. The phantom vertex `p[3]` is required to be at `2 * p[2] - p[1]`. + phantomBasis1[x][0] = m_memberData->basis.matrix[x][0]; + phantomBasis1[x][1] = m_memberData->basis.matrix[x][1] - m_memberData->basis.matrix[x][3]; + phantomBasis1[x][2] = m_memberData->basis.matrix[x][2] + 2 * m_memberData->basis.matrix[x][3]; + phantomBasis1[x][3] = 0.0f; + // Basis for single segment in which both endpoints are pinned. + phantomBasis2[x][0] = 0.0f; + phantomBasis2[x][1] = m_memberData->basis.matrix[x][1] + 2 * m_memberData->basis.matrix[x][0] - m_memberData->basis.matrix[x][3]; + phantomBasis2[x][2] = m_memberData->basis.matrix[x][2] - m_memberData->basis.matrix[x][0] + 2 * m_memberData->basis.matrix[x][3]; + phantomBasis2[x][3] = 0.0f; + } + + geometryShaderSetup->addUniformParameter( "phantomBasis0", new IECore::M44fData( phantomBasis0 ) ); + geometryShaderSetup->addUniformParameter( "phantomBasis1", new IECore::M44fData( phantomBasis1 ) ); + geometryShaderSetup->addUniformParameter( "phantomBasis2", new IECore::M44fData( phantomBasis2 ) ); + } + m_memberData->geometrySetups.push_back( MemberData::GeometrySetup( shader, geometryShaderSetup, linear, ribbons ) ); return geometryShaderSetup.get(); @@ -430,7 +491,7 @@ void CurvesPrimitive::ensureVertIds() const vertIds.push_back( vertIndex ); vertIds.push_back( ++vertIndex ); } - if( m_memberData->periodic ) + if( m_memberData->wrap == IECoreScene::CurvesPrimitive::Wrap::Periodic ) { vertIds.push_back( vertIndex ); vertIds.push_back( vertIndex - numSegments ); @@ -459,10 +520,40 @@ void CurvesPrimitive::ensureAdjacencyVertIds() const for( vector::const_iterator it = vertsPerCurve.begin(), eIt = vertsPerCurve.end(); it != eIt; it++ ) { int numPoints = *it; - int numSegments = IECoreScene::CurvesPrimitive::numSegments( m_memberData->basis, m_memberData->periodic, numPoints ); + int numSegments = IECoreScene::CurvesPrimitive::numSegments( m_memberData->basis, m_memberData->wrap, numPoints ); unsigned pi = 0; for( int i=0; iwrap == IECoreScene::CurvesPrimitive::Wrap::Pinned ) + { + // Pinned curve. We give the first and last segment special + // treatment so as to interpolate all the way to the endpoint. + // This could be achieved by adding "phantom vertices", but we + // don't want to have to preprocess all the vertex primitive + // variables to add them. Instead we use custom basis matrices + // which have the same effect, by distributing the endpoint + // weights onto the two vertices from which the phantom vertex + // would be constructed. This means that the endpoint vertices + // are actually unused because their weights are zero. + if( i == 0 ) + { + vertIds.push_back( baseIndex + pi ); // Unused + vertIds.push_back( baseIndex + pi ); + vertIds.push_back( baseIndex + pi + 1 ); + vertIds.push_back( baseIndex + pi + ( numSegments > 1 ? 2 : 1 ) ); + continue; + } + else if( i == numSegments - 1 ) + { + vertIds.push_back( baseIndex + pi ); + vertIds.push_back( baseIndex + pi + 1 ); + vertIds.push_back( baseIndex + pi + 2 ); + vertIds.push_back( baseIndex + pi + 2 ); // Unused + continue; + } + } + + // Regular segment. vertIds.push_back( baseIndex + ( pi % numPoints ) ); vertIds.push_back( baseIndex + ( (pi+1) % numPoints ) ); vertIds.push_back( baseIndex + ( (pi+2) % numPoints ) ); @@ -471,7 +562,6 @@ void CurvesPrimitive::ensureAdjacencyVertIds() const } baseIndex += numPoints; - } m_memberData->numAdjacencyVertIds = vertIds.size(); @@ -495,11 +585,11 @@ void CurvesPrimitive::ensureLinearAdjacencyVertIds() const for( vector::const_iterator it = vertsPerCurve.begin(), eIt = vertsPerCurve.end(); it != eIt; it++ ) { unsigned int numPoints = *it; - int numSegments = IECoreScene::CurvesPrimitive::numSegments( m_memberData->basis, m_memberData->periodic, numPoints ); + int numSegments = IECoreScene::CurvesPrimitive::numSegments( m_memberData->basis, m_memberData->wrap, numPoints ); int pi = 0; for( int i=0; iperiodic ) + if( m_memberData->wrap == IECoreScene::CurvesPrimitive::Wrap::Periodic ) { vertIds.push_back( baseIndex + ( (pi-1) % numPoints ) ); vertIds.push_back( baseIndex + ( (pi) % numPoints ) ); diff --git a/src/IECoreGL/Shader.cpp b/src/IECoreGL/Shader.cpp index d4f8e097ba..24a281d64a 100644 --- a/src/IECoreGL/Shader.cpp +++ b/src/IECoreGL/Shader.cpp @@ -985,12 +985,14 @@ const std::string &Shader::defaultVertexSource() in vec3 vertexN; in vec2 vertexuv; in vec3 vertexCs; + in int vertexIsCurveEndPoint; out vec3 geometryI; out vec3 geometryP; out vec3 geometryN; out vec2 geometryuv; out vec3 geometryCs; + out int geometryIsCurveEndPoint; out vec3 fragmentI; out vec3 fragmentP; @@ -1024,6 +1026,7 @@ const std::string &Shader::defaultVertexSource() geometryuv = vertexuv; geometryCs = mix( Cs, vertexCs, float( vertexCsActive ) ); + geometryIsCurveEndPoint = vertexIsCurveEndPoint; fragmentI = geometryI; fragmentP = geometryP; diff --git a/src/IECoreGL/ToGLCurvesConverter.cpp b/src/IECoreGL/ToGLCurvesConverter.cpp index 43fb89fb21..15804ee406 100644 --- a/src/IECoreGL/ToGLCurvesConverter.cpp +++ b/src/IECoreGL/ToGLCurvesConverter.cpp @@ -125,7 +125,7 @@ IECore::RunTimeTypedPtr ToGLCurvesConverter::doConversion( IECore::ConstObjectPt width = widthData->readable(); } - CurvesPrimitive::Ptr result = new CurvesPrimitive( curves->basis(), curves->periodic(), curves->verticesPerCurve(), width ); + CurvesPrimitive::Ptr result = new CurvesPrimitive( curves->basis(), curves->wrap(), curves->verticesPerCurve(), width ); for ( IECoreScene::PrimitiveVariableMap::const_iterator pIt = curves->variables.begin(); pIt != curves->variables.end(); ++pIt ) { diff --git a/src/IECoreGL/bindings/CurvesPrimitiveBinding.cpp b/src/IECoreGL/bindings/CurvesPrimitiveBinding.cpp index 8cd71e9903..257a9ef657 100644 --- a/src/IECoreGL/bindings/CurvesPrimitiveBinding.cpp +++ b/src/IECoreGL/bindings/CurvesPrimitiveBinding.cpp @@ -62,6 +62,19 @@ void bindCurvesPrimitive() arg_( "width" ) = 1.0f ) ) ) + .def( + init< + const IECore::CubicBasisf &, + IECoreScene::CurvesPrimitive::Wrap, + IECore::ConstIntVectorDataPtr, + float + >( ( + arg_( "basis" ), + arg_( "wrap" ), + arg_( "vertsPerCurve" ), + arg_( "width" ) = 1.0f + ) ) + ) ; bindTypedStateComponent( "IgnoreBasis" ); From 2bb0c5f3b944116b741d0b8368d1ce8e017959b9 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Thu, 5 Mar 2026 10:54:47 +0000 Subject: [PATCH 10/15] CurvesAlgo : Add utilities for dealing with pinned curves --- Changes | 1 + include/IECoreScene/CurvesAlgo.h | 8 + src/IECoreScene/CurvesAlgoConvertPinned.cpp | 180 ++++++++++++++++++ .../bindings/CurvesAlgoBinding.cpp | 8 + test/IECoreScene/CurvesAlgoTest.py | 93 +++++++++ 5 files changed, 290 insertions(+) create mode 100644 src/IECoreScene/CurvesAlgoConvertPinned.cpp diff --git a/Changes b/Changes index b4f819662a..7ebf6f96b3 100644 --- a/Changes +++ b/Changes @@ -8,6 +8,7 @@ Improvements - Added `Wrap` enum, with `Periodic`, `NonPeriodic` and `Pinned`. This generalises the previous `periodic` boolean property. - Added argument names to `variableSize()` and `numSegments()` Python bindings, so they can be passed as keywords. - USDScene : Added support for pinned curves. +- CurvesAlgo : Added `isPinned()` and `convertPinnedToNonPeriodic()` utilities. Fixes ----- diff --git a/include/IECoreScene/CurvesAlgo.h b/include/IECoreScene/CurvesAlgo.h index 56755b95a0..e475b459ba 100644 --- a/include/IECoreScene/CurvesAlgo.h +++ b/include/IECoreScene/CurvesAlgo.h @@ -64,7 +64,15 @@ IECORESCENE_API CurvesPrimitivePtr deleteCurves( const CurvesPrimitive *curvesPr /// completely segmententing the curves based on the unique values in a primitive variable. IECORESCENE_API std::vector segment( const CurvesPrimitive *curves, const PrimitiveVariable &primitiveVariable, const IECore::Data *segmentValues = nullptr, const IECore::Canceller *canceller = nullptr ); +/// Returns true if wrap is pinned and basis is BSpline or CatmullRom. +IECORESCENE_API bool isPinned( const CurvesPrimitive *curves ); + +/// If `Curves::wrap()` is `pinned`, converts it to `NonPeriodic`, adding "phantom" endpoints as required. +/// If wrap is not pinned, does nothing. +IECORESCENE_API void convertPinnedToNonPeriodic( CurvesPrimitive *curves, const IECore::Canceller *canceller = nullptr ); + /// Update the number of replicated end points based on the basis. +/// \deprecated Use pinned curves instead. IECORESCENE_API CurvesPrimitivePtr updateEndpointMultiplicity( const CurvesPrimitive *curves, const IECore::CubicBasisf& cubicBasis, const IECore::Canceller *canceller = nullptr ); } // namespace CurveAlgo diff --git a/src/IECoreScene/CurvesAlgoConvertPinned.cpp b/src/IECoreScene/CurvesAlgoConvertPinned.cpp new file mode 100644 index 0000000000..964308f10d --- /dev/null +++ b/src/IECoreScene/CurvesAlgoConvertPinned.cpp @@ -0,0 +1,180 @@ +////////////////////////////////////////////////////////////////////////// +// +// Copyright (c) 2025, Cinesite VFX Ltd. All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// +// * Redistributions in binary form must reproduce the above copyright +// notice, this list of conditions and the following disclaimer in the +// documentation and/or other materials provided with the distribution. +// +// * Neither the name of Image Engine Design nor the names of any +// other contributors to this software may be used to endorse or +// promote products derived from this software without specific prior +// written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +// IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +// THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +// PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR +// CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +// EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +// PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +// LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +// NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +////////////////////////////////////////////////////////////////////////// + +#include "IECoreScene/CurvesAlgo.h" + +#include "IECore/DataAlgo.h" +#include "IECore/Interpolator.h" +#include "IECore/TypeTraits.h" + +using namespace IECore; +using namespace IECoreScene; +using namespace Imath; +using namespace std; + +namespace +{ + +template +typename DataType::Ptr addPhantomPoints( const DataType *sourceData, const vector &verticesPerCurve ) +{ + const auto &source = sourceData->readable(); + typename DataType::Ptr resultData = new DataType; + auto &result = resultData->writable(); + result.reserve( source.size() + verticesPerCurve.size() * 2 ); + + using ElementType = typename DataType::ValueType::value_type; + LinearInterpolator interpolator; + ElementType phantomPoint; + auto sourceIt = source.begin(); + + for( int numVertices : verticesPerCurve ) + { + if constexpr( Interpolate && TypeTraits::IsInterpolable::value ) + { + interpolator( *(sourceIt+1), *sourceIt, 2, phantomPoint ); + result.push_back( phantomPoint ); + } + else + { + result.push_back( *sourceIt ); + } + + std::copy( sourceIt, sourceIt + numVertices, std::back_inserter( result ) ); + sourceIt += numVertices; + + if constexpr( Interpolate && TypeTraits::IsInterpolable::value ) + { + interpolator( *(sourceIt-2), *(sourceIt-1), 2, phantomPoint ); + result.push_back( phantomPoint ); + } + else + { + result.push_back( *(sourceIt-1) ); + } + } + + if constexpr( TypeTraits::IsGeometricTypedData::value ) + { + resultData->setInterpretation( sourceData->getInterpretation() ); + } + + return resultData; +} + +} // namespace + +bool CurvesAlgo::isPinned( const CurvesPrimitive *curves ) +{ + if( curves->wrap() != CurvesPrimitive::Wrap::Pinned ) + { + return false; + } + auto basis = curves->basis().standardBasis(); + return basis == StandardCubicBasis::BSpline || basis == StandardCubicBasis::CatmullRom; +} + +void CurvesAlgo::convertPinnedToNonPeriodic( CurvesPrimitive *curves, const IECore::Canceller *canceller ) +{ + if( curves->wrap() != CurvesPrimitive::Wrap::Pinned ) + { + return; + } + + const StandardCubicBasis standardBasis = curves->basis().standardBasis(); + if( standardBasis != StandardCubicBasis::BSpline && standardBasis != StandardCubicBasis::CatmullRom ) + { + IECore::Canceller::check( canceller ); + curves->setTopology( curves->verticesPerCurve(), curves->basis(), CurvesPrimitive::Wrap::NonPeriodic ); + return; + } + + for( auto &[name, primitiveVariable] : curves->variables ) + { + IECore::Canceller::check( canceller ); + if( primitiveVariable.interpolation == PrimitiveVariable::Vertex ) + { + if( primitiveVariable.indices ) + { + // It seems highly unlikely that we'll encounter an indexed + // curve. The only hypothetical use case I can recall + // considering was one where `P` was indexed to indicate sharing + // of vertices between connected curves. + // + // We have two choices : + // + // 1. Add additional _values_ with the true extrapolated + // position of the phantom points, and add indices to + // reference those. This gives the exact curve shape we'd get + // for a non-indexed curve, but at additional cost. + // 2. Add additional _indices_ that reference the existing + // vertices. This gives a subtly different curve shape, but + // is cheaper and preserves connectivity of shared vertices. + // + // For simplicity, and because we think this is hypothetical, we + // choose option 2 for now. + primitiveVariable.indices = addPhantomPoints( + primitiveVariable.indices.get(), curves->verticesPerCurve()->readable() + ); + } + else + { + dispatch( + primitiveVariable.data.get(), + /// \todo Capture can just be `[&]` when we get C++20. + [&, &primitiveVariable=primitiveVariable] ( auto data ) { + using DataType = typename std::remove_const_t>; + if constexpr( TypeTraits::IsVectorTypedData::value ) + { + primitiveVariable.data = addPhantomPoints( data, curves->verticesPerCurve()->readable() ); + } + + } + ); + } + } + } + + IECore::Canceller::check( canceller ); + IntVectorDataPtr newVerticesPerCurveData = new IntVectorData; + vector &newVerticesPerCurve = newVerticesPerCurveData->writable(); + newVerticesPerCurve.reserve( curves->numCurves() ); + for( auto numVertices : curves->verticesPerCurve()->readable() ) + { + newVerticesPerCurve.push_back( numVertices + 2 ); + } + + IECore::Canceller::check( canceller ); + curves->setTopology( newVerticesPerCurveData.get(), curves->basis(), CurvesPrimitive::Wrap::NonPeriodic ); +} diff --git a/src/IECoreScene/bindings/CurvesAlgoBinding.cpp b/src/IECoreScene/bindings/CurvesAlgoBinding.cpp index b7842d9edc..b252949492 100644 --- a/src/IECoreScene/bindings/CurvesAlgoBinding.cpp +++ b/src/IECoreScene/bindings/CurvesAlgoBinding.cpp @@ -77,6 +77,12 @@ boost::python::list segmentWrapper(const CurvesPrimitive *curves, const Primitiv return returnList; } +void convertPinnedToNonPeriodicWrapper( CurvesPrimitive &curves, const IECore::Canceller *canceller ) +{ + IECorePython::ScopedGILRelease gilRelease; + return CurvesAlgo::convertPinnedToNonPeriodic( &curves, canceller ); +} + CurvesPrimitivePtr updateEndpointMultiplicityWrapper( const CurvesPrimitive *curves, const IECore::CubicBasisf& cubicBasis, const IECore::Canceller *canceller ) { IECorePython::ScopedGILRelease gilRelease; @@ -100,6 +106,8 @@ void bindCurvesAlgo() def( "resamplePrimitiveVariable", &resamplePrimitiveVariableWrapper, ( arg_( "curvesPrimitive" ), arg_( "primitiveVariable" ), arg_( "interpolation" ), arg_( "canceller" ) = object() ) ); def( "deleteCurves", &deleteCurvesWrapper, ( arg_( "curvesPrimitive" ), arg_( "curvesToDelete" ), arg_( "invert" ) = false, arg_( "canceller" ) = object() ) ); def( "segment", ::segmentWrapper, segmentOverLoads()); + def( "isPinned", &CurvesAlgo::isPinned ); + def( "convertPinnedToNonPeriodic", &convertPinnedToNonPeriodicWrapper, ( arg_( "curves" ), arg_( "canceller" ) = object() ) ); def( "updateEndpointMultiplicity", &updateEndpointMultiplicityWrapper, ( arg_( "curves" ), arg_( "cubicBasis" ), arg_( "canceller" ) = object() ) ); } diff --git a/test/IECoreScene/CurvesAlgoTest.py b/test/IECoreScene/CurvesAlgoTest.py index 0d96f92a70..32985fe96e 100644 --- a/test/IECoreScene/CurvesAlgoTest.py +++ b/test/IECoreScene/CurvesAlgoTest.py @@ -1827,6 +1827,99 @@ def testDeletePeriodicWithVarying( self ) : ) ) +class CurvesAlgoConvertPinnedToNonPeriodicTest( unittest.TestCase ): + + def test( self ) : + + sourceCurves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 3, 3 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Pinned, + IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ), imath.V3f( 2 ), imath.V3f( 10 ), imath.V3f( 11 ), imath.V3f( 12 ) ] ) + ) + sourceCurves["varyingFloats"] = IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Varying, + IECore.FloatVectorData( [ 0, 1, 2, 10, 11, 12 ] ) + ) + sourceCurves["vertexStrings"] = IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Vertex, + IECore.StringVectorData( [ "0", "1", "2", "10", "11", "12" ] ) + ) + sourceCurves["indexedFloats"] = IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Vertex, + IECore.FloatVectorData( [ 10, 11 ] ), + IECore.IntVectorData( [ 0, 1, 0, 1, 0, 1 ] ) + ) + self.assertTrue( sourceCurves.arePrimitiveVariablesValid() ) + + convertedCurves = sourceCurves.copy() + IECoreScene.CurvesAlgo.convertPinnedToNonPeriodic( convertedCurves ) + self.assertEqual( convertedCurves.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + self.assertEqual( convertedCurves.verticesPerCurve(), IECore.IntVectorData( [ 5, 5 ] ) ) + self.assertTrue( convertedCurves.arePrimitiveVariablesValid() ) + self.assertEqual( + convertedCurves["P"], + IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Vertex, + IECore.V3fVectorData( + [ + imath.V3f( -1 ), imath.V3f( 0 ), imath.V3f( 1 ), imath.V3f( 2 ), imath.V3f( 3 ), + imath.V3f( 9 ), imath.V3f( 10 ), imath.V3f( 11 ), imath.V3f( 12 ), imath.V3f( 13 ), + ], + IECore.GeometricData.Interpretation.Point + ) + ) + ) + self.assertEqual( + convertedCurves["varyingFloats"], + sourceCurves["varyingFloats"] + ) + self.assertEqual( + convertedCurves["vertexStrings"], + IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Vertex, + IECore.StringVectorData( [ "0", "0", "1", "2", "2", "10", "10", "11", "12", "12" ] ), + ) + ) + self.assertEqual( + convertedCurves["indexedFloats"], + IECoreScene.PrimitiveVariable( + IECoreScene.PrimitiveVariable.Interpolation.Vertex, + IECore.FloatVectorData( [ 10, 11 ] ), + IECore.IntVectorData( [ 0, 0, 1, 0, 0, 1, 1, 0, 1, 1 ] ) + ) + ) + + def testSharedData( self ) : + + sourceCurves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 2 ] ), IECore.CubicBasisf.bSpline(), IECoreScene.CurvesPrimitive.Wrap.Pinned, + IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ) ] ) + ) + sourceCurves["PRef"] = sourceCurves["P"] + self.assertTrue( sourceCurves.arePrimitiveVariablesValid() ) + + convertedCurves = sourceCurves.copy() + IECoreScene.CurvesAlgo.convertPinnedToNonPeriodic( convertedCurves ) + self.assertTrue( convertedCurves.arePrimitiveVariablesValid() ) + + def testNonPinnableCurves( self ) : + + # Pinning shouldn't have any effect on linear or bezier curves + for basis in ( IECore.CubicBasisf.linear(), IECore.CubicBasisf.bezier() ) : + + with self.subTest( basis = basis.standardBasis() ) : + + sourceCurves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 4 ] ), basis, IECoreScene.CurvesPrimitive.Wrap.Pinned, + IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ), imath.V3f( 2 ) ] ) + ) + + convertedCurves = sourceCurves.copy() + IECoreScene.CurvesAlgo.convertPinnedToNonPeriodic( convertedCurves ) + self.assertEqual( convertedCurves.wrap(), IECoreScene.CurvesPrimitive.Wrap.NonPeriodic ) + # Nothing except the wrap should have changed. + convertedCurves.setTopology( convertedCurves.verticesPerCurve(), convertedCurves.basis(), IECoreScene.CurvesPrimitive.Wrap.Pinned ) + self.assertEqual( convertedCurves, sourceCurves ) + class CurvesAlgoUpdateEndpointMultiplicityTest( unittest.TestCase ): def createLinearCurves(self): From a544b0e6ec868353675c4f515e00ce8625b4468d Mon Sep 17 00:00:00 2001 From: John Haddon Date: Tue, 24 Mar 2026 11:13:12 +0000 Subject: [PATCH 11/15] CurvesPrimitiveEvalutor : Make `m_curvesPrimitive` const We have no need of modifying it. --- include/IECoreScene/CurvesPrimitiveEvaluator.h | 2 +- src/IECoreScene/CurvesPrimitiveEvaluator.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/IECoreScene/CurvesPrimitiveEvaluator.h b/include/IECoreScene/CurvesPrimitiveEvaluator.h index 0c309a5f5e..c0aa63a019 100644 --- a/include/IECoreScene/CurvesPrimitiveEvaluator.h +++ b/include/IECoreScene/CurvesPrimitiveEvaluator.h @@ -184,7 +184,7 @@ class IECORESCENE_API CurvesPrimitiveEvaluator : public PrimitiveEvaluator float integrateCurve( unsigned curveIndex, float vStart, float vEnd, int samples, Result& typedResult ) const; - CurvesPrimitivePtr m_curvesPrimitive; + ConstCurvesPrimitivePtr m_curvesPrimitive; const std::vector &m_verticesPerCurve; std::vector m_vertexDataOffsets; // one value per curve std::vector m_varyingDataOffsets; // one value per curve diff --git a/src/IECoreScene/CurvesPrimitiveEvaluator.cpp b/src/IECoreScene/CurvesPrimitiveEvaluator.cpp index b25e5672a9..ff63a705b2 100644 --- a/src/IECoreScene/CurvesPrimitiveEvaluator.cpp +++ b/src/IECoreScene/CurvesPrimitiveEvaluator.cpp @@ -376,7 +376,7 @@ CurvesPrimitiveEvaluator::CurvesPrimitiveEvaluator( ConstCurvesPrimitivePtr curv varyingDataOffset += m_curvesPrimitive->variableSize( PrimitiveVariable::Varying, i ); } - PrimitiveVariableMap::iterator pIt = m_curvesPrimitive->variables.find( "P" ); + auto pIt = m_curvesPrimitive->variables.find( "P" ); if( pIt==m_curvesPrimitive->variables.end() ) { throw InvalidArgumentException( "No PrimitiveVariable named P on CurvesPrimitive." ); From bd7516e588d29e9bc790ff0bb0d03bf852e9ac28 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Tue, 24 Mar 2026 11:24:27 +0000 Subject: [PATCH 12/15] CurvesPrimitiveEvaluator : Support pinned curves This is currently taking the cheap and cheerful approach of converting to NonPeriodic on construction, which does mean that the client has to be careful to use `CurvesPrimitiveEvaluator::primitive()` for retrieving primitive variables rather than the original primitive. An alternative here would be to throw if the curves are pinned, and require the client to do the conversion externally. That's currently what MeshPrimitiveEvaluator does for non-triangle meshes, so there is some precedent. --- .../IECoreScene/CurvesPrimitiveEvaluator.h | 4 ++++ src/IECoreScene/CurvesPrimitiveEvaluator.cpp | 21 ++++++++++++++++++- .../CurvesPrimitiveEvaluatorTest.py | 17 +++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/include/IECoreScene/CurvesPrimitiveEvaluator.h b/include/IECoreScene/CurvesPrimitiveEvaluator.h index c0aa63a019..36ee9d3134 100644 --- a/include/IECoreScene/CurvesPrimitiveEvaluator.h +++ b/include/IECoreScene/CurvesPrimitiveEvaluator.h @@ -118,6 +118,10 @@ class IECORESCENE_API CurvesPrimitiveEvaluator : public PrimitiveEvaluator }; IE_CORE_DECLAREPTR( Result ); + /// > Note : If `curves` are Pinned, then they are converted to NonPeriodic + /// > internally. PrimitiveVariable queries must therefore be made using + /// > `CurvesPrimitiveEvaluator::primitive()` rather than the original `curves` + /// > object. CurvesPrimitiveEvaluator( ConstCurvesPrimitivePtr curves ); ~CurvesPrimitiveEvaluator() override; diff --git a/src/IECoreScene/CurvesPrimitiveEvaluator.cpp b/src/IECoreScene/CurvesPrimitiveEvaluator.cpp index ff63a705b2..17168ccf55 100644 --- a/src/IECoreScene/CurvesPrimitiveEvaluator.cpp +++ b/src/IECoreScene/CurvesPrimitiveEvaluator.cpp @@ -34,6 +34,7 @@ #include "IECoreScene/CurvesPrimitiveEvaluator.h" +#include "IECoreScene/CurvesAlgo.h" #include "IECoreScene/CurvesPrimitive.h" #include "IECore/Exception.h" @@ -49,6 +50,24 @@ using namespace IECoreScene; using namespace Imath; using namespace std; +namespace +{ + +/// \todo Deal with pinned curves directly in CurvesPrimitiveEvaluator, so we +/// don't generate any additional data. `IECoreGL::CurvesPrimitive` demonstrates +/// one approach. +ConstCurvesPrimitivePtr conformedCurves( const ConstCurvesPrimitivePtr &curves ) +{ + CurvesPrimitivePtr result = curves->copy(); + if( CurvesAlgo::isPinned( result.get() ) ) + { + CurvesAlgo::convertPinnedToNonPeriodic( result.get() ); + } + return result; +} + +} // namespace + IE_CORE_DEFINERUNTIMETYPED( CurvesPrimitiveEvaluator ); PrimitiveEvaluator::Description CurvesPrimitiveEvaluator::g_evaluatorDescription; @@ -361,7 +380,7 @@ struct CurvesPrimitiveEvaluator::Line ////////////////////////////////////////////////////////////////////////// CurvesPrimitiveEvaluator::CurvesPrimitiveEvaluator( ConstCurvesPrimitivePtr curves ) - : m_curvesPrimitive( curves->copy() ), m_verticesPerCurve( m_curvesPrimitive->verticesPerCurve()->readable() ), m_haveTree( false ) + : m_curvesPrimitive( conformedCurves( curves ) ), m_verticesPerCurve( m_curvesPrimitive->verticesPerCurve()->readable() ), m_haveTree( false ) { m_vertexDataOffsets.reserve( m_verticesPerCurve.size() ); m_varyingDataOffsets.reserve( m_verticesPerCurve.size() ); diff --git a/test/IECoreScene/CurvesPrimitiveEvaluatorTest.py b/test/IECoreScene/CurvesPrimitiveEvaluatorTest.py index ed422b218a..91c2cd021d 100644 --- a/test/IECoreScene/CurvesPrimitiveEvaluatorTest.py +++ b/test/IECoreScene/CurvesPrimitiveEvaluatorTest.py @@ -1225,6 +1225,23 @@ def testParallelClosestPoint( self ) : IECoreScene.testCurvesPrimitiveEvaluatorParallelClosestPoint() + def testPinnedCurves( self ) : + + curves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 5 ] ), IECore.CubicBasisf.bSpline(), + IECoreScene.CurvesPrimitive.Wrap.Pinned, IECore.V3fVectorData( [ imath.V3f( x ) for x in range( 0, 5 ) ] ) + ) + evaluator = IECoreScene.CurvesPrimitiveEvaluator( curves ) + result = evaluator.createResult() + + evaluator.pointAtV( 0, 0.0, result ) + self.assertEqual( result.point(), imath.V3f( 0 ) ) + self.assertEqual( result.vectorPrimVar( evaluator.primitive()["P"] ), imath.V3f( 0 ) ) + + evaluator.pointAtV( 0, 1.0, result ) + self.assertEqual( result.point(), imath.V3f( 4 ) ) + self.assertEqual( result.vectorPrimVar( evaluator.primitive()["P"] ), imath.V3f( 4 ) ) + if __name__ == "__main__": unittest.main() From c97e337b6009b8fab1206ff8a74dd439b756f932 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Tue, 24 Mar 2026 11:38:10 +0000 Subject: [PATCH 13/15] CurvesAlgo : Throw for pinned curves in `updateEndpointMultiplicity()` --- src/IECoreScene/CurvesAlgoUpdateEndpointMultiplicity.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/IECoreScene/CurvesAlgoUpdateEndpointMultiplicity.cpp b/src/IECoreScene/CurvesAlgoUpdateEndpointMultiplicity.cpp index 61d0078710..75f7a4037e 100644 --- a/src/IECoreScene/CurvesAlgoUpdateEndpointMultiplicity.cpp +++ b/src/IECoreScene/CurvesAlgoUpdateEndpointMultiplicity.cpp @@ -156,6 +156,13 @@ struct DuplicateEndPoints CurvesPrimitivePtr IECoreScene::CurvesAlgo::updateEndpointMultiplicity( const IECoreScene::CurvesPrimitive *curves, const IECore::CubicBasisf &cubicBasis, const Canceller *canceller ) { + if( CurvesAlgo::isPinned( curves ) ) + { + // Pinned curves provide a superior alternative to manually managing endpoint + // multiplicity, so it doesn't make sense to combine the two. + throw IECore::InvalidArgumentException( "Pinned curves not supported" ); + } + CurvesPrimitivePtr newCurves = new IECoreScene::CurvesPrimitive(); int vertexAdjustment = requiredMultiplicity( cubicBasis ) - requiredMultiplicity( curves->basis() ); From 4aa0331f08ef225a9ce5b70815ca5479a5dc5aeb Mon Sep 17 00:00:00 2001 From: John Haddon Date: Tue, 24 Mar 2026 11:56:27 +0000 Subject: [PATCH 14/15] CurvesAlgo : Maintain wrap in `deleteCurves()` --- src/IECoreScene/CurvesAlgo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IECoreScene/CurvesAlgo.cpp b/src/IECoreScene/CurvesAlgo.cpp index 27bb09beab..a9f8d75521 100644 --- a/src/IECoreScene/CurvesAlgo.cpp +++ b/src/IECoreScene/CurvesAlgo.cpp @@ -402,7 +402,7 @@ CurvesPrimitivePtr deleteCurves( IntVectorDataPtr verticesPerCurve = IECore::runTimeCast( outputVertsPerCurve.data ); - CurvesPrimitivePtr outCurvesPrimitive = new CurvesPrimitive( verticesPerCurve, curvesPrimitive->basis(), curvesPrimitive->periodic() ); + CurvesPrimitivePtr outCurvesPrimitive = new CurvesPrimitive( verticesPerCurve.get(), curvesPrimitive->basis(), curvesPrimitive->wrap() ); for (PrimitiveVariableMap::const_iterator it = curvesPrimitive->variables.begin(), e = curvesPrimitive->variables.end(); it != e; ++it) { From a07334f98c1a1b34c106514b25bcf036c3df50ca Mon Sep 17 00:00:00 2001 From: John Haddon Date: Tue, 24 Mar 2026 12:40:40 +0000 Subject: [PATCH 15/15] CurvesAlgo : Support pinned curves in `resamplePrimitiveVariable()` These are nice and simple, because Varying and Vertex have the same variable size. This also revealed a bug in our handling of linear curves, where the same applies. --- Changes | 1 + src/IECoreScene/CurvesAlgo.cpp | 14 +++++++--- test/IECoreScene/CurvesAlgoTest.py | 41 +++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/Changes b/Changes index 7ebf6f96b3..9dc380817d 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,7 @@ Fixes ----- - CurvesAlgo : Fixed handling of periodic curves in `deleteCurves()`. +- CurvesAlgo : Fixed `resamplePrimitiveVariable()` to handle Vertex<->Varying conversion for linear curves correctly. Breaking Changes ---------------- diff --git a/src/IECoreScene/CurvesAlgo.cpp b/src/IECoreScene/CurvesAlgo.cpp index a9f8d75521..6484749832 100644 --- a/src/IECoreScene/CurvesAlgo.cpp +++ b/src/IECoreScene/CurvesAlgo.cpp @@ -469,6 +469,16 @@ void resamplePrimitiveVariable( const CurvesPrimitive *curves, PrimitiveVariable return; } + if( curves->variableSize( primitiveVariable.interpolation ) == curves->variableSize( interpolation ) ) + { + // Various topologies have variable sizes that are compatible. Varying + // and FaceVarying are always identical. For linear curves and pinned + // cubic curves, they are also the same as Vertex. In these cases + // there is no need to resample at all. + primitiveVariable.interpolation = interpolation; + return; + } + DataPtr dstData = nullptr; DataPtr srcData = nullptr; @@ -559,10 +569,6 @@ void resamplePrimitiveVariable( const CurvesPrimitive *curves, PrimitiveVariable CurvesVertexToVarying fn( curves, canceller ); dstData = despatchTypedData( const_cast< Data * >( srcData.get() ), fn ); } - else if ( primitiveVariable.interpolation == PrimitiveVariable::Varying || primitiveVariable.interpolation == PrimitiveVariable::FaceVarying ) - { - dstData = srcData; - } } if( primitiveVariable.indices ) diff --git a/test/IECoreScene/CurvesAlgoTest.py b/test/IECoreScene/CurvesAlgoTest.py index 32985fe96e..b7e31dedf4 100644 --- a/test/IECoreScene/CurvesAlgoTest.py +++ b/test/IECoreScene/CurvesAlgoTest.py @@ -915,7 +915,7 @@ def testLinearCurvesVaryingToVertex( self ) : IECoreScene.CurvesAlgo.resamplePrimitiveVariable(curves, p, IECoreScene.PrimitiveVariable.Interpolation.Vertex) self.assertEqual( p.interpolation, IECoreScene.PrimitiveVariable.Interpolation.Vertex ) - self.assertEqual( p.data, IECore.FloatVectorData( [ 0, 0.5, 2, 2.5 ] ) ) + self.assertEqual( p.data, IECore.FloatVectorData( [ 0, 1, 2, 3 ] ) ) def testLinearCurvesVaryingToUniform( self ) : curves = self.curvesLinear() @@ -945,9 +945,8 @@ def testLinearCurvesFaceVaryingToVertex( self ) : curves = self.curvesLinear() p = curves["e"] IECoreScene.CurvesAlgo.resamplePrimitiveVariable(curves, p, IECoreScene.PrimitiveVariable.Interpolation.Vertex) - self.assertEqual( p.interpolation, IECoreScene.PrimitiveVariable.Interpolation.Vertex ) - self.assertEqual( p.data, IECore.FloatVectorData( [ 0, 0.5, 2, 2.5 ] ) ) + self.assertEqual( p.data, IECore.FloatVectorData( [ 0, 1, 2, 3 ] ) ) def testLinearCurvesFaceVaryingToUniform( self ) : curves = self.curvesLinear() @@ -1015,6 +1014,42 @@ def testLinearCurvesPeriodic( self ) : self.assertTrue( curves.isPrimitiveVariableValid( p ) ) self.assertEqual( p.data, origP.data ) + def testPinnedVertexToVarying( self ) : + + curves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 2 ] ), IECore.CubicBasisf.bSpline(), + IECoreScene.CurvesPrimitive.Wrap.Pinned, + IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ) ] ) + ) + + primVar = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Vertex, IECore.FloatVectorData( [ 0, 1 ] ) ) + self.assertTrue( curves.isPrimitiveVariableValid( primVar ) ) + + originalData = primVar.data + IECoreScene.CurvesAlgo.resamplePrimitiveVariable( curves, primVar, IECoreScene.PrimitiveVariable.Interpolation.Varying ) + self.assertEqual( primVar.interpolation, IECoreScene.PrimitiveVariable.Interpolation.Varying ) + self.assertEqual( primVar.data, IECore.FloatVectorData( [ 0, 1 ] ) ) + self.assertTrue( primVar.data.isSame( originalData ) ) + self.assertTrue( curves.isPrimitiveVariableValid( primVar ) ) + + def testPinnedVaryingToVertex( self ) : + + curves = IECoreScene.CurvesPrimitive( + IECore.IntVectorData( [ 2 ] ), IECore.CubicBasisf.bSpline(), + IECoreScene.CurvesPrimitive.Wrap.Pinned, + IECore.V3fVectorData( [ imath.V3f( 0 ), imath.V3f( 1 ) ] ) + ) + + primVar = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Varying, IECore.FloatVectorData( [ 0, 1 ] ) ) + self.assertTrue( curves.isPrimitiveVariableValid( primVar ) ) + + originalData = primVar.data + IECoreScene.CurvesAlgo.resamplePrimitiveVariable( curves, primVar, IECoreScene.PrimitiveVariable.Interpolation.Vertex ) + self.assertEqual( primVar.interpolation, IECoreScene.PrimitiveVariable.Interpolation.Vertex ) + self.assertEqual( primVar.data, IECore.FloatVectorData( [ 0, 1 ] ) ) + self.assertTrue( primVar.data.isSame( originalData ) ) + self.assertTrue( curves.isPrimitiveVariableValid( primVar ) ) + def testCanSegmentUsingIntegerPrimvar( self ) : curves = self.curvesLinear()