godot
godot copied to clipboard
Add an option to jitter shadow map dithering pattern across frames
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 |
---|---|
![]() |
![]() |
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 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 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.
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
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.
I think when TAA is enabled, we should disable updating after N frames to allow convergence.
Rebased and tested again, it works as expected in all configurations (directional/omni/spot, with and without PCSS shadows, Forward+/Mobile).
The check don´t seem to work and can´t download artifact.
It failed CI so no artifacts are available, try checking out the branch and building yourself if you want to test it
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
Jitter off
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.
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
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?
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
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
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
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:
TAA Off:
This Quality is also maintained in all sorts of motion I've tested
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
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.