Merge pull request #1526 from johnhaddon/curvePinning

CurvesPrimitive : Add WrapMode enum

Author: johnhaddon

Changes

@@ -1,7 +1,25 @@
 10.7.x.x (relative to 10.7.0.0a7)
 ========
 
+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.
+- CurvesAlgo : Added `isPinned()` and `convertPinnedToNonPeriodic()` utilities.
+
+Fixes
+-----
+
+- CurvesAlgo : Fixed handling of periodic curves in `deleteCurves()`.
+- CurvesAlgo : Fixed `resamplePrimitiveVariable()` to handle Vertex<->Varying conversion for linear curves correctly.
+
+Breaking Changes
+----------------
 
+- CurvesPrimitive : Made protected members private.
 
 10.7.0.0a7 (relative to 10.7.0.0a6)
 ==========

contrib/IECoreUSD/src/IECoreUSD/CurvesAlgo.cpp

@@ -56,15 +56,15 @@ 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;
 	curves.GetCurveVertexCountsAttr().Get( &vertexCountsArray, 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() )

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],

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;
 
@@ -89,10 +93,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;

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;

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<CurvesPrimitivePtr> 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

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,27 +86,25 @@ 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.
 		static Ptr createBox( const Imath::Box3f &b );
 
 		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;
-		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;
 
-	private :
-
 		static const unsigned int m_ioVersion;
 
 };

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;
 
@@ -184,7 +188,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<int> &m_verticesPerCurve;
 		std::vector<int> m_vertexDataOffsets; // one value per curve
 		std::vector<int> m_varyingDataOffsets; // one value per curve

include/IECoreScene/private/PrimitiveVariableAlgos.h

@@ -291,8 +291,7 @@ class DeleteFlaggedVaryingFunctor : public DeleteFlagged<U>
 			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 )