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

Map view lags behind annotation views when panning #5489

Closed
Paladinfeng opened this issue Jun 27, 2016 · 45 comments
Closed

Map view lags behind annotation views when panning #5489

Paladinfeng opened this issue Jun 27, 2016 · 45 comments
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

Comments

@Paladinfeng
Copy link

*_Platform: iOS 9.3.2
*_Mapbox SDK version: v3.3.0-beta2

I found the MGLAnnotationView scrolling like jello when I use the new iOS SDK. The MGLAnnotationView can't follow the MapView smoothly. I have tested at iPhone 5s and iPhone 6.

@friedbunny
Copy link
Contributor

Please post more information about the view you’re using — code and a gif would be ideal. Thanks.

@Paladinfeng
Copy link
Author

ok, I'll save something as a gif video tomorrow.
it just like rolling shutter in the field of photography.

@Paladinfeng
Copy link
Author

rolling_shutter
can u see it? The MGLAnnotationView can't follow the mapView totally.

@friedbunny
Copy link
Contributor

Thanks for the image, @Paladinfeng. Syncing the movement of native views with the map is an ongoing challenge, certainly.

/cc @boundsj @1ec5

@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Jun 28, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jun 28, 2016

I suspect #5245 may yield some improvement, specifically disabling animations while updating annotations and coalescing the frame updates into one Core Animation transaction. I’ll need to pull those changes out into a separate PR and see if they improve things on their own, without the other matrix-related changes.

@1ec5 1ec5 changed the title AnnotationView scrolling like jello Annotation views lag behind map view when panning Jul 3, 2016
@1ec5 1ec5 changed the title Annotation views lag behind map view when panning Map view lags behind annotation views when panning Jul 3, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jul 3, 2016

The changes in #5245 don’t address this issue after all.

As far as I can tell, the map is lagging behind the annotation views by one frame every other frame or so. In the screen recording below, keep an eye on the annotation view that initially covers up the word “Washington”. When I scroll up, you can occasionally see “Washington” appear to the south of the annotation view; when I scroll down, you can occasionally see “Washington” appear to the north of the annotation view:

gelatin

@boundsj boundsj added the annotations Annotations on iOS and macOS or markers on Android label Jul 6, 2016
@boundsj boundsj added this to the ios-v3.4.0 milestone Jul 6, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jul 7, 2016

One potential way to eliminate the "tearing" effect might be to put the annotation updating phase into a custom style layer drawing callback. This would ensure that the Core Graphics and Core Animation drawing commands occur closer to the OpenGL drawing commands and in the same run loop iteration. Ironically, the annotation views would be less performant, but the perceived performance would improve.

@incanus
Copy link
Contributor

incanus commented Jul 19, 2016

Naive post to related issue, before further investigation: #1813

@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

#1813 in some respects addresses the opposite issue: the map was leading the UIView because the map was given the opportunity to advance one step (for a transition) before the UIView got a chance to update. The annotation view implementation addresses this by updating the frame on the "map did finish rendering frame" notification.

I think what we're seeing is that Core Animation / Core Graphics manages to maintain 60fps drawing of the annotation views even as MGLMapView struggles to do the same with the underlying map. We mitigated that for the user dot in #3683 by disabling implicit animation, but the root cause can still be reexposed by adding multiple views onto the map.

incanus referenced this issue Jul 23, 2016
Removed unused annotation image code from iosapp, now that bulk-added point annotations are backed by annotation views.
@boundsj boundsj assigned boundsj and unassigned boundsj Aug 12, 2016
@incanus
Copy link
Contributor

incanus commented Aug 12, 2016

Here's a good debug snippet against 0ae941d that easily shows the problem on panning:

diff --git a/platform/ios/app/MBXViewController.m b/platform/ios/app/MBXViewController.m
index df1c446..173bb39 100644
--- a/platform/ios/app/MBXViewController.m
+++ b/platform/ios/app/MBXViewController.m
@@ -75,6 +75,13 @@ static NSString * const MBXViewControllerAnnotationViewReuseIdentifer = @"MBXVie
     [self restoreState:nil];

     self.debugLoggingEnabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"MGLMapboxMetricsDebugLoggingEnabled"];
+
+    self.mapView.pitchEnabled = NO;
+    self.mapView.rotateEnabled = NO;
+
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(38.913343209724644, -77.033041982152554) zoomLevel:11 animated:NO];
+
+    [self parseFeaturesAddingCount:100 usingViews:YES];
 }

 - (void)viewDidAppear:(BOOL)animated
@@ -709,7 +716,7 @@ static NSString * const MBXViewControllerAnnotationViewReuseIdentifer = @"MBXVie
     if (!annotationView)
     {
         annotationView = [[MBXAnnotationView alloc] initWithReuseIdentifier:MBXViewControllerAnnotationViewReuseIdentifer];
-        annotationView.frame = CGRectMake(0, 0, 10, 10);
+        annotationView.frame = CGRectMake(0, 0, 50, 50);
         annotationView.backgroundColor = [UIColor whiteColor];

         // uncomment to make the annotation view draggable

@incanus
Copy link
Contributor

incanus commented Aug 12, 2016

Probably minor compared to actual proper view syncing, but we very frequently run this pointless conversion for nil annotations:

annotationView.center = [self convertCoordinate:annotationContext.annotation.coordinate toPointToView:self];
2016-08-12 12:22:06.772 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x1393298f0 1
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x139447e20 1
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x13932ddf0 1
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x13932ef30 1
2016-08-12 12:22:06.773 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.774 Mapbox GL[3947:1796076] 0x139330070 1
2016-08-12 12:22:06.775 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.775 Mapbox GL[3947:1796076] 0x139331070 1
2016-08-12 12:22:06.775 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.775 Mapbox GL[3947:1796076] 0x1393321c0 1
2016-08-12 12:22:06.775 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.776 Mapbox GL[3947:1796076] 0x139449650 1
2016-08-12 12:22:06.776 Mapbox GL[3947:1796076] 0x0 2
2016-08-12 12:22:06.776 Mapbox GL[3947:1796076] 0x13944a790 2

1 is here and 2 is here.

@boundsj
Copy link
Contributor

boundsj commented Aug 12, 2016

we very frequently run this pointless conversion for nil annotations

As we've noted noted in #5787, we've been waiting for #5165 to land (it was just merged) so that we can optimize the MGLAnnotationView part of this issue (1 above). While working on that, I will also short circuit updateAnnotationViews: so that it does not deal in MGLAnnotationImage backed annotations (2 above)

@incanus
Copy link
Contributor

incanus commented Aug 16, 2016

I've done a number of tests with CATransaction, -actionForLayer:forKey:, and even annotation view layer custom subclassing, but in the end, I think this might be a rounding and/or precision problem. We are dealing in sub-pixel values for -[MGLAnnotationView center]:

2016-08-15 17:29:04.867 Mapbox GL[412:78842] ==>: (280.808557, 409.468855) for 0x133021bc0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] <==: (281.000000, 409.000000) for 0x133021bc0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] ==>: (312.938138, 413.440941) for 0x133022df0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] <==: (313.000000, 413.000000) for 0x133022df0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] ==>: (462.486913, 257.307288) for 0x132a53aa0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] <==: (462.000000, 257.000000) for 0x132a53aa0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] ==>: (340.970596, 405.615242) for 0x132a53dc0
2016-08-15 17:29:04.867 Mapbox GL[412:78842] <==: (341.000000, 406.000000) for 0x132a53dc0

The ==> values represent those passed into -[MGLAnnotationView setCenter:] and the <== ones represent values that have had round() applied to their double values. When I do this, the jitter is slightly more pronounced, making me think that this is a lead.

Chasing the rabbit hole of

-[MGLMapView convertCoordinate:toPointToView:]
   -[MGLMapView convertLatLng:toPointToView:]
      Map::pixelForLatLng()
         -[UIView convertPoint:]

seems to be double-safe the whole way down, though.

@incanus
Copy link
Contributor

incanus commented Aug 16, 2016

The above doesn't make sense in the context of the current, pre-annotation view merged MGLUserLocationAnnotationView movement, though, which uses the same conversion routines. Currently trying some updates more in line with how we do things there (which is actually an explicit UIView animation block with zero duration).

@incanus
Copy link
Contributor

incanus commented Aug 16, 2016

The primary difference is that we update the user location annotation right inside -[MGLMapView glkView:drawInRect:], but wait for a MapChangeDidFinishRenderingFrame or MapChangeDidFinishRenderingFrameFullyRendered to call -updateAnnotationViews. We used to have this problem with the user dot early on, which is why we changed this behavior.

@incanus
Copy link
Contributor

incanus commented Aug 16, 2016

We used to have this problem with the user dot early on, which is why we changed this behavior.

Specifically, a67e802.

@incanus
Copy link
Contributor

incanus commented Aug 16, 2016

While I can see the potential of what @1ec5 states in #5489 (comment), I can't see why the user dot would behave differently. I even tried just a single annotation view to more closely match the scenarios. But moving things into -glkView:drawInRect: has the same problems.

@incanus
Copy link
Contributor

incanus commented Aug 20, 2016

focus on pan behavior, utilizing a CAScrollLayer to move all annotations together rather than each independently

The more I think about this, the more I think it will have the same effect as adding just one annotation view to the map (or the user dot). We've still got to test that scrolling with -[CAScrollLayer scrollToPoint:] doesn't behave differently than layer.position updates, though.

@incanus
Copy link
Contributor

incanus commented Aug 20, 2016

Punting on this for now—couple higher priority issues in the hopper.

@incanus incanus removed their assignment Aug 20, 2016
@1ec5
Copy link
Contributor

1ec5 commented Aug 20, 2016

The macOS SDK also sees poorer base map rendering performance compared to the GLFW app, except when you turn off CALayer compositing. Considering that you can't turn off CALayer compositing on iOS, what approaches can we take to speed up the map? Tear out CADisplayLink? What approaches can we take to slow down annotation views? Tie them to CADisplayLink with a slower refresh rate?

@mb12
Copy link

mb12 commented Aug 22, 2016

@incanus and @1ec5 . The following WWDC session talks about exactly the same problem (synchronizing opengl map view with UIViews). Can you please also try out that as well? I've copy pasted the relevant links and sections below.

WWDC video link
https://developer.apple.com/videos/play/wwdc2015/233/

Text notes here

http://asciiwwdc.com/2015/sessions/233

"Now this may be a problem if you want those two to be in sync, and that could happen, for example, if you had an OpenGL map view, that you wanted to draw UIKit content on top of.

In that case, you may want to synchronize those updates in something like this, and there's a property that allows you to do that called Presents With Transaction.

It's on CAeagllayer and CAMetalLayer.

When this is set to False, which is the default value, then it will get your GPU content to the display as quickly as possible.

But when you set it to True, then we synchronize your GPU content with the Core Animation content so they both appear on the display at the same time.
"

@incanus
Copy link
Contributor

incanus commented Aug 22, 2016

there's a property that allows you to do that called Presents With Transaction.

Yes, but per http://stackoverflow.com/a/30722276/977220 it's iOS 9+ 😞

@mb12
Copy link

mb12 commented Aug 22, 2016

@incanus This should be fine. iOS 9 is over an year old and installed on over 75% devices(as of Jan 2016). We can use respondsToSelector to make sure that things work well on iOS9 devices (while lagging behind on older devices).

@incanus
Copy link
Contributor

incanus commented Aug 22, 2016

Yeah, good point. We are currently in discussions about dropping iOS 7 but opportunistically using this for 9+ may be an option.

@incanus incanus self-assigned this Aug 22, 2016
@incanus incanus modified the milestones: ios-future, ios-v3.4.0 Aug 22, 2016
@incanus
Copy link
Contributor

incanus commented Aug 22, 2016

Will continue to investigate this, but for a lot of use cases (particularly small view annotations) current performance is acceptable.

@1ec5
Copy link
Contributor

1ec5 commented Aug 22, 2016

@mb12, thanks for the reminder. We previously looked into presentsWithTransaction in #1125 (comment), but there was no effect. In hindsight, that was because of an implicit animation, which we disabled in #3683. Having exhausted that lead for this issue, we should look into presentsWithTransaction at some point.

@incanus
Copy link
Contributor

incanus commented Aug 23, 2016

The good news: I have figured out the problem and have a local version working nearly perfectly.

The bad news: We need to get a higher frame rate on our core GL draws to solve it. This is essentially what @1ec5 has been hypothesizing since #5489 (comment), but I think I have confirmed it.

I tried a number of approaches to get the annotations & Core Animation to slow down and/or draw in sync with the OpenGL frame updates:

  • Calling -updateAnnotationViews only every other GL render (i.e. every other CADisplayLink update, i.e. at essentially half its current frame rate)
  • Experimenting with -[CATransaction flush] and -[CATransaction lock]
  • Bracketing both the call to -updateAnnotationViews and _mbglMap->render() in a CATransaction with varying options (-setDisableActions:YES, varying animation durations including 0)
  • Checking out ((CAEAGLLayer *)_glView.layer).presentsWithTransaction = YES; after watching the WWDC session (again), despite poor specifics in both that and the docs around how to use it (but which is what led me to experimenting with the above bracketing around the GL render)

None of these reduced jitter since what we perceive as jitter is the position relative to the map, and the map isn't moving as smoothly as it could.

Also, just to be sure, I experimented with:

  • Rounding annotation latitude & longitude to varying levels (2-5 decimal places)
  • Rounding annotation position values to whole numbers instead of their default fractional CGPoint values
  • Experimenting with calling updateAnnotationViews from the drawingHandler of a custom layer
  • Ensuring that a map-less view with annotations still looked smooth
  • Ensuring that an annotation-less map view still looked smooth

It was that last one that still looked suspect to me, especially when using Apple Maps to compare, and profiling confirmed that I was only ever seeing ~30fps for a default Streets style (on a couple years old but respectable retina iPad mini). I pared back a style in Studio until I could get one that was consistently 55-60fps:

screen shot 2016-08-22 at 6 27 17 pm

Portland

This mostly involved getting rid of all labels, symbols, and roads and just leaving fills like water and parks.

But: once I did this, and got the OpenGL frame rate to about 60fps, I got some good-looking sync:

'

You can still see this jitter a tiny bit when we dip into the ~55fps range, which is to be expected.

screen shot 2016-08-22 at 6 14 23 pm

The test style URL in question is mapbox://styles/justin/cis6rzo41000egpnt2julnz92 if you'd like to try this yourself.

So, again I think we punt on this for now as there is no magic Core Animation bullet to help us. We need to get our frame rate up with arbitrary styles, which is a more difficult proposition than what Apple Maps does with its one or two default styles.

/cc @jfirebaugh @kkaefer

@incanus
Copy link
Contributor

incanus commented Aug 23, 2016

To be complete: I also changed the MGLTargetFrameInterval to trigger 30fps max GL draws, in combination with the CATransaction experimentation in case that would be enough to cause Core Animation to slow down and sync, but no dice. Our map looks the same (since we're at about 30fps already) and Core Animation was still out of sync since it's faster.

@1ec5
Copy link
Contributor

1ec5 commented Aug 23, 2016

You can also see the map lagging behind the view in the GIF (!) in #5489 (comment) – that effect wouldn't be possible if not for the map rendering at far less than 60 fps.

We added CADisplayLink in order to throttle map redraws (originally to 30 fps, later to 60 fps, nominally). Should we revisit our use of that API? It has its own performance downsides: #2985.

@incanus
Copy link
Contributor

incanus commented Aug 23, 2016

Will read up on CADisplayLink further.

As a clarification, when I say Core Animation in the context of this problem, I mean the mechanism by which native views are composited to the frame buffer on the device. Even though we no longer actually animate our annotation views, the mere movement of them pixel-by-pixel is governed by internal OpenGL rendering on Core Animation's part and that is a solid 60fps, thus highlighting weaknesses in our own frame draws.

@1ec5
Copy link
Contributor

1ec5 commented Nov 1, 2016

Same case is with Android too. UserLocation dot and ViewMarkers are not synced with map panning.

@Paladinfeng, FYI, the equivalent issue on Android is being tracked in #6139. mapbox/mapbox-gl-js#3273 is a conceptually similar issue in GL JS.

@tabsong
Copy link

tabsong commented Jun 30, 2017

@incanus in the end, can you resolve the sync problem?

@1ec5
Copy link
Contributor

1ec5 commented Aug 25, 2018

I can't see why the user dot would behave differently.

I’ve split out #12740 to track a separate synchronization issue that only affects the user dot but not other annotation views. That issue includes a possible explanation as to why the user dot behaves differently than other annotation views.

@1ec5
Copy link
Contributor

1ec5 commented Dec 10, 2018

Fixed in #12895.

@1ec5 1ec5 closed this as completed Dec 10, 2018
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 a pull request may close this issue.

9 participants