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

Improve collision detection for labels along lines by switching to runtime evaluation of collision circle's pixelated radius #10918

Merged
merged 2 commits into from Aug 10, 2021

Conversation

zmiao
Copy link
Contributor

@zmiao zmiao commented Aug 9, 2021

Launch Checklist

Collision circles are used during collision detection for line labels.
Previously collision circle's pixel radius was calculated in symbol layout time with a constant value. This patch moves the calculation into placement time, so the collision circle's radius would be more aligned with the text's size in different zoom levels, thus leading to a better collision detection behavior.

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • write tests for all new functionality
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes
  • tagged @mapbox/gl-native if this PR includes shader changes or needs a native port
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Improve collision detection for labels along lines by switching to runtime evaluation of collision circle's pixelated radius</changelog>

@zmiao
Copy link
Contributor Author

zmiao commented Aug 9, 2021

Benchmark test result:
Screen Shot 2021-08-09 at 10 23 29 AM
Screen Shot 2021-08-09 at 10 23 14 AM

@zmiao zmiao requested a review from ansis August 9, 2021 07:04
@zmiao zmiao marked this pull request as ready for review August 9, 2021 07:04
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

This looks great! Nit: we should use PR titles and changelog descriptions in a way that describes user impact if possible, e.g. in this case I'd name it "Improve collision detection for labels along lines", because it's not clear what collision circle pixelated radius means for the end user.

@zmiao zmiao changed the title Runtime evaluation of collision circle's pixelated radius Improve collision detection for labels along lines by switching to runtime evaluation of collision circle's pixelated radius Aug 9, 2021
@zmiao
Copy link
Contributor Author

zmiao commented Aug 9, 2021

@mourner Thank you for the suggestion! I will update the title and changelog entry accordingly.

Copy link
Contributor

@ansis ansis left a comment

Choose a reason for hiding this comment

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

Looks great!

@zmiao zmiao merged commit 7d2f272 into main Aug 10, 2021
@zmiao zmiao deleted the zmiao-runtime-collision-circle branch August 10, 2021 06:42
@SnailBones SnailBones added this to the v2.4.2 milestone Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants