Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Set Tile loaded/rendered instead of marking tile as optional. #11985

Merged
merged 1 commit into from
May 24, 2018

Conversation

asheemmamoowala
Copy link
Contributor

Fixes #11957.

When a tile has no features CustomGeometryTile::setTileData marks it as TileNecessity::optional, before calling GeometryTile:setData. This call to setData triggers parse and layout on the GeometryTileWorker and the worker will subsequently update the tile, which requests a new render frame. In the next frame, the tile is marked as TileNecessity::Required again, triggerring a reload of the tile data and subsequent repeat layout of the empty feature data - creating a never-ending loop.

The fix here is to mark the tile as loaded and renderable instead of optional. This way the tile skips layout (and placement) and does not incur future reloads until evicted fro the tile cache.

Verified the fix on iOS, macOS, and Android. CPU consumption drops back to 0 and tiles behave correctly.

The one thing I am not sure of in this approach is using the loaded and renderable protected members from the Tile parent class.

cc @ChrisLoer @fabian-guerra

@ChrisLoer
Copy link
Contributor

It's OK for classes derived from Tile to modify loaded and renderable, but I think the easier way to manage the state here is to call setData with an empty data set -- we already support that pathway, and it will generate a renderable tile with no features in it, using the same code as the rest of GeometryTile. And I don't think you need to change the tile necessity here.

I haven't looked into why all those tests timed out, but it does seem suspicious...

Copy link
Contributor

@fabian-guerra fabian-guerra left a comment

Choose a reason for hiding this comment

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

I tried this fix and is working. Also ran the tests and is timing out. So this change affected somehow the Source.CustomGeometrySourceSetTileData test.

@asheemmamoowala
Copy link
Contributor Author

The Source.CustomGeometrySourceSetTileData was legitimately failing. The fix suggested by @ChrisLoer fixes it though, so I've pushed that change.

Copy link
Contributor

@ChrisLoer ChrisLoer left a comment

Choose a reason for hiding this comment

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

🚀

@ChrisLoer ChrisLoer merged commit b1f6961 into master May 24, 2018
@ChrisLoer ChrisLoer deleted the fix-11957 branch May 24, 2018 18:01
@friedbunny friedbunny added Core The cross-platform C++ core, aka mbgl rendering needs changelog Indicates PR needs a changelog entry prior to merging. labels May 24, 2018
@friedbunny
Copy link
Contributor

iOS changelog added in #12086.

@friedbunny friedbunny removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Jun 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ComputedShapeSource empty array causes high cpu utilisation
4 participants