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

optimization of replacement refinement selection/loading: only request visible tile children #3449

Conversation

e-andersson
Copy link
Contributor

This commit implements part of the optimizations discussed in #3424 (comment).

The selectTiles function will now be slightly more expensive as we dynamically need to find out whether a tile is refinable every time it comes off the stack. Finding this out requires checking the current visibility of its children, as opposed to before when we merely kept count of how many children had their contents loaded.

@pjcozzi and @lilleyse please review.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 22, 2016

@lilleyse can you please do the initial review?

Finding this out requires checking the current visibility of its children, as opposed to before when we merely kept count of how many children had their contents loaded.

Perhaps the visibility of the child can be cached so it can be used later in the tree traversal, e.g., so it is only ever computed once per frame just like we only compute the distance to the tile once per frame.

Perhaps we can optimize the additive refinement case?

Also, this reminds me of an optimization we want to try, but it will require some careful analysis (and may having varying performance depending on the dataset):

Performance idea: cull/LOD select with union of children bounding volumes instead of the tile's BV. Does the extra CPU overhead result in fewer tiles on average? Is it a performance win?

From #3241

if (child.visibility(cullingVolume) !== CullingVolume.MASK_OUTSIDE) {
visibleChildren.push(child);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be made into a prototype function called getVisibleChildren.

Instead of creating a new array every time, maybe store the array as part of Cesium3DTile, or even as a scratch array which may be safe now but maybe not later.

@lilleyse
Copy link
Contributor

Overall I agree with @pjcozzi that the more we can cache results and optimize the most common cases the better.

I've been trying to think of ways to cleanup selectTiles since I think it got a bit confusing since #3367.

@e-andersson
Copy link
Contributor Author

@lilleyse I've addressed the issues, except for caching of visibleChildren. We could compute the visibility of children (traversing down the tree) in selectTiles but if we do it every time a tile comes off the stack the cost would be the same as it is now. Computing distanceToCamera seems easier because it doesn't need to take children further down into account. Any ideas on how caching of visibleChildren could be done? How do we know if the current values can still be trusted or are too old?

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 11, 2017

I believe this was added in #4287.

@lilleyse please let me know otherwise.

@pjcozzi pjcozzi closed this Jan 11, 2017
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.

3 participants