ImageEngine / cortex

Libraries for visual effects software development
Other
528 stars 123 forks source link

USDScene : Support round-tripping of non-UsdLux lights #1406

Closed johnhaddon closed 7 months ago

johnhaddon commented 7 months ago

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?