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

Layers flash on map when one is removed and reapplied in 0.29.0 #3895

Closed
DannyDelott opened this issue Jan 5, 2017 · 5 comments · Fixed by #4010
Closed

Layers flash on map when one is removed and reapplied in 0.29.0 #3895

DannyDelott opened this issue Jan 5, 2017 · 5 comments · Fixed by #4010
Assignees
Labels

Comments

@DannyDelott
Copy link
Contributor

DannyDelott commented Jan 5, 2017

In the latest version of mapbox-gl-js, all layers that share a tile source will flash if any of them are removed, then reapplied with the same id.

This is problematic in my application, where there is a large vector tile source for census blocks with many layers consuming it simultaneously. We need to be able to add and remove the layers if the user changes what they want to see, without flashing all of the layers the wish to keep.

mapbox-gl-js version:
0.29.0

Steps to Trigger Behavior

jsbin example: https://jsbin.com/muyayuzeva/edit?js,output

Expected Behavior

The maine_layer style layer should be removed and reapplied under the hood, but cause no flash of layers.

Also, the new_york_layer style layer should be unaffected by the removing/adding of the maine_layer style layer.

See same jsbin example w/ mapbox-gl@0.28.0 for expected behavior - https://jsbin.com/fabotihala/edit?html,js,output

Actual Behavior

Both the new_york_layer and maine_layer flash on across the map.

@AndyMoreland
Copy link
Contributor

AndyMoreland commented Jan 6, 2017

Geojson sources (and their layers) exhibit the same behavior for me in 0.30.0.

@jfirebaugh
Copy link
Contributor

Thank you for the detailed bug report and test case! It made bisecting this easy. Bisect landed on fc49a2c, which sounds very plausible.

cc @anandthakker

@anandthakker
Copy link
Contributor

anandthakker commented Jan 18, 2017

Thanks for the clear reproduction @DannyDelott and the bisect @jfirebaugh!

Yeah, fc49a2c is a very likely culprit. It was a too-far attempt to solve #3633 -- wherein removing a layer and immediately re-adding it as a different type violated assumptions about the buffer in the renderer.

Potential fix is to relax the conditions under which we clear the underlying source tiles (rather than just reloading them). Right now, we do this anytime a layer of a given name is removed and re-added. I think we can instead do it when a layer is removed and re-added with a different type.

@jfirebaugh
Copy link
Contributor

I think we can instead do it when a layer is removed and re-added with a different type.

That may be a good short term fix. Long term I think we should make the clear operation more granular, so it can remove data at the level of individual style layers rather than full tiles. That would prevent flickering of other layers even when a layer is removed and re-added with a different type.

@anandthakker
Copy link
Contributor

Long term I think we should make the clear operation more granular, so it can remove data at the level of individual style layers rather than full tiles.

👍 good point, agreed. Not sure where #2875 will land on the roadmap, but that might be a natural place for this, if there's no pressing need before

anandthakker added a commit that referenced this issue Jan 23, 2017
* Relax clearing of tiles on layer removed/readded

Fixes #3895

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

Successfully merging a pull request may close this issue.

5 participants