Open jjhembd opened 6 days ago
Thank you for the pull request, @jjhembd!
:white_check_mark: We can confirm we have a CLA on file for you.
Thanks @jjhembd! Excited to get this in!
Is the code ready for review yet, or should we wait for the uniform changes to go in first?
The code is now ready for performance testing with different sample counts. There are 2 parameters:
directionCount
: The number of directions in which to raytrace away from a given pixelstepCount
: The number of steps to take in the ray tracing along a given pixelSee this local Sandcastle for a demonstration of the parameters.
Is the code ready for review yet, or should we wait for the uniform changes to go in first?
I think it would make sense to wait until the uniform changes are done. I still have one change to implement: the step size should be automatically computed from the window size (lengthCap
) and stepCount
. This will take a little more thought since the step size is in pixels, while the Gaussian window is currently defined in meters.
@jjhembd I'm consistently seeing a band along the top of the screen, any idea what may be causing that? Is it the transition between multiple frustums maybe?
I'm also seeing some flickering due to occlusion when I look up at the sky. Is that just me?
@jjhembd What's the plan for blur? I noticed your sandcastle disables it entirely.
I agree the look of the blur is not great. Would it make sense to remove it completely? Maybe we could use MSAA to remove "noise" from few samples?
I'm seeing good results and a stable 60 FPS with
stepCount: 32,
directionCount: 8,
I think stepCount could potentially go even lower to 16
, if we're looking fewer samples overall, and still look good.
That looks to be working well at other scales as well, such as buildings:
No AO, for comparison:
@ggetz thanks for the initial feedback!
I reworked the uniforms to compute the step length from the window size and step count. This value also works as the scalar to account for discrete sampling of the Gaussian, so we don't have to compute normalization factors in the inner loop.
I'm consistently seeing a band along the top of the screen
This does look like a frustum boundary. The occlusion should be neglecting this, since it appears in the depth buffer as a huge jump--much bigger than the window length. It's possible we're not correctly handling zero values in the depth buffer. I'm looking at this now.
What's the plan for blur?
We discussed using a bilateral filtering at one point, which would better preserve edges. I will take a look today to see how much work that would be.
We still need to add some kind of deprecation for the old uniform names. I updated the TODO list in the description.
Description
Ambient Occlusion (AO) for geospatial data needs to produce good results at a wide range of scales, without requiring the user to adjust parameters for each zoom level. This PR redefines the weight function for AO as a normalized Gaussian window, with a variance that scales with the distance from the camera.
Before this PR, the AO effect disappears when zoomed out:
After this PR, the AO effect blurs wider, remaining visible (though lower intensity) when far from the camera:
Issue number and link
Fixes #10106.
Testing plan
Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my changeTODO