Use roughness-dependent fresnel when rendering with multiscattering #16559
Conversation
| @@ -268,17 +279,15 @@ vec3 BRDF_Specular_GGX_Environment( const in GeometricContext geometry, const in | |||
| // http://www.jcgt.org/published/0008/01/03/ | |||
| void BRDF_Specular_Multiscattering_Environment( const in GeometricContext geometry, const in vec3 specularColor, const in float roughness, inout vec3 singleScatter, inout vec3 multiScatter ) { | |||
|
|
|||
| float dotNV = saturate( dot( geometry.normal, geometry.viewDir ) ); | |||
| float dotNV = max( 0.01, dot( geometry.normal, geometry.viewDir ) ); | |||
There was a problem hiding this comment.
Can you please explain your reason for this redefinition?
There was a problem hiding this comment.
Idea was this should never actually be 0, but that isn't true in some projections(?), and regardless doesn't change the outcome here. Will remove.
|
|
||
| vec3 F = F_Schlick( specularColor, dotNV ); | ||
| vec3 F = F_Schlick_RoughnessDependent( specularColor, dotNV, roughness ); |
There was a problem hiding this comment.
You are passing in dotNV to your method, but the method as written expects dotLH.
There was a problem hiding this comment.
The Fdez-Aguera paper uses the normal vector rather than the halfway vector. Not sure if that's required, or if this should use the halfway vector like other instances. I'll do some side by side tests.
Either way, as this is a new schlick function, the signature needs to be updated. Good catch!
|
@robertlong Could you give this a try if you still have that hot-fresnel in Hubs? |
jsantell
force-pushed
the
roughness-dependent-fresnel
branch
from
af43249
to
b14bbaf
Compare
|
Addressed comments, and trying more scenarios with the changes. This could also be a preference, with prior art of PBR models (Disney's) exposing some "fresnel" parameter that isn't based on physics. Similarly, with the previous standard shader changes in r101/ #15644, any sort of change can be unexpected. Maybe parameterizing a fresnel value by mapping directly to the roughness dampening seems like an obvious parameter to consider if anything, although I haven't given this more thought. Side by side comparison of change; dielectrics on top, conductors bottom, roughness 0 -> 1
|
|
There's also the related work of potentially reverting #15673 -- the rationale was that it produced a dark fresnel, but that could just be that scenario (there's no dark fresnel when the torus is reflecting the lighter sky, rather than distorting the dark building). This is mostly noticeable in smooth dielectrics (metalness:0,roughness:0). Reverting that patch, along with this patch makes dielectrics energy-preserving as well (i.e. passing the furnace test), and may be preferable. This is what it looks like:
Note that this test scene is WIP, the view angles for the objects on the side are more extreme than in the middle. I can kick this to a new issue as well |
|
Have you tried comparing how the same scene looks like in babylon.js? |
Not yet, although here are some comparisons, roughness increasing rightward, metalness increasing upward in a furnace test; any visible surfaces indicate energy gain or loss: r100 r104, with energy compensation for metals, note the top row is invisible (#15644, #15673). Note the newly hottened fresnel, especially on rough materials. This is what's currently on dev. Reverting #15673 makes dielectrics also energy preserving however, completely hiding the row at the bottom: Applying this patch, roughness dependent fresnel; note the reduced hotness compared to the r104 renders: Applying this patch, on top of reverting #15673:
What is the ideal outcome, or goal? Targeting another renderer (e.g. Filament, Babylon), or manufactured tests like the furnace tests above, or by arbitrary examples in the repo? I'm not sure which one I'd even suggest. At least subjectively, this patch makes rough matte objects possible again, subduing the very hot fresnel, and matching the paper's implementation |
|
#15673 does not look right, so I'd revert it. This PR is correcting an error in a former PR, right? So, it has to be fixed -- unless you argue that the change is not perceptible (e.g., your last image above). You may be seeing what happens when we cobble together techniques from multiple sources that do not fit together as a coherent whole. |
The original rational was a dark fresnel in some cases, but I think that may have been due to some specific examples (and think they were correctly reflecting dark environments). // @mrdoob
This PR is correcting the original energy compensation one by applying roughness-dependent fresnel. This patch, and the reverting are two additional changes -- when applying both (last image), the bottom row is now invisible and overall darkening for non-dielectric/metal materials (due to revert), and additionally, there is less fresnel glow (due to this patch). Right now, without either patch, the fresnel is very hot (second picture). Either of these would reduce that delta, but I believe both would be the most consistently correct in terms of furnace test at least. Handling the inbetween (0<metalness<1) values can be done with Filament's technique + LUT IIRC, but everything would be consistent with the Fdez paper with both of these.
I was thinking the same thing, hence why I'm mentioning reverting #15673 at all, since they're all related. (Edit: thinking more about that especially with the newer gltf materials/Dassault specs) and wondering how that could be applied here) |
|
Thanks! |
|
Yeah, that fresnel was a bit too much... Before:
After:
|
Follow up from the multiscattering handling in PBR rendering: #15644 #15673
The fresnel was not taking into account roughness, resulting in bright fresnel that is especially noticible on rough, dark materials.
(0x000, roughness:1, metalness:0) With changes (top), and r105 (bottom):
(0x000, roughness:1, metalness:0) With changes (top), and r105 (bottom):