[ios] Invalidate CADisplayLink

CADisplayLink holds a strong reference to its target, forming a cycle that must be broken with -[CADisplayLink invalidate] when the animation is complete. I don’t yet have enough faith that will-change and did-change notifications are always coming from mbgl in pairs, so this change limits CADisplayLink to when MGLMapView is in the view hierarchy. It also pauses the CADisplayLink when the view is hidden or the application is in the background. Finally, -[MGLMapView invalidate] has been renamed because that term tends not to mean “redraw” in Cocoa but is rather tied to timers. Fixes #3130. [ios] Also invalidate CADisplayLink on removal from window [ios] Also shut down CADisplayLink when view is hidden
mapbox · Jan 8, 2016 · 6fcb9f4 · 6fcb9f4
1 parent 52844d4
commit 6fcb9f4
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,6 +38,7 @@ Known issues:
 
 ## iOS master
 
+- Fixed an issue causing the entire MGLMapView to leak. ([#3447](https://github.com/mapbox/mapbox-gl-native/pull/3447))
 - `MGLMapView` methods that alter the viewport now accept optional completion handlers. ([#3090](https://github.com/mapbox/mapbox-gl-native/pull/3090))
 - You can now modify an annotation’s image after adding the annotation to the map. ([#3146](https://github.com/mapbox/mapbox-gl-native/pull/3146))
 - Tapping now selects annotations more reliably. Tapping near the top of a large annotation image now selects that annotation. An annotation image’s alignment insets influence how far away the user can tap and still select the annotation. For example, if your annotation image has a large shadow, you can keep that shadow from being tappable by excluding it from the image’s alignment rect. ([#3261](https://github.com/mapbox/mapbox-gl-native/pull/3261))

diff --git a/ios/benchmark/MBXBenchViewController.mm b/ios/benchmark/MBXBenchViewController.mm
@@ -11,7 +11,7 @@ @interface MGLMapView (MBXBenchmarkAdditions)
 #pragma mark - Debugging
 
 /** Triggers another render pass even when it is not necessary. */
-- (void)invalidate;
+- (void)setNeedsGLDisplay;
 
 /** Returns whether the map view is currently loading or processing any assets required to render the map */
 - (BOOL)isFullyLoaded;
@@ -119,7 +119,7 @@ - (void)mapViewDidFinishRenderingFrame:(MGLMapView *)mapView fullyRendered:(__un
             idx++;
             [self startBenchmarkIteration];
         } else {
-            [mapView invalidate];
+            [mapView setNeedsGLDisplay];
         }
         return;
     }
@@ -134,7 +134,7 @@ - (void)mapViewDidFinishRenderingFrame:(MGLMapView *)mapView fullyRendered:(__un
             started = Clock::now();
             NSLog(@"- Benchmarking for %d frames...", benchmarkDuration);
         }
-        [mapView invalidate];
+        [mapView setNeedsGLDisplay];
         return;
     }
 
@@ -146,7 +146,7 @@ - (void)mapViewDidFinishRenderingFrame:(MGLMapView *)mapView fullyRendered:(__un
             state = State::WarmingUp;
             [self.mapView emptyMemoryCache];
             NSLog(@"- Warming up for %d frames...", warmupDuration);
-            [mapView invalidate];
+            [mapView setNeedsGLDisplay];
         }
         return;
     }

diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
@@ -287,12 +287,6 @@ - (void)commonInit
     // setup mbgl map
     _mbglMap = new mbgl::Map(*_mbglView, *_mbglFileSource, mbgl::MapMode::Continuous);
 
-    // setup refresh driver
-    _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(updateFromDisplayLink)];
-    _displayLink.frameInterval = MGLTargetFrameInterval;
-    [_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
-    _needsDisplayRefresh = YES;
-
     // start paused if in IB
     if (_isTargetingInterfaceBuilder || background) {
         self.dormant = YES;
@@ -489,6 +483,8 @@ - (void)dealloc
 {
     [[NSNotificationCenter defaultCenter] removeObserver:self];
     [[MGLAccountManager sharedManager] removeObserver:self forKeyPath:@"accessToken"];
+
+    [self validateDisplayLink];
 
     if (_mbglMap)
     {
@@ -785,7 +781,7 @@ - (void)updateFromDisplayLink
     }
 }
 
-- (void)invalidate
+- (void)setNeedsGLDisplay
 {
     MGLAssertIsMainThread();
 
@@ -798,21 +794,53 @@ - (void)willTerminate
 
     if ( ! self.isDormant)
     {
+        [self validateDisplayLink];
         self.dormant = YES;
         _mbglMap->pause();
         [self.glView deleteDrawable];
     }
 }
 
+- (void)validateDisplayLink
+{
+    BOOL isVisible = self.superview && self.window;
+    if (isVisible && ! _displayLink)
+    {
+        _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(updateFromDisplayLink)];
+        _displayLink.frameInterval = MGLTargetFrameInterval;
+        [_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
+        _needsDisplayRefresh = YES;
+    }
+    else if ( ! isVisible && _displayLink)
+    {
+        [_displayLink invalidate];
+        _displayLink = nil;
+    }
+}
+
+- (void)didMoveToWindow
+{
+    [self validateDisplayLink];
+    [super didMoveToWindow];
+}
+
+- (void)didMoveToSuperview
+{
+    [self validateDisplayLink];
+    [super didMoveToSuperview];
+}
+
 - (void)sleepGL:(__unused NSNotification *)notification
 {
     MGLAssertIsMainThread();
 
-    if ( ! self.isDormant)
+    if ( ! self.dormant)
     {
         self.dormant = YES;
 
         [MGLMapboxEvents flush];
+
+        _displayLink.paused = YES;
 
         if ( ! self.glSnapshotView)
         {
@@ -843,7 +871,7 @@ - (void)wakeGL:(__unused NSNotification *)notification
 {
     MGLAssertIsMainThread();
 
-    if (self.isDormant && [UIApplication sharedApplication].applicationState != UIApplicationStateBackground)
+    if (self.dormant && [UIApplication sharedApplication].applicationState != UIApplicationStateBackground)
     {
         self.dormant = NO;
 
@@ -857,9 +885,17 @@ - (void)wakeGL:(__unused NSNotification *)notification
         [self.glView bindDrawable];
 
         _mbglMap->resume();
+
+        _displayLink.paused = NO;
     }
 }
 
+- (void)setHidden:(BOOL)hidden
+{
+    super.hidden = hidden;
+    _displayLink.paused = hidden;
+}
+
 - (void)tintColorDidChange
 {
     for (UIView *subview in self.subviews) [self updateTintColorForView:subview];
@@ -3423,7 +3459,7 @@ void deactivate() override
 
     void invalidate() override
     {
-        [nativeView performSelectorOnMainThread:@selector(invalidate)
+        [nativeView performSelectorOnMainThread:@selector(setNeedsGLDisplay)
                                      withObject:nil
                                   waitUntilDone:NO];
     }