PBR extension: clearcoat - #33
Conversation
…o pbr-ext-clearcoat # Conflicts: # vclib/render/include/vclib/bgfx/drawable/mesh/mesh_render_buffers_macros.h # vclib/render/include/vclib/bgfx/drawable/mesh/pbr_macros.h # vclib/render/include/vclib/bgfx/drawable/uniforms/drawable_mesh_uniforms.h # vclib/render/include/vclib/bgfx/drawable/uniforms/material_uniforms.h # vclib/render/shaders/vclib/bgfx/drawable/drawable_mesh/surface/fs_surface_uber_pbr.sc # vclib/render/shaders/vclib/bgfx/drawable/uniforms/drawable_mesh_texture_uniforms.sh # vclib/render/shaders/vclib/bgfx/pbr_common.sh
…o pbr-ext-clearcoat
# Conflicts: # vclib/render/include/vclib/bgfx/drawable/uniforms/drawable_mesh_uniforms.h
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 20 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| float NoV = clampedDot(normal, V); | ||
| float clearcoatNoV = clampedDot(clearcoatNormal, V); | ||
|
|
||
| vec3 clearcoatFresnel = clearcoat * F_Schlick(f0_dielectric, f90, clearcoatNoV); | ||
|
|
There was a problem hiding this comment.
F_Schlick is documented to take VoH, but clearcoat Fresnel is currently computed once using clearcoatNoV and then reused for all lights. This makes the clearcoat response independent of the per-light half-vector and will produce incorrect highlights. Compute clearcoat Fresnel per-light using VoH (or otherwise align the computation with the intended Schlick parameter) and avoid reusing a single value across the loop.
There was a problem hiding this comment.
I put clearcoatNoV out of the loop since both the clearcoat N and V are constant w.r.t. the loop, as for the clearcoatFresnel, it is computed this way in the khronos viewer
|
|
||
| // Fresnel | ||
| vec2 brdf = brdfLutTex(vec2(NoV, roughness)).rg; | ||
| vec3 metalFresnel = iblGgxFresnel(brdf, NoV, roughness, baseColor.rgb); | ||
| vec3 dielectricFresnel = iblGgxFresnel(brdf, NoV, roughness, f0_dielectric); | ||
| vec3 clearcoatFresnel = clearcoat * F_Schlick(f0_dielectric, f90, clearcoatNoV); |
There was a problem hiding this comment.
For IBL, clearcoat Fresnel is computed with F_Schlick(..., clearcoatNoV) and the clearcoat specular term is taken directly from the specular cubemap sample. Unlike the base layer, this doesn’t use the BRDF LUT integration (NoV/roughness) and will diverge noticeably from glTF reference viewers. Consider computing a clearcoat IBL term using the BRDF LUT (e.g., derive a Fresnel/specular response via iblGgxFresnel with clearcoatNoV and clearcoatRoughness) before mixing it into pbrColorIbl.
| vec3 clearcoatFresnel = clearcoat * F_Schlick(f0_dielectric, f90, clearcoatNoV); | |
| vec2 brdfClearcoat = brdfLutTex(vec2(clearcoatNoV, clearcoatRoughness)).rg; | |
| vec3 clearcoatFresnel = clearcoat * iblGgxFresnel(brdfClearcoat, clearcoatNoV, clearcoatRoughness, f0_dielectric); |
There was a problem hiding this comment.
Again, this is how the khronos viewer does it, there should be no need to change things
…rmal or clearcoat textures
# Conflicts: # vclib/render/shaders/vclib/bgfx/drawable/uniforms/drawable_mesh_texture_uniforms.sh
# Conflicts: # vclib/render/include/vclib/bgfx/drawable/uniforms/material_uniforms.h # vclib/render/shaders/vclib/bgfx/drawable/drawable_mesh/material_uniforms.sh # vclib/render/shaders/vclib/bgfx/drawable/drawable_mesh/surface_pbr/fs_surface_pbr.sc # vclib/render/shaders/vclib/bgfx/drawable/drawable_mesh/texture_uniforms.sh # vclib/render/shaders/vclib/bgfx/pbr_common.sh
# Conflicts: # tests/render/006-mesh-pbr-headless/main.cpp # vclib/render/include/vclib/bgfx/drawable/uniforms/material_uniforms.h
TODO:
could it be the same mipmap normal problem of PBR extension: anisotropy #32?ClearCoatCarPaint uses KHR_texture_transform extension which is (still) not implemented in our viewer.