External tileset traversal fix #7035
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
force-traversal-external
branch
from
b931536
to
3ff4d23
Compare
|
Also opened CesiumGS/3d-tiles-validator#154 with the tileset updates. |
| @@ -438,6 +438,18 @@ define([ | |||
| return tile._screenSpaceError > baseScreenSpaceError; | |||
| } | |||
|
|
|||
| function canTraverse(tileset, tile) { | |||
| if (tile.children.length === 0) { | |||
| return false; | |||
There was a problem hiding this comment.
So the consensus from #7007 is not to throw a warning, correct?
It seems like it would be fairly trivial to include here with a one-time warning.
There was a problem hiding this comment.
Yeah I think we should wait and see if it comes up again. But agreed it should be easy to add if we decide to.
| @@ -307,7 +307,7 @@ define([ | |||
|
|
|||
| // Use parent's geometric error with child's box to see if the tile already meet the SSE | |||
| var parent = tile.parent; | |||
| if (defined(parent) && (parent.refine === Cesium3DTileRefine.ADD) && getScreenSpaceError(tileset, parent.geometricError, tile, frameState) <= tileset._maximumScreenSpaceError) { | |||
| if (defined(parent) && !parent.hasTilesetContent && (parent.refine === Cesium3DTileRefine.ADD) && getScreenSpaceError(tileset, parent.geometricError, tile, frameState) <= tileset._maximumScreenSpaceError) { | |||
There was a problem hiding this comment.
This line is getting a bit unwieldy. Can we account for this in updateVisibility? Or if not just pull out
getScreenSpaceError(tileset, parent.geometricError, tile, frameState)
or
getScreenSpaceError(tileset, parent.geometricError, tile, frameState) <= tileset._maximumScreenSpaceError
into its own var?
There was a problem hiding this comment.
Moved to its own function.
Unfortunately it can't be handled in updateVisibility because it shouldn't get run when checking children visibility for the cull-with-children-bounds optimization.
|
@ggetz Updated. |
|
Looks good! Thanks @lilleyse! |
|
Congratulations on closing the issue! I found these Cesium forum links in the comments above: https://groups.google.com/d/msg/cesium-dev/DBbw-uHNY2k/jmZn9cl-BwAJ If this issue affects any of these threads, please post a comment like the following:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
I was starting to notice a common theme from here, here, and here where external tilesets would not refine to their root tile in the same way that they did pre traversal refactor.
In certain cases the traversal would stop at the external tileset and not move along to the tileset's root tile.
This isn't really the correct thing to do. If a tile refines to an external tileset tile we would expect it to refine to the external tileset's root instead of stopping early. The fix is to always traverse to the external tileset root and in general use the SSE of the external tileset root's parent instead of itself.
The linked issues should now work as they did pre traversal. This does not fix the main problem in #7007. I'm not sure if this fixes #7026, as that may just be a duplicate of #7007.
@ggetz can you review?