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

[ios] Optimize MGLAnnotationView scale transform when not pitched #10951

Merged
merged 1 commit into from
Jan 18, 2018

Conversation

friedbunny
Copy link
Contributor

@friedbunny friedbunny commented Jan 18, 2018

Skips 3D transform (pitch scaling) logic if an MGLAnnotationView with scalesWithViewingDistance = YES is being shown on an unpitched map.

This change yields a modest 5-10% reduction in the amount of time spent updating view annotation positions in such cases. Since scalesWithViewingDistance is enabled by default, this seems worthwhile.

screen shot 2018-01-17 at 8 16 32 pm
Moving around an unpitched map, post-optimization.

/cc @boundsj @1ec5

@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage annotations Annotations on iOS and macOS or markers on Android labels Jan 18, 2018
@friedbunny friedbunny added this to the ios-v4.0.0 milestone Jan 18, 2018
@friedbunny friedbunny self-assigned this Jan 18, 2018
@friedbunny friedbunny requested review from boundsj and 1ec5 January 18, 2018 01:23
@friedbunny friedbunny force-pushed the fb-unpitched-annotation-view-scaling branch from 5749158 to 2bb6bc0 Compare January 18, 2018 01:50
@friedbunny
Copy link
Contributor Author

friedbunny commented Jan 18, 2018

Added a note to MGLAnnotationView.scalesWithViewingDistance about performance — judging by the above Instruments trace, this property still comes with a ~10% performance cost.

screen shot 2018-01-17 at 8 49 51 pm

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

This is worth a blurb in the changelog.

@@ -171,6 +171,10 @@ MGL_EXPORT

The default value of this property is `YES`. Set this property to `NO` if the
view’s legibility is important.

@note Scaling many on-screen view annotations can contribute to poor map
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: “view-backed annotations” or “annotation views”, but not “view annotations”. (All annotations go inside a view, the map view.)

@friedbunny friedbunny force-pushed the fb-unpitched-annotation-view-scaling branch from 2bb6bc0 to 42845ad Compare January 18, 2018 19:01
@friedbunny friedbunny merged commit 42845ad into master Jan 18, 2018
@friedbunny friedbunny deleted the fb-unpitched-annotation-view-scaling branch January 18, 2018 19:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
annotations Annotations on iOS and macOS or markers on Android iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants