Merge branch 'RB-10.6'

Author: murraystevenson

Changes

@@ -1,7 +1,12 @@
 10.7.x.x (relative to 10.7.0.0a11)
 ========
 
+Improvements
+------------
 
+- USDScene : Added `IECOREUSD_WRITE_CONFORMANT_OSL_SHADERS` environment variable. If set to a value of `1`, this causes OSL shaders to be written in the format expected by RenderMan. Set the `RMAN_SHADERPATH` environment variable appropriately to ensure RenderMan can find the shaders [^1].
+
+[^1]: To be omitted from the notes for the final 10.7.0.0 release.
 
 10.7.0.0a11 (relative to 10.7.0.0a10)
 ===========
@@ -223,10 +228,18 @@ Breaking Changes
   - Removed deprecated functions componentConnectionAdapterLabel, convertOSLComponentConnections, collapseSplineParameters, expandSplineParameters.
   - Switch support for Spline parameters to support for Ramp parameters.
 
-10.6.x.x (relative to 10.6.5.0)
+10.6.x.x (relative to 10.6.6.0)
+========
+
+
+
+10.6.6.0 (relative to 10.6.5.0)
 ========
 
+Improvements
+------------
 
+- USDScene : Added `IECOREUSD_WRITE_CONFORMANT_OSL_SHADERS` environment variable. If set to a value of `1`, this causes OSL shaders to be written in the format expected by RenderMan. Set the `RMAN_SHADERPATH` environment variable appropriately to ensure RenderMan can find the shaders.
 
 10.6.5.0 (relative to 10.6.4.0)
 ========

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -55,35 +55,78 @@
 #include "boost/algorithm/string/replace.hpp"
 #include "boost/pointer_cast.hpp"
 
+#include <filesystem>
 #include <regex>
 
 namespace
 {
 
 const pxr::TfToken g_blindDataToken( "cortex:blindData" );
+const pxr::TfToken g_shaderNameToken( "cortex:shaderName" );
+const pxr::TfToken g_shaderTypeToken( "cortex:shaderType" );
 
 // Hardcoded to match an old name used by Cortex when writing USD with OSL version earlier than 1.10 (
 // ie. a pre-2021 version of gafferDependencies ). We no longer expose this in the API, but I'm not sure
 // if we're ready to drop support for loading these old files.
 IECore::InternedString g_legacyAdapterLabelString( "cortex_autoAdapter" );
 pxr::TfToken g_legacyAdapterLabelToken( g_legacyAdapterLabelString.string() );
 
-std::pair<pxr::TfToken, std::string> shaderIdAndType( const pxr::UsdShadeConnectableAPI &connectable )
+bool writeConformantOSLShaders()
+{
+	if( const char *e = getenv( "IECOREUSD_WRITE_CONFORMANT_OSL_SHADERS" ) )
+	{
+		return strcmp( e, "0" );
+	}
+	return false;
+}
+
+// Recover a Cortex-style shader name and type, essentially reversing the
+// transformation done by `createShaderPrim()`.
+std::pair<std::string, std::string> shaderNameAndType( const pxr::UsdShadeConnectableAPI &connectable )
 {
-	pxr::TfToken id;
-	std::string type;
 	if( auto shader = pxr::UsdShadeShader( connectable ) )
 	{
-		shader.GetShaderId( &id );
-		type = "surface";
+		std::string name;
+		std::string type;
+
+		pxr::VtValue nameVtValue = shader.GetPrim().GetCustomDataByKey( g_shaderNameToken );
+		if( !nameVtValue.IsEmpty()  )
+		{
+			name = nameVtValue.Get<std::string>();
+		}
+		else
+		{
+			pxr::TfToken id;
+			shader.GetShaderId( &id );
+			name = id.GetString();
+			type = "surface";
+			const size_t colonPos = name.find( ":" );
+			if( colonPos != std::string::npos )
+			{
+				std::string prefix = name.substr( 0, colonPos );
+				name = name.substr( colonPos + 1 );
+				if( prefix == "arnold" )
+				{
+					prefix = "ai";
+				}
+				type = prefix + ":shader";
+			}
+		}
+
+		pxr::VtValue typeVtValue = shader.GetPrim().GetCustomDataByKey( g_shaderTypeToken );
+		if( !typeVtValue.IsEmpty() )
+		{
+			type = typeVtValue.Get<std::string>();
+		}
+
+		return { name, type };
 	}
 	else if( auto light = pxr::UsdLuxLightAPI( connectable ) )
 	{
-		light.GetShaderIdAttr().Get( &id );
-		type = "light";
+		return { light.GetShaderId( {} ).GetString(), "light" };
 	}
 
-	return std::make_pair( id, type );
+	return { "", "" };
 }
 
 bool writeNonStandardLightParameter( const std::string &name, const IECore::Data *value, pxr::UsdShadeConnectableAPI usdShader )
@@ -225,24 +268,7 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 		return handle;
 	}
 
-	auto [id, shaderType] = shaderIdAndType( usdShader );
-	std::string shaderName = "defaultsurface";
-	if( id.size() )
-	{
-		std::string name = id.GetString();
-		size_t colonPos = name.find( ":" );
-		if( colonPos != std::string::npos )
-		{
-			std::string prefix = name.substr( 0, colonPos );
-			name = name.substr( colonPos + 1 );
-			if( prefix == "arnold" )
-			{
-				prefix = "ai";
-			}
-			shaderType = prefix + ":shader";
-		}
-		shaderName = name;
-	}
+	// Read parameter values and connections.
 
 	IECore::CompoundDataPtr parametersData = new IECore::CompoundData();
 	IECore::CompoundDataMap &parameters = parametersData->writable();
@@ -288,6 +314,9 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 
 	readNonStandardLightParameters( usdShader.GetPrim(), parameters );
 
+	// Create shader.
+
+	auto [shaderName, shaderType] = shaderNameAndType( usdShader );
 	IECoreScene::ShaderPtr newShader = new IECoreScene::Shader( shaderName, shaderType, parametersData );
 
 	// General purpose support for any Cortex blind data.
@@ -388,35 +417,73 @@ pxr::UsdShadeConnectableAPI createShaderPrim( const IECoreScene::Shader *shader,
 		throw IECore::Exception( "Could not create shader at " + path.GetAsString() );
 	}
 
-	const std::string type = shader->getType();
+	// We need to declare the shader in a form corresponding to entries in USD's
+	// Sdr registry, so that if rendered in `usdview` or another Hydra-based
+	// app, the render delegates can find the shaders. We could potentially do
+	// that done by finding a shader in the Sdr registry by name and then using
+	// `SdrShaderNode.GetIdentifier()` or
+	// `SdrShaderNode.GetResolvedImplementationURI()`, although it's not clear
+	// how you'd decide which to use. Anyway, for now at least, we want to be
+	// able to operate in environments without renderer-specific USD plugins
+	// installed. So instead of querying the registry we use some heuristics of
+	// our own.
 
-	std::string typePrefix;
 	if( boost::starts_with( shader->getName(), "Pxr" ) || boost::starts_with( shader->getName(), "Lama" ) )
 	{
-		// Leave the type prefix empty. This should be the default, but we are currently only doing this
-		// for a small number of shaders that we can be completely confident require it, in order to
-		// preserve backwards compatibility.
+		// Could be either an OSL or a C++ shader, but either way, RenderMan
+		// registers it in the SdrRegistry by name.
+		usdShader.SetShaderId( pxr::TfToken( shader->getName() ) );
+	}
+	else if( boost::starts_with( shader->getType(), "ai:" ) )
+	{
+		// Arnold registers all plugins at startup, so also only needs the name
+		// to be able to create a shader. It uses an `arnold:` prefix to avoid
+		// clashes with the names of other shaders.
+		usdShader.SetShaderId( pxr::TfToken( "arnold:" + shader->getName() ) );
+	}
+	else if(
+		boost::starts_with( shader->getType(), "osl:" ) &&
+		writeConformantOSLShaders()
+	)
+	{
+		// Arbitrary OSL shader. Arnold would want that written as an
+		// `arnold:osl` shader with `input:shadername` pointing to the shader.
+		// But that will never work in another renderer. RenderMan takes a
+		// slightly more generic approach by registering each OSL shader from
+		// `RMAN_SHADERPATH` into the Sdr registry, so we follow that in the hope
+		// that Arnold and other renderers might fall in line in future.
+		if( shader->getName().find( '/' ) != std::string::npos )
+		{
+			// Unfortunately, RenderMan's SdrDiscoveryPlugin uses only the leaf
+			// name, even though the Riley API will accept `{directory}/{file}`.
+			// So we write the leaf name, and accept that we can no longer
+			// round-trip exactly (we are also losing the shader type).
+			//
+			// Our plan is that in future we'll flatten our shader libraries
+			// into a single directory, and remove the few remaining spots in
+			// the codebase that rely on `Shader::getType()`.
+			usdShader.SetShaderId( pxr::TfToken( std::filesystem::path( shader->getName() ).stem().string() ) );
+		}
+		else
+		{
+			usdShader.SetShaderId( pxr::TfToken( shader->getName() ) );
+		}
 	}
 	else
 	{
-		size_t typeColonPos = type.find( ":" );
+		const std::string &type = shader->getType();
+		const size_t typeColonPos = type.find( ":" );
 		if( typeColonPos != std::string::npos )
 		{
-			// According to our current understanding, this is almost completely wrong. Renderer's like
-			// PRMan won't accept shaders with type prefixes, and Arnold apparently requires all shaders
-			// to be prefixed with "arnold:", including OSL. This code prefixes OSL shaders with "osl:",
-			// which fails in all renderers we're aware of - we're keeping this behaviour for now for
-			// backwards compatibility reasons.
-			typePrefix = type.substr( 0, typeColonPos ) + ":";
-
-			// This is the one case that actually works
-			if( typePrefix == "ai:" )
-			{
-				typePrefix = "arnold:";
-			}
+			// We don't currently know of any renderers where this is right, but
+			// it is our historic behaviour, which we are keeping until we know better.
+			usdShader.SetShaderId( pxr::TfToken( type.substr( 0, typeColonPos ) + ":" + shader->getName() ) );
+		}
+		else
+		{
+			usdShader.SetShaderId( pxr::TfToken( shader->getName() ) );
 		}
 	}
-	usdShader.SetShaderId( pxr::TfToken( typePrefix + shader->getName() ) );
 
 	return usdShader.ConnectableAPI();
 }

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -485,7 +485,6 @@ void populateMaterial( pxr::UsdShadeMaterial &mat, const boost::container::flat_
 	for( const auto &[output, shaderNetwork] : shaders )
 	{
 		pxr::UsdShadeOutput matOutput = mat.CreateOutput( output, pxr::SdfValueTypeNames->Token );
-
 		std::string shaderContainerName = boost::replace_all_copy( output.GetString(), ":", "_" ) + "_shaders";
 		pxr::UsdGeomScope shaderContainer = pxr::UsdGeomScope::Define( mat.GetPrim().GetStage(), mat.GetPath().AppendChild( pxr::TfToken( shaderContainerName ) ) );
 		pxr::UsdShadeOutput networkOut = ShaderAlgo::writeShaderNetwork( shaderNetwork.get(), shaderContainer.GetPrim() );

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -4967,5 +4967,78 @@ def testMaterialTerminalFromSubgraph( self ) :
 
 		self.assertEqual( shaderNetwork.outputShader().name, "LamaSurface" )
 
+	def testOSLShaderForHDPrman( self ) :
+
+		self.addCleanup( os.environ.__delitem__, "IECOREUSD_WRITE_CONFORMANT_OSL_SHADERS" )
+
+		for conformant in True, False :
+
+			with self.subTest( conformant = conformant ) :
+
+				os.environ["IECOREUSD_WRITE_CONFORMANT_OSL_SHADERS"] = str( int( conformant ) )
+
+				fileName = os.path.join( self.temporaryDirectory(), f"testConformance{conformant}.usda" )
+
+				shaderNetwork = IECoreScene.ShaderNetwork(
+					shaders = {
+						"output" : IECoreScene.Shader( "PxrDiffuse", "ri:surface" ),
+						"texture" : IECoreScene.Shader( "Pattern/Noise", "osl:shader" ),
+						"scale" : IECoreScene.Shader( "floatAttribute", "osl:shader" ),
+					},
+					connections = [
+						( ( "scale", "out" ), ( "texture", "scale" ) ),
+						( ( "texture", "out" ), ( "output", "diffuseColor" ) ),
+					],
+					output = ( "output", "bxdf_out" ),
+				)
+
+				# Test writing to USD.
+
+				scene = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+				scene.createChild( "test" ).writeAttribute( "ri:surface", shaderNetwork, 0 )
+				del scene
+
+				stage = pxr.Usd.Stage.Open( fileName )
+
+				outputShader = pxr.UsdShade.Shader(
+					next( p for p in stage.Traverse() if p.IsA( pxr.UsdShade.Shader ) and p.GetName() == "output" )
+				)
+				self.assertEqual( outputShader.GetShaderId(), "PxrDiffuse" )
+
+				textureShader = pxr.UsdShade.Shader(
+					next( p for p in stage.Traverse() if p.IsA( pxr.UsdShade.Shader ) and p.GetName() == "texture" )
+				)
+				if conformant :
+					self.assertEqual( textureShader.GetShaderId(), "Noise" )
+				else :
+					self.assertEqual( textureShader.GetShaderId(), "osl:Pattern/Noise" )
+
+				scaleShader = pxr.UsdShade.Shader(
+					next( p for p in stage.Traverse() if p.IsA( pxr.UsdShade.Shader ) and p.GetName() == "scale" )
+				)
+				if conformant :
+					self.assertEqual( scaleShader.GetShaderId(), "floatAttribute" )
+				else :
+					self.assertEqual( scaleShader.GetShaderId(), "osl:floatAttribute" )
+
+				# Test loading back to Cortex.
+
+				scene = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+				loadedShaderNetwork = scene.child( "test" ).readAttribute( "ri:surface", 0 )
+
+				if conformant :
+					# Can't round trip. We will need to adjust our usage to avoid shaders
+					# nested in directories.
+					self.assertEqual( loadedShaderNetwork.getShader( "texture" ).name, "Noise" )
+					self.assertEqual( loadedShaderNetwork.getShader( "texture" ).type, "surface" )
+					self.assertEqual( loadedShaderNetwork.getShader( "scale" ).name, "floatAttribute" )
+					self.assertEqual( loadedShaderNetwork.getShader( "scale" ).type, "surface" )
+				else :
+					# Round tripped exactly.
+					self.assertEqual( loadedShaderNetwork.getShader( "texture" ).name, "Pattern/Noise" )
+					self.assertEqual( loadedShaderNetwork.getShader( "texture" ).type, "osl:shader" )
+					self.assertEqual( loadedShaderNetwork.getShader( "scale" ).name, "floatAttribute" )
+					self.assertEqual( loadedShaderNetwork.getShader( "scale" ).type, "osl:shader" )
+
 if __name__ == "__main__":
 	unittest.main()