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

Placement order matches viewport-y sort #8180

Merged
merged 1 commit into from
May 8, 2019

Conversation

pozdnyakov
Copy link
Contributor

Symbols are placed accordingly to their viewport Y order,
if the style symbol-z-order is explicitly set to viewport-y.

This improves rendering of symbol layers, where icons are allowed
to overlap but not text.

@pozdnyakov pozdnyakov force-pushed the mikhail_sorted_y_placement branch from 31b0f7a to 1d22882 Compare April 24, 2019 13:21
Symbols are placed accordingly to their viewport Y order,
if the style `symbol-z-order` is explicitly set to `viewport-y`.

This improves rendering of symbol layers, where icons are allowed
to overlap but not text.
@pozdnyakov pozdnyakov force-pushed the mikhail_sorted_y_placement branch from 1d22882 to d8bc48a Compare April 24, 2019 13:31
@pozdnyakov pozdnyakov marked this pull request as ready for review April 26, 2019 07:20
@pozdnyakov
Copy link
Contributor Author

@ansis @asheemmamoowala PTAL

Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

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

👍 from me, but I think a 👀 from @ansis would still be helpful.


result.sort((aIndex, bIndex) => {
return (rotatedYs[aIndex] - rotatedYs[bIndex]) ||
(featureIndexes[bIndex] - featureIndexes[aIndex]);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Docs should be updated to include fallback to source order for symbols with same Y. Especially useful is symbol-sort-key is also provided, and the expectation would be to fallback to the sort key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will make a separate PR for that, thanks!

@pozdnyakov
Copy link
Contributor Author

👍 from me, but I think a 👀 from @ansis would still be helpful.

mapbox/mapbox-gl-native#14486 <- the peer PR in gl-native, approved by @ansis

@pozdnyakov pozdnyakov merged commit f791754 into master May 8, 2019
@pozdnyakov pozdnyakov deleted the mikhail_sorted_y_placement branch May 8, 2019 09:33
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.

2 participants