Merge branch 'RB-10.4' into RB-10.5

Author: johnhaddon

Changes

@@ -9,12 +9,15 @@ Improvements
 Fixes
 -----
 
-- USDScene : Fixed handling of invalid values on the following attributes :
-  - PointBased : `positions`, `normals`, `velocities`, `accelerations`.
-  - Curves : `widths`.
-  - PointInstancer : `ids`, `protoIndices`, `orientations`, `scales`, `velocities`, `accelerations`, `angularVelocities`.
-  - Points : `ids`, `widths`.
-  Invalid values are now ignored with a warning, instead of loading as invalid primitive variables.
+- USDScene :
+  - Fixed handling of invalid values on the following attributes :
+    - PointBased : `positions`, `normals`, `velocities`, `accelerations`.
+    - Curves : `widths`.
+    - PointInstancer : `ids`, `protoIndices`, `orientations`, `scales`, `velocities`, `accelerations`, `angularVelocities`.
+    - Points : `ids`, `widths`.
+    Invalid values are now ignored with a warning, instead of loading as invalid primitive variables.
+  - Fixed treatment of unconnected material outputs during reading. If they were "authored" but not connected to a source, they were incorrectly being treated as valid attributes, and loading as empty ShaderNetworks which caused problems elsewhere.
+
 - ToMayaMeshConverter : No longer locks normals set on the Mesh from the scc.
 
 Breaking Changes
@@ -83,6 +86,14 @@ Breaking Changes
   - Changed function signature.
   - Bug fixes mean subtle changes to the resulting points.
 
+10.4.10.3 (relative to 10.4.10.2)
+=========
+
+Fixes
+-----
+
+- USDScene : Fixed treatment of unconnected material outputs during reading. If they were "authored" but not connected to a source, they were incorrectly being treated as valid attributes, and loading as empty ShaderNetworks which caused problems elsewhere.
+
 10.4.10.2 (relative to 10.4.10.1)
 =========
 

contrib/IECoreUSD/include/IECoreUSD/ShaderAlgo.h

@@ -57,7 +57,11 @@ namespace ShaderAlgo
 IECOREUSD_API pxr::UsdShadeOutput writeShaderNetwork( const IECoreScene::ShaderNetwork *shaderNetwork, pxr::UsdPrim shaderContainer );
 
 /// Reads a ShaderNetwork from a material output, typically obtained from `UsdShadeMaterial::GetOutput()`.
+/// Returns `nullptr` if `canReadShaderNetwork() == false`, usually because the output has no connected source.
 IECOREUSD_API IECoreScene::ShaderNetworkPtr readShaderNetwork( const pxr::UsdShadeOutput &output );
+/// Returns true if `readShaderNetwork()` will return `nullptr`, usually because the output has no
+/// connected source.
+bool canReadShaderNetwork( const pxr::UsdShadeOutput &output );
 
 #if PXR_VERSION >= 2111
 /// Writes a UsdLuxLight from a shader network.

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -359,6 +359,22 @@ pxr::UsdShadeOutput IECoreUSD::ShaderAlgo::writeShaderNetwork( const IECoreScene
 	return networkOutUsd;
 }
 
+bool IECoreUSD::ShaderAlgo::canReadShaderNetwork( const pxr::UsdShadeOutput &output )
+{
+	pxr::UsdShadeConnectableAPI usdSource;
+	pxr::TfToken usdSourceName;
+	pxr::UsdShadeAttributeType usdSourceType;
+	if(
+		!output.GetConnectedSource( &usdSource, &usdSourceName, &usdSourceType ) ||
+		usdSourceType != pxr::UsdShadeAttributeType::Output
+	)
+	{
+		return false;
+	}
+
+	return true;
+}
+
 IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const pxr::UsdShadeOutput &output  )
 {
 	pxr::UsdShadeConnectableAPI usdSource;
@@ -369,7 +385,7 @@ IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const px
 		usdSourceType != pxr::UsdShadeAttributeType::Output
 	)
 	{
-		return new IECoreScene::ShaderNetwork();
+		return nullptr;
 	}
 
 	IECoreScene::ShaderNetworkPtr result = new IECoreScene::ShaderNetwork();

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -480,7 +480,7 @@ class ShaderNetworkCache : public LRUCache<pxr::SdfPath, IECoreScene::ConstShade
 		static IECoreScene::ConstShaderNetworkPtr getter( const ShaderNetworkCacheGetterKey &key, size_t &cost )
 		{
 			IECoreScene::ConstShaderNetworkPtr result = ShaderAlgo::readShaderNetwork( key );
-			cost = result->Object::memoryUsage();
+			cost = result ? result ->Object::memoryUsage() : 0;
 			return result;
 		}
 
@@ -961,7 +961,7 @@ bool USDScene::hasAttribute( const SceneInterface::Name &name ) const
 		{
 			if( pxr::UsdShadeOutput o = mat.GetOutput( output ) )
 			{
-				return o.GetAttr().IsAuthored();
+				return ShaderAlgo::canReadShaderNetwork( o );
 			}
 		}
 		return false;
@@ -1025,6 +1025,10 @@ void USDScene::attributeNames( SceneInterface::NameList &attrs ) const
 		{
 			for( pxr::UsdShadeOutput &o : mat.GetOutputs( /* onlyAuthored = */ true ) )
 			{
+				if( !ShaderAlgo::canReadShaderNetwork( o ) )
+				{
+					continue;
+				}
 				InternedString attrName = AttributeAlgo::nameFromUSD( { o.GetBaseName() , false } );
 				if( !purpose.IsEmpty() )
 				{
@@ -1118,8 +1122,7 @@ ConstObjectPtr USDScene::readAttribute( const SceneInterface::Name &name, double
 		const auto &[output, purpose] = materialOutputAndPurpose( name.string() );
 		if( pxr::UsdShadeMaterial mat = m_root->computeBoundMaterial( m_location->prim, purpose ) )
 		{
-			pxr::UsdShadeOutput o = mat.GetOutput( output );
-			if( o && o.GetAttr().IsAuthored() )
+			if( pxr::UsdShadeOutput o = mat.GetOutput( output ) )
 			{
 				return m_root->readShaderNetwork( o );
 			}

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -2686,7 +2686,8 @@ def testShaders( self ) :
 		oneShaderNetwork.addShader( "foo", surface )
 		oneShaderNetwork.setOutput( IECoreScene.ShaderNetwork.Parameter( "foo", "" ) )
 
-		# A network with no output can be written out, but it will read back in as empty
+		# A network with no output can be written out, but not read back in, because
+		# it will not have been connected to a material output.
 		noOutputNetwork = IECoreScene.ShaderNetwork()
 		noOutputNetwork.addShader( "foo", surface )
 
@@ -2873,14 +2874,14 @@ def testShaders( self ) :
 
 		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
 
-		self.assertEqual( set( root.child( "shaderLocation" ).attributeNames() ), set( ['ai:disp_map', 'ai:surface', 'complex:surface', 'testBad:surface', 'volume', 'componentConnection:surface', 'manualComponent:surface' ] ) )
+		self.assertEqual( set( root.child( "shaderLocation" ).attributeNames() ), set( ['ai:disp_map', 'ai:surface', 'complex:surface', 'volume', 'componentConnection:surface', 'manualComponent:surface' ] ) )
 
 		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ai:surface", 0 ).outputShader().parameters, oneShaderNetwork.outputShader().parameters )
 		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ai:surface", 0 ).outputShader(), oneShaderNetwork.outputShader() )
 		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ai:surface", 0 ), oneShaderNetwork )
-		self.assertTrue( root.child( "shaderLocation" ).hasAttribute( "testBad:surface" ) )
+		self.assertFalse( root.child( "shaderLocation" ).hasAttribute( "testBad:surface" ) )
 
-		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "testBad:surface", 0 ), IECoreScene.ShaderNetwork() )
+		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "testBad:surface", 0 ), None )
 		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ai:disp_map", 0 ), pickOutputNetwork )
 		self.assertEqual( root.child( "shaderLocation" ).hasAttribute( "ai:volume" ), False )
 		self.assertEqual( root.child( "shaderLocation" ).readAttribute( "volume", 0 ), oneShaderNetwork )
@@ -3898,5 +3899,17 @@ def testWriteVDBObject( self ) :
 		self.assertEqual( field.GetFilePathAttr().Get( 0 ), vdbFileName )
 		self.assertEqual( field.GetFieldClassAttr().Get( 0 ), "GRID_FOG_VOLUME" )
 
+	def testUnconnectedMaterialOutput( self ) :
+
+		root = IECoreScene.SceneInterface.create(
+			os.path.join( os.path.dirname( __file__ ), "data", "unconnectedMaterialOutput.usda" ),
+			IECore.IndexedIO.OpenMode.Read
+		)
+
+		sphere = root.child( "sphere" )
+		self.assertFalse( sphere.hasAttribute( "cycles:surface" ) )
+		self.assertNotIn( "cycles:surface", sphere.attributeNames() )
+		self.assertIsNone( sphere.readAttribute( "cycles:surface", 0 ) )
+
 if __name__ == "__main__":
 	unittest.main()

contrib/IECoreUSD/test/IECoreUSD/data/unconnectedMaterialOutput.usda

@@ -0,0 +1,13 @@
+#usda 1.0
+
+def Sphere "sphere" (
+    prepend apiSchemas = ["MaterialBindingAPI"]
+)
+{
+    rel material:binding = </myMaterial>
+}
+
+def Material "myMaterial"
+{
+    token outputs:cycles:surface
+}