Ensure pending completion blocks, even if rendering is not strictly r…

…equired. (#463)
mapbox · Sep 23, 2020 · ba78dcd · ba78dcd
1 parent 3501f47
commit ba78dcd
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 103 deletions.
diff --git a/Makefile b/Makefile
@@ -151,7 +151,7 @@ $(IOS_OUTPUT_PATH):
 $(NETRC_FILE):
 	@echo "$$NETRC" > $(NETRC_FILE)
 
-$(CARTHAGE_DEPS): $(NETRC_FILE) | $(IOS_OUTPUT_PATH)
+$(CARTHAGE_DEPS): | $(NETRC_FILE) $(IOS_OUTPUT_PATH)
 	carthage bootstrap --platform iOS --use-netrc
 	@echo "Finishing bootstrapping"
 

diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.
 
+## master
+
+### 🐞 Bug fixes
+
+* Fixed an issue where completion blocks were not called until the map was rendered. ([#463](https://github.com/mapbox/mapbox-gl-native-ios/pull/463))
+
 ## 6.2.0 - September 17, 2020
 
 **This release supports iOS 14 and Xcode 12**

diff --git a/...ation Tests/MGLMapViewPendingBlockTests.m → ...tion Tests/MGLMapViewPendingBlockTests.mm b/...ation Tests/MGLMapViewPendingBlockTests.m → ...tion Tests/MGLMapViewPendingBlockTests.mm
@@ -1,5 +1,6 @@
 #import "MGLMapViewIntegrationTest.h"
 #import "MGLTestUtility.h"
+#import "MGLMapView_Private.h"
 
 @interface MGLMapView (MGLMapViewPendingBlockTests)
 @property (nonatomic) NSMutableArray *pendingCompletionBlocks;
@@ -14,75 +15,89 @@ @interface MGLMapViewPendingBlockTests : MGLMapViewIntegrationTest
 @implementation MGLMapViewPendingBlockTests
 
 - (void)testSetCenterCoordinate {
-    __typeof__(self) weakSelf = self;
-
-    void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
-        __typeof__(self) strongSelf = weakSelf;
-
-        if (strongSelf) {
-            [strongSelf.mapView setCenterCoordinate:CLLocationCoordinate2DMake(10.0, 10.0)
-                                          zoomLevel:10.0
-                                          direction:0
-                                           animated:NO
-                                  completionHandler:completion];
-        }
-        else {
-            completion();
-        }
-    };
-
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
-                                                    transition:transition
-                                          addToPendingCallback:nil];
+    [self internal_testSetCenterCoordinate:CLLocationCoordinate2DMake(10.0, 20.0)
+                                 zoomLevel:10.0
+                                  animated:NO
+                                  testName:NSStringFromSelector(_cmd)];
 }
 
 - (void)testSetCenterCoordinateAnimated {
+    [self internal_testSetCenterCoordinate:CLLocationCoordinate2DMake(10.0, 20.0)
+                                 zoomLevel:10.0
+                                  animated:NO
+                                  testName:NSStringFromSelector(_cmd)];
+}
+
+- (void)internal_testSetCenterCoordinate:(CLLocationCoordinate2D)coord
+                               zoomLevel:(double)zoomLevel
+                                animated:(BOOL)animated
+                                testName:(NSString*)testName {
     __typeof__(self) weakSelf = self;
 
     void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
         __typeof__(self) strongSelf = weakSelf;
 
         if (strongSelf) {
-            [strongSelf.mapView setCenterCoordinate:CLLocationCoordinate2DMake(10.0, 10.0)
-                                          zoomLevel:10.0
+            [strongSelf.mapView setCenterCoordinate:coord
+                                          zoomLevel:zoomLevel
                                           direction:0
-                                           animated:YES
+                                           animated:animated
                                   completionHandler:completion];
         }
         else {
             completion();
         }
     };
 
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
+    [self internalTestCompletionBlockAddedToPendingForTestName:testName
                                                     transition:transition
                                           addToPendingCallback:nil];
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+- (void)testSetSameCenterCoordinateBug461 {
+    [self internal_testSetSameCenterCoordinateBug461Animated:NO testName:NSStringFromSelector(_cmd)];
+}
+
+- (void)testSetSameCenterCoordinateBug461Animated {
+    [self internal_testSetSameCenterCoordinateBug461Animated:YES testName:NSStringFromSelector(_cmd)];
+}
+
+- (void)internal_testSetSameCenterCoordinateBug461Animated:(BOOL)animated testName:(NSString*)testName {
+
+    CLLocationCoordinate2D coord = CLLocationCoordinate2DMake(10.0, 20.0);
+    double zoomLevel = 10.0;
+    // Set the map zoom level to a specific constant
+    // Set the map center coordinate to a specific constant
+    [self.mapView setCenterCoordinate:coord
+                            zoomLevel:zoomLevel
+                             animated:NO];
+
+    [self.mapView setNeedsRerender];
+    [self waitForMapViewToIdleWithTimeout:1];
+
+    XCTAssert(self.mapView.zoomLevel == zoomLevel);
+    XCTAssertEqualWithAccuracy(self.mapView.centerCoordinate.latitude, coord.latitude, 0.000001);
+    XCTAssertEqualWithAccuracy(self.mapView.centerCoordinate.longitude, coord.longitude, 0.000001);
+
+    [self internal_testSetCenterCoordinate:coord
+                                 zoomLevel:zoomLevel
+                                  animated:NO
+                                  testName:testName];
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
 - (void)testSetVisibleCoordinateBounds {
-    __typeof__(self) weakSelf = self;
-
-    void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
-        __typeof__(self) strongSelf = weakSelf;
-
-        if (strongSelf) {
-            MGLCoordinateBounds unitBounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0, 0), CLLocationCoordinate2DMake(1, 1));
-            [strongSelf.mapView setVisibleCoordinateBounds:unitBounds
-                                               edgePadding:UIEdgeInsetsZero
-                                                  animated:NO
-                                         completionHandler:completion];
-        }
-        else {
-            completion();
-        }
-    };
-
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
-                                                    transition:transition
-                                          addToPendingCallback:nil];
+    [self internal_testSetVisibleCoordinateBoundsAnimated:NO testName:NSStringFromSelector(_cmd)];
 }
 
 - (void)testSetVisibleCoordinateBoundsAnimated {
+    [self internal_testSetVisibleCoordinateBoundsAnimated:YES testName:NSStringFromSelector(_cmd)];
+}
+
+- (void)internal_testSetVisibleCoordinateBoundsAnimated:(BOOL)animated testName:(NSString*)testName {
     __typeof__(self) weakSelf = self;
 
     void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
@@ -92,42 +107,30 @@ - (void)testSetVisibleCoordinateBoundsAnimated {
             MGLCoordinateBounds unitBounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0, 0), CLLocationCoordinate2DMake(1, 1));
             [strongSelf.mapView setVisibleCoordinateBounds:unitBounds
                                                edgePadding:UIEdgeInsetsZero
-                                                  animated:YES
+                                                  animated:animated
                                          completionHandler:completion];
         }
         else {
             completion();
         }
     };
 
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
+    [self internalTestCompletionBlockAddedToPendingForTestName:testName
                                                     transition:transition
                                           addToPendingCallback:nil];
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
 - (void)testSetCamera {
-    __typeof__(self) weakSelf = self;
-
-    void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
-        __typeof__(self) strongSelf = weakSelf;
-
-        if (strongSelf) {
-            MGLCoordinateBounds unitBounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0, 0), CLLocationCoordinate2DMake(1, 1));
-            MGLMapCamera *camera = [strongSelf.mapView cameraThatFitsCoordinateBounds:unitBounds];
-
-            [strongSelf.mapView setCamera:camera withDuration:0.0 animationTimingFunction:nil completionHandler:completion];
-        }
-        else {
-            completion();
-        }
-    };
-
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
-                                                    transition:transition
-                                          addToPendingCallback:nil];
+    [self internal_testSetCameraAnimated:NO testName:NSStringFromSelector(_cmd)];
 }
 
 - (void)testSetCameraAnimated {
+    [self internal_testSetCameraAnimated:YES testName:NSStringFromSelector(_cmd)];
+}
+
+- (void)internal_testSetCameraAnimated:(BOOL)animated testName:(NSString*)testName {
     __typeof__(self) weakSelf = self;
 
     void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
@@ -137,41 +140,32 @@ - (void)testSetCameraAnimated {
             MGLCoordinateBounds unitBounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0, 0), CLLocationCoordinate2DMake(1, 1));
             MGLMapCamera *camera = [strongSelf.mapView cameraThatFitsCoordinateBounds:unitBounds];
 
-            [strongSelf.mapView setCamera:camera withDuration:0.3 animationTimingFunction:nil completionHandler:completion];
+            [strongSelf.mapView setCamera:camera
+                             withDuration:animated ? 0.3 : 0.0
+                  animationTimingFunction:nil
+                        completionHandler:completion];
         }
         else {
             completion();
         }
     };
 
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
+    [self internalTestCompletionBlockAddedToPendingForTestName:testName
                                                     transition:transition
                                           addToPendingCallback:nil];
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
 - (void)testFlyToCamera {
-    __typeof__(self) weakSelf = self;
-
-    void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) {
-        __typeof__(self) strongSelf = weakSelf;
-
-        if (strongSelf) {
-            MGLCoordinateBounds unitBounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0, 0), CLLocationCoordinate2DMake(1, 1));
-            MGLMapCamera *camera = [strongSelf.mapView cameraThatFitsCoordinateBounds:unitBounds];
-
-            [strongSelf.mapView flyToCamera:camera withDuration:0.0 completionHandler:completion];
-        }
-        else {
-            completion();
-        }
-    };
-
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
-                                                    transition:transition
-                                          addToPendingCallback:nil];
+    [self internal_testFlyToCameraAnimated:NO testName:NSStringFromSelector(_cmd)];
 }
 
 - (void)testFlyToCameraAnimated {
+    [self internal_testFlyToCameraAnimated:YES testName:NSStringFromSelector(_cmd)];
+}
+
+- (void)internal_testFlyToCameraAnimated:(BOOL)animated testName:(NSString*)testName {
 
     __typeof__(self) weakSelf = self;
 
@@ -182,19 +176,20 @@ - (void)testFlyToCameraAnimated {
             MGLCoordinateBounds unitBounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0, 0), CLLocationCoordinate2DMake(1, 1));
             MGLMapCamera *camera = [strongSelf.mapView cameraThatFitsCoordinateBounds:unitBounds];
 
-            [strongSelf.mapView flyToCamera:camera withDuration:0.3 completionHandler:completion];
+            [strongSelf.mapView flyToCamera:camera
+                               withDuration:animated ? 0.3 : 0.0
+                          completionHandler:completion];
         }
         else {
             completion();
         }
     };
 
-    [self internalTestCompletionBlockAddedToPendingForTestName:NSStringFromSelector(_cmd)
+    [self internalTestCompletionBlockAddedToPendingForTestName:testName
                                                     transition:transition
                                           addToPendingCallback:nil];
 }
 
-
 #pragma mark - test interrupting regular rendering
 
 - (void)testSetCenterCoordinateSetHidden {

diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj
@@ -403,7 +403,7 @@
 		CA3402782495E53900415EEE /* testQueryRoadsAroundDC.json in Resources */ = {isa = PBXBuildFile; fileRef = CA3402772495E53900415EEE /* testQueryRoadsAroundDC.json */; };
 		CA4C54FE2324948100A81659 /* MGLSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA4C54FD2324948100A81659 /* MGLSourceTests.swift */; };
 		CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */; };
-		CA4F3BDE230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m */; };
+		CA4F3BDE230F74C3008BAFEA /* MGLMapViewPendingBlockTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.mm */; };
 		CA4F3BE223107793008BAFEA /* MGLCameraTransitionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA4F3BE123107793008BAFEA /* MGLCameraTransitionTests.mm */; };
 		CA4F3BE4231077B9008BAFEA /* MGLCameraTransitionFinishTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA4F3BE3231077B9008BAFEA /* MGLCameraTransitionFinishTests.mm */; };
 		CA55CD41202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
@@ -1082,7 +1082,7 @@
 		CA3402772495E53900415EEE /* testQueryRoadsAroundDC.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = testQueryRoadsAroundDC.json; sourceTree = "<group>"; };
 		CA4C54FD2324948100A81659 /* MGLSourceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MGLSourceTests.swift; sourceTree = "<group>"; };
 		CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleLayerIntegrationTests.m; sourceTree = "<group>"; };
-		CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MGLMapViewPendingBlockTests.m; sourceTree = "<group>"; };
+		CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLMapViewPendingBlockTests.mm; sourceTree = "<group>"; };
 		CA4F3BE123107793008BAFEA /* MGLCameraTransitionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionTests.mm; sourceTree = "<group>"; };
 		CA4F3BE3231077B9008BAFEA /* MGLCameraTransitionFinishTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionFinishTests.mm; sourceTree = "<group>"; };
 		CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = "<group>"; };
@@ -1470,7 +1470,7 @@
 				CA88DC2F21C85D900059ED5A /* MGLStyleURLIntegrationTest.m */,
 				077061DB215DA11F000FEF62 /* MGLTestLocationManager.h */,
 				077061D9215DA00E000FEF62 /* MGLTestLocationManager.m */,
-				CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m */,
+				CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.mm */,
 				CA4C54FD2324948100A81659 /* MGLSourceTests.swift */,
 				CA17464D23D8A93C008B7A43 /* MGLNetworkConfigurationIntegrationTests.mm */,
 				CA600148242BD8360041BBEC /* MGLIntegrationTestCase.m */,
@@ -3115,7 +3115,7 @@
 				CA7766842229C11A0008DE9E /* SMCalloutView.m in Sources */,
 				CA176AD72507F98900D47E3D /* MGLMockApplication.m in Sources */,
 				16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.mm in Sources */,
-				CA4F3BDE230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m in Sources */,
+				CA4F3BDE230F74C3008BAFEA /* MGLMapViewPendingBlockTests.mm in Sources */,
 				CA88DC3021C85D900059ED5A /* MGLStyleURLIntegrationTest.m in Sources */,
 				CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */,
 				CA7766832229C10E0008DE9E /* MGLCompactCalloutView.m in Sources */,

diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
@@ -1104,24 +1104,21 @@ - (void)updateViewsPostMapRendering {
     [self updateUserLocationAnnotationView];
     [self updateAnnotationViews];
     [self updateCalloutView];
-
-    // Call any pending completion blocks. This is primarily to ensure
-    // that annotations are in the expected position after core rendering
-    // and map update.
-    //
-    // TODO: Consider using this same mechanism for delegate callbacks.
-    [self processPendingBlocks];
 }
 
 - (BOOL)renderSync
 {
-    if (!self.needsDisplayRefresh) {
+    BOOL hasPendingBlocks = (self.pendingCompletionBlocks.count > 0);
+
+    if (!self.needsDisplayRefresh && !hasPendingBlocks) {
         return NO;
     }
 
+    BOOL needsRender = self.needsDisplayRefresh;
+
     self.needsDisplayRefresh = NO;
 
-    if (!self.dormant)
+    if (!self.dormant && needsRender)
     {
         MGL_SIGNPOST_BEGIN(_log, _signpost, "renderSync", "render");
         if (_rendererFrontend) {
@@ -1149,6 +1146,16 @@ - (BOOL)renderSync
         [self updateViewsPostMapRendering];
         MGL_SIGNPOST_END(_log, _signpost, "renderSync", "update");
     }
+
+    if (hasPendingBlocks) {
+        // Call any pending completion blocks. This is primarily to ensure
+        // that annotations are in the expected position after core rendering
+        // and map update.
+        //
+        // TODO: Consider using this same mechanism for delegate callbacks.
+        [self processPendingBlocks];
+    }
+
     return YES;
 }