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

Significantly improve zoom performance #220

Merged
merged 2 commits into from
Dec 16, 2013
Merged

Significantly improve zoom performance #220

merged 2 commits into from
Dec 16, 2013

Conversation

mourner
Copy link
Member

@mourner mourner commented Dec 16, 2013

With this pull in, zooming becomes significantly faster, especially noticeable when zooming out a lot. The code in datasource.js file was pretty inefficient. Here's the summary of changes:

  • the code for retaining existing and caching tiles if they're children of a missing tile happened in a loop for no reason (you can see that z variable isn't used anywhere there), and it still didn't filter out tiles that have higher zoom that needed — this fix alone made a big difference
  • sorting tiles from center out was slow because of Tile.fromID calls where they could be avoided
  • tile ids where kept in arrays which was inefficient because of the need to keep the values unique (using lots of indexOf calls, utils.unique, etc.) — now it uses objects (and difference can also be calculated much faster)
  • after all the fixes, I could reenable tile caching and pan tile without seeing any visual glitches, so I did

ansis added a commit that referenced this pull request Dec 16, 2013
Significantly improve zoom performance
@ansis ansis merged commit 5c36053 into gl Dec 16, 2013
@ansis
Copy link
Contributor

ansis commented Dec 16, 2013

great

@mourner mourner deleted the optimize-zoom branch December 16, 2013 22:46
@kkaefer
Copy link
Member

kkaefer commented Dec 17, 2013

the code for retaining existing and caching tiles if they're children of a missing tile happened in a loop for no reason (you can see that z variable isn't used anywhere there), and it still didn't filter out tiles that have higher zoom that needed — this fix alone made a big difference

At one point, we gradually climbed up zoom levels to find the highest covering tile. Not sure where that got lost.

@mourner
Copy link
Member Author

mourner commented Dec 17, 2013

@kkaefer I thought so. It seems that to do this efficiently with the current code, the tiles need to be indexed by zoom.

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