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

Fix tile refinement when leaf is empty #8996

Merged
merged 5 commits into from
Jul 9, 2020
Merged

Conversation

lilleyse
Copy link
Contributor

We got a support issue about a tileset that doesn't refine property when skipLevelOfDetail is false, which is the default since CesiumJS 1.67. The tileset has empty leaf tiles which exposed an edge case in executeEmptyTraversal in Cesium3DTilesetTraversal in which a tile with empty leaf descendants would not refine. The fix is to refine even if the tile would refine to empty leaf tiles. Note that this only applies to leaf tiles - if there's a tile chain that consists of a content tile, an empty tile, and a content tile, it will wait for the final content tile to load before refining the top-level content tile, which is the main purpose of executeEmptyTraversal- to prevent holes from appearing while tiles stream in.

Technically this fix could lead to situations where the parent will refine and there will be holes wherever there's empty leaf tiles, but I think that's acceptable and in the spirit of how the tileset is constructed.

Local sandcastle with the fix
Hosted sancastle

@lilleyse lilleyse requested a review from loshjawrence June 28, 2020 02:06
@cesium-concierge
Copy link

Thanks for the pull request @lilleyse!

  • ✔️ 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.

@OmarShehata
Copy link
Contributor

Forum user confirmed this branch fixes their 3D Tileset generated by Context Capture: https://community.cesium.com/t/3dtiles-are-only-partially-displayed-with-skiplevelofdetail-disabled/10066/5?u=omar

@mramato
Copy link
Contributor

mramato commented Jul 9, 2020

@loshjawrence Can you please take a look at this?

@loshjawrence
Copy link
Contributor

Looks ok, waiting for ci to pass.

@loshjawrence loshjawrence merged commit 5e0cce6 into master Jul 9, 2020
@loshjawrence loshjawrence deleted the fix-empty-traversal branch July 9, 2020 15:11
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.

5 participants