godot icon indicating copy to clipboard operation
godot copied to clipboard

Add an option to jitter shadow map dithering pattern across frames

Open Calinou opened this issue 2 years ago • 9 comments

If set to Auto (the default), jittering is enabled whenever TAA is also enabled. With TAA, jittering will look the best regardless of the monitor refresh rate, framerate or monitor response time.

If set to Always, when TAA is disabled, this makes the dithering pattern less noticeable as the monitor will smear across frames (due to its reponse time not being instant). When TAA is disabled, the effect works best on high refresh rate monitors (> 80 Hz) and at high framerates (> 80 FPS). That said, even at lower framerates, jittering the shadow pattern may still be desired to prevent the dithering pattern from "following" the camera as it moves and rotates.

If set to Never, the existing behavior before this commit is used. The shadow filtering quality won't improve when TAA is enabled. This is mostly useful for troubleshooting, but may also be desired when image stability needs to be maximized.

This closes https://github.com/godotengine/godot-proposals/issues/4179 and closes https://github.com/godotengine/godot/issues/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
2022-06-08_19 44 09 2022-06-09_19 25 33

OmniLight3D with PCSS shadows

Before After
2022-06-08_19 44 04 2022-06-09_19 25 12
Older revision of this PR

DirectionalLight3D with non-PCSS shadows

Before After
2022-06-08_19 44 09 2022-06-08_19 43 15

OmniLight3D with PCSS shadows

Before After
2022-06-08_19 44 04 2022-06-08_19 43 08

Calinou avatar Jun 09 '22 00:06 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.

Calinou avatar Jun 28 '22 23:06 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.

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.

clayjohn avatar Jun 28 '22 23:06 clayjohn

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):

https://user-images.githubusercontent.com/180032/176327021-a8d0665d-8065-4d6b-8a9f-f239d0ebc830.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:

https://user-images.githubusercontent.com/180032/176327227-e9e1ade1-991a-416c-9976-5aa556eee694.mp4

Calinou avatar Jun 29 '22 00:06 Calinou

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.

clayjohn avatar Jun 29 '22 01:06 clayjohn

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

reduz avatar Aug 06 '22 22:08 reduz

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

Calinou avatar May 26 '23 21:05 Calinou

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

Saul2022 avatar Jul 10 '23 06:07 Saul2022

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

AThousandShips avatar Jul 10 '23 12:07 AThousandShips

Tested it and it removes the weird noise for me when i see the shadow without moving on 1024 resolution,,though it still gives some circle patterm at resolution like 524 when using the suft ultra, but that another issue(will post screenshot later when i have time ), good step forward and lets see how it can improve later.

Edit: done , it is on 524 resolution, default atlas quadrant and soft medium filter.

Jitter on editor_screenshot_2023-07-10T184302

Jitter off editor_screenshot_2023-07-10T184321

Saul2022 avatar Jul 10 '23 16:07 Saul2022

Tested it and it removes the weird noise for me when i see the shadow without moving on 1024 resolution,,though it still gives some circle patterm at resolution like 524 when using the suft ultra,

This PR only aims to make the dithering pattern less visible. It can't fix aliasing in the shadow map if it has low resolution or lacks blurring. https://github.com/godotengine/godot-proposals/issues/4632 can help with this, but only to an extent and it'll have a performance cost as it prevents caching opportunities.

Calinou avatar Jul 10 '23 20:07 Calinou

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

jcostello avatar Oct 12 '23 03:10 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?

clayjohn avatar Oct 12 '23 17:10 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?

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

jcostello avatar Oct 12 '23 17:10 jcostello

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

https://github.com/godotengine/godot/assets/1168582/a975836a-41ca-46a5-9fdc-4b2d7cf54cfd

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

jcostello avatar Oct 12 '23 17:10 jcostello

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

clayjohn avatar Oct 12 '23 17:10 clayjohn

This Might soon be only relevant to FSR2.. Here's the current state of https://github.com/godotengine/godot/pull/85462 results with TAA on vs off

TAA On:

image

TAA Off:

image

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

mrjustaguy avatar Nov 30 '23 21:11 mrjustaguy

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 avatar Jan 04 '24 19:01 clayjohn

Okay, https://github.com/godotengine/godot/pull/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.

clayjohn avatar Jan 05 '24 00:01 clayjohn