Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Polylabel-based "pole of inaccessibility" symbol placement #7465

Merged
merged 2 commits into from
Dec 21, 2016

Conversation

jfirebaugh
Copy link
Contributor

Fixes #6115

@mention-bot
Copy link

@jfirebaugh, thanks for your PR! By analyzing this pull request, we identified @tmpsantos, @ansis and @springmeyer to be potential reviewers.

@jfirebaugh jfirebaugh force-pushed the polylabel branch 2 times, most recently from bc6e768 to 70eaab7 Compare December 20, 2016 16:32
@jfirebaugh jfirebaugh changed the base branch from 7416 to master December 20, 2016 16:32
}

// 16 here represents 2 pixels
Copy link
Member

Choose a reason for hiding this comment

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

why?

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 copied this comment from mapbox/mapbox-gl-js@5318f35#diff-621f09c210ab28c3dbf7bc1f0f03c6b2R365. I suppose it derives from the relationship between normalized tile extent of 8192 and nominal tile pixel size of 512. @lucaswoj?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, exactly.

Copy link
Member

Choose a reason for hiding this comment

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

Ok; can we replace this with the symbolic constants that we're using throughout GL native?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't 16 equal 1 pixel though? 8192/512=16

Copy link
Contributor

Choose a reason for hiding this comment

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

This code originates from mapbox/mapbox-gl-js#2678 by @blanchg. I don't have any knowledge beyond what's been discussed here so far.

Copy link

Choose a reason for hiding this comment

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

That parameter is the accuracy threshold, I.e. stop searching if the target size reaches 16 or less. @mourner helped me to identify that. Is it possible at the time we were working off of 256 tiles?

@jfirebaugh jfirebaugh merged commit 029805f into master Dec 21, 2016
@jfirebaugh jfirebaugh deleted the polylabel branch December 21, 2016 21:42
1ec5 added a commit that referenced this pull request Jan 16, 2017
1ec5 added a commit that referenced this pull request Jan 19, 2017
Updated changelogs to mention #7446, #7356, #7465, #7616, #7445, #7444, #7526, #7586, #7574, and #7770. Also corrected the blurb about #7711.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants