-
-
Notifications
You must be signed in to change notification settings - Fork 21.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jitter shadow map dithering pattern across frames when TAA is enabled #97428
Jitter shadow map dithering pattern across frames when TAA is enabled #97428
Conversation
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. Co-authored-by: Clay John <[email protected]>
Will the improved documentation of this comment of yours in #61834
be included in another PR? I think it wouldn't be obvious why these artifacts will be found in various strengths in different locations in a level so documenting it would be great. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is co-authored by Clay and Hugo so it's pretty much peer reviewed already. Seems simple enough for me to approve for merge.
Yes, the docs are hosted in a separate repository. So the documentation update will necessarily be in another PR |
Thanks! |
@@ -275,7 +275,7 @@ float sample_directional_pcf_shadow(texture2D shadow, vec2 shadow_pixel_size, ve | |||
|
|||
mat2 disk_rotation; | |||
{ | |||
float r = quick_hash(gl_FragCoord.xy) * 2.0 * M_PI; | |||
float r = quick_hash(gl_FragCoord.xy + vec2(taa_frame_count * 5.588238)) * 2.0 * M_PI; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a specific reason we went for 5.588238
? I can't find this value anywhere else in the codebase. For context, I'm asking because of #101961.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the value suggested by the author of IGN. There is a nice explanation here: https://blog.demofox.org/2022/01/01/interleaved-gradient-noise-a-different-kind-of-low-discrepancy-sequence/
Supersedes: #61834
This closes godotengine/godot-proposals#4179 and closes #53534.
Credit to @Calinou who did most of the work. I just did some final cleanup to improve the quality before it was ready to merge
I ended up rebasing while working on the improvements described in #61834 (comment) so I figured it was less work overall for us to open a new PR.
This PR builds on the current state of #61834, see that PR for a full description/history