Basic UsdLux read support #1256
Conversation
- Simplify `readShaderNetwork()` public interface, and move "does the output have a connection?" logic from USDScene into ShaderAlgo. - Introduce `readShaderNetworkWalk()` overload which takes an output and returns a handle for that output. This removes the repetition of the `DEFAULT_OUTPUT` special case. - Simplify connection handling by building `vector<Connection>` directly, rather than packing and unpacking a recipe for making a connection. - Remove unused `inputs` variable. - Remove bogus comment - there is no such variable as `handles[0]`.
We only support this for USD 21.11 and greater, because that version introduces UsdLuxLightAPI which lets us load lights using the same UsdShadeConnectable API that we use for materials. At this point we're making no attempt to conform the lights to Gaffer's conventions for Arnold (or any other renderer), so in the short term this is mostly useful for "data smithing" - converting stuff manually after loading. An upcoming ShaderQuery node in Gaffer should provide a useful tool for doing that.
danieldresser-ie
left a comment
danieldresser-ie
left a comment
There was a problem hiding this comment.
Looks good, with one question about naming.
I wouldn't have necessarily expected it to be a reasonable requirement that all shading groups can be referred to by an output they're connected to, as an API for readShaderNetwork, but it's definitely cleaner, and appears to cover all use cases, so LGTM.
| } | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
The naming of this compared to the previous function doesn't feel particularly clear ( I guess we do need both, we need to check for cameras being a schema vs lights having a schema, rather than using HasAPI for both? ).
I'm wondering about something like readDescendentsMatchSchema vs readDescendantsHaveSchema?
There was a problem hiding this comment.
Yep, we definitely need both - IsA() won't consider mix-in API types like UsdLuxLightAPI, and HasAPI has static asserts that limit it to mix-in API types. I've tried something along the lines of what you suggested in 798e7af.
There was a problem hiding this comment.
How's this looking to you now @danieldresser-ie? OK to merge?
|
LGTM |
2 participants
No description provided.