Improve layering in the solari BRDF#24243
Conversation
|
Btw would love to know the code you used for the white furnace test for solari - I lost mine. |
|
Thanks for the review! I think I've addressed everything, with the only logic change being the mirror pdf handling. |
| if material.roughness <= MIRROR_ROUGHNESS_THRESHOLD { | ||
| return EvaluateAndSampleBrdfResult( | ||
| wi, | ||
| rho.specular / specular_weight, |
There was a problem hiding this comment.
I don't think rho.specular / specular_weight is right here. You need to evaluate the BRDF.
There was a problem hiding this comment.
It should be the same thing, as the total reflected energy for a mirror is the same as the brdf evaluated for the reflected ray.
Doing a brdf evaluation does feel more readable though, and the lut at 0 roughness probably isn't that great.
JMS55
left a comment
There was a problem hiding this comment.
One last potential issues (not sure which it should be, my brain is a little fried).
Also, we have a couple usages of F_AB, which samples a texture. Can we hoist that out now?
| if material.roughness <= MIRROR_ROUGHNESS_THRESHOLD { | ||
| return EvaluateAndSampleBrdfResult( | ||
| wi, | ||
| evaluate_specular_brdf(wo, wi, world_normal, material) / specular_weight, |
There was a problem hiding this comment.
Are we sure that we should be using evaluate_specular_brdf and not evaluate_brdf here?
There was a problem hiding this comment.
Old code evaluated the full BRDF, but not sure if that was right.
| #import bevy_render::maths::{PI, orthonormalize} | ||
| #import bevy_render::view::View | ||
| #import bevy_solari::brdf::{evaluate_brdf, evaluate_and_sample_brdf, fresnel} | ||
| #import bevy_solari::brdf::{evaluate_brdf, evaluate_and_sample_brdf, brdf_pdf, lobe_reflectances} |
|
|
||
| let diffuse_pdf = wi_tangent.z / PI; | ||
| let specular_pdf = ggx_vndf_pdf(wo_tangent, wi_tangent, material.roughness); | ||
| return specular_weight * specular_pdf + diffuse_weight * diffuse_pdf; |
There was a problem hiding this comment.
Nit: Reverse these to match evaluate_and_sample_brdf.
Objective
Improve the solari BRDF. Split off from #23818.
Solution
The layering approach used in the current brdf to distribute energy between the specular and diffuse lobes doesn't work, for reasons I honestly don't quite understand.
The suggested changes are roughly inspired by OpenPBR's equation 42. We're not using EON though, so this uses Filament's multiscattering correction instead of equation 39.
This formulation has the downside of breaking reciprocity. As far as I can tell the path tracer is unidirectional, so this shouldn't cause issues.
Showcase
White furnace
Main:
This PR:
PICA PICA pathtraced
Main:
This PR:
