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

Blazing fast layer update #2174

Merged
merged 5 commits into from
Feb 24, 2016
Merged

Blazing fast layer update #2174

merged 5 commits into from
Feb 24, 2016

Conversation

mourner
Copy link
Member

@mourner mourner commented Feb 23, 2016

Makes setLayoutProperty, setFilter and setLayerZoomRange much faster by limiting the style->worker transfer to just the updated layers instead of the whole style.

Closes #2082 and makes this hover example https://jsfiddle.net/0Lk8awwp/1/ fast and smooth again: https://jsfiddle.net/Mourner/0Lk8awwp/3/

cc @lyzidiamond @lucaswoj @scothis

@tmcw
Copy link
Contributor

tmcw commented Feb 23, 2016

Awesome, could you do a jsfiddle that includes this patch so we can compare?

@mourner
Copy link
Member Author

mourner commented Feb 23, 2016

@tmcw yes, was waiting for the build to complete.

@mourner
Copy link
Member Author

mourner commented Feb 23, 2016

JSFiddle with the new build (inlined) that demonstrates the performance: https://jsfiddle.net/Mourner/0Lk8awwp/3/

@mourner
Copy link
Member Author

mourner commented Feb 23, 2016

Added another optimization that affects the example — avoid triggering an update when you set filter, layout property or zoom range to values that are already set.

@@ -14,7 +14,8 @@ function styleBatch(style, work) {

batch._style = style;
batch._groupLayers = false;
batch._broadcastLayers = false;
batch._broadcastAllLayers = false;
batch._broadcastLayers = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a little tricky that this._broadcastLayers and this._style._broadcastLayers both exist and are of different types

@lucaswoj
Copy link
Contributor

I disagree that this 🔥 is so bad that you don't have time to

  • rename one property
  • move one function to another module
  • refactor a unit test to use public rather than private methods

If you have too much on your plate, I can take care of these tweaks.

@lucaswoj
Copy link
Contributor

This looks great! :shipit:

Makes `setLayoutProperty`, `setFilter` and `setLayerZoomRange` much
faster by limiting the style->worker transfer to just the updated
layers instead of the whole style.

Makes this hover example https://jsfiddle.net/0Lk8awwp/1/ fast and
smooth again.

cc @lyzidiamond @lucaswoj @scothis
This is especially helpful for cases where `setFilter` or
`setLayoutProperty` are called on mousemove.
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