USDScene : Fix loading of animated materials and lights

Changes

@@ -4,6 +4,7 @@
 Fixes
 -----
 
+- USDScene : Fixed reading of animated parameters on materials and lights.
 - Boost : Fixed compatibility with Boost 1.85.
 
 10.6.2.0 (relative to 10.6.1.0)

contrib/IECoreUSD/include/IECoreUSD/ShaderAlgo.h

@@ -58,16 +58,19 @@ IECOREUSD_API pxr::UsdShadeOutput writeShaderNetwork( const IECoreScene::ShaderN
 
 /// 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 );
+IECOREUSD_API IECoreScene::ShaderNetworkPtr readShaderNetwork( const pxr::UsdShadeOutput &output, pxr::UsdTimeCode timeCode = pxr::UsdTimeCode::Default() );
 /// Returns true if `readShaderNetwork()` will return `nullptr`, usually because the output has no
 /// connected source.
 bool canReadShaderNetwork( const pxr::UsdShadeOutput &output );
+IECOREUSD_API bool shaderNetworkMightBeTimeVarying( const pxr::UsdShadeOutput &output );
 
 #if PXR_VERSION >= 2111
 /// Writes a UsdLuxLight from a shader network.
 IECOREUSD_API void writeLight( const IECoreScene::ShaderNetwork *shaderNetwork, pxr::UsdPrim prim );
 /// Reads a ShaderNetwork from a light.
-IECOREUSD_API IECoreScene::ShaderNetworkPtr readLight( const pxr::UsdLuxLightAPI &light );
+IECOREUSD_API IECoreScene::ShaderNetworkPtr readLight( const pxr::UsdLuxLightAPI &light, pxr::UsdTimeCode timeCode = pxr::UsdTimeCode::Default() );
+IECOREUSD_API bool lightMightBeTimeVarying( const pxr::UsdLuxLightAPI &light );
+
 #endif
 
 } // namespace ShaderAlgo

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -162,6 +162,45 @@ void readNonStandardLightParameters( const pxr::UsdPrim &prim, IECore::CompoundD
 #endif
 }
 
+bool nonStandardLightParametersMightBeTimeVarying( const pxr::UsdPrim &prim )
+{
+#if PXR_VERSION >= 2111
+	if( auto sphereLight = pxr::UsdLuxSphereLight( prim ) )
+	{
+		if( sphereLight.GetTreatAsPointAttr().ValueMightBeTimeVarying() )
+		{
+			return true;
+		}
+	}
+	else if( auto cylinderLight = pxr::UsdLuxCylinderLight( prim ) )
+	{
+		if( cylinderLight.GetTreatAsLineAttr().ValueMightBeTimeVarying() )
+		{
+			return true;
+		}
+	}
+
+	if( auto light = pxr::UsdLuxLightAPI( prim ) )
+	{
+		pxr::UsdGeomPrimvarsAPI primVarsAPI( prim );
+		for( const auto &primVar : primVarsAPI.GetPrimvarsWithAuthoredValues() )
+		{
+			pxr::TfToken name = primVar.GetPrimvarName();
+			if( !boost::starts_with( name.GetString(), "arnold:" ) )
+			{
+				continue;
+			}
+
+			if( primVar.ValueMightBeTimeVarying() )
+			{
+				return true;
+			}
+		}
+	}
+#endif
+	return false;
+}
+
 const std::regex g_arrayIndexFromUSDRegex( ":i([0-9]+)$" );
 const std::string g_arrayIndexFromUSDFormat( "[$1]" );
 IECore::InternedString fromUSDParameterName( const pxr::TfToken &usdName )
@@ -181,9 +220,9 @@ pxr::TfToken toUSDParameterName( IECore::InternedString cortexName )
 	);
 }
 
-IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath &anchorPath, const pxr::UsdShadeOutput &output, IECoreScene::ShaderNetwork &shaderNetwork );
+IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath &anchorPath, const pxr::UsdShadeOutput &output, pxr::UsdTimeCode timeCode, IECoreScene::ShaderNetwork &shaderNetwork );
 
-IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, const pxr::UsdShadeConnectableAPI &usdShader, IECoreScene::ShaderNetwork &shaderNetwork )
+IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, const pxr::UsdShadeConnectableAPI &usdShader, pxr::UsdTimeCode timeCode, IECoreScene::ShaderNetwork &shaderNetwork )
 {
 	IECore::InternedString handle( usdShader.GetPath().MakeRelativePath( anchorPath ).GetString() );
 
@@ -226,7 +265,7 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 			if( usdSourceType == pxr::UsdShadeAttributeType::Output )
 			{
 				const IECoreScene::ShaderNetwork::Parameter sourceHandle = readShaderNetworkWalk(
-					anchorPath, usdSource.GetOutput( usdSourceName ), shaderNetwork
+					anchorPath, usdSource.GetOutput( usdSourceName ), timeCode, shaderNetwork
 				);
 				connections.push_back( {
 					sourceHandle, { handle, fromUSDParameterName( i.GetBaseName() ) }
@@ -241,7 +280,7 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 			}
 		}
 
-		if( IECore::DataPtr d = IECoreUSD::DataAlgo::fromUSD( pxr::UsdAttribute( valueAttribute ) ) )
+		if( IECore::DataPtr d = IECoreUSD::DataAlgo::fromUSD( pxr::UsdAttribute( valueAttribute ), timeCode ) )
 		{
 			parameters[fromUSDParameterName( i.GetBaseName() )] = d;
 		}
@@ -281,9 +320,9 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 	return handle;
 }
 
-IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath &anchorPath, const pxr::UsdShadeOutput &output, IECoreScene::ShaderNetwork &shaderNetwork )
+IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath &anchorPath, const pxr::UsdShadeOutput &output, pxr::UsdTimeCode timeCode, IECoreScene::ShaderNetwork &shaderNetwork )
 {
-	IECore::InternedString shaderHandle = readShaderNetworkWalk( anchorPath, pxr::UsdShadeConnectableAPI( output.GetPrim() ), shaderNetwork );
+	IECore::InternedString shaderHandle = readShaderNetworkWalk( anchorPath, pxr::UsdShadeConnectableAPI( output.GetPrim() ), timeCode, shaderNetwork );
 	if( output.GetBaseName() != "DEFAULT_OUTPUT" )
 	{
 		return IECoreScene::ShaderNetwork::Parameter( shaderHandle, output.GetBaseName().GetString() );
@@ -294,6 +333,45 @@ IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath
 	}
 }
 
+bool shaderNetworkMightBeTimeVaryingWalk( const pxr::UsdShadeConnectableAPI &usdShader, std::unordered_set<pxr::UsdPrim, pxr::TfHash> &visited )
+{
+	if( !visited.insert( usdShader.GetPrim() ).second )
+	{
+		return false;
+	}
+
+	std::vector<IECoreScene::ShaderNetwork::Connection> connections;
+	for( pxr::UsdShadeInput &i : usdShader.GetInputs() )
+	{
+		pxr::UsdShadeConnectableAPI usdSource;
+		pxr::TfToken usdSourceName;
+		pxr::UsdShadeAttributeType usdSourceType;
+
+		pxr::UsdAttribute valueAttribute = i;
+		if( i.GetConnectedSource( &usdSource, &usdSourceName, &usdSourceType ) )
+		{
+			if( usdSourceType == pxr::UsdShadeAttributeType::Output )
+			{
+				if( shaderNetworkMightBeTimeVaryingWalk( usdSource, visited ) )
+				{
+					return true;
+				}
+			}
+			else
+			{
+				valueAttribute = usdSource.GetInput( usdSourceName );
+			}
+		}
+
+		if( valueAttribute.ValueMightBeTimeVarying() )
+		{
+			return true;
+		}
+	}
+
+	return nonStandardLightParametersMightBeTimeVarying( usdShader.GetPrim() );
+}
+
 IECoreScene::ConstShaderNetworkPtr adaptShaderNetworkForWriting( const IECoreScene::ShaderNetwork *shaderNetwork )
 {
 	IECoreScene::ShaderNetworkPtr result = shaderNetwork->copy();
@@ -480,7 +558,7 @@ bool IECoreUSD::ShaderAlgo::canReadShaderNetwork( const pxr::UsdShadeOutput &out
 	return true;
 }
 
-IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const pxr::UsdShadeOutput &output  )
+IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const pxr::UsdShadeOutput &output, pxr::UsdTimeCode timeCode )
 {
 	pxr::UsdShadeConnectableAPI usdSource;
 	pxr::TfToken usdSourceName;
@@ -494,7 +572,7 @@ IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const px
 	}
 
 	IECoreScene::ShaderNetworkPtr result = new IECoreScene::ShaderNetwork();
-	IECoreScene::ShaderNetwork::Parameter outputHandle = readShaderNetworkWalk( usdSource.GetPrim().GetParent().GetPath(), usdSource.GetOutput( usdSourceName ), *result );
+	IECoreScene::ShaderNetwork::Parameter outputHandle = readShaderNetworkWalk( usdSource.GetPrim().GetParent().GetPath(), usdSource.GetOutput( usdSourceName ), timeCode, *result );
 
 	// If the output shader has type "ai:shader" then set its type to
 	// "ai:surface" or "ai:light" as appropriate. This is just a heuristic,
@@ -529,6 +607,23 @@ IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const px
 	return result;
 }
 
+bool IECoreUSD::ShaderAlgo::shaderNetworkMightBeTimeVarying( 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;
+	}
+
+	std::unordered_set<pxr::UsdPrim, pxr::TfHash> visited;
+	return shaderNetworkMightBeTimeVaryingWalk( usdSource, visited );
+}
+
 #if PXR_VERSION >= 2111
 
 // This is very similar to `writeShaderNetwork` but with these key differences :
@@ -587,13 +682,19 @@ void IECoreUSD::ShaderAlgo::writeLight( const IECoreScene::ShaderNetwork *shader
 	writeShaderConnections( shaderNetwork, usdShaders );
 }
 
-IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readLight( const pxr::UsdLuxLightAPI &light )
+IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readLight( const pxr::UsdLuxLightAPI &light, pxr::UsdTimeCode timeCode )
 {
 	IECoreScene::ShaderNetworkPtr result = new IECoreScene::ShaderNetwork();
-	IECoreScene::ShaderNetwork::Parameter lightHandle = readShaderNetworkWalk( light.GetPath().GetParentPath(), pxr::UsdShadeConnectableAPI( light ), *result );
+	IECoreScene::ShaderNetwork::Parameter lightHandle = readShaderNetworkWalk( light.GetPath().GetParentPath(), pxr::UsdShadeConnectableAPI( light ), timeCode, *result );
 	result->setOutput( lightHandle );
 	IECoreScene::ShaderNetworkAlgo::removeComponentConnectionAdapters( result.get() );
 	return result;
 }
 
+bool IECoreUSD::ShaderAlgo::lightMightBeTimeVarying( const pxr::UsdLuxLightAPI &light )
+{
+	std::unordered_set<pxr::UsdPrim, pxr::TfHash> visited;
+	return shaderNetworkMightBeTimeVaryingWalk( pxr::UsdShadeConnectableAPI( light ), visited );
+}
+
 #endif

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -531,43 +531,60 @@ std::tuple<pxr::TfToken, pxr::TfToken> materialOutputAndPurpose( const std::stri
 	return { AttributeAlgo::nameToUSD( attributeName ).name, pxr::UsdShadeTokens->allPurpose };
 }
 
-/// SdfPath is the appropriate cache key for _storage_, but we need a
-/// `UsdShadeOutput` for computation. This struct provides the implicit
-/// conversion that LRUCache needs to make that possible.
-struct ShaderNetworkCacheGetterKey : public pxr::UsdShadeOutput
+using ShaderNetworkCacheKey = std::pair<pxr::SdfPath, pxr::UsdTimeCode>;
+
+struct ShaderNetworkCacheGetterKey : public ShaderNetworkCacheKey
 {
-	ShaderNetworkCacheGetterKey( const pxr::UsdShadeOutput &output )
-		:	pxr::UsdShadeOutput( output )
+	ShaderNetworkCacheGetterKey( const pxr::UsdShadeOutput &output, pxr::UsdTimeCode time = pxr::UsdTimeCode::Default() )
+		:	ShaderNetworkCacheKey( output.GetAttr().GetPath(), time ), output( output )
 	{
 	}
 
-	operator pxr::SdfPath () const
-	{
-		return GetAttr().GetPath();
-	}
+	const pxr::UsdShadeOutput output;
 };
 
-class ShaderNetworkCache : public LRUCache<pxr::SdfPath, IECoreScene::ConstShaderNetworkPtr, LRUCachePolicy::Parallel, ShaderNetworkCacheGetterKey>
+class ShaderNetworkCache : public LRUCache<ShaderNetworkCacheKey, IECoreScene::ConstShaderNetworkPtr, LRUCachePolicy::Parallel, ShaderNetworkCacheGetterKey>
 {
 
 	public :
 
 		ShaderNetworkCache( size_t maxBytes )
-			:	LRUCache<pxr::SdfPath, IECoreScene::ConstShaderNetworkPtr, LRUCachePolicy::Parallel, ShaderNetworkCacheGetterKey>( getter, maxBytes )
+			:	LRUCache<ShaderNetworkCacheKey, IECoreScene::ConstShaderNetworkPtr, LRUCachePolicy::Parallel, ShaderNetworkCacheGetterKey>( getter, maxBytes )
 		{
 		}
 
 	private :
 
 		static IECoreScene::ConstShaderNetworkPtr getter( const ShaderNetworkCacheGetterKey &key, size_t &cost )
 		{
-			IECoreScene::ConstShaderNetworkPtr result = ShaderAlgo::readShaderNetwork( key );
+			IECoreScene::ConstShaderNetworkPtr result = ShaderAlgo::readShaderNetwork( key.output, key.second );
 			cost = result ? result ->Object::memoryUsage() : 0;
 			return result;
 		}
 
 };
 
+class ShaderNetworkMightBeTimeVaryingCache : public LRUCache<ShaderNetworkCacheKey, bool, LRUCachePolicy::Parallel, ShaderNetworkCacheGetterKey>
+{
+
+	public :
+
+		ShaderNetworkMightBeTimeVaryingCache( size_t maxBytes )
+			:	LRUCache<ShaderNetworkCacheKey, bool, LRUCachePolicy::Parallel, ShaderNetworkCacheGetterKey>( getter, maxBytes )
+		{
+		}
+
+	private :
+
+		static bool getter( const ShaderNetworkCacheGetterKey &key, size_t &cost )
+		{
+			bool result = ShaderAlgo::shaderNetworkMightBeTimeVarying( key.output );
+			cost = 1;
+			return result;
+		}
+
+};
+
 Imath::M44d localTransform( const pxr::UsdPrim &prim, pxr::UsdTimeCode time )
 {
 	pxr::UsdGeomXformable transformable( prim );
@@ -669,6 +686,7 @@ class USDScene::IO : public RefCounted
 				m_rootPrim( m_stage->GetPseudoRoot() ),
 				m_timeCodesPerSecond( m_stage->GetTimeCodesPerSecond() ),
 				m_shaderNetworkCache( 10 * 1024 * 1024 ), // 10Mb
+				m_shaderNetworkMightBeTimeVaryingCache( 1000 ),
 				m_uniqueId( g_usdFileCounter.fetch_add( 1, std::memory_order_relaxed ) )
 		{
 			// Although the USD API implies otherwise, we need a different
@@ -802,9 +820,23 @@ class USDScene::IO : public RefCounted
 			return material;
 		}
 
-		IECoreScene::ConstShaderNetworkPtr readShaderNetwork( const pxr::UsdShadeOutput &output )
+		bool shaderNetworkMightBeTimeVarying( const pxr::UsdShadeOutput &output )
 		{
-			return m_shaderNetworkCache.get( output );
+			return m_shaderNetworkMightBeTimeVaryingCache.get( output );
+		}
+
+		IECoreScene::ConstShaderNetworkPtr readShaderNetwork( const pxr::UsdShadeOutput &output, pxr::UsdTimeCode timeCode )
+		{
+			if( shaderNetworkMightBeTimeVarying( output ) )
+			{
+				return m_shaderNetworkCache.get( ShaderNetworkCacheGetterKey( output, timeCode ) );
+			}
+			else
+			{
+				// Omit time, so we don't produce duplicate cache entries
+				// for non-time-varying shaders.
+				return m_shaderNetworkCache.get( output );
+			}
 		}
 
 		inline int uniqueId()
@@ -863,6 +895,7 @@ class USDScene::IO : public RefCounted
 		pxr::UsdShadeMaterialBindingAPI::CollectionQueryCache m_usdCollectionQueryCache;
 
 		ShaderNetworkCache m_shaderNetworkCache;
+		ShaderNetworkMightBeTimeVaryingCache m_shaderNetworkMightBeTimeVaryingCache;
 
 		// Used to identify a file uniquely ( including between different openings of the same filename,
 		// since closing and reopening a file may cause USD to shuffle the contents ).
@@ -1234,7 +1267,7 @@ ConstObjectPtr USDScene::readAttribute( const SceneInterface::Name &name, double
 #if PXR_VERSION >= 2111
 	else if( name == g_lightAttributeName )
 	{
-		return ShaderAlgo::readLight( pxr::UsdLuxLightAPI( m_location->prim ) );
+		return ShaderAlgo::readLight( pxr::UsdLuxLightAPI( m_location->prim ), m_root->timeCode( time ) );
 	}
 #endif
 	else if( name == g_kindAttributeName )
@@ -1267,7 +1300,7 @@ ConstObjectPtr USDScene::readAttribute( const SceneInterface::Name &name, double
 		{
 			if( pxr::UsdShadeOutput o = mat.GetOutput( output ) )
 			{
-				return m_root->readShaderNetwork( o );
+				return m_root->readShaderNetwork( o, m_root->timeCode( time ) );
 			}
 		}
 		return nullptr;
@@ -1719,8 +1752,7 @@ void USDScene::attributesHash( double time, IECore::MurmurHash &h ) const
 #if PXR_VERSION >= 2111
 	if( m_location->prim.HasAPI<pxr::UsdLuxLightAPI>() )
 	{
-		/// \todo Consider time-varying lights - see comment below
-		/// for materials.
+		mightBeTimeVarying = mightBeTimeVarying || ShaderAlgo::lightMightBeTimeVarying( pxr::UsdLuxLightAPI( m_location->prim ) );
 		haveAttributes = true;
 	}
 #endif
@@ -1751,9 +1783,11 @@ void USDScene::attributesHash( double time, IECore::MurmurHash &h ) const
 	{
 		if( pxr::UsdShadeMaterial mat = m_root->computeBoundMaterial( m_location->prim, purpose ) )
 		{
-			// \todo - This does not consider the possibility that the material could contain time-varying
-			// attributes
 			append( mat.GetPrim().GetPath(), h );
+			for( pxr::UsdShadeOutput &o : mat.GetOutputs( /* onlyAuthored = */ true ) )
+			{
+				mightBeTimeVarying = mightBeTimeVarying || m_root->shaderNetworkMightBeTimeVarying( o );
+			}
 			haveMaterials = true;
 		}
 	}