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

sort renderable tiles by y and x to match -native #5601

Merged
merged 1 commit into from
Nov 6, 2017
Merged

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Nov 6, 2017

This changes the tile sort order to match -native. This is important to make sure the render order of symbols that cross tile boundaries is the same in -native and -js.

This will let us fix symbol-spacing/line-close for the viewport collision pr in -native.

regressions/mapbox-gl-native#7357/expected.png changes. I think the new rendering is fine, but if we shouldn't change this I can change this so that only symbol rendering order is affected.

Launch Checklist

@ChrisLoer

@ansis ansis requested review from jfirebaugh and ChrisLoer November 6, 2017 21:38
@ChrisLoer
Copy link
Contributor

This looks good to me -- my one concern was that compareKeyZoom ends up getting called pretty heavily, so the more expensive calculation might make a difference. I ran the "paint" and "layout" benchmarks several times and saw this branch as between 53% and 71% likely to be slower than master. I also ran a test on an animated full screen map with the Chrome javascript profiler running and saw getIds taking between .4-1% of CPU time on both branches. So.. seems fine.

@jfirebaugh jfirebaugh deleted the tile-sort-order branch February 22, 2018 20:22
mourner added a commit that referenced this pull request Jul 20, 2018
#5601 introduced the sorting to match native, but in the latter, the fancy sorting is only used for symbol layers.
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