[FIX] Fix KHR_materials_specular incorrectly affecting metals and diffuse

[willeastcott]

This PR fixes an issue where specularityFactor from the KHR_materials_specular glTF extension was incorrectly applied to all materials, including metals. When specularFactor: 0 was set, metallic materials would appear completely black because their specular reflections were incorrectly zeroed out.

glTF Sample Viewer (Reference):

Before Fix (PlayCanvas Model Viewer):

After Fix (PlayCanvas Model Viewer):

Changes

• Modified getSpecularModulate() to incorporate specularityFactor directly into the dielectric F0 calculation
• Removed the post-multiplication of dSpecularLight and dReflection by specularityFactor
• For metals (metalness=1), F0 is now correctly derived from the base color, unaffected by specularityFactor
• For dielectrics (metalness=0), F0 is correctly scaled by specularityFactor

Technical Details

According to the KHR_materials_specular specification:

dielectric_f0 = specularFactor * ior_f0 * specularColor
F0 = lerp(dielectric_f0, baseColor, metalness)

For metals, F0 should always be the base color regardless of specularFactor. The previous implementation applied specularityFactor as a post-multiplier to all specular outputs, which incorrectly affected metals.

Before (incorrect)

// F0 calculated without specularityFactor
litArgs_specularity = getSpecularModulate(specularity, albedo, metalness, f0);

// ... later, specularityFactor applied to ALL specular (including metals)
dSpecularLight *= litArgs_specularityFactor;
dReflection.rgb *= litArgs_specularityFactor;

After (correct)

// specularityFactor incorporated into dielectric F0 only
litArgs_specularity = getSpecularModulate(specularity, albedo, metalness, f0, specularityFactor);
// mix(dielectricF0 * specularityFactor, albedo, metalness) ensures metals use albedo as F0

Testing

• All 91 existing tests pass
• Tested with glTF assets using KHR_materials_specular extension
• Metallic materials with specularFactor: 0 now correctly show their base color as specular
• Dielectric materials with specularFactor: 0 now correctly show full diffuse with no specular

API Changes

None - this is a bugfix with no public API changes.

Fixes #7304

Checklist

• I have read the contributing guidelines
• My code follows the project's coding standards
• This PR focuses on a single change

[Copilot]

Pull request overview

This PR fixes a critical bug in the implementation of the KHR_materials_specular glTF extension where specularityFactor was incorrectly applied to all materials, causing metallic materials with specularFactor: 0 to render completely black. The fix ensures the factor only affects dielectric F0 calculations, not metallic materials.

Key Changes:

• Modified getSpecularModulate() to incorporate specularityFactor into the dielectric F0 calculation rather than applying it as a post-multiplier
• Removed incorrect post-multiplication of specular outputs that was affecting all materials including metals
• Added clear inline documentation referencing the KHR_materials_specular glTF specification

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/scene/shader-lib/glsl/chunks/lit/frag/metalnessModulate.js	Updated getSpecularModulate() function to accept specularityFactor parameter and apply it only to dielectric F0 calculation
src/scene/shader-lib/wgsl/chunks/lit/frag/metalnessModulate.js	WGSL version of the same getSpecularModulate() function update
src/scene/shader-lib/glsl/chunks/lit/frag/pass-forward/litForwardBackend.js	Updated function calls to pass specularityFactor to getSpecularModulate() and removed incorrect post-multiplication of dSpecularLight and dReflection
src/scene/shader-lib/wgsl/chunks/lit/frag/pass-forward/litForwardBackend.js	WGSL version of the same forward rendering backend updates

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.