Merge pull request #1420 from johnhaddon/usdFixes

USD fixes
ImageEngine · Jun 5, 2024 · a5e12c9 · a5e12c9
2 parents 9668070 + 1d2bfd6
commit a5e12c9
Show file tree

Hide file tree

Showing 7 changed files with 150 additions and 50 deletions.
diff --git a/Changes b/Changes
@@ -1,11 +1,20 @@
 10.5.x.x (relative to 10.5.7.1)
 ========
 
+Improvements
+------------
+
+- IECoreUSD::DataAlgo : Added binding for `toUSD()` function.
+
 Fixes
 -----
 
 - ShaderNetworkAlgo : Fixed crash caused by cyclic connections in `removeUnusedShaders()`.
 - ShaderStateComponent : Fixed GL rendering failures caused by unsupported values for texture parameters.
+- USDScene :
+  - Fixed exceptions caused by attempt to write shader parameters with unsupported value types.
+  - Fixed duplicate loading of `arnold:*` primvars on lights as attributes. These are now only loaded as parameters on the light shader.
+- IECoreUSD::DataAlgo : Fixed exceptions thrown by `toUSD()` and `valueTypeName()` when passed datatypes not supported by `dispatch()`. An empty VtValue or SdfValueTypeName is now returned instead.
 
 Build
 -----

diff --git a/contrib/IECoreUSD/src/IECoreUSD/AttributeAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/AttributeAlgo.cpp
@@ -40,6 +40,8 @@
 IECORE_PUSH_DEFAULT_VISIBILITY
 #include "pxr/usd/usdGeom/primvar.h"
 #include "pxr/usd/usdGeom/primvarsAPI.h"
+
+#include "pxr/usd/usdLux/lightAPI.h"
 IECORE_POP_DEFAULT_VISIBILITY
 
 using namespace IECoreUSD;
@@ -92,6 +94,17 @@ bool IECoreUSD::AttributeAlgo::isCortexAttribute( const pxr::UsdGeomPrimvar &pri
 		}
 	}
 
+	// Check for `arnold:*` primvars on lights. These will be loaded as
+	// parameters in `ShaderAlgo::readLight()`.
+
+	if(
+		boost::starts_with( primVar.GetPrimvarName().GetString(), "arnold:" ) &&
+		pxr::UsdLuxLightAPI( primVar.GetAttr().GetPrim() )
+	)
+	{
+		return false;
+	}
+
 	// Everything else should be loaded as a Cortex attribute.
 
 	return true;

diff --git a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp
@@ -400,7 +400,15 @@ struct VtValueFromData
 
 pxr::VtValue IECoreUSD::DataAlgo::toUSD( const IECore::Data *data, bool arrayRequired )
 {
-	return IECore::dispatch( data, VtValueFromData(), arrayRequired );
+	try
+	{
+		return IECore::dispatch( data, VtValueFromData(), arrayRequired );
+	}
+	catch( const IECore::Exception & )
+	{
+		// Type not supported by `dispatch()`.
+		return VtValue();
+	}
 }
 
 pxr::TfToken IECoreUSD::DataAlgo::role( GeometricData::Interpretation interpretation )
@@ -503,5 +511,13 @@ struct VtValueTypeNameFromData
 
 pxr::SdfValueTypeName IECoreUSD::DataAlgo::valueTypeName( const IECore::Data *data )
 {
-	return IECore::dispatch( data, VtValueTypeNameFromData() );
+	try
+	{
+		return IECore::dispatch( data, VtValueTypeNameFromData() );
+	}
+	catch( const IECore::Exception & )
+	{
+		// Type not supported by `dispatch()`.
+		return SdfValueTypeName();
+	}
 }
diff --git a/contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp
@@ -322,10 +322,16 @@ void writeShaderParameterValues( const IECoreScene::Shader *shader, pxr::UsdShad
 		pxr::UsdShadeInput input = usdShader.GetInput( usdParameterName );
 		if( !input )
 		{
-			input = usdShader.CreateInput(
-				toUSDParameterName( p.first ),
-				IECoreUSD::DataAlgo::valueTypeName( p.second.get() )
-			);
+			pxr::SdfValueTypeName typeName = IECoreUSD::DataAlgo::valueTypeName( p.second.get() );
+			if( !typeName )
+			{
+				IECore::msg( IECore::Msg::Warning, "ShaderAlgo",
+					boost::format( "Shader parameter `%1%.%2%` has unsupported type `%3%`" )
+						% shader->getName() % p.first % p.second->typeName()
+				);
+				continue;
+			}
+			input = usdShader.CreateInput( toUSDParameterName( p.first ), typeName );
 		}
 		if( auto *s = IECore::runTimeCast<IECore::StringData>( p.second.get() ) )
 		{

diff --git a/contrib/IECoreUSD/src/IECoreUSD/bindings/IEUSDModule.cpp b/contrib/IECoreUSD/src/IECoreUSD/bindings/IEUSDModule.cpp
@@ -96,6 +96,7 @@ BOOST_PYTHON_MODULE( _IECoreUSD )
 		scope dataAlgoModuleScope( dataAlgoModule );
 
 		def( "role", &DataAlgo::role );
+		def( "toUSD", (pxr::VtValue (*)( const IECore::Data *, bool ))&DataAlgo::toUSD, ( arg( "data" ), arg( "arrayRequired" ) = false ) );
 		def( "valueTypeName", &DataAlgo::valueTypeName );
 	}
 

diff --git a/contrib/IECoreUSD/test/IECoreUSD/DataAlgoTest.py b/contrib/IECoreUSD/test/IECoreUSD/DataAlgoTest.py
@@ -35,70 +35,88 @@
 import unittest
 import imath
 
+import pxr.Sdf
+
 import IECore
 import IECoreUSD
 
 class DataAlgoTest( unittest.TestCase ) :
 
-    def testRoleBinding( self ) :
-
-        self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Point ), "Point" )
-        self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Vector ), "Vector" )
-        self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Normal ), "Normal" )
-        self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.UV ), "TextureCoordinate" )
-        self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Color ), "Color" )
-        self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Rational ), "" )
+	def testRoleBinding( self ) :
+
+		self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Point ), "Point" )
+		self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Vector ), "Vector" )
+		self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Normal ), "Normal" )
+		self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.UV ), "TextureCoordinate" )
+		self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Color ), "Color" )
+		self.assertEqual( IECoreUSD.DataAlgo.role( IECore.GeometricData.Interpretation.Rational ), "" )
+
+	def testValueTypeNameBinding( self ) :
+
+		v3 = IECore.V3fData( imath.V3f( 0.0 ) )
+		v2 = IECore.V2fData( imath.V2f( 0.0 ) )
+
+		v3.setInterpretation( IECore.GeometricData.Interpretation.Point )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v3 ).type.typeName, "GfVec3f" )
+
+		v3.setInterpretation( IECore.GeometricData.Interpretation.Vector )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v3 ).type.typeName, "GfVec3f" )
+
+		v3.setInterpretation( IECore.GeometricData.Interpretation.Normal )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v3 ).type.typeName, "GfVec3f" )
 
-    def testValueTypeNameBinding( self ) :
+		v2.setInterpretation( IECore.GeometricData.Interpretation.UV )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v2 ).type.typeName, "GfVec2f" )
 
-        v3 = IECore.V3fData( imath.V3f( 0.0 ) )
-        v2 = IECore.V2fData( imath.V2f( 0.0 ) )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( IECore.Color3fData( imath.Color3f( 0.0 ) ) ).type.typeName, "GfVec3f" )
 
-        v3.setInterpretation( IECore.GeometricData.Interpretation.Point )
-        self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v3 ).type.typeName, "GfVec3f" )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( IECore.PathMatcherData() ), pxr.Sdf.ValueTypeName() )
+		self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( IECore.CompoundData() ), pxr.Sdf.ValueTypeName() )
 
-        v3.setInterpretation( IECore.GeometricData.Interpretation.Vector )
-        self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v3 ).type.typeName, "GfVec3f" )
+	def testToUSDBinding( self ) :
 
-        v3.setInterpretation( IECore.GeometricData.Interpretation.Normal )
-        self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v3 ).type.typeName, "GfVec3f" )
+		# Note : On the C++ side we are converting to VtValue, but the
+		# bindings for that convert back to native Python types.
 
-        v2.setInterpretation( IECore.GeometricData.Interpretation.UV )
-        self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( v2 ).type.typeName, "GfVec2f" )
+		for data, value in [
+			( IECore.IntData( 10 ), 10 ),
+			( IECore.FloatData( 2.5 ), 2.5 ),
+			( IECore.IntVectorData( [ 1, 2, 3 ] ), [ 1, 2, 3 ] ),
+			( IECore.PathMatcherData(), None ),
+			( IECore.CompoundData(), None ),
+		] :
+			self.assertEqual( IECoreUSD.DataAlgo.toUSD( data ), value )
 
-        self.assertEqual( IECoreUSD.DataAlgo.valueTypeName( IECore.Color3fData( imath.Color3f( 0.0 ) ) ).type.typeName, "GfVec3f" )
+	def testToFromInternalName( self ) :
 
-    def testToFromInternalName( self ) :
+		a = "a-name(that-is-bad)"
 
-        a = "a-name(that-is-bad)"
+		b = IECoreUSD.SceneCacheDataAlgo.toInternalName( a )
+		self.assertEqual( b, "a_____name____that_____is_____bad___" )
 
-        b = IECoreUSD.SceneCacheDataAlgo.toInternalName( a )
-        self.assertEqual( b, "a_____name____that_____is_____bad___" )
+		c = IECoreUSD.SceneCacheDataAlgo.fromInternalName( b )
+		self.assertEqual( a , c )
 
-        c = IECoreUSD.SceneCacheDataAlgo.fromInternalName( b )
-
-        self.assertEqual( a , c )
+	def testToFromInternalPath( self ) :
 
-    def testToFromInternalPath( self ) :
+		a = ["a-path", "(that)", "(is)", "(bad)"]
 
-        a = ["a-path", "(that)", "(is)", "(bad)"]
+		b = IECoreUSD.SceneCacheDataAlgo.toInternalPath( a )
+		self.assertEqual(
+			b,
+			[
+				"__IECOREUSD_ROOT",
+				"a_____path",
+				"____that___",
+				"____is___",
+				"____bad___",
+			]
+		)
 
-        b = IECoreUSD.SceneCacheDataAlgo.toInternalPath( a )
-        self.assertEqual(
-            b,
-            [
-                "__IECOREUSD_ROOT",
-                "a_____path",
-                "____that___",
-                "____is___",
-                "____bad___",
-            ]
-        )
+		c = IECoreUSD.SceneCacheDataAlgo.fromInternalPath( b )
 
-        c = IECoreUSD.SceneCacheDataAlgo.fromInternalPath( b )
+		self.assertEqual( a, c )
 
-        self.assertEqual( a, c )
-
 
 if __name__ == "__main__":
-    unittest.main()
+	unittest.main()
diff --git a/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py b/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py
@@ -3181,6 +3181,42 @@ def testShadersAcrossDifferentScopes( self ):
 			) ]
 		)
 
+	def testUnsupportedShaderParameterTypes( self ) :
+
+		sourceNetwork = IECoreScene.ShaderNetwork(
+			shaders = {
+				"test" : IECoreScene.Shader(
+					"test",
+					parameters = {
+						"unsupported1" : IECore.CompoundData(),
+						"unsupported2" : IECore.PathMatcherData(),
+						"supported" : "abc"
+					}
+				)
+			},
+			output = "test"
+		)
+
+		fileName = os.path.join( self.temporaryDirectory(), "test.usda" )
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+
+		with IECore.CapturingMessageHandler() as mh :
+			root.createChild( "test" ).writeAttribute( "surface", sourceNetwork, 0 )
+
+		self.assertEqual(
+			{ m.message for m in mh.messages },
+			{
+				"Shader parameter `test.unsupported1` has unsupported type `CompoundData`",
+				"Shader parameter `test.unsupported2` has unsupported type `PathMatcherData`",
+			}
+		)
+
+		del root
+
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+		network = root.child( "test" ).readAttribute( "surface", 0 )
+		self.assertEqual( network.outputShader().parameters, IECore.CompoundData( { "supported" : "abc" } ) )
+
 	def testHoudiniVaryingLengthArrayPrimVar( self ) :
 
 		root = IECoreScene.SceneInterface.create(
@@ -4129,6 +4165,7 @@ def testArnoldSpecificLightInputs( self ) :
 
 		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
 		self.assertEqual( root.child( "light" ).readAttribute( "light", 0 ), lightShader )
+		self.assertEqual( root.child( "light" ).attributeNames(), [ "light" ] )
 
 	def testTreatLightAsPointOrLine( self ) :