Support concurrent node definition in the same document#2883
Support concurrent node definition in the same document#2883AdrienHerubel wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
Conversation
|
Can the requirement to have identical nodedef names with different versions be explained? Feels like this should not be required as the existing mechanisms will support multiple versions. I put this into Slack as reply to the question on identifiers ( For nodedef versioning, seems like what's in the PR breaks the unique name requirement for nodedefs (and any node in general) in the spec. Unsure why the same names are being used ? (*) -- with additional patching to try and auto create unique names, which is also not something defined in the spec, and looks like it could break USD shader registration. Custom Node Declaration NodeDef Elements Each custom node must be explicitly declared with a element, with child , and elements specifying the expected names and types of the node’s ports. (*) If this is to try and make things work with the existing shader translation logic, then I'd suggest that that logic be extended to support versions. As a further note, standard surface already has 2 versions and both can be instantiated without any addition logic changes. There is a USD/MTLX Slack thread with a note on this as well. |
| <!-- OpenPBR Surface v1.1.1 --> | ||
| <!-- ============================================================ --> | ||
|
|
||
| <nodedef name="ND_open_pbr_surface_surfaceshader" node="open_pbr_surface" nodegroup="pbr" version="1.1.1" isdefaultversion="false" |
There was a problem hiding this comment.
I think we just want to call this ND_open_pbr_surface_surfaceshader_111 or maybe ND_open_pbr_surface_surfaceshader_1_1_1
So we have unique node def names - as we were discussing at the TSC - my hope would be if you do that - then you wouldn't need any changes in MaterialXCore or MaterialXFormat at all - this really boils down to a new version of the node, and some code changes on the application side (MaterialXGraphEditor)
There was a problem hiding this comment.
I'd vote for that. If we follow what was done for std surface than it's ND_open_pbr_surface_surfaceshader_111, but not sure if this should be the recommended "convention" -- I don't recall the discussion anymore on why we chose this.
There was a problem hiding this comment.
I'll revisit the PR to use different nodedef names as discussed at the TSC. Though for backward compatibility with USD where we explicitly use a nodedef name and no nodedef versioning, should we not keep the current name for the 1.1.1 version to maintain the look ?
3 participants
This PR adds support for having nodes pointing at multiple versions of the same node definition using different node graphs in the same document. This also adds the draft version of OpenPBR 1.2 as a practical example.