Skip to content

Conversation

UX3D-nopper
Copy link
Contributor

No description provided.

@UX3D-nopper UX3D-nopper requested a review from emackey March 2, 2020 19:20
@UX3D-nopper UX3D-nopper changed the title Fix for blending Fix for clearcoat blending Mar 2, 2020
@emackey
Copy link
Member

emackey commented Mar 2, 2020

@romainguy @sebavan @proog128 I've been following the email discussion. Are you guys OK with this PR?

@emackey
Copy link
Member

emackey commented Mar 2, 2020

I'm trying to get an A/B screenshot of the difference. I'm doing something wrong though because all of my clearcoat is washed out & overexposed now...

@emackey
Copy link
Member

emackey commented Mar 2, 2020

So in my implementation, f_clearcoat did not already contain a Fresnel term, and removing it from the formula like this has obvious bad effects.

Also it looks like the sample clearcoat shader in Substance Painter matches what's in master here, not this current change.

Still investigating...

@romainguy
Copy link

We used a squared Fresnel attenuation for a while in Filament as an attempt to account for loss of energy when entering and exiting the clear coat layer. I believe Unity HDRP does this still. May Substance does the same?

@emackey
Copy link
Member

emackey commented Mar 2, 2020

It's not square though. There's no Fresnel term on the IBL being reflected in f_clearcoat, as far as I can tell.

There is a Fresnel term on a punctual light reflected in clearcoat, but that's Fresnel(0.04, VdotH) for H that comes from the light source in question. That's a very different term from the Fresnel(0.04, NdotV) that is used for blending.

For me, this PR makes IBL on the coat come in at full strength, with no Fresnel effect at all.

@sebavan
Copy link
Contributor

sebavan commented Mar 2, 2020

Is the formula not implying it should be done for both analytical and ibl the same ?

In Babylon we actually use the exact same in IBL :

// IBL clear coat energy conservation
float fresnelIBLClearCoat = fresnelSchlickGGX(clearCoatNdotV, clearCoatIOR);
fresnelIBLClearCoat *= clearCoatIntensity;

float conservationFactor = (1. - fresnelIBLClearCoat);

environmentDiffuse *= conservationFactor;
environmentSpecular *= conservationFactor;

@sebavan
Copy link
Contributor

sebavan commented Mar 2, 2020

but as a small diff, we do not apply it to emissive that we consider taking over everything else

@emackey
Copy link
Member

emackey commented Mar 2, 2020

I found the difference! My own code was following the Substance Painter example, which uses a clearcoat specular color of 1.0 instead of 0.04 as this extension calls for. Oddly, pairing this much brighter specular color with the additional Fresnel term yields very similar results to using the darker specular color without the Fresnel term.

@emackey
Copy link
Member

emackey commented Mar 2, 2020

I think I'm good with this PR. Let's leave it open until @proog128 and others have had a day to process and comment.

@proog128
Copy link
Contributor

proog128 commented Mar 3, 2020

Just to make sure: f_clearcoat includes the Fresnel term, right? So it's

f_clearcoat = fresnel(0.04, VdotH) * G * D / (4 * NdotL * NdotV)

coated = f_base * (1 - clearcoatTexture.r * clearcoatFactor * fresnel(0.04, NdotV)) + f_clearcoat * clearcoatTexture.r * clearcoatFactor

This will produce some energy (because the base layer Fresnel is computed with VdotH), but the general structure is OK (matches this comment). Alternatives:

  • Compute f_clearcoat with NdotV in the Fresnel. This is energy conserving, but less "correct", because the Fresnel should act on the facets. (Although the error is not high, because the facet orientation for small roughness values does not deviate much from the macro-surface normal).
  • Apply Fresnel two times for base layer: max(fresnel(0.04, NdotV), fresnel(0.04, NdotL)). This is energy conserving, but loses energy at grazing angles for high roughness (not common for clearcoat).This is used in Enterprise PBR and MDL. Instead of max(...) you could also use (1-fresnel(0.04, NdotV)) * (1-fresnel(0.04, NdotL)), but loses even more energy. This is used in AxF.
  • Albedo-scaling technique as in the sheen BRDF (KHR_materials_sheen #1688).
  • More fancy techniques: 1 2, ...

@emackey
Copy link
Member

emackey commented Mar 3, 2020

Yes. I wonder if the PR needs to capture some of this discussion, particularly that f_clearcoat is expected to already have the Fresnel in it.

@emackey
Copy link
Member

emackey commented Mar 3, 2020

This has enough approvals though, so I'll merge it and make some additional tweaks in a new PR.

@emackey emackey merged commit a4f9ce3 into KhronosGroup:master Mar 3, 2020
@emackey
Copy link
Member

emackey commented Mar 3, 2020

Small follow-up in #1776.

@donmccurdy donmccurdy added this to the PBR Next milestone Apr 28, 2020
@UX3D-nopper UX3D-nopper deleted the fix/KHR_materials_clearcoat branch July 11, 2022 09:21
@emackey emackey added PBR Physically Based Rendering and removed extension:KHR_materials_clearcoat labels Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension PBR Physically Based Rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants