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 a crash when loading OSM buildings with shadows enabled #9172

Merged
merged 1 commit into from
Sep 26, 2020

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Sep 26, 2020

Fixes #9137

This bug was exposed by the sandcastle in #9137 in which a single 3D Tiles command was behind the camera and caused shadowNear and shadowFar to both equal the camera's near distance, creating a zero-sized frustum and a divide-by-zero crash when computing cascade splits. The fix was to enforce shadow far always being greater than shadow near in ShadowMap.

@lilleyse lilleyse requested a review from IanLilleyT September 26, 2020 16:58
@cesium-concierge
Copy link

Thanks for the pull request @lilleyse!

  • ✔️ Signed CLA found.
  • ❔ 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.

@IanLilleyT IanLilleyT merged commit 759e8f4 into master Sep 26, 2020
@IanLilleyT IanLilleyT deleted the fix-shadow-crash branch September 26, 2020 17:31
@IanLilleyT
Copy link
Contributor

Changes look good and it fixes the crash. Thanks @lilleyse !

@jtorresfabra
Copy link
Contributor

Nice! Indeed changes look good.

@lilleyse I have a question, but just for curiosity: Why does it make sense to do those calculations when the 3DTiles command is behind the camera?

@lilleyse
Copy link
Contributor Author

lilleyse commented Sep 28, 2020

@jtorresfabra This is an edge case where a tile's bounding volume is significantly larger than the draw command's bounding volume.

I guess some background on how 3D Tiles culling happens is needed here.

DrawCommand has a cull property to indicate whether View should do frustum culling on the command. All draw commands coming from 3D Tiles will have this set to false since 3D Tiles does its own internal frustum culling during traversal and doesn't expect View to have to do this check again. The problem is that a tile might be considered visible during traversal but its content might not actually be visible, and this is likely to occur if the bounding volume in tileset.json is significantly larger than the bounding volume computed for the content (which is computed from the glTF position accessor min/max). So the command is visible from the tileset's perspective, gets pushed to the command list, goes through View, modifies camera near/far and shadow near/far, but ultimately doesn't get placed into any frustum command lists because it's behind all the frustums. As a result it doesn't get rendered but it does move the near plane of the camera and shadows much nearer than necessary.

I was able to get some debug views. The red is the bounding region from the tileset.json and the yellow is the bounding sphere for the draw command. By the last image the tile is still visible but the content is not and this is when the app used to crash.

region
region3
region2

Debug sandcastle

@lilleyse
Copy link
Contributor Author

Opened #9178 to keep track of this issue.

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.

Can't turn on shadows during viewer construction with Cesium OSM Buildings?
4 participants