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

Fixed shadow volume regression for mobile #9108

Merged
merged 2 commits into from
Aug 31, 2020
Merged

Fixed shadow volume regression for mobile #9108

merged 2 commits into from
Aug 31, 2020

Conversation

IanLilleyT
Copy link
Contributor

@IanLilleyT IanLilleyT commented Aug 23, 2020

Fixes #8953

#8854 clamped vertices to the near plane in order to create a watertight shadow volume. While this happened to work well for orthographic mode, and worked sometimes on desktop (curiously, only when ANGLE was enabled), it broke down for mobile, and was flawed to begin with. This is because the way we do shadow volumes doesn't actually require the front of the volume to be closed, just the back.

Ultimately we want a solution that emulates GL_DEPTH_CLAMP, which is not available in WebGL 1 or 2. GL_DEPTH_CLAMP clamps geometry that is outside the near and far planes, capping the shadow volume. More information here.

We emulate GL_DEPTH_CLAMP by ensuring no geometry gets clipped by setting the clip space z value to 0.0 and then sending the unaltered screen space z value (using emulated noperspective interpolation) to the frag shader where it is clamped to [0,1] and then written with gl_FragDepth. This is the solution when GL_EXT_frag_depth is available and works for both perspective and orthographic. It is based off of https://stackoverflow.com/questions/5960757/how-to-emulate-gl-depth-clamp-nv.

When GL_EXT_frag_depth is not available, which is the case on some mobile devices, we must attempt to fix this only in the vertex shader. Our approach is to clamp the z value to the far plane, which closes the shadow volume but also distorts the geometry, so there can still be artifacts on frustum seams but it won't be as severe as the code in #8854 which also clamps the near plane unnecessarily.

Screen Shot 2021-06-08 at 6 50 22 PM

Before:
Screenshot 2020-08-22 at 10 29 48 PM

After:
Screenshot 2020-08-22 at 10 30 46 PM

Sandcastle

Here are some examples of frustum seam artifacts that could not be fixed in this PR when GL_EXT_frag_depth is not available. These artifacts have existed since shadow volumes were first implemented:

Screenshot 2020-08-22 at 8 51 44 PM

Screenshot 2020-08-22 at 9 15 44 PM

Sandcastle for testing more options

@lilleyse and I worked on this fix together so someone else should review. @kring?

CC @OmarShehata

@cesium-concierge
Copy link

Thanks for the pull request @IanLilleyT!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@Brightboy666
Copy link

Banga

@lilleyse
Copy link
Contributor

lilleyse commented Aug 26, 2020

This PR fixes some other artifacts when log depth is disabled:

Sandcastle

ezgif com-optimize

@dennisadams
Copy link
Contributor

This PR fixes some other artifacts when log depth is disabled:

This is certainly an improvement but I still have some problems on my Android:

Before:
Peek 2020-08-28 13-21

After:
Peek 2020-08-28 13-16

@lilleyse
Copy link
Contributor

Ah I should've mentioned I had only tested the ellipse sandcastle on desktop linux where it is now fixed. I see those same artifacts on Android where frag depth isn't supported (in the WebGL 1 context at least).

@kring kring merged commit 61ff7da into master Aug 31, 2020
@kring kring deleted the shadowVolumeMobile branch August 31, 2020 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Artifacts in classification example in 1.70?
6 participants