Merge pull request #1416 from johnhaddon/arnoldLightPrimvars

USD ShaderAlgo : Fix `treatAs*` and `arnold:*` light parameters

Changes

@@ -1,6 +1,13 @@
 10.5.x.x (relative to 10.5.7.0)
 ========
 
+Fixes
+-----
+
+- USDScene :
+  - Adding mapping of `arnold:*` light parameters to and from the non-standard `primvars:arnold:*` attributes preferred by the `arnold-usd` project.
+  - Fixed writing of `treatAsPoint` and `treatAsLine` light parameters, which were being written as generic `inputs:*` attributes and not the specific
+    attributes defined by the SphereLight and CylinderLight schemas.
 
 10.5.7.0 (relative to 10.5.6.2)
 ========

contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp

@@ -49,6 +49,8 @@
 #include "pxr/usd/usd/schemaRegistry.h"
 #endif
 
+#include "pxr/usd/usdGeom/primvarsAPI.h"
+
 #include "boost/algorithm/string/predicate.hpp"
 #include "boost/algorithm/string/replace.hpp"
 #include "boost/pointer_cast.hpp"
@@ -84,7 +86,43 @@ std::pair<pxr::TfToken, std::string> shaderIdAndType( const pxr::UsdShadeConnect
 	return std::make_pair( id, type );
 }
 
-void readAdditionalLightParameters( const pxr::UsdPrim &prim, IECore::CompoundDataMap &parameters )
+bool writeNonStandardLightParameter( const std::string &name, const IECore::Data *value, pxr::UsdShadeConnectableAPI usdShader )
+{
+#if PXR_VERSION >= 2111
+
+	if( auto sphereLight = pxr::UsdLuxSphereLight( usdShader.GetPrim() ) )
+	{
+		if( name == "treatAsPoint" )
+		{
+			sphereLight.GetTreatAsPointAttr().Set( IECoreUSD::DataAlgo::toUSD( value ) );
+			return true;
+		}
+	}
+	else if( auto cylinderLight = pxr::UsdLuxCylinderLight( usdShader.GetPrim() ) )
+	{
+		if( name == "treatAsLine" )
+		{
+			cylinderLight.GetTreatAsLineAttr().Set( IECoreUSD::DataAlgo::toUSD( value ) );
+			return true;
+		}
+	}
+
+	if( pxr::UsdLuxLightAPI( usdShader.GetPrim() ) )
+	{
+		if( boost::starts_with( name, "arnold:" ) )
+		{
+			const pxr::SdfValueTypeName valueTypeName = IECoreUSD::DataAlgo::valueTypeName( value );
+			pxr::UsdGeomPrimvar primVar = pxr::UsdGeomPrimvarsAPI( usdShader.GetPrim() ).CreatePrimvar( pxr::TfToken( name ), valueTypeName );
+			primVar.Set( IECoreUSD::DataAlgo::toUSD( value ) );
+			return true;
+		}
+	}
+
+#endif
+	return false;
+}
+
+void readNonStandardLightParameters( const pxr::UsdPrim &prim, IECore::CompoundDataMap &parameters )
 {
 	// Just to keep us on our toes, not all light parameters are stored as UsdShade inputs,
 	// so we have special-case code for loading those here.
@@ -101,6 +139,25 @@ void readAdditionalLightParameters( const pxr::UsdPrim &prim, IECore::CompoundDa
 		cylinderLight.GetTreatAsLineAttr().Get( &treatAsLine );
 		parameters["treatAsLine"] = new IECore::BoolData( treatAsLine );
 	}
+
+	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;
+			}
+
+			pxr::VtValue value;
+			if( primVar.Get( &value ) )
+			{
+				parameters[name.GetString()] = IECoreUSD::DataAlgo::fromUSD( value, primVar.GetTypeName() );
+			}
+		}
+	}
 #endif
 }
 
@@ -189,7 +246,7 @@ IECore::InternedString readShaderNetworkWalk( const pxr::SdfPath &anchorPath, co
 		}
 	}
 
-	readAdditionalLightParameters( usdShader.GetPrim(), parameters );
+	readNonStandardLightParameters( usdShader.GetPrim(), parameters );
 
 	IECoreScene::ShaderPtr newShader = new IECoreScene::Shader( shaderName, shaderType, parametersData );
 	pxr::VtValue metadataValue;
@@ -256,6 +313,11 @@ void writeShaderParameterValues( const IECoreScene::Shader *shader, pxr::UsdShad
 {
 	for( const auto &p : shader->parametersData()->readable() )
 	{
+		if( writeNonStandardLightParameter( p.first.string(), p.second.get(), usdShader ) )
+		{
+			continue;
+		}
+
 		const pxr::TfToken usdParameterName = toUSDParameterName( p.first );
 		pxr::UsdShadeInput input = usdShader.GetInput( usdParameterName );
 		if( !input )

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -866,7 +866,7 @@ def testSubdOptions( self ) :
 			( "interpolateBoundary", allowedIB ),
 			( "faceVaryingLinearInterpolation", allowedFVLI ),
 			( "triangleSubdivisionRule", allowedTS ),
-			
+
 		]:
 			for value in allowed:
 
@@ -889,7 +889,7 @@ def testSubdOptions( self ) :
 
 				if property == "triangleSubdivisionRule":
 					mesh.CreateTriangleSubdivisionRuleAttr().Set( value, 0.0 )
-				
+
 				stage.GetRootLayer().Save()
 				del stage
 
@@ -4091,5 +4091,88 @@ def testRoundTripArnoldLight( self ) :
 		self.assertIn( "__lights", root.setNames() )
 		self.assertEqual( root.readSet( "__lights" ), IECore.PathMatcher( [ "/light" ] ) )
 
+	def testArnoldSpecificLightInputs( self ) :
+
+		# The `arnold-usd` project doesn't represent Arnold-specific UsdLux
+		# extensions as `inputs:arnold:*` attributes as it logically should :
+		# instead it uses `primvars:arnold:*` attributes. In Cortex/Gaffer we
+		# wish to use regular `arnold:*` shader parameters rather than primvars,
+		# so must convert to and from the less logical form in USDScene.
+
+		lightShader = IECoreScene.ShaderNetwork(
+			shaders = {
+				"light" : IECoreScene.Shader(
+					"RectLight", "light",
+					parameters = {
+						"exposure" : 1.0,
+						"arnold:roundness" : 2.0,
+					}
+				)
+			},
+			output = "light",
+		)
+
+		fileName = os.path.join( self.temporaryDirectory(), "test.usda" )
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+		light = root.createChild( "light" )
+		light.writeAttribute( "light", lightShader, 0 )
+		del root, light
+
+		stage = pxr.Usd.Stage.Open( fileName )
+		shadeAPI = pxr.UsdShade.ConnectableAPI( stage.GetPrimAtPath( "/light" ) )
+		self.assertTrue( shadeAPI.GetInput( "exposure" ) )
+		self.assertFalse( shadeAPI.GetInput( "arnold:roundness" ) )
+		primvarsAPI = pxr.UsdGeom.PrimvarsAPI( stage.GetPrimAtPath( "/light" ) )
+		self.assertTrue( primvarsAPI.HasPrimvar( "arnold:roundness" ) )
+		self.assertEqual( primvarsAPI.GetPrimvar( "arnold:roundness" ).Get(), 2.0 )
+		del stage, shadeAPI, primvarsAPI
+
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+		self.assertEqual( root.child( "light" ).readAttribute( "light", 0 ), lightShader )
+
+	def testTreatLightAsPointOrLine( self ) :
+
+		# `treatAsPoint` and `treatAsLine` aren't defined as UsdShade inputs but we store
+		# them as regular shader parameter, so they need special handling when writing to USD.
+
+		sphereLightShader = IECoreScene.ShaderNetwork(
+			shaders = {
+				"sphereLight" : IECoreScene.Shader(
+					"SphereLight", "light",
+					parameters = {
+						"treatAsPoint" : True,
+					}
+				)
+			},
+			output = "sphereLight",
+		)
+
+		cylinderLightShader = IECoreScene.ShaderNetwork(
+			shaders = {
+				"cylinderLight" : IECoreScene.Shader(
+					"CylinderLight", "light",
+					parameters = {
+						"treatAsLine" : True,
+					}
+				)
+			},
+			output = "cylinderLight",
+		)
+
+		fileName = os.path.join( self.temporaryDirectory(), "test.usda" )
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
+		root.createChild( "sphereLight" ).writeAttribute( "light", sphereLightShader, 0 )
+		root.createChild( "cylinderLight" ).writeAttribute( "light", cylinderLightShader, 0 )
+		del root
+
+		stage = pxr.Usd.Stage.Open( fileName )
+		self.assertEqual( pxr.UsdLux.SphereLight( stage.GetPrimAtPath( "/sphereLight" ) ).GetTreatAsPointAttr().Get(), True )
+		self.assertEqual( pxr.UsdLux.CylinderLight( stage.GetPrimAtPath( "/cylinderLight" ) ).GetTreatAsLineAttr().Get(), True )
+		del stage
+
+		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+		self.assertEqual( root.child( "sphereLight" ).readAttribute( "light", 0 ), sphereLightShader )
+		self.assertEqual( root.child( "cylinderLight" ).readAttribute( "light", 0 ), cylinderLightShader )
+
 if __name__ == "__main__":
 	unittest.main()