Refactoring MaterialX implementation to align more closely with the spec

[portsmouth]

Here is an attempt to write the MaterialX implementation in a way which aligns as closely as possible, given the current form of the MaterialX spec, with the formalism in the current OpenPBR spec. To make it easier to digest (since XML is tough to read), I wrote the functional form of it in Latex as screen-shotted below.

The changes i've made roughly amount to:

• making the layers align with the OpenPBR spec, both the structure and the naming conventions.

• adding the thin-walled subsurface model, as close as I can get it.

• fixing up all the parameter names to align.

This still differs in various important respects to the spec:

• the way the coat absorption is implemented is something of a hack (and actually does not exactly correspond to what is done in either standard surface or OpenPBR). Doing it properly requires a deeper discussion about how to account for the medium of a dielectric layer in MaterialX (so beyond the scope of this PR). At least the way it is done is no worse than what was done for the MaterialX standard surface implementation.

• there is no way to specify how the subsurface_bsdf parameters are mapped into the volumetric medium coefficients, so our "similarity-theory" based mapping is not expressible.

• there is no way to implement the volumetric medium of the "translucent base" (transmission_depth etc.), as the dielectric BSDF in MaterialX does not have the concept of the "interior medium". Similarly for the dispersion properties (transmission_dispersion_abbe_number etc.). This can maybe be done with the "VDF" construct, but it's unclear how to do it.

• there is no facility for the (very useful) "thin-walled glass" model (e.g. see slide 34 of "Revisiting Physically Based Shading at Imageworks" 2017). This is implemented in Arnold for example (MicrofacetThinRefractionBSDF), but not in the standard surface spec (hence it did not get into MaterialX either I assume).

• For the thin-walled SSS model, ideally there would be a R/T mode for the Oren-Nayar BSDF, which would make more sense than the ad-hoc translucent_bsdf I think. (NB, in the MaterialX spec, the only way to know that translucent_bsdf refers to a diffuse lobe oriented into the transmission hemisphere, is by guessing based on the names. That's not ideal).

• I did not attempt to implement in nodes the formulas for specular_ior_level / coat_ior_level yet. This should be fairly easily doable though. (Can be done in a separate PR).

• Unlike MaterialX, thin-film is not represented as a separate layer/BSDF in OpenPBR (or standard surface), it just modifies the microfacet BSDF of the base metal/dielectric. It seems reasonable to model it as a layer though. Technically the IOR of the thin-film should alter the Fresnel of the underlying layer (and in turn the thin-film effect should depend on the IOR of the layer above it), as noted e.g. in https://github.com/AcademySoftwareFoundation/OpenPBR/issues/79. This would be a bit more clear if the thin-film was an explicit layer (though whether we attempt to explain how to account for the adjacent IORs, I'm not sure). I would be open to modifying the OpenPBR spec, to align with MaterialX, in this instance.

• There is no "specular layer" in OpenPBR, as physically this is just the interface of the base dielectric. In MaterialX, I take it that layer(specular_bsdf, specular_btdf) is just a convenient shorthand for approximating a dielectric BSDF via albedo scaling (of the BRDF "on top" of the BTDF), even though there's not actually any layering happening (except in the sense that there's an upper and lower hemisphere..). I find this ugly, but obviously we have to work with it.

• It's not very clear how the albedo scaling for the fuzz layer works in MaterialX. It could also be clearer in OpenPBR (as requested in https://github.com/AcademySoftwareFoundation/OpenPBR/issues/84), but we do try to make it explicit that fuzz_color does not tint the base, whereas this is unstated in MaterialX.

• the emission model uses generalized_schlick_edf with color90 = coat_ior_to_F0. I'm not sure what this is trying to model, but it doesn't (so far) appear in the OpenPBR spec. We could augment the spec perhaps if this is something worth having.

• the coat_affect parameters and their (rather ad-hoc) effect taken from the standard surface spec, is coded up in the graph -- but at this moment not mentioned in the OpenPBR spec. We may wish to leave this in (the spec needs to be updated if we want to keep them), or try to improve the formulas, not sure.

[jstone-lucasfilm]

Thanks for getting this conversation started, @portsmouth, though I believe we'll want to simplify this down to a set of purely functional changes, leaving any formatting improvements for a future proposal. This will allow us to track the changes to our reference implementation robustly over time, and will allow us to consider new formatting ideas independently, since it's important to maintain the clarity of parallels between OpenPBR and existing shading models such as Autodesk Standard Surface, glTF PBR, and UsdPreviewSurface. It's the clarity of these parallels between models that allows developers to author and maintain shader translation graphs, which will be an important part of future OpenPBR development.

I'm also noticing that this new version no longer renders at all in MaterialX validation applications, and I just get a series of validation warnings and a uniform black output in MaterialXView and MaterialXGraphEditor.

Rather than fixing each of these individually, though, I would recommend starting fresh with the absolute minimum set of functional changes that you'd like to see

If you'd like, you can just propose each individual change as a GitHub Issue, and I can help out with the concrete changes to the reference implementation, validating each one in a MaterialX rendering environment (and posting example renders) to make sure it behaves as we'd like.

[portsmouth]

Fair enough about the formatting changes (I assume you mean e.g. indentation and comments), I'll remove them. I do think they make the XML file a bit easier to read (more clear section headings and structure), but agreed it's best to decouple that from the functional changes. (Things like the names of the nodes though, I think are fair game as it impacts how the correspondence is made between the XML and the spec.)

I'd prefer to do the refactor of the functionality in one PR, otherwise it's going to be too long a process of minor increments. It should be easy enough to figure out why it's not passing the checks, probably just some minor errors. I will shortly push a version with the formatting changes omitted.

Also I was hoping to discuss the various things we are not able to express, as outlined above. If preferred, I can move that over to a GitHub issue though. Though I would like to have all these points engaged with by the MaterialX people. If Niklas has access, perhaps we can get the conversation going. (edited)

[portsmouth]

Closing in order to refactor as more incremental changes.