Jitter shadow map dithering pattern across frames when TAA is enabled

[Calinou]

This improves shadow quality by reducing the visibility of the noisy pattern caused by dithering.

This jittering also applies when FSR2 is enabled, as it provides its own form of temporal antialiasing.

This closes godotengine/godot-proposals#4179 and closes #53534.

Testing project: test_shadow_jitter.zip

Preview

Click to view at full size (this is required for accurate preview). TAA is enabled on all screenshots.

DirectionalLight3D with non-PCSS shadows

Before	After

OmniLight3D with PCSS shadows

Before	After

Older revision of this PR

DirectionalLight3D with non-PCSS shadows

Before	After

OmniLight3D with PCSS shadows

Before	After

[clayjohn]

Testing this out locally I noticed that the temporal supersampling seemed to introduce some artifacts in the penumbra that weren't their before. So I returned to http://www.iryoku.com/next-generation-post-processing-in-call-of-duty-advanced-warfare to see what we are missing and noticed something interesting. The author suggests an approach for use with 2X SMAA, he says you should use IGN() * 0.5 + frame where frame flips between 0.0 and 0.5. I realized that as implemented the rotation ranges from 0 to 4 * PI because both IGN and frame range between 0 and 1. This creates an issue where it is possible to sample the same (or essentially the same) rotation twice. Removing the 2.0 multiplier restores the range to 0 to 2 * PI and improves the result.

One final optimization, since we are only using frame to calculate this rotation. And since we are using 32 bits anyway, we might as well pass frame as a float with JITTER_FRACTION baked in.

[Calinou]

@clayjohn I've tried the above change and optimization, but it makes shadows flicker wildly while TAA is enabled and the camera is moving. It looks good when still, but it's worse than before when the camera is moving, both for PCSS and non-PCSS shadows.

I committed it separately so you can take a look.

[clayjohn]

@clayjohn I've tried the above change and optimization, but it makes shadows flicker wildly while TAA is enabled and the camera is moving. It looks good when still, but it's worse than before when the camera is moving, both for PCSS and non-PCSS shadows.

I committed it separately so you can take a look.

I tried out your newest commit, but I can't reproduce the shadows flickering wildly while in motion. Keep in mind, there will also be some extra movement during motion from the jitter frames as TAA won't be able to fully reproject and resolve the image. But what I am seeing now is no worse than in the previous commit.

[Calinou]

I tried out your newest commit, but I can't reproduce the shadows flickering wildly while in motion. Keep in mind, there will also be some extra movement during motion from the jitter frames as TAA won't be able to fully reproject and resolve the image. But what I am seeing now is no worse than in the previous commit.

At realistic viewing distances, it's probably not too bad, so I guess we can move forward with this PR. I pushed a squashed version.

I still remember the flickering situation being better with the earliest revision of this PR (constant offset for the noise pattern every frame). Here's the flickering when TAA is enabled, shadow quality is set to Soft Very Low and shadow blur is set to 4 on the DirectionalLight3D (non-PCSS):

simplescreenrecorder-2022-06-29_02.38.47.mp4

This is an extreme situation; it won't be as noticeable in most scenes (and with the default shadow quality settings), but it shows the issue I was encountering.

If you set the update mode to Update When Changed, you can notice the rotation pattern when you don't move the camera but rotate it slowly:

simplescreenrecorder-2022-06-29_02.41.02.mp4

[clayjohn]

It would be good to compare the results with your earlier commit to verify whether they indeed got worse with the changes.

Keep in mind, a blur scale of 4 is an absurdly high value. Blur scale was designed to tweak the blur radius by a small amount and anything in that kind of range is bound to cause issues. In this case the issue is that the radius is being increased to 4 times its size and the distance between samples is directly related to the radius size. In other words, pixels on the far end of the radius travel quite far when the sampling disk is rotated. This is compounded by the fact that shadow quality very low only takes a few samples, so you end up not really benefiting from TAA.

[reduz]

I think when TAA is enabled, we should disable updating after N frames to allow convergence.

[Calinou]

Rebased and tested again, it works as expected in all configurations (directional/omni/spot, with and without PCSS shadows, Forward+/Mobile).

[Saul2022]

The check don´t seem to work and can´t download artifact.

[AThousandShips]

It failed CI so no artifacts are available, try checking out the branch and building yourself if you want to test it

[clayjohn]

As we have discussed on Rocketchat, I think we should remove the project setting entirely and just always jitter when using TAA and never jitter when not using TAA. The project setting is confusing and enabling temporal jitter without TAA is pointless.

While testing the changes locally I updated the PR to include support for FSR2 and remove the project setting. commit here: db4d2a3

[jcostello]

Works fine but the camera has to be still in order to this to happen. Does it make sense? Is not that common to have a fixed camera

[clayjohn]

Works fine but the camera has to be still in order to this to happen. Does it make sense? Is not that common to have a fixed camera

This doesn't require a fixed camera? Are you saying that when you tested it, this didn't work while the camera was moving?

[jcostello]

Works fine but the camera has to be still in order to this to happen. Does it make sense? Is not that common to have a fixed camera

This doesn't require a fixed camera? Are you saying that when you tested it, this didn't work while the camera was moving?

Exactly, only when I stop moving the camera I can see the shadow filtered. Let me record a video

[jcostello]

Here are two examples. One with moving camera and moving object

Peek.2023-10-12.14-09.mp4

One only with moving camera:

Peek.2023-10-12.14-10.mp4

In the last one you can see close the ramp that the filter is lost when the camera is moving

[clayjohn]

Here are two examples. One with moving camera and moving object

Peek.2023-10-12.14-09.mp4
One only with moving camera: https://github.com/godotengine/godot/assets/1168582/ff2a07be-b06e-4520-beed-c558714816f3

In the last one you can see close the ramp that the filter is lost when the camera is moving

When moving the object, the shadow will alias as we can't provide good motion vectors for shadows. So the TAA will lose track of it.

For the moving camera only, it should resolve better than that.

Perhaps there is an issue with how we calculate motion vectors from the camera rotation? cc @DarioSamo

[mrjustaguy]

This Might soon be only relevant to FSR2.. Here's the current state of #85462 results with TAA on vs off

TAA On:

TAA Off:

This Quality is also maintained in all sorts of motion I've tested

[clayjohn]

Testing this again after recent updates and I can reproduce the issue pointed out by jcostello. All accumulation is lost when the camera moves and it takes a noticeable amount of time to converge. FSR has similar artifacts, but also has unavoidable pixel swimming

I think we need to investigate what is wrong with TAA before we can go ahead with this PR

[clayjohn]

Okay, #86809 fixes the issue with motion throwing out the accumulation buffer when using TAA. This looks really good with TAA now.

We just need to figure out why it looks so bad with FSR. For now I suggest we only enable this when using non-FSR TAA.

[Calinou]

Rebased and tested again with all rendering methods, it works as expected. Note that this PR only has an effect on Forward+ as this is the only rendering method where TAA/FSR2 is implemented.

[clayjohn]

Rebased and tested again with all rendering methods, it works as expected. Note that this PR only has an effect on Forward+ as this is the only rendering method where TAA/FSR2 is implemented.

Does it still look bad with FSR2? If so, let's just disable it for now when using FSR2

[Calinou]

Does it still look bad with FSR2? If so, let's just disable it for now when using FSR2

It still doesn't look great with FSR2, but unfortunately I don't know how to disable this specifically when FSR2 is used. The jitter fraction is defined based on the TAA frame count, which depends on the antialiasing method in use. We can't just set the TAA frame count to 0 when FSR2 is used as this will break the rest of the FSR2 jittering for its own TAA.

This is where the jitter fraction is set:

https://github.com/Calinou/godot/blob/37b97b8af76bd9aa96824afc86cce306cfd36f90/servers/rendering/renderer_rd/storage_rd/render_scene_data_rd.cpp#L116-L122

I don't see a way to access the viewport to check whether the scaling mode is FSR2 here.

Edit: I tried FSR2 after #91799 (comment) was merged. Even if I bring back white noise-based dithering from #45326, it still doesn't look great. I wonder which dithering pattern I'm supposed to use along with FSR2 – AMD's documentation doesn't seem to have anything on this.

Edit 2: I pushed a second commit that keeps using interleaved gradient noise, but aims to rework how the TAA jitter fraction is calculated based on the Halton sequence. This should provide better results, but it needs further tweaking to the formula.