Merge branch 'RB-10.4' into main

Author: johnhaddon

Changes

@@ -6,6 +6,31 @@ Breaking Changes
 
 - Primitive : Changed `variableIndexedView()` return type from `boost::optional` to `std::optional`.
 
+10.4.x.x (relative to 10.4.7.0)
+========
+
+
+10.4.7.0 (relative to 10.4.6.0)
+========
+
+Improvements
+------------
+
+- IECoreGL::PointsPrimitive : Added `renderUsesGLPoints()` method (#1347).
+- IECoreGL::CurvesPrimitive : Added `renderUsesGLLines()` method (#1347).
+- StringAlgo : Added `concat()` and `toInt()` methods (#1343).
+- IECoreScene::ShaderNetworkAlgo (#1343) :
+  - Added `convertToOSLConventions()`, `expandSplines()` and `collapseSplines()`.
+  - Deprecated `convertOSLComponentConnections()`, `expandSplineParameters()` and `collapseSplineParameters()`.
+- USDScene : Added support for roundtripping connections to SplineData parameters.
+
+Fixes
+-----
+
+- IECoreUSD::PrimitiveAlgo (#1346) :
+  - Fixed reading of primitives containing `primvars:normals`. These are now correctly loaded as a primitive variable called `N`, taking precedence over the UsdGeomPointBased `normals` attribute.
+  - Fixed writing of indexed normals so that the indexing is retained on load. Note that this means that normals are now _always_ written as `primvars:normals` and never via the UsdGeomPointBased `normals` attribute.
+
 10.4.6.0 (relative to 10.4.5.0)
 ========
 

SConstruct

@@ -56,7 +56,7 @@ SConsignFile()
 
 ieCoreMilestoneVersion = 10 # for announcing major milestones - may contain all of the below
 ieCoreMajorVersion = 4 # backwards-incompatible changes
-ieCoreMinorVersion = 6 # new backwards-compatible features
+ieCoreMinorVersion = 7 # new backwards-compatible features
 ieCorePatchVersion = 0 # bug fixes
 ieCoreVersionSuffix = "" # used for alpha/beta releases. Example: "a1", "b2", etc.
 

contrib/IECoreAppleseed/src/IECoreAppleseed/ShaderNetworkAlgo.cpp

@@ -65,13 +65,9 @@ namespace ShaderNetworkAlgo
 
 renderer::ShaderGroup *convert( const IECoreScene::ShaderNetwork *shaderNetwork )
 {
-	ShaderNetworkPtr networkCopy;
-	if( true ) // todo : make conditional on OSL < 1.10
-	{
-		networkCopy = shaderNetwork->copy();
-		IECoreScene::ShaderNetworkAlgo::convertOSLComponentConnections( networkCopy.get() );
-		shaderNetwork = networkCopy.get();
-	}
+	ShaderNetworkPtr networkCopy = shaderNetwork->copy();
+	IECoreScene::ShaderNetworkAlgo::convertToOSLConventions( networkCopy.get(), 10900 );
+	shaderNetwork = networkCopy.get();
 
 	asf::auto_release_ptr<asr::ShaderGroup> shaderGroup;
 	shaderGroup = asr::ShaderGroupFactory::create( "shader_group" );
@@ -89,11 +85,7 @@ renderer::ShaderGroup *convert( const IECoreScene::ShaderNetwork *shaderNetwork
 				shaderType += 4;
 			}
 
-			IECore::ConstCompoundDataPtr expandedParameters = IECoreScene::ShaderNetworkAlgo::expandSplineParameters(
-				shader->parametersData()
-			);
-
-			asr::ParamArray params( ParameterAlgo::convertShaderParameters( expandedParameters->readable() ) );
+			asr::ParamArray params( ParameterAlgo::convertShaderParameters( shader->parametersData()->readable() ) );
 			shaderGroup->add_shader( shaderType, shader->getName().c_str(), handle.c_str(), params );
 
 			for( const auto &c : shaderNetwork->inputConnections( handle ) )

contrib/IECoreAppleseed/test/IECoreAppleseed/ShaderNetworkAlgoTest.py

@@ -48,7 +48,7 @@ def testSplines( self ) :
 
 		n = IECoreScene.ShaderNetwork(
 			shaders = {
-				"test" : IECoreScene.Shader( "test", "test",
+				"test" : IECoreScene.Shader( "test", "osl:shader",
 					IECore.CompoundData(
 						{
 							"testColorSpline" : IECore.SplinefColor3fData( IECore.SplinefColor3f( IECore.CubicBasisf.linear(), ( ( 0, imath.Color3f(1) ), ( 10, imath.Color3f(2) ), ( 20, imath.Color3f(0) ) ) ) ),

contrib/IECoreUSD/src/IECoreUSD/PrimitiveAlgo.cpp

@@ -97,6 +97,11 @@ void IECoreUSD::PrimitiveAlgo::writePrimitiveVariable( const std::string &name,
 		writePrimitiveVariable( "st", primitiveVariable, primvarsAPI, time );
 		return;
 	}
+	else if( name == "N" && runTimeCast<const V3fVectorData>( primitiveVariable.data ) )
+	{
+		writePrimitiveVariable( "normals", primitiveVariable, primvarsAPI, time );
+		return;
+	}
 
 	const pxr::SdfValueTypeName valueTypeName = DataAlgo::valueTypeName( primitiveVariable.data.get() );
 	pxr::UsdGeomPrimvar usdPrimVar = primvarsAPI.CreatePrimvar( pxr::TfToken( name ), valueTypeName );
@@ -121,11 +126,6 @@ void IECoreUSD::PrimitiveAlgo::writePrimitiveVariable( const std::string &name,
 	{
 		pointBased.CreatePointsAttr().Set( PrimitiveAlgo::toUSDExpanded( value ), time );
 	}
-	else if( name == "N" )
-	{
-		pointBased.CreateNormalsAttr().Set( PrimitiveAlgo::toUSDExpanded( value ), time );
-		pointBased.SetNormalsInterpolation( PrimitiveAlgo::toUSD( value.interpolation ) );
-	}
 	else if( name == "velocity" )
 	{
 		pointBased.CreateVelocitiesAttr().Set( PrimitiveAlgo::toUSDExpanded( value ), time );
@@ -469,6 +469,23 @@ void IECoreUSD::PrimitiveAlgo::readPrimitiveVariables( const pxr::UsdGeomPrimvar
 			primitive->variables.erase( it );
 		}
 	}
+
+	// USD uses "normals" for normals and we use "N".
+
+	it = primitive->variables.find( "normals" );
+	if( it != primitive->variables.end() )
+	{
+		if( auto d = runTimeCast<V3fVectorData>( it->second.data ) )
+		{
+			// Force the interpretation, since some USD files
+			// use `vector3f` rather than `normal3f`. I'm looking
+			// at you, `arnold-usd`.
+			d->setInterpretation( GeometricData::Normal );
+			primitive->variables["N"] = it->second;
+			primitive->variables.erase( it );
+		}
+	}
+
 }
 
 void IECoreUSD::PrimitiveAlgo::readPrimitiveVariables( const pxr::UsdGeomPointBased &pointBased, pxr::UsdTimeCode time, IECoreScene::Primitive *primitive, const Canceller *canceller )
@@ -485,9 +502,14 @@ void IECoreUSD::PrimitiveAlgo::readPrimitiveVariables( const pxr::UsdGeomPointBa
 		}
 
 		Canceller::check( canceller );
-		if( auto n = boost::static_pointer_cast<V3fVectorData>( DataAlgo::fromUSD( pointBased.GetNormalsAttr(), time ) ) )
+		if( !primitive->variables.count( "N" ) )
 		{
-			primitive->variables["N"] = IECoreScene::PrimitiveVariable( PrimitiveAlgo::fromUSD( pointBased.GetNormalsInterpolation() ), n );
+			// Only load `PointBased::GetNormalsAttr()` if we didn't already load `primvars:normals`.
+			// From the USD API docs : "If normals and primvars:normals are both specified, the latter has precedence."
+			if( auto n = boost::static_pointer_cast<V3fVectorData>( DataAlgo::fromUSD( pointBased.GetNormalsAttr(), time ) ) )
+			{
+				primitive->variables["N"] = IECoreScene::PrimitiveVariable( PrimitiveAlgo::fromUSD( pointBased.GetNormalsInterpolation() ), n );
+			}
 		}
 	}
 

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -187,8 +187,6 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 
 	readAdditionalLightParameters( usdShader.GetPrim(), parameters );
 
-	parametersData = boost::const_pointer_cast< IECore::CompoundData >( IECoreScene::ShaderNetworkAlgo::collapseSplineParameters( parametersData ) );
-
 	IECoreScene::ShaderPtr newShader = new IECoreScene::Shader( shaderName, shaderType, parametersData );
 	pxr::VtValue metadataValue;
 	if( usdShader.GetPrim().GetMetadata( g_adapterLabelToken, &metadataValue ) && metadataValue.Get<bool>() )
@@ -224,6 +222,7 @@ IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath
 pxr::UsdShadeOutput IECoreUSD::ShaderAlgo::writeShaderNetwork( const IECoreScene::ShaderNetwork *shaderNetwork, pxr::UsdPrim shaderContainer )
 {
 	IECoreScene::ShaderNetworkPtr shaderNetworkWithAdapters = shaderNetwork->copy();
+	IECoreScene::ShaderNetworkAlgo::expandSplines( shaderNetworkWithAdapters.get() );
 	IECoreScene::ShaderNetworkAlgo::addComponentConnectionAdapters( shaderNetworkWithAdapters.get() );
 
 	IECoreScene::ShaderNetwork::Parameter networkOutput = shaderNetworkWithAdapters->getOutput();
@@ -258,11 +257,7 @@ pxr::UsdShadeOutput IECoreUSD::ShaderAlgo::writeShaderNetwork( const IECoreScene
 		}
 		usdShader.SetShaderId( pxr::TfToken( typePrefix + shader.second->getName() ) );
 
-
-		IECore::ConstCompoundDataPtr expandedParameters = IECoreScene::ShaderNetworkAlgo::expandSplineParameters(
-			shader.second->parametersData()
-		);
-		for( const auto &p : expandedParameters->readable() )
+		for( const auto &p : shader.second->parametersData()->readable() )
 		{
 			pxr::UsdShadeInput input = usdShader.CreateInput(
 				toUSDParameterName( p.first ),
@@ -355,6 +350,7 @@ IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const px
 	result->setOutput( outputHandle );
 
 	IECoreScene::ShaderNetworkAlgo::removeComponentConnectionAdapters( result.get() );
+	IECoreScene::ShaderNetworkAlgo::collapseSplines( result.get() );
 
 	return result;
 }

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -1370,7 +1370,9 @@ def testPointBasedPrimvars( self ) :
 
 		# Make sure we redirect Cortex primitive variables to the correct
 		# attributes of UsdGeomPointBased instead of writing them to
-		# arbitrary primvars.
+		# arbitrary primvars. We don't use the `normals` attribute though,
+		# since USD documents `primvars:normals` to take precedence, and
+		# it is the only way we can preserve indexed normals.
 
 		stage = pxr.Usd.Stage.Open( fileName )
 		primvarsAPI = pxr.UsdGeom.PrimvarsAPI( stage.GetPrimAtPath( "/test" ) )
@@ -1382,7 +1384,8 @@ def testPointBasedPrimvars( self ) :
 
 		usdMesh = pxr.UsdGeom.Mesh( stage.GetPrimAtPath( "/test" ) )
 		self.assertTrue( usdMesh.GetPointsAttr().HasAuthoredValue() )
-		self.assertTrue( usdMesh.GetNormalsAttr().HasAuthoredValue() )
+		self.assertFalse( usdMesh.GetNormalsAttr().HasAuthoredValue() )
+		self.assertTrue( primvarsAPI.GetPrimvar( "normals" ) )
 		self.assertTrue( usdMesh.GetVelocitiesAttr().HasAuthoredValue() )
 		if pxr.Usd.GetVersion() >= ( 0, 19, 11 ) :
 			self.assertTrue( usdMesh.GetAccelerationsAttr().HasAuthoredValue() )
@@ -2632,6 +2635,12 @@ def testShaders( self ) :
 		dest.parameters["a"] = IECore.Color3fData( imath.Color3f( 0.0 ) )
 		dest.parameters["b"] = IECore.Color3fData( imath.Color3f( 0.0 ) )
 		dest.parameters["c"] = IECore.FloatData( 0.0 )
+		dest.parameters["sf"] = IECore.SplineffData( IECore.Splineff( IECore.CubicBasisf.catmullRom(),
+			( ( 0, 1 ), ( 10, 2 ), ( 20, 0 ), ( 30, 1 ) )
+		) )
+		dest.parameters["sc"] = IECore.SplinefColor3fData( IECore.SplinefColor3f( IECore.CubicBasisf.linear(),
+			( ( 0, imath.Color3f(1) ), ( 10, imath.Color3f(2) ), ( 20, imath.Color3f(0) ) )
+		) )
 
 		componentConnectionNetwork = IECoreScene.ShaderNetwork()
 		componentConnectionNetwork.addShader( "source1", add1 )
@@ -2664,6 +2673,24 @@ def testShaders( self ) :
 		) )
 		componentConnectionNetwork.setOutput( IECoreScene.ShaderNetwork.Parameter( "dest", "" ) )
 
+		# Float to spline element connection
+		componentConnectionNetwork.addConnection( IECoreScene.ShaderNetwork.Connection(
+			IECoreScene.ShaderNetwork.Parameter( "source1", "out" ),
+			IECoreScene.ShaderNetwork.Parameter( "dest", "sf[3].y" )
+		) )
+
+		# Color to spline element connection
+		componentConnectionNetwork.addConnection( IECoreScene.ShaderNetwork.Connection(
+			IECoreScene.ShaderNetwork.Parameter( "source3", "out" ),
+			IECoreScene.ShaderNetwork.Parameter( "dest", "sc[2].y" )
+		) )
+
+		# Float to spline element component connection
+		componentConnectionNetwork.addConnection( IECoreScene.ShaderNetwork.Connection(
+			IECoreScene.ShaderNetwork.Parameter( "source1", "out" ),
+			IECoreScene.ShaderNetwork.Parameter( "dest", "sc[0].y.g" )
+		) )
+
 		# If we manually create the shaders that are used as adapters for component connections,
 		# they should not be automatically removed on import.  ( This is implemented using
 		# a label for automatically created adapters, stored as blindData in Cortex that is
@@ -3333,5 +3360,51 @@ def testInvalidPrimitiveVariables( self ) :
 		del goodCube["uv"]
 		self.assertEqual( goodCube, badCube )
 
+	def testNormalsPrimVar( self ) :
+
+		root = IECoreScene.SceneInterface.create( os.path.dirname( __file__ ) + "/data/normalsPrimVar.usda", IECore.IndexedIO.OpenMode.Read )
+		mesh = root.child( "mesh" ).readObject( 0 )
+
+		self.assertNotIn( "normals", mesh )
+		self.assertIn( "N", mesh )
+		self.assertEqual( mesh["N"].interpolation, IECoreScene.PrimitiveVariable.Interpolation.FaceVarying )
+		self.assertIsNotNone( mesh["N"].indices )
+
+	def testNormalsPrimVarBeatsNormalsAttribute( self ) :
+
+		root = IECoreScene.SceneInterface.create( os.path.dirname( __file__ ) + "/data/normalsAttributeAndPrimVar.usda", IECore.IndexedIO.OpenMode.Read )
+		mesh = root.child( "mesh" ).readObject( 0 )
+
+		self.assertNotIn( "normals", mesh )
+		self.assertIn( "N", mesh )
+		self.assertEqual( mesh["N"].data, IECore.V3fVectorData( [ imath.V3f( 0, 0, 1 ) ] * 3, IECore.GeometricData.Interpretation.Normal ) )
+		self.assertEqual( mesh["N"].interpolation, IECoreScene.PrimitiveVariable.Interpolation.FaceVarying )
+		self.assertIsNone( mesh["N"].indices )
+
+	def testRoundTripIndexedNormals( self ) :
+
+		mesh = IECoreScene.MeshPrimitive(
+			IECore.IntVectorData( [ 3, 3 ] ),
+			IECore.IntVectorData( [ 0, 2, 1, 2, 3, 1 ] ),
+			"linear",
+			IECore.V3fVectorData( [
+				imath.V3f( 0, 1, 0 ), imath.V3f( 1, 1, 0 ), imath.V3f( 0, 0, 0 ), imath.V3f( 1, 0, 0 )
+			] )
+		)
+
+		mesh["N"] = IECoreScene.PrimitiveVariable(
+			IECoreScene.PrimitiveVariable.Interpolation.FaceVarying,
+			IECore.V3fVectorData( [ imath.V3f( 0, 0, 1 ) ], IECore.GeometricData.Interpretation.Normal ),
+			IECore.IntVectorData( [ 0, 0, 0, 0, 0, 0 ] )
+		)
+
+		fileName = os.path.join( self.temporaryDirectory(), "indexedNormals.usda" )
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+		root.createChild( "mesh" ).writeObject( mesh, 0.0 )
+		del root
+
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+		self.assertEqual( root.child( "mesh").readObject( 0.0 ), mesh )
+
 if __name__ == "__main__":
 	unittest.main()

contrib/IECoreUSD/test/IECoreUSD/data/normalsAttributeAndPrimVar.usda

@@ -0,0 +1,16 @@
+#usda 1.0
+
+def Mesh "mesh"
+{
+    int[] faceVertexCounts = [3]
+    int[] faceVertexIndices = [0, 1, 3]
+    point3f[] points = [(0, 0, 0), (0, 1, 0), (1, 0, 0)]
+    normal3f[] primvars:normals = [(0, 0, 1), (0, 0, 1), (0, 0, 1)] (
+        elementSize = 3
+        interpolation = "faceVarying"
+    )
+    normal3f[] normals = [(0, 0, -1), (0, 0, -1), (0, 0, -1)] (
+        interpolation = "vertex"
+    )
+    uniform token subdivisionScheme = "none"
+}