Fix depth sampling for ambient occlusion #12201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 5 commits
September 17, 2024 16:35
|
Thank you for the pull request, @jjhembd! ✅ We can confirm we have a CLA on file for you. |
ggetz
requested changes
Sep 18, 2024
| @@ -6,109 +8,107 @@ uniform float lengthCap; | |||
| uniform float stepSize; | |||
| uniform float frustumLength; | |||
|
|
|||
| in vec2 v_textureCoordinates; | |||
There was a problem hiding this comment.
I recall we talked offline that this is probably not the best solution for post processing across the board. If that's correct, could you make sure we have an open issue documenting the suggested for other post processing stages?
Co-authored-by: Gabby Getz <[email protected]>
|
@ggetz thanks for the feedback! I think I addressed it above. |
|
Awesome, thanks @jjhembd! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes sampling of the depth texture in
AmbientOcclusionGenerate.glsl.Our ambient occlusion shader was developed when depth buffers still used linear sampling. After #9966, the depth buffer uses nearest sampling. And in a WebGL2 context, linear sampling of a depth buffer is not allowed. This meant that:
depthTextureactually corresponded to the center of the pixel.The inconsistency between depth and XY values produced enough geometry error to create a background occlusion factor everywhere, including on flat surfaces which should have no occlusion.
This PR adjusts the shader calculations to explicitly shift all sampling points to pixel centers.
Along the way, I rewrote the shader to better reflect the geometry of the various inputs.
Rendering results
Before this PR:
After this PR, with a WebGL2 context (default):
After this PR, with a WebGL1 context:
Issue number and link
Partially addresses #10106.
Testing plan
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change