This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
Improve user location annotation performance #6019
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
friedbunny
added
iOS
| @@ -291,13 +292,21 @@ - (void)drawDot | |||
| if (accuracyRingSize > MGLUserLocationAnnotationDotSize + 15) | |||
| { | |||
| _accuracyRingLayer.hidden = NO; | |||
|
|
|||
| id shouldNotAnimateAccuracyRingChange = (_oldHorizontalAccuracy == self.userLocation.location.horizontalAccuracy) ? (id)kCFBooleanTrue : (id)kCFBooleanFalse; | |||
There was a problem hiding this comment.
Nit: always express variable names in the positive, for easier reasoning. How about shouldDisableActions, along with a comment?
There was a problem hiding this comment.
Sounds good to me... fought with myself a bit here and I like your solution. 👍
friedbunny
force-pushed
the
fb-user-dot-performance
branch
from
83721f2
to
2f58d22
Compare
friedbunny
force-pushed
the
5039-MGLUserLocationAnnotationView-refactor
branch
from
824c09f
to
634f680
Compare
friedbunny
changed the base branch from
5039-MGLUserLocationAnnotationView-refactor
to
master
Improves perceived and actual performance by eliminating the implicit animation that occurred when the accuracy ring changed size during zooms. This previously would pile up animations as the zoom changed, causing an unsightly wobbling effect.
Supposed performance enhancement and also improves edge smoothness.
friedbunny
force-pushed
the
fb-user-dot-performance
branch
from
2f58d22
to
1e5b15f
Compare
|
Changing the target branch was a bit rougher than I hoped — it didn’t recognize that #5882 had been squash-merged and showed all of the squashed commits again here. Fixed and rebased onto master. Will merge after tests go green. |
|
Merged without Travis (re-)approval because Travis is having a major outage today. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User location annotation performance wasn’t bad previously, but there are some small tweaks we can make to improve both perceived and actual performance.
This PR is proposed against
5039-MGLUserLocationAnnotationView-refactoruntil #5882 lands. We can now change the base branch of existing pull requests, so I’ll try that out before merging this.Accuracy ring
The biggest improvement is to the accuracy ring: instead of queueing up implicit animations while zooming, now we only animate changes to the ring’s size (i.e., when the horizontal accuracy changes). This fixes the wobbling effect you could see at the edges of the ring when zooming.
We’re able to disable the implicit animation during zooms because we already update the user dot ourselves in
-[MGLMapView updateUserLocationAnnotationViewAnimatedWithDuration:]— sometimes almost every frame.Shadow paths
Per Apple, pre-calculating the paths of shadows can provide better performance.
Rasterize pulsating inner dot
This stops the dot from being rendered off-screen and smoothes its edges (which could at times appear squarish).
/cc @1ec5 @boundsj @frederoni @incanus