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

Thin-film improvements #167

Conversation

portsmouth
Copy link
Contributor

@portsmouth portsmouth commented Feb 28, 2024

This addresses:

We still don't give much detail about the implementation, but at least we make the physics that is supposed to be implemented reasonably clear -- or as clear as it can be given our usage of the not-strictly-physical F82-tint conductor model:

Modeling of the precise effect is implementation-dependent, however as described in the Metal section the Fresnel factor for metal is defined according to the Schlick-based "F82-tint" parametrization, which does not specify the underlying (complex) physical IOR. We suggest here that some reasonable approximation is employed to map the Fresnel factor to effective IOR and absorption coefficients, for example that described by [#Gulbrandsen2014].

As discussed on Slack and in the tickets above, it makes a great deal of sense to add a thin_film_weight parameter. This improves the expressivity of the model (in some use-cases, it could be essential to smoothly blend the effect in), and is a fairly trivial change to the implementation.

Also in a sense it didn't make sense to have the default thin_film_thickness = 0, as in fact the effect doesn't smoothly turn off as thickness goes to zero, it causes darkening for any finite thickness. I suggest instead we change the default thin_film_thickness to e.g. 1000 (mid-range), and have the thin_film_weight default to 0. Then on dialing this weight parameter the effect turns on smoothly. We may wish to tweak the default from 1000 though, depending on the look.

image

image

It seems better to break the thin-film discussion out into its own sub-section of the "Base Substrate" section.

@portsmouth
Copy link
Contributor Author

portsmouth commented Feb 28, 2024

Another issue to contend with is that currently the default thin_film_ior and specular_ior are both 1.5. According to the physical description, this means that the thin-film would have no effect (i.e. no color fringes appear) if metalness=0, since it is index-matched with the underlying dielectric. Similarly, the fringes should also disappear if the coat is present but index-matched with the film.

An implementation could just ignore the IOR of the underlying dielectric or overlying coat (e.g. Arnold does, currently), but this would technically be an incorrect interpretation of the physics. Also of course the actual fringes generated by the model would be fake as they should change as the IOR of the base changes.

Instead, I suggest we tweak the default IORs so that the thin-film fringes appear (to some reasonable extent) when used with those defaults. The thin-film IOR default simply has to be sufficiently different to both the specular and coat IORs. So perhaps e.g.:

  • specular_ior=1.6 ($\approx$ polycarbonate)
  • thin_film=1.4 ($\approx$ oil)
  • coat_ior=1.3 ($\approx$ water)

For reference, the current defaults are:

  • specular_ior=1.5
  • thin_film=1.5 // thus technically thin-film effect disappears
  • coat_ior=1.6

Having the coat IOR higher than the specular IOR also produces the TIR artifact/gotcha, unless dealt with.

@portsmouth portsmouth changed the title Improvements/clarification of the thin-film physics Clarification of the thin-film physics Feb 28, 2024
@portsmouth portsmouth changed the title Clarification of the thin-film physics Thin-film improvements Feb 28, 2024
@jstone-lucasfilm
Copy link
Member

@portsmouth Would it make sense to update our reference implementation as part of this change, so that the specification and implementation remain in sync?

@portsmouth
Copy link
Contributor Author

@portsmouth Would it make sense to update our reference implementation as part of this change, so that the specification and implementation remain in sync?

Yes it would, and should be pretty straightforward. I'll add that ASAP.

@portsmouth
Copy link
Contributor Author

portsmouth commented Mar 18, 2024

NB, we should also clarify the behavior in the case of a dielectric base, as then the transmitted light will also exhibit the color fringes.

Belcour & Barla say:

Note that with a dielectric base ... we should also consider transmission through both layers. The simplest approach is then to use energy conservation and define an Airy transmittance term by Tj = 1 − Rj , which may then be plugged in any microfacet-based BTDF model.

The most obvious use case is a thin-walled bubble. This image from B&B shows that neglecting the transmission through the film (e.g. TRRT mode) produces a much less convincing result.

Note that they model this as a non-thin-walled bubble, with a thin-film of IOR 1.7, whose interior dielectric is just air! (Quite an elegant way to represent it IMO). We should check that works in our implementations.

image

If this bubble (presumably supposed to be a homogeneous "thin wall" of soap, with thickness comparable to the wavelength) was actually modeled as a thin-wall, then technically the thin-film should be duplicated on each side of the wall, according to our "mirroring" assumption, and sit on top of the dielectric wall of equal IOR. This would work in theory, but is less physically reasonable as in fact the entire wall is supposed to function as the thin-film. Thin-wall mode should really be reserved only for cases where the wall is much thicker than a wavelength, e.g. panes of glass or leaves, etc.

Copy link
Contributor

@virtualzavie virtualzavie left a comment

Choose a reason for hiding this comment

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

I suggest instead we change the default thin_film_thickness to e.g. 1000 (mid-range), and have the thin_film_weight default to 0.

I agree. Parameters should have reasonable default values. To me this is like choosing a default roughness for the coat or a default colour for the subsurface scattering.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 465 to 467
**`thin_film_weight`** | Weight | `float` | $ [0, 1] $ | | $ 0 $ | Coverage weight of the thin film
**`thin_film_thickness`** | Thickness | `float` | $ [0, \infty) $ | $ [0, 2000] $ | $ 1000 $ | Thickness of the film in nanometers
**`thin_film_ior`** | IOR | `float` | $ (0, \infty) $ | $ [1, 3] $ | $ 1.5 $ | Refractive index of the film
Copy link
Contributor

Choose a reason for hiding this comment

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

I like @sharktacos's proposal (#150) to have a normalised parameter. This can be done in a separate PR though if there isn't consensus there.

Copy link
Contributor Author

@portsmouth portsmouth Apr 10, 2024

Choose a reason for hiding this comment

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

I think you could probably get away making the width be in micrometers, with a soft-max at 1. So have it be [0,1] range, with default at 0.5 say (i.e. then the film width defaults to roughly the wavelength of visible light).

In the Belcour & Barla paper, they use the parameter $\mathcal{D} = 2 \eta d$ where $\eta$ is IOR, $d$ is width. In their tests, $\mathcal{D}$ seems to vary in range [0, 3200], though with maximal effect at the mid-range i.e. 1600nm. So that implies even if $d$ is (soft) capped at 1 micrometer, the useful range is captured. There's no harm if someone needs a thicker film, as it's a soft-max anyway.

We can bring it up at the next meeting. If there's a liking for this idea, it would be easy to put in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, sounds good.

portsmouth and others added 6 commits April 10, 2024 19:40
Co-authored-by: Julien Guertault <9511025+virtualzavie@users.noreply.github.com>
Signed-off-by: Jamie Portsmouth <jamports@mac.com>
Co-authored-by: Julien Guertault <9511025+virtualzavie@users.noreply.github.com>
Signed-off-by: Jamie Portsmouth <jamports@mac.com>
@portsmouth
Copy link
Contributor Author

@portsmouth Would it make sense to update our reference implementation as part of this change, so that the specification and implementation remain in sync?

Done in f8538ec.

Copy link
Contributor

@virtualzavie virtualzavie left a comment

Choose a reason for hiding this comment

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

LGTM

@portsmouth portsmouth marked this pull request as draft April 13, 2024 02:52
@portsmouth
Copy link
Contributor Author

portsmouth commented Apr 16, 2024

What about this:

#167 (comment)

Should we change the default IORs? As currently the thin-film will be invisible if base_metalness=0, since its IOR is equal to the dielectric IOR.

Just to note, in the model there are 8 different possible IOR configurations, depending on which layers are present, as shown below. The BRDFs of the lobes in each case differ due to the Fresnel factors having different relative IORs. The implementation needs to take all that into account in principle, and it's not that trivial to do correctly.

image

@AdrienHerubel
Copy link
Contributor

Let's move the default value changes to a separate PR and discuss there, let's go ahead with the unit change.

@portsmouth
Copy link
Contributor Author

In 984bff1 I added the units change.

I also added more detail to the text regarding the implementation, and a clarifying diagram based on the one above.

I've refactored the section layout also, to make the thin-film discussion separate from the "Base substrate" section. This seems to make more logical sense, since now we cover the entire base (metal and dielectric slabs), then the thin film which sits on top of it, then the coat, then the fuzz.

image

image

Copy link
Contributor

@virtualzavie virtualzavie left a comment

Choose a reason for hiding this comment

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

I made a change suggestion for the MaterialX implementation, but other than that the PR looks good to me.

reference/open_pbr_surface.mtlx Outdated Show resolved Hide resolved
\begin{equation}
S_\mathrm{metal} = \mathrm{Slab}(f_\mathrm{conductor}) \ .
\end{equation}
_Iridescence_ is the occurrence of rainbow-like color fringes in the reflection when a thin dielectric film with thickness on the order of the wavelength of light is placed on top of a material, due to wave interference between the various electromagnetic reflection modes within the film. To model this, there is assumed to be such a thin-film sitting on top of the base substrate (whether metal or dielectric), parametrized only by:
Copy link
Contributor

Choose a reason for hiding this comment

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

Bit of side comment: I understand bird feathers exhibit iridescence not because of a thin-film, but mainly because of feather nanostructures. I don't know if our thin-film model would be sufficient to represent them. It would be interesting to explore this topic with artists in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems likely to be a special case deserving of a specific model, but probably thin-film iridescence is a reasonable fake approximation to it at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

Birds seem a common enough case to me to deserve a bit of attention. I'm making a mental note to test whether the thin-film model is sufficient for that case.

Co-authored-by: Julien Guertault <9511025+virtualzavie@users.noreply.github.com>
Signed-off-by: Jamie Portsmouth <jamports@mac.com>
@portsmouth
Copy link
Contributor Author

portsmouth commented Apr 29, 2024

Does this sound dumb?

In principle the implementation should model all these configurations correctly, though modeling of the precise effect is implementation-dependent.

Tweaked to:

In principle the implementation should deal with all these physical configurations correctly, though modeling of the precise effect is implementation-dependent. In practice, this wave-optics effect is most easily incorporated directly into the Fresnel factor of the microfacet BSDFs of both the metal and dielectric-base layers. (For this reason, this effect is not represented by incorporating an explicit thin-film Slab into the model).

# Conflicts:
#	index.html
#	parametrization.md.html
#	reference/open_pbr_surface.mtlx
@portsmouth portsmouth marked this pull request as ready for review April 29, 2024 19:04
Copy link

linux-foundation-easycla bot commented Apr 30, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

index.html Outdated
@@ -545,22 +545,23 @@
\begin{equation}
\mathbf{F}_{82}(\mu) = \mathbf{F}_{\mathrm{Schlick}}(\mu) - \frac{\mu (1 - \mu)^6}{\bar{\mu}(1 - \bar{\mu})^6} \Bigl(\mathbf{F}_{\mathrm{Schlick}}(\bar{\mu}) - \mathbf{F}(\bar{\mu})\Bigr)
\end{equation}
where $\bar{\mu} = 1/7$, and $\mathbf{F}(\bar{\mu})$ is the desired metallic reflectivity at that "grazing edge" angle cosine corresponding roughly to $82^\circ$ (i.e. around silhouettes), ensuring $\mathbf{F}_{82}(\bar{\mu}) = \mathbf{F}(\bar{\mu})$. This desired edge reflectivity is user-specified as a fractional tint of the Schlick curve, i.e.
where $\bar{\mu} = 1/7$, and $\mathbf{F}(\bar{\mu})$ is the desired metallic reflectivity at that "grazing edge" angle cosine corresponding roughly to $82^\circ$ (i.e. around silhouettes), ensuring $\mathbf{F}_{82}(\bar{\mu}) = \mathbf{F}(\bar{\mu})$. This desired edge reflectivity is user-specified as a fractional tint of the Schlick curve control via **`specular_color`**, i.e.
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean "tint of the Schlick curve control, via specular_color" or "tint of the Schlick curve, controlled via specular_color"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes should be the latter, good catch!

index.html Outdated Show resolved Hide resolved
@virtualzavie
Copy link
Contributor

Following the reply from Jamie, I'm proposing a minor change.
Other than that, the PR looks good to me and ready to merge.

@jstone-lucasfilm
Copy link
Member

@portsmouth @virtualzavie This looks really promising, and make sure you've updated the OpenPBR example materials (e.g. open_pbr_pearl, open_pbr_soap_bubble) so that they render as expected with this change. We'll be releasing those example materials as part of each future version of OpenPBR, and they're a great way for artists to understand the visual impact of control changes.

@virtualzavie
Copy link
Contributor

Addendum: the open_pbr_pearl.mtlx, open_pbr_soapbubble.mtlx and open_pbr_surface_default.mtlx files need to be updated as the parametrisation have changed.

portsmouth and others added 3 commits April 30, 2024 22:52
Co-authored-by: Julien Guertault <9511025+virtualzavie@users.noreply.github.com>
Signed-off-by: Jamie Portsmouth <jamports@mac.com>
@portsmouth
Copy link
Contributor Author

Addendum: the open_pbr_pearl.mtlx, open_pbr_soapbubble.mtlx and open_pbr_surface_default.mtlx files need to be updated as the parametrisation have changed.

Done in ac476eb.

@virtualzavie
Copy link
Contributor

All looks good to me and I'd say the PR is ready to merge. 👍

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.

This looks good to me as well, thanks @portsmouth and @virtualzavie!

@jstone-lucasfilm jstone-lucasfilm merged commit 71d27bf into AcademySoftwareFoundation:main May 7, 2024
1 check passed
@portsmouth
Copy link
Contributor Author

portsmouth commented May 7, 2024

It seems the .svg file fails to render in the live version of the spec, though it did in my local browser for some reason.

I think we need to convert it to PDF.

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