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

Sharing StyleLayers between WorkerTiles is dangerous #3479

Open
jfirebaugh opened this issue Oct 28, 2016 · 3 comments
Open

Sharing StyleLayers between WorkerTiles is dangerous #3479

jfirebaugh opened this issue Oct 28, 2016 · 3 comments

Comments

@jfirebaugh
Copy link
Contributor

StyleLayers are shared between WorkerTiles, via the worker StyleLayerIndex. But StyleLayers contain state consisting of the evaluated result of style properties at a certain zoom level, and WorkerTiles may have different zoom levels.

Currently, we avoid any issues that this shared state could cause by carefully calling StyleLayer#recalculate at the entry point to every async callback in WorkerTile. But this is relatively fragile and error prone, and also requires reevaluation of previously-evaluated properties.

A better solution would be to have each WorkerTile call StyleLayer#recalculate once for each necessary layer, and store the logical result state independently of any other WorkerTiles. This would likely require a significant refactor of the internals of style recalculation (refs #2739, #3044).

@DannyDelott
Copy link
Contributor

This is interesting.

I'm noticing a bug where Map#queryRenderedFeatures throws an AssertionError: false === true at lower zoom levels on my custom layer. When I zoom in closer on the map and try again, it is able to get the features from my custom layer no problem. It's hard to reproduce, so I haven't filed a bug for it-- but this might be related?

What areas of the library are affected by this issue?

@jfirebaugh
Copy link
Contributor Author

I don't believe this is causing any bugs currently -- this is a refactoring issue noting how we could improve the code in the future. An assertion error in queryRenderedFeatures sounds like a separate issue.

@kristfal
Copy link

@DannyDelott just encountered the same issue while running V0.23.0. The assertion error should be fixed by #3233.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants