USDScene : Fix writing of UsdLux lights

Changes

@@ -9,6 +9,7 @@ Improvements
 Fixes
 -----
 
+- USDScene : Fixed writing of lights so they are represented as appropriate `UsdLux*Light` prims in USD.
 - FrameRange : Prevented creation of FrameRanges with negative steps
 - IECore.dataTypeFromElement : Fixed support for list to Vector conversions
 - LinkedScene : Fixed bug where `linkLocations` attribute was baked incorrectly if the link target location wasn't the ROOT

contrib/IECoreUSD/include/IECoreUSD/ShaderAlgo.h

@@ -57,11 +57,13 @@ 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()`.
-IECoreScene::ShaderNetworkPtr readShaderNetwork( const pxr::UsdShadeOutput &output );
+IECOREUSD_API IECoreScene::ShaderNetworkPtr readShaderNetwork( 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.
-IECoreScene::ShaderNetworkPtr readShaderNetwork( const pxr::UsdLuxLightAPI &light );
+IECOREUSD_API IECoreScene::ShaderNetworkPtr readLight( const pxr::UsdLuxLightAPI &light );
 #endif
 
 } // namespace ShaderAlgo

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -43,7 +43,10 @@
 
 #if PXR_VERSION >= 2111
 #include "pxr/usd/usdLux/cylinderLight.h"
+#include "pxr/usd/usdLux/nonboundableLightBase.h"
 #include "pxr/usd/usdLux/sphereLight.h"
+
+#include "pxr/usd/usd/schemaRegistry.h"
 #endif
 
 #include "boost/algorithm/string/replace.hpp"
@@ -217,87 +220,89 @@ IECoreScene::ShaderNetwork::Parameter readShaderNetworkWalk( const pxr::SdfPath
 	}
 }
 
-} // namespace
-
-pxr::UsdShadeOutput IECoreUSD::ShaderAlgo::writeShaderNetwork( const IECoreScene::ShaderNetwork *shaderNetwork, pxr::UsdPrim shaderContainer )
+IECoreScene::ConstShaderNetworkPtr adaptShaderNetworkForWriting( const IECoreScene::ShaderNetwork *shaderNetwork )
 {
-	IECoreScene::ShaderNetworkPtr shaderNetworkWithAdapters = shaderNetwork->copy();
-	IECoreScene::ShaderNetworkAlgo::expandSplines( shaderNetworkWithAdapters.get() );
-	IECoreScene::ShaderNetworkAlgo::addComponentConnectionAdapters( shaderNetworkWithAdapters.get() );
+	IECoreScene::ShaderNetworkPtr result = shaderNetwork->copy();
+	IECoreScene::ShaderNetworkAlgo::expandSplines( result.get() );
+	IECoreScene::ShaderNetworkAlgo::addComponentConnectionAdapters( result.get() );
+	return result;
+}
 
-	IECoreScene::ShaderNetwork::Parameter networkOutput = shaderNetworkWithAdapters->getOutput();
-	if( networkOutput.shader.string() == "" )
+pxr::UsdShadeConnectableAPI createShaderPrim( const IECoreScene::Shader *shader, const pxr::UsdStagePtr &stage, const pxr::SdfPath &path )
+{
+	pxr::UsdShadeShader usdShader = pxr::UsdShadeShader::Define( stage, path );
+	if( !usdShader )
 	{
-		// This could theoretically happen, but a shader network with no output is not useful in any way
-		IECore::msg(
-			IECore::Msg::Warning, "IECoreUSD::ShaderAlgo::writeShaderNetwork",
-			"No output shader in network"
-		);
+		throw IECore::Exception( "Could not create shader at " + path.GetAsString() );
 	}
-
-	pxr::UsdShadeOutput networkOutUsd;
-	for( const auto &shader : shaderNetworkWithAdapters->shaders() )
+	const std::string type = shader->getType();
+	std::string typePrefix;
+	size_t typeColonPos = type.find( ":" );
+	if( typeColonPos != std::string::npos )
 	{
-		pxr::SdfPath usdShaderPath = shaderContainer.GetPath().AppendChild( pxr::TfToken( pxr::TfMakeValidIdentifier( shader.first.string() ) ) );
-		pxr::UsdShadeShader usdShader = pxr::UsdShadeShader::Define( shaderContainer.GetStage(), usdShaderPath );
-		if( !usdShader )
+		typePrefix = type.substr( 0, typeColonPos ) + ":";
+		if( typePrefix == "ai:" )
 		{
-			throw IECore::Exception( "Could not create shader at: " + shaderContainer.GetPath().GetString() + " / " + shader.first.string() );
-		}
-		std::string type = shader.second->getType();
-		std::string typePrefix;
-		size_t typeColonPos = type.find( ":" );
-		if( typeColonPos != std::string::npos )
-		{
-			typePrefix = type.substr( 0, typeColonPos ) + ":";
-			if( typePrefix == "ai:" )
-			{
-				typePrefix = "arnold:";
-			}
+			typePrefix = "arnold:";
 		}
-		usdShader.SetShaderId( pxr::TfToken( typePrefix + shader.second->getName() ) );
+	}
+	usdShader.SetShaderId( pxr::TfToken( typePrefix + shader->getName() ) );
+
+	return usdShader.ConnectableAPI();
+}
 
-		for( const auto &p : shader.second->parametersData()->readable() )
+void writeShaderParameterValues( const IECoreScene::Shader *shader, pxr::UsdShadeConnectableAPI usdShader )
+{
+	for( const auto &p : shader->parametersData()->readable() )
+	{
+		const pxr::TfToken usdParameterName = toUSDParameterName( p.first );
+		pxr::UsdShadeInput input = usdShader.GetInput( usdParameterName );
+		if( !input )
 		{
-			pxr::UsdShadeInput input = usdShader.CreateInput(
+			input = usdShader.CreateInput(
 				toUSDParameterName( p.first ),
-				DataAlgo::valueTypeName( p.second.get() )
+				IECoreUSD::DataAlgo::valueTypeName( p.second.get() )
 			);
-			input.Set( DataAlgo::toUSD( p.second.get() ) );
 		}
-
-		if( networkOutput.shader == shader.first )
+		if( auto *s = IECore::runTimeCast<IECore::StringData>( p.second.get() ) )
 		{
-			pxr::TfToken outName( networkOutput.name.string() );
-			if( outName.GetString().size() == 0 )
+			// USD has several "stringy" types - convert if necessary.
+			if( input.GetTypeName() == pxr::SdfValueTypeNames->Token )
 			{
-				outName = pxr::TfToken( "DEFAULT_OUTPUT" );
+				input.Set( pxr::TfToken( s->readable() ) );
+				continue;
+			}
+			else if( input.GetTypeName().GetType().IsA<pxr::SdfAssetPath>() )
+			{
+				input.Set( pxr::SdfAssetPath( s->readable() ) );
+				continue;
 			}
-
-			// \todo - we should probably be correctly tracking the output type if it is typed?
-			// Currently, we don't really track output types in Gaffer.
-			networkOutUsd = usdShader.CreateOutput( outName, pxr::SdfValueTypeNames->Token );
 		}
+		input.Set( IECoreUSD::DataAlgo::toUSD( p.second.get() ) );
+	}
 
-		const IECore::BoolData* adapterMeta = shader.second->blindData()->member<IECore::BoolData>( IECoreScene::ShaderNetworkAlgo::componentConnectionAdapterLabel() );
-		if( adapterMeta && adapterMeta->readable() )
-		{
-			usdShader.GetPrim().SetMetadata( g_adapterLabelToken, true );
-		}
+	const IECore::BoolData *adapterMeta = shader->blindData()->member<IECore::BoolData>( IECoreScene::ShaderNetworkAlgo::componentConnectionAdapterLabel() );
+	if( adapterMeta && adapterMeta->readable() )
+	{
+		usdShader.GetPrim().SetMetadata( g_adapterLabelToken, true );
 	}
+}
 
-	for( const auto &shader : shaderNetworkWithAdapters->shaders() )
+using ShaderMap = std::unordered_map<IECore::InternedString, pxr::UsdShadeConnectableAPI>;
+void writeShaderConnections( const IECoreScene::ShaderNetwork *shaderNetwork, const ShaderMap &usdShaders )
+{
+	for( const auto &shader : shaderNetwork->shaders() )
 	{
-		pxr::UsdShadeShader usdShader = pxr::UsdShadeShader::Get( shaderContainer.GetStage(), shaderContainer.GetPath().AppendChild( pxr::TfToken( pxr::TfMakeValidIdentifier( shader.first.string() ) ) ) );
-		for( const auto &c : shaderNetworkWithAdapters->inputConnections( shader.first ) )
+		pxr::UsdShadeConnectableAPI usdShader = usdShaders.at( shader.first );
+		for( const auto &c : shaderNetwork->inputConnections( shader.first ) )
 		{
 			pxr::UsdShadeInput dest = usdShader.GetInput( pxr::TfToken( c.destination.name.string() ) );
-			if( ! dest.GetPrim().IsValid() )
+			if( !dest )
 			{
 				dest = usdShader.CreateInput( toUSDParameterName( c.destination.name ), pxr::SdfValueTypeNames->Token );
 			}
 
-			pxr::UsdShadeShader sourceUsdShader = pxr::UsdShadeShader::Get( shaderContainer.GetStage(), shaderContainer.GetPath().AppendChild( pxr::TfToken( pxr::TfMakeValidIdentifier( c.source.shader.string() ) ) ) );
+			pxr::UsdShadeShader sourceUsdShader = usdShaders.at( c.source.shader );
 			std::string sourceOutputName = c.source.name.string();
 			if( sourceOutputName.size() == 0 )
 			{
@@ -306,9 +311,51 @@ pxr::UsdShadeOutput IECoreUSD::ShaderAlgo::writeShaderNetwork( const IECoreScene
 			pxr::UsdShadeOutput source = sourceUsdShader.CreateOutput( pxr::TfToken( sourceOutputName ), dest.GetTypeName() );
 			dest.ConnectToSource( source );
 		}
+	}
+}
+
+} // namespace
+
+pxr::UsdShadeOutput IECoreUSD::ShaderAlgo::writeShaderNetwork( const IECoreScene::ShaderNetwork *shaderNetwork, pxr::UsdPrim shaderContainer )
+{
+	IECoreScene::ConstShaderNetworkPtr adaptedNetwork = adaptShaderNetworkForWriting( shaderNetwork );
+	shaderNetwork = adaptedNetwork.get();
 
+	IECoreScene::ShaderNetwork::Parameter networkOutput = shaderNetwork->getOutput();
+	if( networkOutput.shader.string() == "" )
+	{
+		// This could theoretically happen, but a shader network with no output is not useful in any way
+		IECore::msg(
+			IECore::Msg::Warning, "IECoreUSD::ShaderAlgo::writeShaderNetwork",
+			"No output shader in network"
+		);
+	}
+
+	ShaderMap usdShaders;
+	pxr::UsdShadeOutput networkOutUsd;
+	for( const auto &shader : shaderNetwork->shaders() )
+	{
+		const pxr::SdfPath usdShaderPath = shaderContainer.GetPath().AppendChild( pxr::TfToken( pxr::TfMakeValidIdentifier( shader.first.string() ) ) );
+		pxr::UsdShadeConnectableAPI usdShader = createShaderPrim( shader.second.get(), shaderContainer.GetStage(), usdShaderPath );
+		writeShaderParameterValues( shader.second.get(), usdShader );
+		usdShaders[shader.first] = usdShader;
+
+		if( networkOutput.shader == shader.first )
+		{
+			pxr::TfToken outName( networkOutput.name.string() );
+			if( outName.GetString().size() == 0 )
+			{
+				outName = pxr::TfToken( "DEFAULT_OUTPUT" );
+			}
+
+			// \todo - we should probably be correctly tracking the output type if it is typed?
+			// Currently, we don't really track output types in Gaffer.
+			networkOutUsd = usdShader.CreateOutput( outName, pxr::SdfValueTypeNames->Token );
+		}
 	}
 
+	writeShaderConnections( shaderNetwork, usdShaders );
+
 	return networkOutUsd;
 }
 
@@ -357,7 +404,63 @@ IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const px
 
 #if PXR_VERSION >= 2111
 
-IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readShaderNetwork( const pxr::UsdLuxLightAPI &light )
+// This is very similar to `writeShaderNetwork` but with these key differences :
+//
+// - The output shader is written as a UsdLight-derived prim rather than a UsdShadeShader.
+// - The other shaders are parented inside the light.
+// - We don't need to create a UsdShadeOutput to return.
+void IECoreUSD::ShaderAlgo::writeLight( const IECoreScene::ShaderNetwork *shaderNetwork, pxr::UsdPrim prim )
+{
+	IECoreScene::ConstShaderNetworkPtr adaptedNetwork = adaptShaderNetworkForWriting( shaderNetwork );
+	shaderNetwork = adaptedNetwork.get();
+
+	// Verify that the light shader corresponds to a valid USD light type.
+
+	const IECoreScene::Shader *outputShader = shaderNetwork->outputShader();
+	if( !outputShader )
+	{
+		IECore::msg( IECore::Msg::Warning, "ShaderAlgo::writeLight", "No output shader" );
+		return;
+	}
+
+	pxr::TfType type = pxr::UsdSchemaRegistry::GetInstance().GetTypeFromName( pxr::TfToken( outputShader->getName() ) );
+	if(
+		!type.IsA<pxr::UsdLuxBoundableLightBase>() &&
+		!type.IsA<pxr::UsdLuxNonboundableLightBase>()
+	)
+	{
+		IECore::msg( IECore::Msg::Warning, "ShaderAlgo::writeLight", boost::format( "Shader `%1%` is not a valid UsdLux light type" ) % outputShader->getName() );
+		return;
+	}
+
+	// Write the light itself onto the prim we've been given.
+
+	ShaderMap usdShaders;
+	prim.SetTypeName( pxr::TfToken( outputShader->getName() ) );
+	writeShaderParameterValues( outputShader, pxr::UsdShadeConnectableAPI( prim ) );
+	usdShaders[shaderNetwork->getOutput().shader] = pxr::UsdShadeConnectableAPI( prim );
+
+	// Then write any other shaders as child prims so they are
+	// encapsulated within the light.
+
+	for( const auto &shader : shaderNetwork->shaders() )
+	{
+		if( shader.second == outputShader )
+		{
+			continue;
+		}
+		const pxr::SdfPath usdShaderPath = prim.GetPath().AppendChild( pxr::TfToken( pxr::TfMakeValidIdentifier( shader.first.string() ) ) );
+		pxr::UsdShadeConnectableAPI usdShader = createShaderPrim( shader.second.get(), prim.GetStage(), usdShaderPath );
+		writeShaderParameterValues( shader.second.get(), usdShader );
+		usdShaders[shader.first] = usdShader;
+	}
+
+	// Finally, connect everything up.
+
+	writeShaderConnections( shaderNetwork, usdShaders );
+}
+
+IECoreScene::ShaderNetworkPtr IECoreUSD::ShaderAlgo::readLight( const pxr::UsdLuxLightAPI &light )
 {
 	IECoreScene::ShaderNetworkPtr result = new IECoreScene::ShaderNetwork();
 	IECoreScene::ShaderNetwork::Parameter lightHandle = readShaderNetworkWalk( light.GetPath().GetParentPath(), pxr::UsdShadeConnectableAPI( light ), *result );

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -1087,7 +1087,7 @@ ConstObjectPtr USDScene::readAttribute( const SceneInterface::Name &name, double
 #if PXR_VERSION >= 2111
 	else if( name == g_lightAttributeName )
 	{
-		return ShaderAlgo::readShaderNetwork( pxr::UsdLuxLightAPI( m_location->prim ) );
+		return ShaderAlgo::readLight( pxr::UsdLuxLightAPI( m_location->prim ) );
 	}
 #endif
 	else if( name == g_kindAttributeName )
@@ -1187,8 +1187,15 @@ void USDScene::writeAttribute( const SceneInterface::Name &name, const Object *a
 	}
 	else if( const IECoreScene::ShaderNetwork *shaderNetwork = runTimeCast<const ShaderNetwork>( attribute ) )
 	{
-		const auto &[output, purpose] = materialOutputAndPurpose( name.string() );
-		m_materials[purpose][output] = shaderNetwork;
+		if( name == g_lightAttributeName )
+		{
+			ShaderAlgo::writeLight( shaderNetwork, m_location->prim );
+		}
+		else
+		{
+			const auto &[output, purpose] = materialOutputAndPurpose( name.string() );
+			m_materials[purpose][output] = shaderNetwork;
+		}
 	}
 	else if( name.string() == "gaffer:globals" )
 	{