Fix tileset traversal #7011
Fix tileset traversal #7011
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. 🌍 🌎 🌏 |
lilleyse
force-pushed
the
refinement-fix
branch
from
fd30df5
to
eb086ee
Compare
lilleyse
force-pushed
the
refinement-fix
branch
from
eb086ee
to
0ee1094
Compare
|
@ggetz can you please review and test? |
There was a problem hiding this comment.
Other than that, the code looks good. Hopefully we hear back in CesiumGS/3d-tiles#336 soon if it fixes the behavior for that tileset.
| selectDesiredTile(tileset, tile, frameState); | ||
| } | ||
| } else { | ||
| // Load tiles that are not skipped or can't refine further. In practice roughly half the tiles stay unloaded. | ||
| // Select tiles that can't refine further. If the tile doesn't have loaded content it will try to select an ancestor with loaded content instead. | ||
| if (!refines) { // eslint-disable-line | ||
| if (stoppedRefining) { // eslint-disable-line |
There was a problem hiding this comment.
You can get rid of the comment disabling eslint if you bring this if statement up a block. I don't think it hurts readability at all.
There was a problem hiding this comment.
I know this is all minor but I prefer this organization because it's easier to see the how the base traversal and skip traversal operate when they are separated like this.
There was a problem hiding this comment.
We have eslint rules for a reason though, adding disable-line because you don't like a rule is kind of a bad precedent. If code needs clarifying, I would indicate that with a comment, not some layout that only makes sense to people who already know the code.
There was a problem hiding this comment.
Alright, made the change and updated the comments to help clarify the code flow.
|
@ggetz Updated. |
|
Hi, Cesium 1.49 Cesium 1.49 with merged pullrequest |
|
Thanks for the confirmation @jbo023, that's good to hear. |
Fixes CesiumGS/3d-tiles#336
There was a skip-lods bug introduced in the recent update to the 3d tiles traversal code.
If the tileset traversal stops at an empty tile it wasn't loading its nearest loaded ancestor, causing nothing to be drawn in the empty space. This becomes really noticeable when using external tilesets where the tile referring to the .json has a different geometric error than the root tile of that external tileset, but can manifest in other situations involving empty tiles.
Until this fix is in, a workaround is to set
tileset.skipLevelOfDetails = false.To do