PCF For Directional Light Shadows

[JMS55]

Objective

• Improve antialiasing for directional light shadow edges.
• Very partially addresses https://github.com/bevyengine/bevy/issues/3628.

Solution

• Implements "The Witness"'s shadow map sampling technique.
  • Ported from @superdump's old branch, all credit to them :)
• Implements "Call of Duty: Advanced Warfare"'s stochastic shadow map sampling technique when the velocity prepass is enabled, for use with TAA.
  • Uses interleaved gradient noise to generate a random angle, and then averages 8 samples in a spiral pattern, rotated by the random angle.
  • I also tried spatiotemporal blue noise, but it was far too noisy to be usable as our current TAA implementation clipped it frequently. I'd like to revisit this when we get DLSS/FSR/better TAA.
  • Along the same lines, the COD presentation has an interesting temporal dithering of the noise for use with temporal supersampling that we should revisit when we get DLSS/FSR/other TSR.

---

Changelog

• Improved directional light shadow edges to be less aliased.

[james7132]

@JMS55 can you update the PR description? This is difficult to understand what this PR is doing and it's action plan from just the PR title.

[JMS55]

@james7132 It's an extremely WIP PR. I'll update it soon, but I'm still working out some details.

[JMS55]

I think the PCF method should be manually configurable and default to The Witness. If the stochastic approach is better when using TAA then it could be the default in that case but it should still be possible to manually change it.

Sure, I can add some more bits to the pipeline key.

I don’t think you added me in ‘Co-authored-by:’ on any of The Witness commits.

My apologies, I must have forgotten. I'll figure out how to add you back in.

[JMS55]

@superdump I checked, I didn't forget to credit you :)

I think this is ready to merge if you don't have further feedback.

[superdump]

Going to review again now. Random question to @JMS55 - how come this PR has soooooo many commits in it?

[JMS55]

how come this PR has soooooo many commits in it?

I started working on this PR before TAA was merged, so it's a fork of the TAA branch, which spans months back, and also includes some SSAO work... I probably should figure out how to avoid this.

[JMS55]

Done!

[superdump]

I think the PCF kernel radius is supposed to scaled to keep the world space filter radius constant across cascades. So each cascade has a different radius in texel units.

[robtfm]

a different radius in texel units would be good for Jimenez14. i think we can't do that with the optimisations for Castano13 since it relies on the texel positions for the optimisation to be valid?

but i think the softening/contracting is to do with srgb. i think a 50% shadow looks like a 25% shadow because of the implicit pow(x, 1/2.2) coming from srgb conversion.

using a 256x shadow map and all the biases set to zero (so ignore the acne), the current version shows shadows contracting when we move to a further cascade:

https://user-images.githubusercontent.com/50659922/229477856-0dc4af83-2663-4351-b1a5-7b7680836ccc.mp4

adding a pow(2.2) makes it look more consistent (to me) :

https://user-images.githubusercontent.com/50659922/229478017-95bc8ab4-2e4f-41fb-bd04-e8dd3f776872.mp4

[superdump]

I see the difference in the videos of course, but I have doubts about sRGB being the problem. The returned value from sampling indicates the proportion of light from that source that reaches the fragment. That's about physical quantities of light. I think maybe what is happening here is that the filter radius is so large in world units that it spans across the entire shadow and back into the light so that the majority of samples are lit and relatively few are in shadow.

I don't remember if I ever had both PCF and CSM on one branch, and yes, you're right the assumptions of Castano 13 break down if we don't use full texel steps, but maybe it still works ok with a variable radius.

[alice-i-cecile]

@superdump @robtfm would you like to merge this as is, or block on further improvements / investigation in this PR?

[superdump]

It needs more fixes. Some depth biasing adjustment is needed but I haven’t had time to come back to it yet.

[JMS55]

Something else we could try: Do 1-2 samples, and if entirely unshadowded, skip further sampling for performance reasons.

[JMS55]

I don't think we ever added ShadowFilteringMethod to the Camera3dBundle by default. We should probably do that.

[DGriffin91]

I think PR currently breaks all 3d examples that use the StandardMaterial on webgl. Here's with the 3d_scene example:

Caused by:
    In Device::create_render_pipeline
      note: label = `pbr_opaque_mesh_pipeline`
    Internal error in ShaderStages(NONE | VERTEX | FRAGMENT) shader: C:\fakepath(5725,12-121): warning X3570: gradient instruction used in a loop with varying iteration, attempting to unroll the loop
C:\fakepath(6504,3-15): error X3511: unable to unroll loop, loop does not appear to terminate in a timely manner (102 iterations) or unrolled loop is too large, use the [unroll(n)] attribute to force an exact higher number

Warning: D3D shader compilation failed with default flags. (ps_5_0)
 Retrying with skip validation
C:\fakepath(5725,12-121): warning X3570: gradient instruction used in a loop with varying iteration, attempting to unroll the loop
C:\fakepath(6504,3-15): error X3511: unable to unroll loop, loop does not appear to terminate in a timely manner (102 iterations) or unrolled loop is too large, use the [unroll(n)] attribute to force an exact higher number

Warning: D3D shader compilation failed with skip validation flags. (ps_5_0)
 Retrying with skip optimization
C:\fakepath(5725,12-121): warning X3570: gradient instruction used in a loop with varying iteration, attempting to unroll the loop
C:\fakepath(6504,3-15): error X3511: unable to unroll loop, loop does not appear to terminate in a timely manner (102 iterations) or unrolled loop is too large, use the [unroll(n)] attribute to force an exact higher number

Warning: D3D shader compilation failed with skip optimization flags. (ps_5_0)

Failed to create D3D Shaders

I tested 3d_scene on current bevy main and it appears to be working as expected.

[nicopap]

Can confirm a panic on WGPU_BACKEND=opengl. But the error in question also occurs on main. I assume it's #8086

[nicopap]

I can't reproduce @DGriffin91 error with --feature webgl2. Given the error message, I assume they meant DX12 rather than webgl?

Examples compile and run fine on WASM in this PR.

[nicopap]

I got a similar panic when developing parallax_mapping. I had to dig out an old windows computer to test and fix it.

I documented the issue in the code:

https://github.com/bevyengine/bevy/blob/84de9e7f28bfaaba8e0eef4ecf185ce795add9e2/crates/bevy_pbr/src/render/parallax_mapping.wgsl#L4-L14

Taking a careful look at the code:

The pbr function in pbr_functions.wgsl does call fetch_directional_shadow in a for loop that has a variable as upper bound.

In fetch_directional_shadow, we call sample_directional_cascade. Which itself will call sample_shadow_map.

sample_shadow_map contains a textureSampleCompareLevel call. However, this call specifically shouldn't trigger this issue, as noted in the comment.

We are left with textureDimensions. a priori, it shouldn't be a gradient instruction.

Furthermore, I see a comment in sample_shadow_map_hardware_2x2 that specifically addresses the issue. So maybe @DGriffin91 was testing an older version of the PR?

[DGriffin91]

@nicopap just re-cloned this PR and tested again. Still failing in chrome and firefox on the 3d_scene example on windows with the same error.

[JMS55]

I don't think we ever added ShadowFilteringMethod to the Camera3dBundle by default. We should probably do that.

I remember why I never did that. ShadowFilteringMethod is part of bevy_pbr as it relates to shadow rendering, but Camera3dBundle is from bevy_core_pipeline... What to do?

EDIT: I remember how I fixed this. queue_material_meshes() will select the default ShadowFilteringMethod when the component does not exist on the camera, so it's effectively on by default.

[Elabajaba]

The Fxc shader compiler is kinda broken and abandoned, and I'm pretty sure we're stuck with it for the foreseeable future on webgl2 because afaik Angle uses it.

On Windows DX12 we can just tell users to use DXC and download the dxc .dlls because it's faster and less broken, but it does add some friction (and users will also have to distribute the DXC .dlls).

[JMS55]

I'm not sure there's any significant blockers left on this PR. If you ran into issues before, can you see if you can reproduce them still? I'd love to get this merged soon. I'd also like confirmation that PCF is actually making a difference, I'm having trouble seeing it's effect in a lot of scenarios, but maybe that's just my lighting setup.

[DGriffin91]

@JMS55 This PR is still failing in chrome and firefox on the 3d_scene example (which doesn't use directional shadows) on windows with the same error I previously posted.

[nicopap]

The PCF is f-ing amazing. (at least last time I tried) I don't see the ugly staircase effect in shadows. I've already reviewed the PR. I find the code quality to be excellent, but I'm withholding approval until DGriffin91's issue is addressed.

[JMS55]

I believe I've now resolved all of the issues besides the cascade blending/softening stuff.

One way we may be able to fix it is scale the PCF kernel/radius size based on cascade.texel_size, or maybe the ratio between the current and next cascade.texel_size.

[JMS55]

Probably better idea: Do PCF in world-space

[JMS55]

Is this a C-Breaking-Change since it adjusts how shadows look by default?

[github-actions[bot]]

Example alien_cake_addict failed to run, please try running it locally and check the result.

[github-actions[bot]]

Example alien_cake_addict failed to run, please try running it locally and check the result.

[github-actions[bot]]

Example alien_cake_addict failed to run, please try running it locally and check the result.