Merge pull request #1438 from johnhaddon/shaderAlgoImprovements

ShaderNetworkAlgo improvements

Author: johnhaddon

Changes

@@ -3,7 +3,15 @@
 
 Improvements
 ------------
+
 - USDScene : PointInstancers are now loaded with invisibleIds and inactiveIds as primitive variables.
+- IECoreUSD::DataAlgo :
+  - Made `valueTypeName` argument to `fromUSD( const VtValue & )` optional. This allows VtValue to be converted without having additional type information available.
+  - Added conversions between `VtDictionary` and `CompoundData`.
+- IECoreUSD::ShaderAlgo :
+  - Stopped writing `cortex_autoAdaptor` metadata, which would cause errors in DCCs without the definition registered.
+  - Added round-tripping of all blind data stored on Shaders.
+- IECoreScene::ShaderNetworkAlgo : Added a mechanism for customising the adapter shaders used by `addComponentConnectionAdapters()`.
 
 10.5.9.5 (relative to 10.5.9.4)
 ========

contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h

@@ -66,13 +66,12 @@ typename USDTypeTraits<T>::CortexType fromUSD( const T &value );
 template<typename T>
 boost::intrusive_ptr< typename USDTypeTraits<T>::CortexVectorDataType > fromUSD( const pxr::VtArray<T> &array );
 
-/// Converts USD `value` to Cortex Data, applying any additional
-/// geometric interpretation implied by `valueTypeName`. If
-/// `arrayAccepted` is false, then converts single element arrays
-/// to simple data and emits a warning and returns nullptr for
-/// all other arrays. Returns nullptr if no appropriate conversion
-/// exists.
-IECOREUSD_API IECore::DataPtr fromUSD( const pxr::VtValue &value, const pxr::SdfValueTypeName &valueTypeName, bool arrayAccepted = true );
+/// Converts USD `value` to Cortex Data, applying any additional geometric
+/// interpretation implied by `valueTypeName` if it is provided. If
+/// `arrayAccepted` is false, then converts single element arrays to simple data
+/// and emits a warning and returns nullptr for all other arrays. Returns
+/// nullptr if no appropriate conversion exists.
+IECOREUSD_API IECore::DataPtr fromUSD( const pxr::VtValue &value, const pxr::SdfValueTypeName &valueTypeName = pxr::SdfValueTypeName(), bool arrayAccepted = true );
 
 /// Converts the value of `attribute` at the specified time, using the attribute's
 /// type name to apply geometric interpretation. The meaning of `arrayAccepted` is

contrib/IECoreUSD/resources/plugInfo.json

@@ -17,6 +17,10 @@
 						"target": "usd"
 					}
 				},
+				## \todo This metadata causes problems because other DCCs are
+				# unlikely to have our `plugInfo.json` available. Replace it
+				# with custom data instead (`UsdObject::SetCustomData()`), since
+				# that doesn't require central registration.
 				"SdfMetadata": {
 					"cortex_isConstantPrimitiveVariable": {
 						"type": "bool",
@@ -37,9 +41,7 @@
 						"type": "bool",
 						"appliesTo": "prims"
 					},
-					# Label a shader that was created automatically to hold
-					# a connection that USD can't support directly - if we
-					# want to round trip, we have to remove these on input
+					# Legacy metadata once used by ShaderAlgo.
 					"cortex_autoAdapter": {
 						"type": "bool",
 						"appliesTo": "prims"

contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp

@@ -34,6 +34,7 @@
 
 #include "IECoreUSD/DataAlgo.h"
 
+#include "IECore/CompoundData.h"
 #include "IECore/DataAlgo.h"
 #include "IECore/MessageHandler.h"
 
@@ -213,6 +214,19 @@ IECore::DataPtr dataFromSdfAssetPath( const pxr::VtValue &value, GeometricData::
 	return dataFromSdfAssetPath( value.UncheckedGet<SdfAssetPath>() );
 }
 
+IECore::DataPtr dataFromDictionary( const pxr::VtValue &value, GeometricData::Interpretation interpretation, bool arrayAccepted )
+{
+	CompoundDataPtr result = new CompoundData;
+	for( const auto &[name, v] : value.Get<VtDictionary>() )
+	{
+		if( IECore::DataPtr d = IECoreUSD::DataAlgo::fromUSD( v, pxr::SdfValueTypeName() ) )
+		{
+			result->writable()[name] = d;
+		}
+	}
+	return result;
+}
+
 static const std::map<pxr::TfType, IECore::DataPtr (*)( const pxr::VtValue &, GeometricData::Interpretation, bool )> g_fromVtValueConverters = {
 
 	// Numeric types
@@ -279,7 +293,11 @@ static const std::map<pxr::TfType, IECore::DataPtr (*)( const pxr::VtValue &, Ge
 	{ TfType::Find<VtArray<string>>(), &dataFromArray<string> },
 	{ TfType::Find<TfToken>(), &dataFromValue<TfToken> },
 	{ TfType::Find<VtArray<TfToken>>(), &dataFromArray<TfToken> },
-	{ TfType::Find<SdfAssetPath>(), &dataFromSdfAssetPath }
+	{ TfType::Find<SdfAssetPath>(), &dataFromSdfAssetPath },
+
+	// Dictionary
+
+	{ TfType::Find<VtDictionary>(), &dataFromDictionary }
 
 };
 
@@ -323,20 +341,32 @@ static const std::map<pxr::TfType, std::function<IECore::DataPtr ( const pxr::Vt
 
 IECore::DataPtr IECoreUSD::DataAlgo::fromUSD( const pxr::VtValue &value, const pxr::SdfValueTypeName &valueTypeName, bool arrayAccepted )
 {
-	const GeometricData::Interpretation i = interpretation( valueTypeName.GetRole() );
+	GeometricData::Interpretation i;
+	TfType type;
+	if( valueTypeName )
+	{
+		i = interpretation( valueTypeName.GetRole() );
+		type = valueTypeName.GetType();
+	}
+	else
+	{
+		i = GeometricData::Interpretation::None;
+		type = value.GetType();
+	}
+
 	if( i == GeometricData::Color )
 	{
 		// Colors can not be identified by TfType because they borrow GfVec3,
 		// so they require their own dispatch table.
-		const auto it = g_fromVtValueColorConverters.find( valueTypeName.GetType() );
+		const auto it = g_fromVtValueColorConverters.find( type );
 		if( it == g_fromVtValueColorConverters.end() )
 		{
 			return nullptr;
 		}
 		return it->second( value, arrayAccepted );
 	}
 
-	const auto it = g_fromVtValueConverters.find( valueTypeName.GetType() );
+	const auto it = g_fromVtValueConverters.find( type );
 	if( it == g_fromVtValueConverters.end() )
 	{
 		return nullptr;
@@ -420,6 +450,26 @@ struct VtValueFromData
 		return VtValue( DataAlgo::toUSD( data->readable() ) );
 	}
 
+	VtValue operator()( const IECore::CompoundData *data, bool arrayRequired )
+	{
+		if( arrayRequired )
+		{
+			return VtValue();
+		}
+
+		VtDictionary result;
+		for( const auto &[name, value] : data->readable() )
+		{
+			VtValue v = DataAlgo::toUSD( value.get() );
+			if( !v.IsEmpty() )
+			{
+				result[name] = v;
+			}
+		}
+
+		return VtValue( result );
+	}
+
 	VtValue operator()( const IECore::Data *data, bool arrayRequired ) const
 	{
 		return VtValue();
@@ -433,7 +483,13 @@ pxr::VtValue IECoreUSD::DataAlgo::toUSD( const IECore::Data *data, bool arrayReq
 {
 	try
 	{
-		return IECore::dispatch( data, VtValueFromData(), arrayRequired );
+		VtValueFromData valueFromData;
+		if( auto cd = runTimeCast<const CompoundData>( data ) )
+		{
+			// Manual dispatch since CompoundData not handled by `dispatch()`.
+			return valueFromData( cd, arrayRequired );
+		}
+		return IECore::dispatch( data, valueFromData, arrayRequired );
 	}
 	catch( const IECore::Exception & )
 	{

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -64,7 +64,8 @@
 namespace
 {
 
-pxr::TfToken g_adapterLabelToken( IECoreScene::ShaderNetworkAlgo::componentConnectionAdapterLabel().string() );
+const pxr::TfToken g_blindDataToken( "cortex:blindData" );
+pxr::TfToken g_legacyAdapterLabelToken( IECoreScene::ShaderNetworkAlgo::componentConnectionAdapterLabel().string() );
 
 std::pair<pxr::TfToken, std::string> shaderIdAndType( const pxr::UsdShadeConnectableAPI &connectable )
 {
@@ -249,11 +250,26 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 	readNonStandardLightParameters( usdShader.GetPrim(), parameters );
 
 	IECoreScene::ShaderPtr newShader = new IECoreScene::Shader( shaderName, shaderType, parametersData );
+
+	// General purpose support for any Cortex blind data.
+
+	const pxr::VtValue blindDataValue = usdShader.GetPrim().GetCustomDataByKey( g_blindDataToken );
+	if( !blindDataValue.IsEmpty() )
+	{
+		if( auto blindData = IECore::runTimeCast<IECore::CompoundData>( IECoreUSD::DataAlgo::fromUSD( blindDataValue ) ) )
+		{
+			newShader->blindData()->writable() = blindData->readable();
+		}
+	}
+
+	// Legacy support for `cortex_autoAdaptor` metadata.
+
 	pxr::VtValue metadataValue;
-	if( usdShader.GetPrim().GetMetadata( g_adapterLabelToken, &metadataValue ) && metadataValue.Get<bool>() )
+	if( usdShader.GetPrim().GetMetadata( g_legacyAdapterLabelToken, &metadataValue ) && metadataValue.Get<bool>() )
 	{
 		newShader->blindData()->writable()[ IECoreScene::ShaderNetworkAlgo::componentConnectionAdapterLabel() ] = new IECore::BoolData( true );
 	}
+
 	shaderNetwork.addShader( handle, std::move( newShader ) );
 
 	// Can only add connections after we've added the shader.
@@ -350,10 +366,12 @@ void writeShaderParameterValues( const IECoreScene::Shader *shader, pxr::UsdShad
 		input.Set( IECoreUSD::DataAlgo::toUSD( p.second.get() ) );
 	}
 
-	const IECore::BoolData *adapterMeta = shader->blindData()->member<IECore::BoolData>( IECoreScene::ShaderNetworkAlgo::componentConnectionAdapterLabel() );
-	if( adapterMeta && adapterMeta->readable() )
+	if( shader->blindData()->readable().size() )
 	{
-		usdShader.GetPrim().SetMetadata( g_adapterLabelToken, true );
+		usdShader.GetPrim().SetCustomDataByKey(
+			g_blindDataToken,
+			IECoreUSD::DataAlgo::toUSD( shader->blindData() )
+		);
 	}
 }
 

contrib/IECoreUSD/test/IECoreUSD/DataAlgoTest.py

@@ -83,7 +83,7 @@ def testToUSDBinding( self ) :
 			( IECore.FloatData( 2.5 ), 2.5 ),
 			( IECore.IntVectorData( [ 1, 2, 3 ] ), [ 1, 2, 3 ] ),
 			( IECore.PathMatcherData(), None ),
-			( IECore.CompoundData(), None ),
+			( IECore.CompoundData(), {} ),
 		] :
 			self.assertEqual( IECoreUSD.DataAlgo.toUSD( data ), value )
 

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -3534,6 +3534,55 @@ def testColor4fShaderParameterComponentConnections( self ) :
 		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
 		self.assertEqual( root.child( "object" ).readAttribute( "ai:surface", 0 ), network )
 
+	def testLegacyComponentConnections( self ) :
+
+		expectedNetwork = IECoreScene.ShaderNetwork(
+			shaders = {
+				"source" : IECoreScene.Shader( "noise" ),
+				"output" : IECoreScene.Shader(
+					"color_correct",
+					parameters = {
+						"input" : imath.Color4f( 1 ),
+					}
+				),
+			},
+			connections = [
+				( ( "source", "r" ), ( "output", "input.g" ) ),
+				( ( "source", "g" ), ( "output", "input.b" ) ),
+				( ( "source", "b" ), ( "output", "input.r" ) ),
+				( ( "source", "r" ), ( "output", "input.a" ) ),
+			],
+			output = "output",
+		)
+
+		root = IECoreScene.SceneInterface.create( os.path.join( os.path.dirname( __file__ ), "data", "legacyComponentConnections.usda" ), IECore.IndexedIO.OpenMode.Read )
+		self.assertEqual( root.child( "object" ).readAttribute( "ai:surface", 0 ), expectedNetwork )
+
+	def testShaderBlindData( self ) :
+
+		shader = IECoreScene.Shader( "test" )
+		shader.blindData()["testInt"] = IECore.IntData( 10 )
+		shader.blindData()["testFloatVector"] = IECore.FloatVectorData( [ 1, 2, 3, ] )
+		shader.blindData()["test:colon"] = IECore.BoolData( True )
+		shader.blindData()["testCompound"] = IECore.CompoundData( {
+			"testString" : "test",
+			"testStringVector" : IECore.StringVectorData( [ "one", "two" ] )
+		} )
+
+		network = IECoreScene.ShaderNetwork(
+			shaders = { "test" : shader },
+			output = ( "test", "out" )
+		)
+
+		fileName = os.path.join( self.temporaryDirectory(), "testShaderBlindData.usda" )
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+		object = root.createChild( "object" )
+		object.writeAttribute( "surface", network, 0.0 )
+		del object, root
+
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+		self.assertEqual( root.child( "object" ).readAttribute( "surface", 0 ), network )
+
 	def testMaterialPurpose( self ) :
 
 		def assertExpected( root ) :

contrib/IECoreUSD/test/IECoreUSD/data/legacyComponentConnections.usda

@@ -0,0 +1,54 @@
+#usda 1.0
+
+def Xform "object" (
+    prepend apiSchemas = ["MaterialBindingAPI"]
+)
+{
+    rel material:binding = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4>
+
+    def Scope "materials" (
+        cortex_autoMaterials = true
+    )
+    {
+        def Material "material_aaad0a40dfc4f67f90ca42cbc732dec4"
+        {
+            token outputs:arnold:surface.connect = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4/arnold_surface_shaders/output.outputs:DEFAULT_OUTPUT>
+
+            def Scope "arnold_surface_shaders"
+            {
+                def Shader "output"
+                {
+                    uniform token info:id = "color_correct"
+                    color4f inputs:input = (1, 1, 1, 1)
+                    color4f inputs:input.connect = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4/arnold_surface_shaders/pack.outputs:out>
+                    token outputs:DEFAULT_OUTPUT
+                }
+
+                def Shader "pack" (
+                    cortex_autoAdapter = true
+                )
+                {
+                    uniform token info:id = "osl:MaterialX/mx_pack_color"
+                    float inputs:in1 = 1
+                    float inputs:in1.connect = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4/arnold_surface_shaders/source.outputs:b>
+                    float inputs:in2 = 1
+                    float inputs:in2.connect = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4/arnold_surface_shaders/source.outputs:r>
+                    float inputs:in3 = 1
+                    float inputs:in3.connect = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4/arnold_surface_shaders/source.outputs:g>
+                    float inputs:in4 = 1
+                    float inputs:in4.connect = </object/materials/material_aaad0a40dfc4f67f90ca42cbc732dec4/arnold_surface_shaders/source.outputs:r>
+                    color4f outputs:out
+                }
+
+                def Shader "source"
+                {
+                    uniform token info:id = "noise"
+                    float outputs:b
+                    float outputs:g
+                    float outputs:r
+                }
+            }
+        }
+    }
+}
+

include/IECoreScene/ShaderNetworkAlgo.h

@@ -63,17 +63,39 @@ IECORESCENE_API void removeUnusedShaders( ShaderNetwork *network );
 template<typename Visitor>
 void depthFirstTraverse( const ShaderNetwork *network, Visitor &&visitor, IECore::InternedString shader = "" );
 
-/// Replace connections between sub components of colors or vectors with connections to whole parameters
-/// on adapter shaders.  Currently uses the OSL shaders mx_pack_color and mx_swizzle_color_float as adapters.
-/// The newly created shaders will be labelled with a blind data so they can be identified.
-/// If `targetPrefix` is given, only translates connections to shaders with a type starting with this string
+/// Replaces connections between sub components of colors or vectors with
+/// connections to whole parameters on adapter shaders. If `targetPrefix` is
+/// given, only translates connections to shaders with a type starting with this
+/// string.
 IECORESCENE_API void addComponentConnectionAdapters( ShaderNetwork *network, std::string targetPrefix = "" );
 
-/// Find adapters that were created by addComponentConnectionAdapters ( based on the blind data label ),
-/// and remove them, replacing them with the original component connections
+/// Finds adapters that were created by addComponentConnectionAdapters, and
+/// removes them, replacing them with the original component connections.
 IECORESCENE_API void removeComponentConnectionAdapters( ShaderNetwork *network );
 
-/// The name of the boolean blindData label used by add/removeComponentConnectionAdapters
+/// Registers an adapter to split a component from a color or vector output, ready for connection into
+/// a scalar input. Used by `addComponentConnectionAdapters()`.
+///
+/// - `destinationShaderType` : The type prefix for the shader receiving the connection - e.g. "ai", "osl".
+/// - `component` : "r", "g", "b", "a", "x", "y", or "z".
+/// - `adapter` : The shader to be used as the adapter.
+/// - `inParameter` : The parameter that receives the color or vector input.
+/// - `outParameter` : The parameter that outputs the component.
+IECORESCENE_API void registerSplitAdapter( const std::string &destinationShaderType, IECore::InternedString component, const IECoreScene::Shader *adapter, IECore::InternedString inParameter, IECore::InternedString outParameter );
+/// Removes an adapter registration.
+IECORESCENE_API void deregisterSplitAdapter( const std::string &destinationShaderType, IECore::InternedString component );
+
+/// Registers an adapter to join multiple scalar components into a color or vector output. Used by `addComponentConnectionAdapters()`.
+///
+/// - `destinationShaderType` : The type prefix for the shader receiving the connection - e.g. "ai", "osl".
+/// - `destinationParameterType` : `(V2i|V3i|V2f|V3f|Color3f|Color4f)DataTypeId`.
+/// - `inParameters` : The parameters that receives the individual components of the vector or color.
+/// - `outParameter` : The parameter that outputs the vector or color.
+IECORESCENE_API void registerJoinAdapter( const std::string &destinationShaderType, IECore::TypeId destinationParameterType, const IECoreScene::Shader *adapter, const std::array<IECore::InternedString, 4> &inParameters, IECore::InternedString outParameter );
+/// Removes an adapter registration.
+IECORESCENE_API void deregisterJoinAdapter( const std::string &destinationShaderType, IECore::TypeId destinationParameterType );
+
+/// \deprecated
 IECORESCENE_API const IECore::InternedString &componentConnectionAdapterLabel();
 
 /// Converts various aspects of how shaders are stored to be ready to pass directly to OSL.