AcademySoftwareFoundation / OpenPBR

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

Add coat darkening and saturation effects #162

Closed portsmouth closed 2 months ago

portsmouth commented 5 months ago

This PR adds a detailed discussion to the spec of the coat darkening effect (incorporating the ideas discussed in https://github.com/AcademySoftwareFoundation/OpenPBR/issues/136 and https://github.com/AcademySoftwareFoundation/OpenPBR/issues/141), with:

I also include a placeholder for a discussion of how the coat roughness should affect the underlying lobes. (Probably to be done in a separate PR, and derivation of a reasonable physical approximation for this is WIP).

To make it easier to read, I broke the Coat section into sub-sections.

image

image

image

image

portsmouth commented 3 months ago

One aspect of the proposed formulas that I worry about is the effect of the base roughness on the darkening, i.e. Equation 61. This is designed so that a smooth base produces less darkening, basically blending between known formulas for the cases of a completely smooth and completely diffuse base. (See the discussion on Slack here, for background to that -- it is based on the Weta "The Path of Water" notes from SIGGRAPH 2023).

I assumed that if the base is dielectric, the roughness of the reflection from it is determined by the specular_roughness. However if the base has a scattering medium (i.e. as it does in the glossy-diffuse case, or if there is a transmission or subsurface volume) then the reflection is effectively roughened due to this scattering, apart from the initial Fresnel reflection. The proposed formula ignores this effect, so probably underestimates the darkening effect of the coat on a glossy-diffuse base or subsurface. We would need to do some proper investigation with Monte Carlo simulations to know.

Perhaps it's fine to still give these formulas merely as a convenient, reasonable approximation.

peterkutz commented 3 months ago

I assumed that if the base is dielectric, the roughness of the reflection from it is determined by the specular_roughness. However if the base has a scattering medium (i.e. as it does in the glossy-diffuse case, or if there is a transmission or subsurface volume) then the reflection is effectively roughened due to this scattering, apart from the initial Fresnel reflection. The proposed formula ignores this effect, so probably underestimates the darkening effect of the coat on a glossy-diffuse base or subsurface.

Is this not addressed by the formula below?

$r_d = \mathrm{lerp}(1, r, \xi_s F_s)$

I thought this was considering the diffuse component to have an effective roughness of $1$.

peterkutz commented 3 months ago

This is looking really good to me at a high level but I would need to do another pass to verify some of the formulas by looking more closely and by looking at the referenced equations and citations. A lot of my comments are questions and clarifications that I could potentially answer myself based on a more careful reading, however I figured I would post them anyway in case they raise any important points and in case the text can be clarified to prevent other readers from initially having the same questions.

portsmouth commented 3 months ago

Is this not addressed by the formula below?

rd=lerp(1,r,ξsFs)

I thought this was considering the diffuse component to have an effective roughness of 1.

I stated it incorrectly, I meant that if scattering medium is actually absent -- e.g. coat on clear glass (or glass with a volume with huge MFP, say), then it seems wrong to assume there is a diffuse component with a roughness of 1, that we should blend in according to the F0 of the glass.

Fixing that would involve the MFP somehow, but it would get a bit too elaborate probably.

portsmouth commented 3 months ago

This is looking really good to me at a high level but I would need to do another pass to verify some of the formulas by looking more closely and by looking at the referenced equations and citations. A lot of my comments are questions and clarifications that I could potentially answer myself based on a more careful reading, however I figured I would post them anyway in case they raise any important points and in case the text can be clarified to prevent other readers from initially having the same questions

Thanks, I appreciate the careful reading and discussion! :smiley:

portsmouth commented 3 months ago

Not sure it's helpful, but my implementation of this in the GLSL viewer might be a useful point of reference. It looks like this, where the darkening is applied to the weight of the "base substrate" lobe (which consists of the mix of metal and dielectric base lobes):

    // Compute base darkening factor due to presence of coat:
    vec3 base_darkening = vec3(1.0);
    if (coated && coat_darkening > 0.0)
    {
        float Kr = 1.0 - (1.0 - average_dielectric_fresnel(coat_ior))/sqr(coat_ior);
        float Ks = FresnelDielectricReflectance(abs(winputL.z), coat_ior);
        float Fr = clamp(specular_weight * fresnel_refl_normal_incidence(), 0.0, 1.0);
        float rd = mix(1.0, specular_roughness, Fr); // estimate of roughness of dielectric base
        float rm = specular_roughness;               // estimate of roughness of metallic base
        float rb = mix(rd, rm, M);  // thus estimated roughness of entire base
        float K = mix(Ks, Kr, rb);  // thus estimated internal diffuse reflection coeff.
        vec3 E_dielectric_base = mix(mix(albedos.m[ID_DIFF_BRDF], albedos.m[ID_SSSC_BTDF], S),
                                         albedos.m[ID_SPEC_BTDF], T);     // dielectric base albedo
        vec3 E_base = mix(E_dielectric_base, albedos.m[ID_META_BRDF], M); // entire base albedo
        vec3 Delta = (1.0 - K) / vec3(1.0 - E_base*K);                    // full darkening factor
        base_darkening = mix(vec3(1.0), Delta, C * coat_darkening);       // modulated darkening factor
    }
    vec3 w_base_substrate = w_coated_base * mix(vec3(1.0), base_darkening * coat_color * (vec3(1.0) - albedos.m[ID_COAT_BRDF]), C);
portsmouth commented 3 months ago

I'm not sure it's practical to implement this in the MaterialX graph. It would be very time consuming to construct all the math in XML without errors.

Also to implement this in the graph would require making various simplifications, as:

I can try to do something minimal, but probably the relevant functions should instead be exposed as built-in utility nodes.

Conceptually, this is really just the physics of the BRDF obtained by layering a dielectric coat on a base slab. So ideally the layer operation should handle this (using the notion in MaterialX that the implementation of layering depends on the specific things being layered). Though then this would require MaterialX to explain the physics it thinks should be implemented, which it makes no attempt to do in the current spec except in a very loose, mathematics-free form.

jstone-lucasfilm commented 3 months ago

@portsmouth Would it be practical to implement this logic in vanilla OSL, using only the set of closures and functions that OSL natively provides? If so, then I could take the step of translating this back to a MaterialX graph that generates the vanilla OSL code.

portsmouth commented 3 months ago

@portsmouth Would it be practical to implement this logic in vanilla OSL, using only the set of closures and functions that OSL natively provides? If so, then I could take the step of translating this back to a MaterialX graph that generates the vanilla OSL code.

It would certainly be a lot easier than trying to write code in XML, yes.

If the native functions are the ones listed here, then it doesn't have them built in (i.e. the formula for dielectric Fresnel factor and its hemispherical albedo). It's also not possible I take it to compute the directional (or hemispherical) albedo of a BSDF closure in OSL (or the MaterialX graph), so those albedos will I assume just have to be be approximated ignoring the view direction.

jstone-lucasfilm commented 3 months ago

@portsmouth Yes, I believe that's the very latest specification for OSL, so that's the set of closures and functions that we would need to use here. Would you like to put together a proposal for a pure OSL definition of the behavior you'd like to see, and I can do the work of translating this to the equivalent MaterialX graph?

portsmouth commented 3 months ago

@jstone-lucasfilm

Here's a first pass at the OSL code, expressed in a self-contained form as a function for the coat darkening factor (which will be multiplied into the base closure combination in the graph -- that part not shown here), taking as input the relevant subset of the OpenPBR parameters:

// Hemispherical (average) albedo of dielectric Fresnel factor
float E_F(float eta)
{
    return log((10893.0*eta - 1438.2)/(-774.4*sqr(eta) + 10212.0*eta + 1.0)); // (NB, this is natural log)
}

float average_dielectric_fresnel(float eta)
{
    // Technically, this is only valid for IOR < 3, unless we use a more elaborate E_F approx.   
    if      (eta > 1.0) return E_F(eta);
    else if (eta < 1.0) return 1.0 - sqr(eta)*(1.0 - E_F(1.0/eta));
    else return 0.0;
}

float dielectric_F0(float eta)
{
   return sqr((eta - 1.0)/(eta + 1.0));
}

color coat_darkening_factor(color base_color, 
                            float base_metalness,
                            float specular_weight,
                            float specular_roughness,
                            float specular_ior,
                            float transmission_weight,
                            color subsurface_color,
                            float subsurface_weight,
                            float coat_ior,
                            float coat_weight,
                            float coat_darkening)
{ 
   // Compute estimate of roughness of base
   const float ambient_ior = 1.0;                                                // (Assume here no nested dielectrics, for simplicity)
   float coat_ior_average = mix(ambient_ior, coat_ior, coat_weight);             // (as described in Coat section of spec)
   float eta_s = specular_ior / coat_ior_average;                                // (as described in Coat section of spec)
   float Fr = clamp(specular_weight * dielectric_F0(eta_s), 0.0, 1.0);           // dielectric specular reflection F0, modulated by specular weight
   float rd = mix(1.0, specular_roughness, Fr);                                  // estimate of roughness of dielectric base
   float rm = specular_roughness;                                                // estimate of roughness of metallic base
   float rb = mix(rd, rm, base_metalness);                                       // thus estimated roughness of entire base
   // Compute "internal diffuse reflection coefficient", taking into account estimated base roughness
   float coat_eta = coat_ior / ambient_ior;
   float Kr = 1.0 - (1.0 - average_dielectric_fresnel(coat_eta))/sqr(coat_eta);  // internal diffuse reflection coefficient, rough case
   float Ks = dielectric_F0(coat_eta);                                           // internal diffuse reflection coefficient, smooth case
                                                                                 // (approx., since we don't have access to wi in the MaterialX graph)
   float K = mix(Ks, Kr, rb);                                                    // thus estimated internal diffuse reflection coeff.
   // Compute estimated total base albedo
   color diffuse_albedo      = color(Fr) + (1.0-Fr)*base_color;                  // approx. diffuse slab albedo
   color subsurface_albedo   = color(Fr) + (1.0-Fr)*subsurface_color;            // approx. subsurface slab albedo
   color transmission_albedo = Fr;                                               // approx. translucent slab albedo (ignore volume or tint, for simplicity)
   color metal_albedo        = specular_weight * base_color;                     // approx. metal slab albedo
   color albedo_opaque_dielectric = mix(diffuse_albedo,           subsurface_albedo,   subsurface_weight);
   color albedo_dielectric        = mix(albedo_opaque_dielectric, transmission_albedo, transmission_weight);
   color albedo_base              = mix(albedo_dielectric,        metal_albedo,        base_metalness);
   // Final darkening factor
   color Delta = color(1.0 - K) / (color(1.0) - albedo_base*K);
   return mix(color(1.0), Delta, coat_weight * coat_darkening);                  // modulate according to coat weight
}

(NB, this assumes the specular weight changes of https://github.com/AcademySoftwareFoundation/OpenPBR/pull/157 will be applied as well).

jstone-lucasfilm commented 3 months ago

Thanks @portsmouth, and that looks straightforward to implement in MaterialX as a collection of new subgraphs, which we can define in the same MaterialX document as the reference graph for OpenPBR. This will definitely add a significant amount of new code to generated shaders (e.g. GLSL, OSL, MDL), but it seems worthwhile to add this new logic and then evaluate the visual benefits versus the performance impact.

jstone-lucasfilm commented 3 months ago

Since this is dependent on PR #157, let's finalize and merge that one first, and then we can start on the MaterialX graph for this proposal.

jstone-lucasfilm commented 2 months ago

I've now merged #157, and the next step would be to resolve conflicts between these two pull requests, after which we can start on the MaterialX-side updates for this change.

portsmouth commented 2 months ago

Conflicts resolved.

portsmouth commented 2 months ago

@jstone-lucasfilm are you still intending to generate the MaterialX graph from the OSL code above?

jstone-lucasfilm commented 2 months ago

@portsmouth I was planning to take this step in preparation for a v1.0 release in late June, which would give us sufficient time for validation and adjustments. But if we need to lock down the codebase in just one week (as suggested by your colleagues in our last meeting), then this would become a significant last-minute change in behavior, which seems less practical to accomplish.

portsmouth commented 2 months ago

We could presumably “lock down” the spec (as far as parameters and ranges) including the darkening, without the MaterialX implementation of the suggested math. If need be, we could put back the old standard surface approximation for the darkening, in the interim till a better MaterialX implementation is done, which at least makes the darkening parameter do something. The math given in the spec is just a suggested approximation, it doesn’t dictate the specific formulas used.

It seems much less disruptive to just change the MaterialX implementation later, than changing the model parametrization.

(I can add the equivalent of the standard surface approximation as the interim MaterialX solution, if that helps).

jstone-lucasfilm commented 2 months ago

@portsmouth I'm not as concerned about the text of the specification changing after a v1.0 release, but we likely won't be able to change our reference implementation without a corresponding version number change, as this is the source document for the shading code that many teams (e.g. Karma, RenderMan, Omniverse) will use in their OpenPBR implementations.

portsmouth commented 2 months ago

So how about we just ship with the interim MaterialX darkening solution borrowed from standard surface? This is easy to add. (And keeps the text of this PR intact).

(Agreed of course it would be better to have more time to implement the better approximation in MaterialX, but if time is limited we can go with the other option temporarily).

jstone-lucasfilm commented 2 months ago

@portsmouth I didn't get the sense that any artists liked the original Standard Surface coat darkening, and it seemed both visually unappealing and counterintuitive as a control system. So that would likely be an unnecessary step backwards for the OpenPBR project overall.

portsmouth commented 2 months ago

I mean it would have the same coat_darkening control, except adapted to turn on the Standard Surface coat darkening (via the MaterialX graph). You mean that that darkening effect is significantly worse than the OpenPBR one? It doesn't respect the IOR of the coat or base roughness, but for default IOR it should look plausible (just raises the base color to a power).

I don't really agree with the logic of "let's release a less powerful model because implementation X doesn't have the time to ship version 1.234 by such and such date".

jstone-lucasfilm commented 2 months ago

I'm hopeful that the new OpenPBR version looks better than the original Standard Surface coat darkening, which was virtually never used by artists, but I don't yet have a real-time implementation that I can test and evaluate! The original Standard Surface version was pretty unappealing, though, from a visual and control system perspective.

portsmouth commented 2 months ago

Or if there is no alternative, I will bite the bullet and implement the MaterialX graph for the darkening logic before the deadline (Probably some script to generate the XML is faster than attempting to do it my hand).

portsmouth commented 2 months ago

I'm hopeful that the new OpenPBR version looks better than the original Standard Surface coat darkening, which was virtually never used by artists, but I don't yet have a real-time implementation that I can test and evaluate! The original Standard Surface version was pretty unappealing, though, from a visual and control system perspective.

We did agree that it looks good verbally in a past meeting, if I recall. We seemed to agree that the darkening looks more natural when turned on than off, which is the default.

As intuitively, one expects e.g. varnished wood to be darker.

jstone-lucasfilm commented 2 months ago

@portsmouth I'm not concerned about the time that it will take to write the coat darkening logic for OpenPBR in MaterialX, but rather the time that it will take to validate it visually and artistically, as one week is not sufficient time to show the functionality to a wide set of artists and get feedback.

portsmouth commented 2 months ago

I think this could be said of almost all the new features in the model. For example, surely the Zeltner sheen, thin film weight, fuzz on top, etc. are not really validated by artists in that sense. I don't think we can hope to achieve that by the 1.0 deadline, for the coat darkening or anything else that changed since Standard Surface.

portsmouth commented 2 months ago

I've now merged https://github.com/AcademySoftwareFoundation/OpenPBR/pull/157, and the next step would be to resolve conflicts between these two pull requests, after which we can start on the MaterialX-side updates for this change.

So has the position changed since this, as seems like now you're pushing to omit the coat darkening from the 1.0? (On the basis that we don't have time to validate it). We've spent quite a lot of time discussing and developing this darkening parametrization, for it to be thrown out at a late stage due to MaterialX concerns. I have also prototyped it in my viewer app, which has been available for months.

jstone-lucasfilm commented 2 months ago

@portsmouth I'd strongly support the idea of including coat darkening for a 1.0 release in late June, but at our last meeting @AdrienHerubel specifically suggested that we omit it due to the new, shorter timetable for OpenPBR release. I'd recommend bringing this up with Adrien!

portsmouth commented 2 months ago

It's worth noting that looking at a proposed visual feature in side-by-side renders is not the same as giving the control system to an artist and asking for their feedback!

The control system just turns the darkening on and off, so not much to it..

NB, we also had Nikie in the meeting, where I recall she agreed the darkening looks good (though unfortunately we don't have the meetings recorded). So there's one artist validation data point, not sure how many we need to be confident.

I did point out on Slack that it's available to test in my viewer, but didn't get feedback on it (apart from when showing it in the meeting, where the feedback was positive as noted).

portsmouth commented 2 months ago

It would be helpful if Adobe/Autodesk implemented/prototyped this in their renderers also.

jstone-lucasfilm commented 2 months ago

I've had a chance to catch up with @portsmouth in a side discussion, and our current recommendation is to include the coat_darkening control in OpenPBR v1.0, but to assign it a default value of zero.

This will give us the flexibility to implement coat darkening using either BSDF node/layer upgrades or graph logic approximations in future versions of MaterialX, and in the meantime we'll have a visual match between our reference implementation and custom Arnold/Eclair shaders for the common case where coat_darkening is left at its default value.

In future versions of OpenPBR, we would then have the option of assigning a new default value of one to coat_darkening, with MaterialX upgrade logic used to transparently handle documents referencing the v1.0 version of OpenPBR.

portsmouth commented 2 months ago

As per the commentary from @jstone-lucasfilm, I've made the change to have coat_darkening default to 0. While I think it's ultimately preferable for the physical darkening effect to happen by default, I think it's reasonable to have the effect be "opt-in" for now, until we have more confidence that it is preferable to change the default.

jstone-lucasfilm commented 2 months ago

This looks good to me, thanks @portsmouth, and my only remaining question is whether we should add the new input to our reference implementation.

Since the coat darkening is not enabled by default, I wouldn't be as concerned about using a very simple reference implementation such as the color squaring of Standard Surface, if that seems like a reasonable compromise for now.

portsmouth commented 2 months ago

This looks good to me, thanks @portsmouth, and my only remaining question is whether we should add the new input to our reference implementation.

Since the coat darkening is not enabled by default, I wouldn't be as concerned about using a very simple reference implementation such as the color squaring of Standard Surface, if that seems like a reasonable compromise for now.

That seems reasonable, I will add it back. Though I'd note that if users decide they don't like the darkening effect on the basis of this, we need to bear in mind that the effect has not been properly implemented yet in MaterialX.

portsmouth commented 2 months ago

@jstone-lucasfilm I added an approximate MaterialX implementation of the coat darkening math (based on the spec, not on the old Standard Surface formula). This should provide a slightly more faithful implementation than the old approximation (e.g. it takes into account the coat IOR (approximately), but not the base roughness).

However, it seems the current MaterialX graph (even before the darkening addition) is failing to compile in the viewer, with error

Error in compiling fragment shader:
0(1723) : error C1503: undefined variable "base_substrate_out"
0(1724) : error C1503: undefined variable "base_substrate_out"
0(1798) : error C1503: undefined variable "base_substrate_out"
0(1799) : error C1503: undefined variable "base_substrate_out"
0(1878) : error C1503: undefined variable "base_substrate_out"
0(1879) : error C1503: undefined variable "base_substrate_out"

It's unclear what causes this, maybe it was introduced in a previous commit. Can you check and help to diagnose it?

jstone-lucasfilm commented 2 months ago

Good catch, @portsmouth, and let me test the reference implementation in main to see what may have broken with respect to the v0.4 version.

jstone-lucasfilm commented 2 months ago

Ok, I've tracked the issue down, and it was actually a limitation of GLSL generation in MaterialX 1.38, which has now been resolved by @niklasharrysson's refactor of the thin-film node in MaterialX 1.39.

Since we're actually targeting MaterialX 1.39 in OpenPBR, I would say that we're safe to move forward with both @virtualzavie's earlier change to thin-film controls and your current implementation of coat darkening, both of which render successfully in 1.39 builds.

In the near future, I'll add support for 1.39 features such as color82 in our reference implementation, and then fully mark this as a MaterialX 1.39 document for clarity.

jstone-lucasfilm commented 2 months ago

For visual confirmation of your proposed change to the reference implementation, here's Anton's carpaint material with coat_roughness set to 1.0, and it looks very convincing to me:

image