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

Clear source tiles for removed/re-added layer #3655

Merged
merged 1 commit into from
Nov 21, 2016
Merged

Clear source tiles for removed/re-added layer #3655

merged 1 commit into from
Nov 21, 2016

Conversation

anandthakker
Copy link
Contributor

Fixes #3633 by distinguishing the cases where the source tiles need to
be reloaded from those where they need to be cleared

Fixes #3633 by distinguishing the cases where the source tiles need to
be _reloaded_ from those where they need to be _cleared_
@anandthakker
Copy link
Contributor Author

Needs mapbox/mapbox-gl-test-suite#179 to be merged and then the test-suite dependency updated here.

#monorepo

// set up for the _previous_ version of this layer, confusing
// https://github.com/mapbox/mapbox-gl-js/issues/3633
delete this._removedLayers[id];
this._updatedSources[layer.source] = 'clear';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think is going to introduce a flash in cases where remove/add was being used to re-order the layers.

Copy link
Contributor

Choose a reason for hiding this comment

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

better than introducing corrupt buffers!

Copy link
Contributor

Choose a reason for hiding this comment

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

Will these tiles still be marked as reloading within SourceCache? As it is defined now, reloading means that there is an asynchronous update underway and the tiles are still renderable.

Is there a need for another state that means there is an asynchronous update underway and the tiles are NOT renderable? Or is there a need to redefine reloading?

@jfirebaugh
Copy link
Contributor

cc @mourner

@anandthakker
Copy link
Contributor Author

^ also re: the 'flash' thing -- I think moveLayer, especially coupled with #3649 , should make this a nonissue.

@anandthakker anandthakker mentioned this pull request Nov 18, 2016
15 tasks
Copy link
Contributor

@lucaswoj lucaswoj left a comment

Choose a reason for hiding this comment

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

This is ready to :shipit:

@lucaswoj lucaswoj merged commit fc49a2c into master Nov 21, 2016
@lucaswoj lucaswoj deleted the fix-3633 branch November 21, 2016 18:52
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