AcademySoftwareFoundation / OpenPBR

Specification and reference implementation for the OpenPBR Surface shading model
Apache License 2.0
407 stars 18 forks source link

Thin-film improvements #167

Closed portsmouth closed 2 months ago

portsmouth commented 4 months ago

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 commented 4 months ago

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.:

For reference, the current defaults are:

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

jstone-lucasfilm commented 4 months ago

@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 commented 4 months ago

@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 commented 4 months ago

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.

portsmouth commented 3 months ago

@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 https://github.com/AcademySoftwareFoundation/OpenPBR/pull/167/commits/f8538ece5a5c89c21e6c2096131ae5ce320ad9f1.

portsmouth commented 3 months ago

What about this:

https://github.com/AcademySoftwareFoundation/OpenPBR/pull/167#issuecomment-1969900347

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 commented 3 months ago

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

portsmouth commented 2 months ago

In https://github.com/AcademySoftwareFoundation/OpenPBR/pull/167/commits/984bff1ccd08444dbd6707ee079c861b62deb82d 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

portsmouth commented 2 months ago

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).

linux-foundation-easycla[bot] commented 2 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

virtualzavie commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 https://github.com/AcademySoftwareFoundation/OpenPBR/pull/167/commits/ac476eb98263c9073f44b520461f6704bfb8e5f1.

virtualzavie commented 2 months ago

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

portsmouth commented 2 months ago

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.