Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Add thinfilm_weight input to supporting BSDF nodes #1859

Open
jstone-lucasfilm opened this issue Jun 1, 2024 · 2 comments
Open

Comments

@jstone-lucasfilm
Copy link
Member

In shading models such as open_pbr_surface and gltf_pbr, we currently end up duplicating each thin-film supporting BSDF node, in order to correctly handle the thin-film and iridescence weights that these shading models require:

https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/libraries/bxdf/open_pbr_surface.mtlx
https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/libraries/bxdf/gltf_pbr.mtlx

Would it make sense to add a thinfilm_weight input to each thin-film supporting BSDF node, so that this duplication would no longer be required?

This would likely provide a significant performance improvement to these shading models, especially in filtered importance sampling contexts where environment lighting must be separately sampled and summed for each of the duplicated BSDFs.

@jstone-lucasfilm
Copy link
Member Author

I'm CC'ing @niklasharrysson and @proog128 for their initial thoughts on this proposal, and members of the OpenPBR team may be interested as well.

@niklasharrysson
Copy link
Contributor

This sounds good to me. Besides the performance improvement, being able to blend in the thin-film effect feels very useful for artists.

Since this effect is achieved by switching in a different fresnel term, I'm not sure if there are any implementation details to work out? I guess both fresnel terms will need to be calculated to blend between. For path tracers it might complicated importance sampling if the fresnel term is accounted for there. But since this parameterization is part of the OpenPBR specification I'm sure there is a reference implementation one could follow there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants