CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.94k stars 3.49k forks source link

Ambient Occlusion doesn't work! #10106

Open happyfeet1996 opened 2 years ago

happyfeet1996 commented 2 years ago

Sandcastle example:Ambient Occlusion

Browser:Chrome

Operating System: macos

image image when i check ambient occlusion,full screen change to black and can not look anything!

ggetz commented 2 years ago

@happyfeet1996 I'm unable to reproduce on MacOS in Chrome. Can you provide some more details about your system by visiting this page and taking a screenshot?

happyfeet1996 commented 2 years ago

@happyfeet1996 I'm unable to reproduce on MacOS in Chrome. Can you provide some more details about your system by visiting this page and taking a screenshot? image image image image

happyfeet1996 commented 2 years ago

it works today,maybe my network go wrong that day

sanjeetsuhag commented 2 years ago

Reopening this issues since it does seem to occur on certain hardware. On my MacBook Pro (16-inch, 2019) running macOS 12.2.1, I see the following results:

Browser Sandcastle WebGL Report
Chrome 100.0.4896.60 Screen Shot 2022-04-04 at 11 24 02 AM Screen Shot 2022-04-04 at 11 24 05 AM
Firefox 98.0.2 Screen Shot 2022-04-04 at 11 23 05 AM Screen Shot 2022-04-04 at 11 23 08 AM
Safari Version 15.3 (17612.4.9.1.8) Screen Shot 2022-04-04 at 11 22 58 AM Screen Shot 2022-04-04 at 11 23 01 AM

Screen Shot 2022-04-04 at 11 52 45 AM

shotamatsuda commented 1 year ago

I also reproduce the same issue on macOS Chrome with eGPU. Specifying fragment alpha in AmbientOcclusionModulate seems to resolve it.

- out_FragColor.rgb = ambientOcclusionOnly ? ao : ao * color;
+ out_FragColor = vec4(ambientOcclusionOnly ? ao : ao * color, 1.0);
ggetz commented 1 year ago

Thanks for the info @shotamatsuda.

@jjhembd Could you please verify the potential fix in the comment above, and recommend a PR if appropriate?

jjhembd commented 1 year ago

Hi @shotamatsuda, thanks for the tip. However, on my machine (Chrome on Windows), I get the same image even if I set the alpha to 0.0

    out_FragColor.rgb = ambientOcclusionOnly ? ao : ao * color;
    out_FragColor.a = 0.0;

image

Does zero alpha also work on your machine, or is the 1.0 value important?

shotamatsuda commented 1 year ago

@jjhembd In my specific hardware, setting the alpha to zero results in a black screen, as well as not setting the value at all.

The importance of 1.0 depends on the blending function of the framebuffer. I haven't fully traced where blending is applied for framebuffers used in the post-processing stage, but setting true as the default value for PassState.blendingEnabled also resolves the issue. https://github.com/CesiumGS/cesium/blob/1.104/packages/engine/Source/Renderer/PassState.js#L36

This suggests to me that the blending function might not be defined, leading to a hardware or driver-dependent issue. Specifying the alpha value explicitly compensates for it.

shotamatsuda commented 1 year ago

For the record, my setup is macOS 13.3.1, Chrome 112.0.5615.49, and AMD Radeon RX 6800 XT GPU. I encountered the issue before Chrome 100 on macOS Monterey with the same eGPU, and it doesn't reproduce in Safari or Firefox. I first noticed the issue around Cesium 1.89.0, but I'm unsure if it is caused by the Chrome version update.

image
jjhembd commented 1 year ago

Hi @shotamatsuda, thanks again for all the info.

I think it was a mistake in AmbientOcclusionModulate.glsl to leave out_FragColor.a undefined. We don't do this in any other shaders.

There are two input textures in AmbientOcclusionModulate:

  1. colorTexture is the rendered scene before the ambient occlusion stage.
  2. ambientOcclusionTexture is the shading factor computed in AmbientOcclusionGenerate.glsl

ambientOcclusionTexture has its alpha value set to a constant 1.0. I think it would make sense to preserve the alpha of colorTexture (which will usually also be 1.0, but not always). This would simplify AmbientOcclusionModulate to read as follows:

    vec4 color = texture(colorTexture, v_textureCoordinates);
    vec4 ao = texture(ambientOcclusionTexture, v_textureCoordinates);
    out_FragColor = ambientOcclusionOnly ? ao : ao * color;

The above code gives the same result as before on my setup. Can you please verify if it works on your hardware? If so, we should be ready for a PR.

shotamatsuda commented 1 year ago

@jjhembd Yes, I confirmed it works with my hardware, and that should solve OP's issue. Deriving alpha from colorTexture makes sense to me.

ggetz commented 1 year ago

Requested on the forum: https://community.cesium.com/t/improve-ambient-occlusion/9639/4

shotamatsuda commented 1 year ago

Major problem is in the quality of stochatic noise and the lack of denoise. I made a library that implements Nvidia's HBAO and cross-bilateral filtering for Cesium. It's not a generic library, but you will get the concept. https://github.com/takram-design-engineering/plateau-view/tree/main/libs/cesium-hbao Live demo: https://plateau.takram.com

HBAO with cross-bilateral filter:

image

HBAO without cross-bilateral filter:

image

No HBAO:

image
ggetz commented 1 year ago

Awesome demo @shotamatsuda! Would you have any interest in contributing this AO algorithm implementation?

jjhembd commented 2 months ago

One issue with our current implementation: it computes a non-zero obscurance on flat surfaces. image

This may be related to error in the depth buffer. We are computing obscurance based on the "horizon angle" relative to the local surface normal, as reconstructed from screen position and depth buffer. On a flat surface, the horizon angle should be 90° from the normal, and obscurance should be zero. However, the reconstructed normals don't look right. Here is a rendering of the difference between the reconstructed normals at adjacent pixels. image

It shows non-zero values at the edges/curves of the model, as expected. But flat surfaces also show significant noise.

ggetz commented 2 months ago

CC https://github.com/CesiumGS/cesium/issues/11067?

jjhembd commented 2 months ago

The reconstructed normals are better if we don't use a log depth buffer.

Current result, with log depth buffer: image

After forcing Scene.defaultLogDepthBuffer = false: image

yoyomule commented 2 months ago

I hope AO will have better effects and performance. It is a good way to improve the effect of Cesium models. At present, it needs to be optimized in terms of performance and effects. Thank you for your efforts.

jjhembd commented 2 months ago

As discussed offline, the depth buffer for post-processing has a precision problem. To address this, we will try the following (in order):

jjhembd commented 2 months ago

Here is an updated Sandcastle displaying the gradient of the depth buffer.

Log depth shows strong (flickering) noise in the distance, as well as some spuriously large gradients on smooth surfaces nearer the camera. image

Linear depth reduces the noise nearer the camera. However, only the frustum nearest the camera is valid. The back frustum is washed out. image

jjhembd commented 2 months ago

I updated the depth buffer to use DEPTH32F_STENCIL8 when in a WebGL2 context. Note that this doubles the size of the depth buffer--because of alignment issues, the values are stored as 64 bits per pixel. Unfortunately, it does not have any effect on the noise.

DEPTH24_STENCIL8: image

DEPTH32F_STENCIL8: image

See the depth-precision branch.

jjhembd commented 1 month ago

@ggetz highlighted this commit, before which AO did not show a noisy background on flat surfaces. Here is a quick screenshot of AO from a prior commit (70fc40e258ce5592d7b00abd0d52cb10e90828c0): image Note the absence of any occlusion on flat surfaces, even with the "Bias" set to zero.

For comparison, here is a screenshot from the current main branch: image

And here is the result using the DEPTH32F_STENCIL8 format and a 1m near plane (instead of 0.1 m), both of which reduce the noise in the depth buffer: image

While some minor artifacts are reduced by the DEPTH32F_STENCIL8 format and a 1m near plane, the main problem is still there: a background noisiness on flat surfaces. This appears to be a problem introduced in https://github.com/CesiumGS/cesium/pull/9966.

I also captured the gradient of the depth buffer prior to #9966. It does not look significantly different from the depth buffer from the current main branch. image

The background noise problem appears to be something more fundamental--perhaps a depth buffer is being overwritten somewhere?

jjhembd commented 1 month ago

The background noise is generated when the HBAO stepping is along a direction not aligned with screen X/Y.

Here is the AO output with stepping only along X/Y. This is achieved by overriding the "randomVal" texture and using a constant rotation of 0 at every sample. image

Note the significant reduction in the background on flat surfaces. The remaining background is the result of the noise in the depth buffer. It is reduced by setting the near plane to 1m (instead of 0.1m): image

This is already approaching the result before #9966. One possible explanation is that before #9966, we had 2 bugs canceling each other:

  1. The random texture was not being read correctly, leading to a rotation of 0 (or nearly 0) being used at every pixel.
  2. The AO was already incorrect along non-screen axes, but this bug was hidden by the random texture problem.

Then #9966 fixed the first bug, exposing the second bug. But this is speculation--actually tracing texture handling through the major refactor in #9966 would take significant time.

Here is the result using a constant rotation of czm_piOverFour: image

Note the (incorrect) non-zero values on every flat surface. The current noisy result in main is a random mix of various non-zero values at different rotations.

jjhembd commented 1 month ago

A remarkably similar-looking bug, which appears to be related to incorrect normals. In this case, I think world-space geometry was leaking into the normal calculation. https://www.reddit.com/r/GraphicsProgramming/comments/sd17k0/trying_to_implement_hbao_shader/ The similar result was what made me suspect our normals, and therefore the depth buffer from which they were computed.

jjhembd commented 1 month ago

There were some incorrect geometry assumptions in our shader. For example,

//Reconstruct Normal Without Edge Removation
vec3 getNormalXEdge(vec3 posInCamera, float depthU, float depthD, float depthL, float depthR, vec2 pixelSize)
{
    vec4 posInCameraUp = clipToEye(v_textureCoordinates - vec2(0.0, pixelSize.y), depthU);
    vec4 posInCameraDown = clipToEye(v_textureCoordinates + vec2(0.0, pixelSize.y), depthD);

Based on what the code is doing, the comment should actually read something like this:

// Reconstruct normal with edge removal

But this line:

    vec4 posInCameraUp = clipToEye(v_textureCoordinates - vec2(0.0, pixelSize.y), depthU);

appears to assume v_textureCoordinates is oriented with y = 0 at the top of the screen, when it actually starts from the bottom of the screen.

Overall, I think our current prototype AO is pretty rough. The choice of algorithm is good, it just needs a rewrite, with careful checking of the input and output of each step.

We now know the depth buffer input is somewhat problematic, but:

jjhembd commented 1 month ago

Some notes from an offline discussion: Our HBAO is generating wrong values along any sampling axis that is not aligned with screen X/Y. This matters because each pixel rotates the sampling axes by a value that is randomized across neighboring pixels.

The error along rotated axes is much larger than the depth buffer error. It is not clear whether:

We will spend a little time reworking and testing each step of the AO implementation, under the preliminary assumption that the depth buffer is not the main problem. Initial tests/QCs will include:

jjhembd commented 1 month ago

One possible explanation is that before https://github.com/CesiumGS/cesium/pull/9966, we had 2 bugs canceling each other

I tried to confirm this, but can't. Before that PR, the result looks fine even with a constant rotation of czm_piOverFour: image

jjhembd commented 1 month ago

Our first test was to check the angular difference between reconstructed normals at adjacent pixels.

Here are some images showing the sine of the angle between the normal at a given screen pixel, and the normal at the pixel just above it (Y + 1 pixel). The sine value is scaled by 11.47, which will make the image fully white if the angular difference exceeds 5 degrees.

From best to worst:

  1. Using linear depth with near plane at 1m image Note how the image is black on flat surfaces, with white outlining the edges of the model. This is as expected.

  2. Using log depth with near plane at 1m image Some linear noise appears on the far-away terrain, but it represents angular differences of less than 5 degrees. There are also some strange artifacts near the edges of the model. But overall, the result is close enough. The depth errors are small enough that we can move ahead with further debugging of the rest of the AO code.

  3. Using log depth with near plane 0.1m (current default setting) image The error between normals at adjacent pixels exceeds 5 degrees in many places. This will be a problem for AO.

I think we can move ahead for now and debug the rest of the AO process, using a near plane at 1m. We will need to come back later to revisit the errors with log depth and closer near planes.

jjhembd commented 1 month ago

Some additional checks just to be sure.

Angular difference between current pixel and the pixel to the right (X + 1 pixel) image

Normals computed from tangents along the screen diagonal, difference along x = -y axis: image

Normals computed from tangents along the screen diagonal, difference along x = y axis: image

jjhembd commented 1 month ago

Difference between normals computed along screen axes vs. along diagonals image

jjhembd commented 1 month ago

I found what changed in #9966: the depth stencil texture was changed from linear to nearest sampling.

We can recreate the old result in the current code by making 2 changes:

Current result with a WebGL2 context: image

Current main with a WebGL1 context, nearest sampling. Note how some of the artifacts are reduced. image

Current main with WebGL1 context, and linear sampling of the depth texture. image

Just for interest: here's the linear sampling result with more angular sampling (16 instead of 4) image

jjhembd commented 1 month ago

I don't think we want to require WebGL1 for AO. Two possible options:

One thing that is not clear to me yet: why does log depth produce fewer artifacts in WebGL1 than in WebGL2?

ggetz commented 1 month ago

I don't think we want to require WebGL1 for AO

I agree. Is doing linear interpolation in the shader a possibility?

@lilleyse Would you be able to weigh in on the solution here?

lilleyse commented 1 month ago

I'm surprised linear sampling looks that much better. Is it because coordinates aren't snapped?

jjhembd commented 1 month ago

I'm surprised linear sampling looks that much better. Is it because coordinates aren't snapped?

Correct, the current shader does not snap coordinates to the texture grid. I am trying a snapping solution now.

Is doing linear interpolation in the shader a possibility?

Yes, though this would add to the cost. I think the main performance hit right now is the texture access, and a manually-coded linear interpolation would make that 4x. Snapping would be faster, if we can correctly account for the snapped geometry everywhere.

jjhembd commented 1 month ago

Here's a quick test fixing the coordinates to account for nearest sampling. This uses some WebGL2-only features like texelFetch. image

The WebGL2 result still has more artifacts than WebGL1.

jjhembd commented 1 month ago

We now have 4 actionable steps to improve AO:

  1. Fix sampling of depth buffer. I have a working code in WebGL2 (ao-debug branch). I will spend another day to see if we can get WebGL1 working too.
  2. Expose parameters (with good defaults) for the number of sample points: both the number of angles, and the number of steps along each angle. Sampling a larger disc can give significantly better results.
  3. Improve scalability: the occlusion radius (lengthCap) is currently a fixed value in meters, which makes the AO effect disappear when zoomed out to city-wide scale. This value should scale with distance.
  4. Use a smarter smoothing on the raw AO result. The cross-bilateral filtering demonstrated by @shotamatsuda looks like a good option.

Other issues arising:

  1. Why do we get more artifacts in the depth buffer in WebGL2?
  2. Are other post-processing steps affected by a similar bug with the depth buffer sampling?
jjhembd commented 1 month ago

Item 1 above is addressed by #12201. Items 2 and 3 are linked, and I think should be addressed together.

At each output pixel, we currently step rays outward along several angles to find the angle of the "horizon". The maximum distance of the ray-stepping defines the distance around the point where we allow nearby geometry to "occlude" the ambient light.

We currently have 3 factors defining this maximum occlusion distance:

  1. The distance-based weighting applied to each sample. The contribution from points far away from the output point is currently weighted by 1 - (distance / lengthCap) ** 2.
  2. Truncation at the lengthCap uniform. As we step to pixels away from the output pixel, we compute the Cartesian distance between the stepping point and the output. If it is above "lengthCap" in meters, the contribution of that pixel is ignored and further stepping along that angle is aborted. Unfortunately, this truncates the occlusion if the raymarching path is interrupted by a small foreground object (such as an overhead wire).
  3. The maximum number of steps along each angle (currently hard-coded to 6) along with the step size (in pixels). This may sometimes result in a hard truncation of the occlusion effect, if we reach the maximum number of steps before reaching a distance of lengthCap.

The main limitation of this approach is that parameters that work at one scale do not work when zooming out or in. For example, when zoomed on some details of the Power Plant model, an occlusion radius of less than 1m makes sense. But when zooming out to city scale, we might reasonably expect a skyscraper or other large building to occlude ambient light on buildings 10m or more away.

I first tried a naive approach: scale the occlusion radius based on distance from the camera. However, this makes a given feature become darker overall as it moves away from the camera, since the shadowed areas maintain their darkness, but their size grows linearly with distance.

One better approach: model the occlusion as something that blurs with distance--i.e., shadowed areas become wider, but less dark. In this way the overall darkness of a given feature would remain constant.

We can achieve a blurring effect by simplifying the flow as follows:

  1. Change the distance-based weighting to a normalized Gaussian distribution, with a variance proportional to distance from the camera. As the variance grows larger, the shadows become wider, but less dark, because the peak of the Gaussian becomes less pronounced.
  2. Compute the step size as scalar * gaussianVariance / maxStepCount. This ensures that every point will sample the entire intended radius.

We do not need to abort the stepping if a given sample point is too far away from the output point, because the Gaussian weighting will already make the contribution from that point negligible.

jjhembd commented 1 month ago

I have a preliminary version of the Gaussian-weighted AO in the ao-sampling-disc branch. The same parameter choices give reasonably consistent results at a wide range of scales.

Here is a quick comparison on the NYC buildings tileset. Some notes:

No AO: image

With AO: image

AO term by itself: image

jjhembd commented 1 month ago

A much closer view of the same scene, with the same parameters.

No AO: image

With AO: image

AO term by itself: image

ggetz commented 1 month ago

@jjhembd Screenshot look fantastic!

I don't see the ao-sampling-disc branch on GitHub. Do you need to push?

jjhembd commented 1 month ago

https://github.com/CesiumGS/cesium/tree/ao-sampling-disc