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

Avoid holes when frustum culling is disabled, even with unconditionally-refined tiles #597

Merged
merged 10 commits into from
Feb 27, 2023

Conversation

kring
Copy link
Member

@kring kring commented Feb 23, 2023

Way back in #488, we added some logic to detect empty tiles with a geometric error greater than their parent's geometric error, and treat these specially.

If such a tile's parent's geometric error is too high, then, by definition, this tile's geometric error will be too (since it's greater), and therefore we should immediately refine past that tile.

Except! the tile's children may not be loaded yet. Prior to #488, we would make the mistake of rendering the placeholder while waiting for the children to load. This created totally unnecessary holes in the tileset, even with forbidHoles enabled. So the special logic added in #488 detected this case and marked the tile "unconditionally refined." And special logic in the traversal guaranteed we wouldn't select such a tile for rendering.

But during the tile content loader refactoring - I think! - this system gained a subtle bug where we accidentally treated these unconditionally-refined tiles as renderable. It didn't cause any problems in the forbidHoles mode, but when disabling frustum culling we would sometimes get these temporary holes in the tileset.

So this PR fixes that bug, making it so any unconditionally-refined tile - not just an external tileset - is considered unrenderable. But then this caused a problem in forbidHoles mode because the check if (!child.isRenderable() && !child.isExternalContent()) went from false to true for these unconditionally-refined tiles, skipping some necessary logic. So this PR fixes that as well.

This is fairly difficult to test well, but I did add some basic tests to confirm it is working correctly both with and without forbidHoles.

Base automatically changed from globe-anchor to main February 26, 2023 23:40
@joseph-kaile
Copy link
Collaborator

In queueChildrenRequiredForForbidHoles, is the check for external content missing? I don't see it checked in isRenderable or isUnconditionallyRefined.

@kring
Copy link
Member Author

kring commented Feb 27, 2023

External tilesets are always marked as unconditionally refined:

tile.setUnconditionallyRefine();

So I don't think we need to check for external tilesets explicitly.

@joseph-kaile joseph-kaile merged commit 64e4303 into main Feb 27, 2023
@joseph-kaile joseph-kaile deleted the unconditionally-refined-is-not-renderable branch February 27, 2023 23:02
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.

2 participants