[HDRP][Path Tracing] Environment (sky) importance sampling

[eturquin]

Added environment importance sampling for the path tracer. When changing the global sky settings, CDF and marginal tables are computed (happens once), and the notion of a sky light has been added to the already existing local and distant lights.

Before:
Sky values would only be accessed by material rays that were not intersecting anything. If there were small, bright spots in the envmap, they would result in extremely noisy images.

Now:
HDRI Skies are importance-sampled, resulting in much better convergence rate when using environments with hot, bright spots (e.g. a sky with a sun).

Tests:
This has already been tested with HDRI images from the dedicated HDRP pack. See examples below, without then with importance sampling, all computed with 256 samples. Note that setting the sky type to anything but HDRI will not use this new importance sampling scheme: although it would work fine, importance-sampling very smooth skies (as is the case with gradient and physically-based skies) is actually counter-productive, so it has been explicitly disabled.

To be tested further with as many sky settings as possible.
Sky Importance Sampling can now be toggled manually. The parameter offers 3 modes:

• HDRI Only: on on HDRI skies, but off on Gradient or Physical skies
• On: Always on
• Off: Always off

Note: This PR includes changes made in this other PR #6525

[github-actions]

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/jobDefinition/.yamato%2Fall-hdrp.yml%23PR_HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

[eturquin]

[eturquin]

[eturquin]

[eturquin]

[iM0ve]

Test status: Completed
The improvement to my test scene is substantial, however found a change that I believe might not be intended. Please verify.

The improvement:
My scene uses a Directional light and an HDRI sky which has multiple tube lights captured. The new importance sampling really shines here. Captures are of 256 samples

Before

After

The concern:
On the template however, where HDRI is continuous and does not contain any bright light sources, we see a loss of highlights (the material ball and the ceiling in the 2nd room). Also the picture is maybe a little more grainy.

For light baking there is a dedicated checkbox to enable and disable importance sampling, maybe having it on all the time can cause negative effects? Captured with 1024 samples and other path tracer settings are default.

Before

After

Whats tested:

• Path tracer on personal project
• Path tracer on template

The intensity clamping has been changed in a way that should better retain the pre-PR values (and as such generate more visible bloom), all the while efficiently filtering out fireflies.

[iM0ve]

The last commit provided some improvements but also introduced some like unwanted effects.

The template now matches better, bloom is again visible. Some light is still lost (under the material ball and red wall in the back)

Before PR

After last commit

Here is the more concerning problem. I have changed the HDRI used in the template to this. So it does not contain even lighting, more representing the target of this PR. With latest commit new streaks of light are visible, maybe the cause of selective clamping. update - after investigation it seems expected result

Before PR

After last commit

[eturquin]

I added a Path Tracing parameter to control when sky importance sampling is performed. Documentation will be added to this PR if the current controls are deemed satisfactory.
On top of that, intensity clamping has been reverted to the Master branch behaviour, i.e. applied only on indirect lighting: let's see how it performs, and if needed we may revisit it.

[iM0ve]

Final round of tests. So the new commit added a new parameter Sky Importance Sampling, allowing to not only turn the feature on/off for HDRI, but also for other types of skies. In both cases there was a reduction in noise.I additionally tested:

• Importance sampling on Physically based sky. Used nasa provided star map but with super hight exposure so the stars could light up the scene.
• Importance sampling on Gradient sky. The sky I made is hight contrast, middle and bottom parts are pure black and the top is blue (so that it could benefit from the feature).
• Turning off the feature for all sky types
• Rechecked previous tests to see nothing broke
• Undo functionality
• Naming and tooltip is clear

Physical sky importance sampling OFF

Physical sky importance sampling ON

Gradient sky importance sampling OFF

Gradient sky importance sampling ON

[remi-chapelain]

Tested a few cases especially with Cloud layer / Volumetric Clouds since adding them to a PBSky or Gradient Sky adds clouds to the skybox directly resulting in high contrast areas.

The default HDRI Only is not perfect since there is some cases when we'd like to use importance sampling for uniform / gradient sky for example when there's no lights but the difference is not substantial enough to counteract the added noise when there's lights.
So it's a good compromise. ✔️

Looks good to me :)

[sebastienlagarde]

is it expected? surfaceData.color?

[eturquin]

Yes