-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Tileset mixed refinement fix #7099
Conversation
Thanks for the pull request @lilleyse!
Reviewers, don't forget to make sure that:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Updated. @ggetz could you review? |
@@ -316,6 +316,13 @@ define([ | |||
return; | |||
} | |||
|
|||
if (tile.hasTilesetContent && tile.children.length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a quick comment to explain why this is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we're already defining var hasChildren = tile.children.length > 0;
on line 334, move it up here?
@ggetz Updated. |
Looks good, thanks @lilleyse! |
Fixes #7084
The problem was a tile referencing an external tileset would pass the visibility test but it's child, the root of the external tileset, would not due to the
cullWithChildrenBounds
optimization. This is why #7084 could only be replicated in views where the root's children were all off screen. The external tileset would not refine further and would select its nearest loaded ancestor, causing the tileset to render mixed LODs.The fix is to use the root tile's visibility in place of its parent's visibility so that the traversal never gets to the parent tile in the first place.
To do: