Support concurrent node definition in the same document

[AdrienHerubel]

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.

[kwokcb]

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.
Attributes for elements:
name(string, required): a unique name for this
There can also be implications of support duplicate names on document read -- which currently has the convention of ignoring duplicates.

(*) 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.

[ld-kerley]

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)

[kwokcb]

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.

[AdrienHerubel]

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 ?

[ld-kerley]

Thats a really interesting point Adrien - and doesn't follow the existing MaterialX convention (although we do only have one data point in standard surface - as far as I'm aware).

I think its a good idea to leave the existing nodedef named as it was - and instead add ND_open_pbr_surface_surfaceshader_1_2_0 - I think I prefer using _ to replace the periods in the version number rather than 120 because it feels less ambiguous.

I think this is how Pixar version their shaders internally too, starting with Taco and then creating Taco_2.

Actually the UsdPreviewSurface texture node in MaterialX got a new version too - and it followed that pattern too.

<nodedef name="ND_UsdUVTexture" node="UsdUVTexture" nodegroup="texture2d" version="2.2" inherit="ND_UsdUVTexture_23">

and then

<nodedef name="ND_UsdUVTexture_23" node="UsdUVTexture" nodegroup="texture2d" version="2.3" isdefaultversion="true">

[kwokcb]

I think you have this situataion:

• 101 was the default version in in 1.39.4, id ND_open_pbr_surface_surfaceshader

• 120 will be default version in 1.39.5, id ND_open_pbr_surface_surfaceshader_120

• Any MTLX docs saved using 101 will stay ND_open_pbr_surface_surfaceshader

• Any newley created instances will be ND_open_pbr_surface_surfaceshader_120 implicitly since MTLX understands "isdefaultversion".

• Any USD docs saved and loaded w/o upgrade will still use ND_open_pbr_surface_surfaceshader. If the USD doc references MTLX then it is also still ND_open_pbr_surface_surfaceshader (1)

• I don't know what USD does and if you can create an instance w/o an explicit id. I don't think it's possible (?)

(1) Thus changing ids now would break USD unless there was some "upgrade" id mechanism which AFAIK does not exist and USD understood "isdefaultversion" tagging.

IMO, all definitions should have a version encoded into their ids, then you don't have ambiguous un-versioned ids (2)

---

(2) I think std surface was okay since the 2 versions were we're added before USD added first MTLX version dependency and that it is using inheritance and only changed a default value -- i.e. it was a complicated special case.

For USDUVTexture this was done after and is an interface change w/o inheritance - thus it's new version is numbered with the latest rev.

@jstone-lucasfilm or @niklasharrysson may have better recollection so take this as only what I can pull out from memory.