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

Fix sampling bug when SSAO is using half size #73420

Merged
merged 1 commit into from Feb 16, 2023

Conversation

clayjohn
Copy link
Member

Fixes: #62881
Fixes: #66643

There are two pieces to this PR:

  1. When downsampling 5 mip levels at once, the 5th mip level was showing some artifacts which lead to invalid samples in the SSAO shader. To fix it, I have removed the invalid averaging code from the 5th mipmap computation. This code path was only hit when using both SSAO and SSIL and only one of them is using half size
  2. Change the half_screen_pixel_size to be based off the full screen size / 2. This was a difficult bug to track down, but essentially when using a screen size that could not be subdivided cleanly 4 times the half res samples would drift away from their intended positions. To fix that, we base our scaling off of the full viewport size.

This also cleans up two unreported bugs:

  1. The SSAO shader was using the wrong sampler (default instead of mirror)
  2. The depth linearization process was doubling the depth value. This could have led to SSAO improperly rejecting valid occlusion (i.e. appearing slightly lighter than it should).

Finally, on investigation it appears that SSIL was suffering from the same issues, it was just less obvious. So I made the changes to SSIL as well

Before
Screenshot from 2023-02-16 00-13-09
After
Screenshot from 2023-02-16 00-13-47

Before

Screenshot from 2023-02-16 00-11-46

After
Screenshot from 2023-02-16 00-12-09

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing further with Clay, agreed this seems like a sound fix

@akien-mga akien-mga merged commit eb1af0d into godotengine:master Feb 16, 2023
@akien-mga
Copy link
Member

Thanks!

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