Proposal: Replace thin_film_bsdf with tf_ior/tf_thickness attributes

[amsXYZ]

Motivation

thin_film_bsdf is one of the BSDF nodes part of PBR spec of MaterialX. Differently to any other of these nodes, it has a lot of particularities that make it a bit unpredictable and hard to integrate in certain pipelines (i.e. realtime renderers).

Some of these issues are:

• As specified on spec, it's not supposed to represent a BSDF per-se, but a thin-layer that interferes with layers below by polarising light that traverses it. Because of this, the thin_film_bsdf node doesn't do anything on its own and needs to be paired with a layer element to have any effect on the output response.
  • This can be quite confusing for users, as any element that accepts a BSDF type as input (i.e. mix) will not output proper results when connected to a thin_film_bsdf element, being up to the user to know this beforehand.
• Using it can lead to graphs that, even when they're valid, present broken results (as already issued here) or have no perceivable change in the material's response (as seen below when layered on top of an oren_nayar_diffuse_bsdf element).
  • It only affects dielectric, conductor and generalized_schlick bsdfs, but can be layered on top of any BSDF. This is not obvious and can be quite confusing for any user without a strong PBR knowledge.
    
• It breaks the assumption that every element encapsulates unique data/logic, since it introduces the concept of modifiers. Because of this, implementations cannot just compute a material's response following element edges from input to output, and are forced to recursively find all BSDF nodes below a layer with a thin_film_bsdf in its top attribute. Once these elements are found, the implementation has two "not-so-great" options:
  • Either modify the ior and thickness for all the found BSDF elements. This could lead to wrong results, if the output of any of these BSDF is used by another element before the thin-film is applied.
  • Duplicate all found BSDF elements and recompute them using the thin-film ior and thickness parameters. This will ensure correct results for elements that use the BSDF outputs before having the thin-film layer applied. However, this doubles their computational cost, which can be critical in certain scenarios (i.e. complex materials, real-time rendering, etc.).

I understand that this element was designed to replicate the mental-model that a thin-film can be applied on top of a surface to modify its appearance, which is fairly user-friendly. But in my opinion, all usability issues presented here (i.e. only specific setup is valid, valid-but-broken graphs, doesn't affect all BSDF elements, etc.) plus the complexity/performance concerns that introduces, overweight the strength of this mental-model... Do we have other alternatives? 👇

Proposal

Since the only elements that get modified by the thin_film_bsdf are the dielectric_bsdf, conductor_bsdf and generalized_schlick_bsdf, we propose the introduction of the tf_ior and tf_thickness attributes to these elements and to remove the thin_film_bsdf element. This change would:

• Remove valid-but-broken situations introduced by the thin_film_bsdf element.
• Ensure that the data/logic encapsulation is satisfied for all nodes again.
• Make it clear which BSDF elements can be modified by thin-film interference (without prior PBR knowledge).
• Be explicit about thin-film usage within these BSDF elements (so the implementation doesn't have to duplicate their computation just to be safe).

Upgrade path

As an upgrade path, all graphs using a thin_film_bsdf should:

• Remove these elements (and their layer element).
• Find all BSDF elements that were connected to the base input of a layer element that used a thin_film_bsdf as their top input.
• Set their tf_ior and tf_thickness values to the top thin_film_bsdf ior and thickness values.
• If their output pre-thin-film was used somewhere, duplicate these BSDF elements and ser their tf_ior and tf_thickness to 1 and 0 respectively.
• Replace the thin-film layer element output by their base input.

[amsXYZ]

Just as a note, this also closely matches the OSL implementation of the conductor, dielectric, and generalized_schlick clojures: https://github.com/AcademySoftwareFoundation/OpenShadingLanguage/blob/main/src/shaders/stdosl.h#L519

[niklasharrysson]

@amsXYZ Thanks for writing up this thorough proposal.

I like this a lot, as it would simplify thin-film handling in our code generators, as well as simplify the shaders produced.

We just need to make sure it works well with all target renderers/languages. As we have already discussed offline it would be a great improvement for the rasterizer targets. And for OSL it maps perfectly to the new MaterialX/OSL closures, as you have noted above.

The one target we need to investigate further is MDL, where the design is close to the current MaterialX design, using a modifier to apply thin-film on other BSDFs. So it would be interesting to get feedback from the MDL team on this. A simple solution would be to always have the thin-film modifier applied on the three BSDF's in question, but using a thickness of zero when thin-film is not used. But I wonder if that would effect performance, or if it would be optimized out by the MDL compiler?

[amsXYZ]

@niklasharrysson Thanks for the response! 🙂

I understand that before making such a drastic spec change, you want to make sure it maps well to all current implementations, so totally get you there!

As you said, if the MDL compiler could optimize this out, we would be in an ideal situation where all targets would get the best UX, simplest implementation and best performance 😊 Is there anyone from NV or an MDL contributor that you think could contribute to this topic?

[niklasharrysson]

A closer look at the MDL implementation reveals that this is not a problem at all. In fact, thin-film for MDL is already implemented in this way 😊
https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/source/MaterialXGenMdl/mdl/materialx/pbrlib.mdl#L155

As far as I can see this way of implementing thin-film was introduced only three months ago, in pull request #1282

This is great, because it means that this proposal should be straight forward to implement, and it would simplify the MDL code generator as well.

@krohmerNV do you see any other issues on MDL side for this proposal to simplify thin-film handling?

If there are no objections I will start looking at this change, so we can get it done for 1.39.

[amsXYZ]

@niklasharrysson That's great news :)

[krohmerNV]

No objections from my side. As @niklasharrysson said, this is pretty much the same transformation I did in the MDL code generator.
I'm interested to see how the upgrade path works.

[jstone-lucasfilm]

This issue has been addressed in #1413 for the in-progress v1.39 of MaterialX. Thanks to @amsXYZ for this proposal, and to @niklasharrysson for the implementation!