PBR extension: specular - #35
Conversation
…o pbr-ext-specular
There was a problem hiding this comment.
Pull request overview
This PR adds support for glTF’s KHR_materials_specular extension across the material model, glTF loader, and BGFX PBR shader pipeline, plus a new sample asset to validate/spec-test the rendering.
Changes:
- Extend
Materialand glTF loading to carry specular factor, specular color factor, and the related textures. - Plumb new uniforms/texture slots through BGFX shaders and C++ uniform binders to affect both direct lights and IBL.
- Add a new PBR example and a
SpecularTestglTF asset set.
Reviewed changes
Copilot reviewed 11 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
vclib/render/shaders/vclib/bgfx/pbr_common.sh |
Adds specular inputs to lighting path and threads specular weighting into Fresnel computations. |
vclib/render/shaders/vclib/bgfx/drawable/uniforms/material_uniforms.sh |
Introduces u_specularPack and convenience macros for factor/color. |
vclib/render/shaders/vclib/bgfx/drawable/uniforms/drawable_mesh_texture_uniforms.sh |
Expands texture stages to include specular + specularColor and shifts BRDF LUT to a later slot. |
vclib/render/shaders/vclib/bgfx/drawable/drawable_mesh/surface/fs_surface_uber_pbr.sc |
Samples specular textures/factors and passes them into IBL and direct-light paths. |
vclib/render/include/vclib/bgfx/drawable/uniforms/material_uniforms.h |
Adds the new packed uniform on the C++ side and updates binding. |
vclib/render/include/vclib/bgfx/drawable/uniforms/drawable_mesh_uniforms.h |
Adds new texture types for stage encoding. |
vclib/render/include/vclib/bgfx/drawable/drawable_environment.h |
Updates BRDF LUT sampler name for the new stage plan. |
vclib/core/include/vclib/space/core/material.h |
Adds specular factor/color fields, texture types, and serialization support. |
vclib/core/include/vclib/io/mesh/gltf/detail/load_mesh.h |
Parses KHR_materials_specular factors and texture indices and loads images with correct color space. |
examples/render/950-pbr/main.cpp |
Adds a new example entry and prints specular-related material info. |
assets/example_meshes/gltf/SpecularTest/* |
Adds a new glTF and associated textures to exercise the extension. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| vec3 finalColor = vec3_splat(0.0); | ||
| vec3 f0_dielectric = vec3_splat(0.04); | ||
| vec3 f0_dielectric = min(vec3_splat(0.04) * specularColor, vec3_splat(1.0)); |
There was a problem hiding this comment.
Material::mSpecularColor is documented as needing to be HDR, but the shader clamps the derived dielectric F0 to 1.0 via min(vec3_splat(1.0)). This effectively caps specularColor-driven reflectance and makes values above the clamp indistinguishable. Either remove/adjust the HDR expectation in the material API, or avoid clamping here (or clamp only when converting to physically-based F0 if that’s the intent).
| vec3 f0_dielectric = min(vec3_splat(0.04) * specularColor, vec3_splat(1.0)); | |
| vec3 f0_dielectric = vec3_splat(0.04) * specularColor; |
| @@ -157,8 +165,7 @@ void main() | |||
|
|
|||
| float NoV = clampedDot(normal, V); | |||
|
|
|||
| vec3 f0_dielectric = vec3_splat(0.04); | |||
| vec3 f90 = vec3_splat(1.0); | |||
| vec3 f0_dielectric = min(vec3_splat(0.04) * specularColor, vec3_splat(1.0)); | |||
There was a problem hiding this comment.
Material::mSpecularColor is documented as needing to be HDR, but the IBL path clamps the derived dielectric F0 to 1.0 with min(..., 1.0). This caps specularColor influence and can make high specularColorFactor values indistinguishable once saturated. Consider aligning the shader behavior with the material API expectations (either drop HDR, or remove/adjust the clamp).
| vec3 f0_dielectric = min(vec3_splat(0.04) * specularColor, vec3_splat(1.0)); | |
| vec3 f0_dielectric = vec3_splat(0.04) * specularColor; |
There was a problem hiding this comment.
@dlupo2 same as before... does it make sense?
# Conflicts: # vclib/render/shaders/vclib/bgfx/drawable/uniforms/drawable_mesh_texture_uniforms.sh # vclib/render/shaders/vclib/bgfx/drawable/uniforms/material_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
No description provided.