Skip to content

Thin out repeated line labels at overscaled tiles #11414

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

Merged
merged 2 commits into from
Jan 19, 2022

Conversation

pozdnyakov
Copy link
Contributor

@pozdnyakov pozdnyakov commented Jan 18, 2022

Prior to this change, the repeated line labels (such as street names) were placed too densely on overscaled tiles, sometimes even closer than the value of the symbol-spacing property.
This caused a noticeable increase in memory usage, and the memory use increased dramatically with higher tiles overscale factors.

This change partially mitigates the issue with the following changes:

  • for "very" overscaled tiles (overscale factor > 2) on high zoom levels (z > 18) we use the tile pixel ratio from the previous zoom level
  • the distance between too repeated line labels is never smaller than the symbol-spacing style property value (tile pixel ratio is clamped to 1)

For example for http://localhost:9966/debug#22/40.79230555/-73.9733581/33.4/67:
memory usage before:
before

memory usage after:
after

So overall memory usage decreased from 75 MB to 26 MB, the amount of allocated objects decreased from 55694 to 21228

Launch Checklist

  • 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>Thin out repeated line labels at overscaled tiles in order to decrease memory and CPU usage</changelog>

Sorry, something went wrong.

@pozdnyakov pozdnyakov added the performance ⚡ Speed, stability, CPU usage, memory usage, or power usage label Jan 18, 2022
@pozdnyakov pozdnyakov self-assigned this Jan 18, 2022
@pozdnyakov pozdnyakov force-pushed the mikhail_thin_out_labels_on_overscaled_tiles branch from 23c2c20 to cd73819 Compare January 18, 2022 16:44
@pozdnyakov pozdnyakov requested a review from ansis January 18, 2022 18:46
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 good!

Can you move the function definition out of the other function so that it isn't nested?

@pozdnyakov pozdnyakov force-pushed the mikhail_thin_out_labels_on_overscaled_tiles branch from e052023 to 444dc9d Compare January 19, 2022 16:54
Prior to this change, the repeated line labels (such as street names) were placed too densely on overscaled tiles, sometimes even closer than the value of the `symbol-spacing` property.
This caused a noticeable increase in memory usage, and the memory use increased dramatically with higher tiles overscale factors.

This change partially mitigates the issue with the following changes:
* for "very" overscaled tiles (overscale factor > 2) on high zoom levels (z > 18) we use the tile pixel ratio from the previous zoom level
* the distance between too repeated line labels is never smaller than the `symbol-spacing` style property value (tile pixel ratio is clamped to 1)
@pozdnyakov pozdnyakov merged commit 203d674 into main Jan 19, 2022
@pozdnyakov pozdnyakov deleted the mikhail_thin_out_labels_on_overscaled_tiles branch January 19, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants