Skip to content
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

Restore and fix post-process shadow blurring in 3D #58771

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Mar 4, 2022

When uncommented, this function resulted in the dithering pattern being less noticeable. However, it also introduced jaggies in distant shadows, which are especially jarring in motion. Its performance cost was fairly low, but it wasn't worth the tradeoff.

The filter was quite effective for blurry shadows that are up close, so maybe it could be resurrected in the future based on shadow texel density relative to the camera.

In the future, temporal antialiasing will be added to optionally hide the dithering pattern by changing it every frame (letting the accumulation buffer do its job of averaging it all). On high refresh rate monitors, the shadow filtering pattern can be jittered to let the monitor's response time perform this averaging (without relying on TAA).

Preview

To see the negative impact of the filter on distant shadows, look at the top of the first screenshot, and compare it with the Filtering uncommented screenshot.

Current shadow rendering

2022-03-04_23 20 36_without_filter

2022-03-04_23 20 36_without_filter_closeup

Filtering uncommented

2022-03-04_23 21 39_with_filter

2022-03-04_23 20 36_with_filter_closeup

@Calinou Calinou requested a review from a team as a code owner March 4, 2022 22:28
@Calinou Calinou added this to the 4.0 milestone Mar 4, 2022
@clayjohn
Copy link
Member

clayjohn commented Jul 26, 2022

I think we can actually fix this by using dFdxFine and dFdyFine and clamp the result so it doesn't exceed 1.0
I tested the below code and it removes the artifacts for me from the previous implementation on Intel and AMD. It would be great if you can test on NVidia. Maybe we can fix this instead of removing it!

	interp_shadow -= dFdxFine(interp_shadow) * (float(fc2.x & 1) - 0.5);
	interp_shadow -= dFdyFine(interp_shadow) * (float(fc2.y & 1) - 0.5);
	interp_shadow = min(interp_shadow, 1.0);

You'll also need to change the call to blur_shadow in the directional light code to shadow=blur_shadow()

Edit: Here is what the effect looks like when working properly

Before

raw-cropped

After

blur-cropped

Before

raw-distance

After

blur-cropped png-distance

@Calinou Calinou force-pushed the remove-unused-postprocess-shadow-blurring branch from e8c14a5 to d32fad0 Compare July 27, 2022 01:01
@Calinou
Copy link
Member Author

Calinou commented Jul 27, 2022

Thanks for the suggestion! I added you as a co-author of the commit 🙂

I've applied your suggestion (and pushed a new commit overriding the old one), but the blurring still seems to look bad. It almost looks like shadow maps are rendered at half resoution in the viewport, which results in distant shadows looking pixelated.

Can you test the project below on your GPU?

Testing project: test_shadow_blur_pass.zip

1024×600

Before After (this PR)
2022-07-27_02 59 00 2022-07-27_03 00 06

2560×1440

Before After (this PR)
2022-07-27_02 59 13 2022-07-27_03 00 13

@Calinou Calinou changed the title Remove unused post-process shadow blurring Restore and fix post-process shadow blurring in 3D Jul 27, 2022
@Calinou Calinou force-pushed the remove-unused-postprocess-shadow-blurring branch from d32fad0 to 3c23c43 Compare July 27, 2022 01:05
@clayjohn
Copy link
Member

What is happening is the blurring applies only within screen aligned 2x2 quads. So in distant areas (where the penumbra is less than 2 pixels wide) it is creating aliasing as the shadow is much higher frequency than 2x2.

@Calinou
Copy link
Member Author

Calinou commented Jul 28, 2022

What is happening is the blurring applies only within screen aligned 2x2 quads. So in distant areas (where the penumbra is less than 2 pixels wide) it is creating aliasing as the shadow is much higher frequency than 2x2.

Could we detect the penumbra's length or use a distance-based metric to avoid this?

@clayjohn
Copy link
Member

clayjohn commented Aug 2, 2022

What is happening is the blurring applies only within screen aligned 2x2 quads. So in distant areas (where the penumbra is less than 2 pixels wide) it is creating aliasing as the shadow is much higher frequency than 2x2.

Could we detect the penumbra's length or use a distance-based metric to avoid this?

Yep! We could probably reuse ddx and ddy (based on vertex position) which are already calculated. I'm not sure what the performance implication would be though.

@mrjustaguy
Copy link
Contributor

I've got a dumb question... is this post proc done in screen space or what?
I mean if it is, wouldn't it maybe be worth seeing if Gaussian Blur has good results?

@clayjohn
Copy link
Member

I've got a dumb question... is this post proc done in screen space or what? I mean if it is, wouldn't it maybe be worth seeing if Gaussian Blur has good results?

It's not actually done as a post process. It's done at render time. And it only works within each 2x2 fragment quad, so guassian blur isn't an option

@Calinou
Copy link
Member Author

Calinou commented Aug 14, 2023

Note that godotengine/godot-proposals#7178 may be worth considering as an alternative, although it aims to resolve a more specific issue (aliasing due to shadow texel density being too high at certain spots of the image).

@mrjustaguy
Copy link
Contributor

Well the Post-processing also reduces the graininess a little, which that wouldn't.

Though what was suggested in the proposal kinda hit a dead end, as the simple angle based blur increase causes it's own issues, and I have no ideas how exactly I'd go about creating my own Anisotropic Filtering shader to use for Shadows or how to Apply Hardware Filtering to it properly (I know Anisotropic Filtering works just fine without mipmaps, I tested, but unsure about how it'd respond to eh, 4 maps in 1 texture even if I figured out how to apply it...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants