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

Improvements to thin-film BSDF #1055

Merged
merged 14 commits into from
Sep 30, 2022

Conversation

proog128
Copy link
Contributor

This PR improves several aspects of the thin film BSDF:

  • Adds support for wavelength-dependent complex refraction indices (relevant for metallic base layer)
  • Adds (limited) support for thin film effects to generalized_schlick_bsdf
  • Adds iridescence to glTF PBR

Wavelength-dependent complex IOR

The current implementation of thin_film_bsdf does not support colored base layers, like for example metals. The new implementation supports this case, as shown in the following image.

colored_base_layer

Left: Conductor BSDF with colored reflection.

Center: Thin film on top of the conductor BSDF, old implementation. The yellow tint of the base layer is gone. Only the thin film is visible.

Right: Thin film on top of the conductor BSDF, new implementation. The yellow tint is preserved.

The new implementation now closely follows the Mitsuba reference implementation given in Belcour and Barla (2017): A Practical Extension to Microfacet Theory for the Modeling of Varying Iridescence. The old implementation was copied from their GLSL shader for BRDF Explorer, which lacked a wavelength-dependent conductor Fresnel term. Note that the authors do not explicitly mention the license of their code. Given the fact that it seemed to be fine to copy their GLSL code, I hope it's fine to copy (parts of) their C++/Mitsuba implementation too.

Thin film for generalized_schlick_bsdf

As mentioned in #1035, the Fresnel term used in generalized_schlick_bsdf does not support thin film yet. This changes with this PR.

Given a dielectric base layer, the results between dielectric_bsdf and generalized_schlick_bsdf are quite close. The following image shows a comparison between thin film over dielectric_bsdf (left) and generalized_schlick_bsdf (right) BSDFs for a thin film with IOR = 1.33 and thickness = 500 nm over a base layer with IOR = 1.59.

dielectric_exact_schlick

However, the implementation has some limitations. It mainly comes from the fact that the Schlick base layer does not consider IOR of the thin film layer. This limitation results in an increase of error if the base layer IOR becomes equal to the thin film IOR. When evaluating the Schlick Fresnel term of the base layer, the incident IOR is assumed to be 1. The following image shows the error by varying the thin film IORs on top of a base layer with IOR = 1.5:

dielectric_ior_range

A similar problem occurs if the base layer is metallic. Here we additionally cannot take the (unknown) complex IOR of the base layer into account for computing the phase shift, thus the error is even worse. Comparison (left: conductor_bsdf, right: generalized_schlick_bsdf):

metal_exact_schlick

Nevertheless, although it has limitations, it conveyes the visual impression of a thin film effect.

Iridescence for glTF PBR

Recently, the glTF PBR was extended by an iridescence effect (KHR_materials_iridescence). Since the MaterialX implementation of glTF PBR is based on generalized_schlick_bsdf, it requires the thin film effect for the Schlick Fresnel. Now that this effect is available, it's straightforward to extend the material by thin film. Straightforward except for the workaround mentioned in #1035, which is required because of limitations in GLSL code generation when referencing the same BSDF multiple times with and without thin-film. Unfortunately, this leads to some duplicated code.

Add support for base layer wavelength-dependent complex refraction index.
Add (limited) support for thin-film to generalized_schlick_bsdf.
@niklasharrysson
Copy link
Contributor

This looks like a great improvement, thanks @proog128!

We're planning to take a closer look in the next couple of days. There seems to be some test failures we should investigate.

@jstone-lucasfilm
Copy link
Member

@proog128 @niklasharrysson Indeed, this looks like a great improvement, and I'll just highlight one of the issues caught by the test suite. This looks to be an ESSL restriction that we'd need to take into account in the new code:

--- Error log:  D:\a\MaterialX\MaterialX\resources\Materials\Examples\StandardSurfaceTiled_Brass.essl.frag

ERROR: 0:4[82](https://github.com/AcademySoftwareFoundation/MaterialX/runs/7931852759?check_suite_focus=true#step:14:83): '*' :  wrong operand types: no operation '*' exists that takes a left-hand operand of type ' const int' and a right operand of type ' in mediump 3-component vector of float' (or there is no acceptable conversion)

Copy link
Contributor

@niklasharrysson niklasharrysson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @proog128 , and I only had a minor note above. We can fix that later.

@jstone-lucasfilm
Copy link
Member

Thanks for putting this proposal together, @proog128, and I've started testing this proposal with existing material examples. One issue that I'm encountering is that Standard Surface materials seem to lose their thin-film rendering effects with this pull request:

Standard Surface Thin Film (main branch):
StandardSurfaceThinFilm_Main

Standard Surface Thin Film (PR 1055)
StandardSurfaceThinFilm_PR1055

@proog128
Copy link
Contributor Author

@jstone-lucasfilm @niklasharrysson Sorry for the delay. Standard Surface Thin Film is fixed now, it was caused by a typo in the latest refactorings. In addition, I fixed another bug that increased overall brightness of the thin-film Fresnel.

@jstone-lucasfilm
Copy link
Member

This looks really promising, thanks @proog128, and I'm now seeing a good visual match with the main branch.

One remaining concern that I have, though, is that the performance for rendering Standard Surface thin-film effects seems to be reduced with respect with the main branch.

To quantify this, I'm attaching screenshots of the standard_surface_thin_film.mtlx material in MaterialXView, with the refresh count set to zero ("--refresh 0"), and with Fraps used to display the frames per second in each build:

Standard Surface Thin Film (main branch):
MaterialXView_ThinFilm_Main

Standard Surface Thin Film (PR 1055)
MaterialXView_ThinFilm_PR1055

@proog128
Copy link
Contributor Author

proog128 commented Sep 18, 2022

The code is quite generic (i.e., it calls the conductor Fresnel function with k = 0 for dielectrics) which is nice but unfortunately problematic for performance. In the latest commits I added a few special cases to improve performance, but it's still not as fast as before. However, given that the BSDF is now more accurate and thus more complicated to compute, I expect it to be slower, at least slightly.

I am measuring the following numbers on my machine:

main branch (52b5339) new impl without optimizations (684e916) new impl with optimizations (81e8e3b)
Standard Surface Thin Film 51 fps 23 fps 44 fps
Thin Film Test 7 (new) 239 fps 86 fps 134 fps

Thin Film Test 7 is a new test to check the case of thin_film_bsdf over conductor_bsdf. It is interesting because it cannot take any shortcuts in the implementation.

Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thanks @proog128! The small performance difference I'm now seeing is restricted to surfaces with thin-film effects, and seem easily justified by the improved accuracy.

@jstone-lucasfilm jstone-lucasfilm merged commit c7a0304 into AcademySoftwareFoundation:main Sep 30, 2022
Michaelredaa pushed a commit to Michaelredaa/MaterialX that referenced this pull request Oct 21, 2023
This PR improves several aspects of the thin film BSDF:

- Adds support for wavelength-dependent complex refraction indices (relevant for metallic base layer)
- Adds (limited) support for thin film effects to generalized_schlick_bsdf
- Adds iridescence to glTF PBR
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

Successfully merging this pull request may close these issues.

4 participants