USDScene : Support round-tripping of non-UsdLux lights

In Cortex/Gaffer, a light is a location which has a `light` or `*:light` attribute containing a suitable ShaderNetwork and which also appears in the `__lights` set. When round-tripping a non-UsdLux light through USD we were losing the membership of the set because on loading we were synthesizing the set from all locations with `UsdLuxLightAPI` applied, rather than loading the  `__lights` set that had been written explicitly. By loading both `UsdLuxLightAPI` locations _and_ members of the `__lights` collection, we can support both UsdLuxLights from other DCCs (where the `__lights` collection won't have been written) and UsdLux and non-UsdLux lights from Gaffer (where the `__lights` collection will have been written).

There are a couple of elephants in the room here :

1. We are still not writing the light's material in a USD-conformant way. It ends up being written as a material assignment just because that's what it is in Cortex/Gaffer, but really it should be written as an ArnoldDistantLight prim.
But doing that wouldn't have helped with the `__lights` set : ArnoldDistantLight is not a UsdLuxLight, so it _still_ wouldn't have been found by the old set reading code.
2. We arguably should be writing the `__lights` set not as a regular collection, but via UsdLuxLightList, for maximum compatibility with other USD consumers. But then we'd be including non-UsdLux lights in the light list (see 1) which isn't ideal either - they wouldn't be included by `UsdLuxLightListAPI::ComputeLightList()`, which is the canonical way of generating such a list.

For now we're just making the simplest possible change to unlock a short term Gaffer->Gaffer workflow involving Arnold-specific lights.

> Note : As a byproduct we are also doing the same merging of the predicate-based and collection based sets for the `__cameras` and `usd:pointInstancers` sets. Is this something we should be concerned about?

Author: johnhaddon

Changes

@@ -1,17 +1,22 @@
 10.5.x.x (relative to 10.5.6.0)
 ========
 
-
 Improvements
 ------------
 
 - DisplayDriverServer : Fixed delays connecting to the server on Windows.
 
+Fixes
+-----
+
+- USDScene : Fixed round-tripping of `__lights` set membership for non-UsdLux lights.
+
 Build
 -----
 
 - SConstruct : Support detection of OpenEXR versions with a patch version containing multiple digits.
 
+
 10.5.6.0 (relative to 10.5.5.0)
 ========
 

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -247,7 +247,6 @@ IECore::PathMatcher localSet( const pxr::UsdPrim &prim, const pxr::TfToken &name
 		{
 			result.addPath( std::vector<IECore::InternedString>() );
 		}
-		return result;
 	}
 
 	const size_t prefixSize = prim.GetPath().GetPathElementCount();

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -3973,5 +3973,33 @@ def testReadFromStageCache( self ) :
 		self.assertEqual( root.childNames(), [ "sphere" ] )
 		self.assertIsInstance( root.child( "sphere" ).readObject( 0 ), IECoreScene.SpherePrimitive )
 
+	def testRoundTripArnoldLight( self ) :
+
+		lightShader = IECoreScene.ShaderNetwork(
+			shaders = {
+				"light" : IECoreScene.Shader( "distant_light", parameters = { "exposure" : 2.0 } )
+			},
+			output = "light",
+		)
+
+		root = IECoreScene.SceneInterface.create(
+			os.path.join( self.temporaryDirectory(), "test.usda" ),
+			IECore.IndexedIO.OpenMode.Write
+		)
+		light = root.createChild( "light" )
+		light.writeAttribute( "ai:light", lightShader, 0 )
+		root.writeSet( "__lights", IECore.PathMatcher( [ "/light" ] ) )
+		del root, light
+
+		root = IECoreScene.SceneInterface.create(
+			os.path.join( self.temporaryDirectory(), "test.usda" ),
+			IECore.IndexedIO.OpenMode.Read
+		)
+		light = root.child( "light" )
+		self.assertIn( "ai:light", light.attributeNames() )
+		self.assertEqual( light.readAttribute( "ai:light", 0 ), lightShader )
+		self.assertIn( "__lights", root.setNames() )
+		self.assertEqual( root.readSet( "__lights" ), IECore.PathMatcher( [ "/light" ] ) )
+
 if __name__ == "__main__":
 	unittest.main()