Correct diffuse component for image-based lighting

[jjhembd]

Description

In our image-based lighting, the diffuse component is constructed from spherical harmonic (SH) coefficients supplied by the user. These coefficients represent the irradiance coming from the environment map as a function of the surface normal. See for example R. Ramamoorthi and P. Hanrahan, SIGGRAPH 2001 (at the top of page 2):

The irradiance E is then a function of the surface normal n only

However, our code was using the reflection vector (the same one used for the specular component) in the calculation, rather than the surface normal. This PR corrects the diffuse calculation to use the surface normal.

During testing, I noticed that the SH coefficients used in our testing Sandcastles were incorrect. These coefficients generated a diffuse lighting that did not align with the specular component. The coefficients appear to have been generated using google filament's cmgen, but without the needed --no-mirror option.

This PR updates the coefficients in the Sandcastles with corrected cmgen results. I also edited the inline comments to specify the use of the --no-mirror option.

SH coefficient results

Here are some snapshots of the Clearcoat Wicker test model, with the clearcoat component turned off for simplicity. The model is viewed from above (camera pointing directly down), using the Kiara 6 Afternoon environment map. Note: these images were captured before correcting the lookup from the reflection vector to the normal vector.

Diffuse + specular components, old coefficients:

Diffuse component only, old coefficients:

Specular component only:

Diffuse component only, new coefficients:

Diffuse and specular components, new coefficients:

Notice the better alignment between the diffuse and specular components, using the new coefficients.

Comparison of using reflection vs normal vectors for diffuse lighting

These captures use the new (corrected) coefficients, with the clearcoat turned back on.

Using reflection vector for diffuse calculation:

Using normal vector for diffuse calculation:

Note the more even lighting when using the normal vector for diffuse calculations.

Issue number and link

Addresses part of #12028.

Testing plan

Load the "Image-based Lighting" and "glTF PBR Extensions" Sandcastles, and compare renderings with the main branch. Note that the Sandcastle code itself has changed, and some models may need to be rotated toward the brighter side. I did not update the Sandcastle orientations yet, because we will need a follow-up PR to address IBL orientation (see #12028).

Author checklist

• I have submitted a Contributor License Agreement
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• [ ] I have added or updated unit tests to ensure consistent code coverage See Add screenshot testing for subtle lighting changes #12065
• I have updated the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[github-actions]

Thank you for the pull request, @jjhembd!

✅ We can confirm we have a CLA on file for you.

[ggetz]

Awesome, looks much better @jjhembd! I can particularly see the benefit in the "Imaged-Based Lighting" example on the diffuse pieces:

main	branch

[jinoumoming]

There seems to be something wrong with this change
Compared to version 1.119, I added NdotL = clamp(NdotL, 0.001, 1.0) to smithVisibilityGGX to fix this issue, but I don't know the reason. I hope this helps.

[ggetz]

Hi @jinoumoming, that artifact was fixed in #12116. It will be out with the next release. Thanks!