AcademySoftwareFoundation / OpenPBR

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

Update behavior of specular_weight, specular_ior_level, coat_ior_level #157

Closed portsmouth closed 2 months ago

portsmouth commented 5 months ago

This implements the changes proposed in

which we agreed to go ahead with in the recent meetings.

The specular_ior_level param is removed, and replaced by specular_weight which works in the same way. I incorporated the facility to increase the specular_weight to values >1, though with a soft-max at 1 -- retaining the facility we had before, though actually more flexible since it doesn't get capped at an arbitrary maximum value of 2 (it just gets capped by the requirement that the Fresnel factor cannot exceed 1). This seems reasonable and potentially convenient (though we may want to verify that it behaves OK in practice).

The specular_weight is now no longer involved in the Fresnel factor multiplier, only specular_color. We state that this is applied only to one specific scattering mode (the initial Fresnel reflection from above), and call it out explicitly as an unphysical convenience.

image

The coat_ior_level param is also removed, simplifying the coat discussion:

image

portsmouth commented 5 months ago

Corresponding changes will be needed in the MaterialX graph (and our native implementations). I'll leave those for a separate PR though.

portsmouth commented 5 months ago

Nice. With this new parametrisation, I believe we need to remove specular_weight from the metal section.

We could do, as it functions currently (for the metal) only as a multiplier of the specular_color edge tint, which is probably not very important to retain, as the specular_color covers the edge-tint perfectly well. Arguably also it's helpful to remove it since if doing e.g. a rusty metal with varying metalness, if the specular_weight is textured it's inconvenient for that to affect the metallic parts and dielectric parts in a different way.

This means we would remove the instances of specular_weight outlined here:

image

portsmouth commented 5 months ago

In https://github.com/AcademySoftwareFoundation/OpenPBR/pull/157/commits/bffd9e1a37260fa6c7b3877e1022e8cc94822055 I changed as follows:

A more physical scheme would amount to somehow lowering the IOR of the metal to 1, as in the dielectric case, producing a narrowing of the Fresnel highlight rather than a suppression, but this is not doable in the standard Schlick representation, so the linear scaling with the weight seems likely the best approach. (Though note that the $F_0$ does actually scale the same way with specular weight, for both metal and dielectric).

image

image

portsmouth commented 3 months ago

specular_weight limited to 1, thus preventing from locally increasing specular reflection. I think a use case would be gemstones on a character outfit. There is also the problem of converting existing assets, given that a modulation up to 2 has been common in several major tools for about a decade.

We've discussed this a fair bit before. I think it would be fine to allow specular_weight to exceed 1 (make it a soft-max at 1, say). We would just need to specify appropriate clamping logic (for both metal and dielectric Fresnel) since the resulting F0 cannot be allowed to exceed 1.

I find the proposed use-cases a bit unconvincing though, as if you're doing gemstones on a costume you could just set the IOR to that of the gemstones, and lower the specular_weight accordingly for the non-gem bits. Or just paint the actual IOR, in a tool where you are not restricted to [0,1] maps.

Also, above I noted:

In the latest commit, I removed that.. [specular_weight > 1] (for both dielectric and metal). As I think it generally means that the Fresnel will at some angle near grazing end up saturating to 1 with a C1 discontinuity, which I would expect to look a bit unnatural (as real Fresnel doesn't saturate like this, except when TIR happens).

Which is true, e.g. if at specular_weight = 1 the F0=0.75, then at specular_weight=1.333 the IOR will saturate to infinity (to achieve F0=1, according to Equation 26 below, assuming we added a clamp so that the thing inside the square root is $\le 1$). So then varying specular_weight > 1.333 does nothing, the Fresnel curve is just maxed at 1. That seems a bit weird.

image

jstone-lucasfilm commented 3 months ago

Once we're happy with these proposed changes to the specification, I'd recommend that we include the matching updates to the reference implementation in this same pull request. This should allow us to validate the new user controls before merging, and will also insure that our specification and reference implementation remain in sync as we approach a 1.0 release.

portsmouth commented 3 months ago

In https://github.com/AcademySoftwareFoundation/OpenPBR/pull/157/commits/0e9560a6dcbd052ee1b5925fb44a8657ad1c79e8 I updated the MaterialX graph to remove specular_ior_level and coat_ior_level, and implemented the various changes required to be consistent with the updates to the spec of this PR.

I verified that this compiles in the MaterialX viewer, and seems to work correctly.

jstone-lucasfilm commented 2 months ago

This looks great, thanks @portsmouth, and my only suggestion is to double-check that @AntonPalmqvist's example materials render correctly using this new parameterization.

jstone-lucasfilm commented 2 months ago

@portsmouth I've looked at each of the example materials, and the only unusual result I see is that open_pbr_honey.mtlx seems to have lost its transmission color, and now looks more like glass. Let's consider this an issue to address in a future change, though, and I think it's fine to move forward with the current pull request.