Merge pull request #1543 from johnhaddon/renderManAttributesFix

IECoreUSD : Conform writing of `ri:` attributes to match `usdRiPxr`

Author: johnhaddon

Changes

@@ -1,7 +1,10 @@
 10.6.x.x (relative to 10.6.6.0)
 ========
 
+Improvements
+------------
 
+- USDScene : Added `IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES` environment variable. If set to a value of `1`, this causes "ri:" prefixed attributes to be written as expected by `usdRiPxr`.
 
 10.6.6.0 (relative to 10.6.5.0)
 ========

contrib/IECoreUSD/src/IECoreUSD/AttributeAlgo.cpp

@@ -55,8 +55,19 @@ static const pxr::TfToken g_cortexPrimitiveVariableMetadataTokenDeprecated( "IEC
 static const std::string g_primVarPrefix = "primvars:";
 static const std::string g_primVarUserPrefix = "primvars:user:";
 static const std::string g_renderPrefix = "render:";
+static const std::string g_riPrefix = "ri:";
+static const std::string g_riAttributesPrefix = "ri:attributes:";
 static const std::string g_userPrefix = "user:";
 
+bool writeConformantRenderManAttributes()
+{
+	if( const char *e = getenv( "IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES" ) )
+	{
+		return strcmp( e, "0" );
+	}
+	return false;
+}
+
 }
 
 bool IECoreUSD::AttributeAlgo::isCortexAttribute( const pxr::UsdGeomPrimvar &primVar )
@@ -122,12 +133,19 @@ pxr::TfToken IECoreUSD::AttributeAlgo::cortexPrimitiveVariableMetadataTokenDepre
 
 IECoreUSD::AttributeAlgo::Name IECoreUSD::AttributeAlgo::nameToUSD( std::string name )
 {
+	if( boost::starts_with( name, g_riPrefix ) && writeConformantRenderManAttributes() )
+	{
+		return { pxr::TfToken( g_riAttributesPrefix + name.substr( g_riPrefix.size() ) ), true };
+	}
+
 	bool isPrimvar = false;
 
 	// The long term plan is to convert only "render:" prefixed attributes to primvars, and it will
 	// be the client's responsibility to ensure everything important gets prefixed with "render:".
 	// But for the moment, Gaffer doesn't do this yet, so we support the two most important prefixes
-	// for Gaffer currently:  "user:" and "ai:"
+	// for Gaffer currently:  "user:" and "ai:".
+	/// \todo I don't think the `render:` plan is working out - it may well be better to just map
+	/// all Cortex attributes to primvars.
 	if( boost::starts_with( name, "render:" ) || boost::starts_with( name, "user:" ) || boost::starts_with( name, "ai:" ) )
 	{
 		isPrimvar = true;
@@ -170,7 +188,12 @@ IECoreUSD::AttributeAlgo::Name IECoreUSD::AttributeAlgo::nameToUSD( std::string
 IECore::InternedString IECoreUSD::AttributeAlgo::nameFromUSD( IECoreUSD::AttributeAlgo::Name name )
 {
 	std::string nameStr = name.name;
-	if( nameStr == "arnold:displacement" )
+
+	if( boost::starts_with( nameStr, g_riAttributesPrefix ) )
+	{
+		return g_riPrefix + nameStr.substr( g_riAttributesPrefix.size() );
+	}
+	else if( nameStr == "arnold:displacement" )
 	{
 		// Special case where the whole name is different, not just prefix
 		nameStr = "ai:disp_map";
@@ -198,7 +221,9 @@ IECore::InternedString IECoreUSD::AttributeAlgo::nameFromUSD( IECoreUSD::Attribu
 
 	// The long term plan is to always prefix primitive variables converted to attributes with "render:".
 	// But for the moment, Gaffer doesn't support this, so we skip the prefix for the two most important prefixes
-	// for Gaffer currently:  "user:" and "ai:"
+	// for Gaffer currently:  "user:" and "ai:".
+	/// \todo I don't think the `render:` plan is working out - it may well be better to just map
+	/// all Cortex attributes to primvars.
 	if ( !boost::starts_with( nameStr, g_userPrefix ) && !boost::starts_with( nameStr, "ai:" ) && name.isPrimvar )
 	{
 		nameStr = "render:" + nameStr;

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -4868,5 +4868,44 @@ def testOSLShaderForHDPrman( self ) :
 					self.assertEqual( loadedShaderNetwork.getShader( "scale" ).name, "floatAttribute" )
 					self.assertEqual( loadedShaderNetwork.getShader( "scale" ).type, "osl:shader" )
 
+	def testRenderManAttributeRoundTrip( self ) :
+
+		self.addCleanup( os.environ.__delitem__, "IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES" )
+
+		for conformant in True, False :
+
+			with self.subTest( conformant = conformant ) :
+
+				os.environ["IECOREUSD_WRITE_CONFORMANT_RENDERMAN_ATTRIBUTES"] = str( int( conformant ) )
+
+				fileName = os.path.join( self.temporaryDirectory(), f"renderManAttributes{conformant}.usda" )
+
+				# Test writing to USD.
+
+				scene = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+				child = scene.createChild( "test" )
+				child.writeAttribute( "ri:trace:maxdiffusedepth", IECore.IntData( 2 ), 0 )
+				del scene, child
+
+				stage = pxr.Usd.Stage.Open( fileName )
+
+				if conformant :
+					primVars = pxr.UsdGeom.PrimvarsAPI( stage.GetPrimAtPath( "/test" ) )
+					diffuseDepth = primVars.GetPrimvar( "ri:attributes:trace:maxdiffusedepth" )
+					self.assertTrue( diffuseDepth.IsDefined() )
+					self.assertEqual( diffuseDepth.GetInterpolation(), "constant" )
+					self.assertTrue( diffuseDepth.HasAuthoredValue() )
+					self.assertTrue( diffuseDepth.Get( 0 ), 2 )
+				else :
+					diffuseDepth = stage.GetPrimAtPath( "/test" ).GetAttribute( "ri:trace:maxdiffusedepth" )
+					self.assertEqual( diffuseDepth.Get( 0 ), 2 )
+
+				# Test loading back to Cortex.
+
+				scene = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+				child = scene.child( "test" )
+				self.assertEqual( child.attributeNames(), [ "ri:trace:maxdiffusedepth" ] )
+				self.assertEqual( child.readAttribute( "ri:trace:maxdiffusedepth", 0 ), IECore.IntData( 2 ) )
+
 if __name__ == "__main__":
 	unittest.main()