From 96581fd3ff4e4239d5498d7fc3d3a490c800d2e0 Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Thu, 5 Sep 2019 00:21:58 -0400 Subject: [core, ios] Call transitionFinishFn for jumpTo. (#15473) --- platform/ios/CHANGELOG.md | 1 + .../Camera Tests/MGLCameraTransitionFinishTests.mm | 109 ++++++ .../Camera Tests/MGLCameraTransitionTests.mm | 394 +++++++++++++++++++++ .../Integration Tests/MGLCameraTransitionTests.mm | 394 --------------------- .../Integration Tests/MGLMapViewIntegrationTest.h | 1 + .../Integration Tests/MGLMapViewIntegrationTest.m | 6 +- .../MGLMapViewPendingBlockTests.m | 3 +- platform/ios/ios.xcodeproj/project.pbxproj | 20 +- src/mbgl/map/transform.cpp | 9 + 9 files changed, 536 insertions(+), 401 deletions(-) create mode 100644 platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionFinishTests.mm create mode 100644 platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionTests.mm delete mode 100644 platform/ios/Integration Tests/MGLCameraTransitionTests.mm diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index bdf855aa3c..2132be5077 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -10,6 +10,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Fixed an issue where the scale bar text would become illegible if iOS 13 dark mode was enabled. ([#15524](https://github.com/mapbox/mapbox-gl-native/pull/15524)) * Enabled use of `MGLSymbolStyleLayer.textOffset` option together with `MGLSymbolStyleLayer.textVariableAnchor` (if `MGLSymbolStyleLayer.textRadialOffset` option is not provided). ([#15542](https://github.com/mapbox/mapbox-gl-native/pull/15542)) * Fixed an issue with the appearance of the compass text in iOS 13. ([#15547](https://github.com/mapbox/mapbox-gl-native/pull/15547)) +* Fixed a bug where the completion block passed to `-[MGLMapView flyToCamera:completionHandler:` (and related methods) wouldn't be called. ([#15473](https://github.com/mapbox/mapbox-gl-native/pull/15473)) ### Performance improvements diff --git a/platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionFinishTests.mm b/platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionFinishTests.mm new file mode 100644 index 0000000000..1527e8dbe5 --- /dev/null +++ b/platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionFinishTests.mm @@ -0,0 +1,109 @@ +#import "MGLMapViewIntegrationTest.h" +#import "MGLTestUtility.h" +#import "../../darwin/src/MGLGeometry_Private.h" + +#include + +@interface MGLCameraTransitionFinishTests : MGLMapViewIntegrationTest +@end + +@implementation MGLCameraTransitionFinishTests + +- (void)testEaseToCompletionHandler { + + MGLCoordinateBounds bounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0.0, 0.0), + CLLocationCoordinate2DMake(1.0, 1.0)); + MGLMapCamera *camera = [self.mapView cameraThatFitsCoordinateBounds:bounds]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Completion block should be called"]; + + [self.mapView setCamera:camera + withDuration:0.0 + animationTimingFunction:nil + completionHandler:^{ + [expectation fulfill]; + }]; + + [self waitForExpectations:@[expectation] timeout:0.5]; +} + +- (void)testEaseToCompletionHandlerAnimated { + + MGLCoordinateBounds bounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0.0, 0.0), + CLLocationCoordinate2DMake(1.0, 1.0)); + MGLMapCamera *camera = [self.mapView cameraThatFitsCoordinateBounds:bounds]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Completion block should be called"]; + + [self.mapView setCamera:camera + withDuration:0.3 + animationTimingFunction:nil + completionHandler:^{ + [expectation fulfill]; + }]; + + [self waitForExpectations:@[expectation] timeout:0.5]; +} + +- (void)testFlyToCompletionHandler { + + MGLCoordinateBounds bounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0.0, 0.0), + CLLocationCoordinate2DMake(1.0, 1.0)); + MGLMapCamera *camera = [self.mapView cameraThatFitsCoordinateBounds:bounds]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Completion block should be called"]; + + [self.mapView flyToCamera:camera + withDuration:0.0 + completionHandler:^{ + [expectation fulfill]; + }]; + + [self waitForExpectations:@[expectation] timeout:0.5]; +} + +- (void)testFlyToCompletionHandlerAnimated { + + MGLCoordinateBounds bounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(0.0, 0.0), + CLLocationCoordinate2DMake(1.0, 1.0)); + MGLMapCamera *camera = [self.mapView cameraThatFitsCoordinateBounds:bounds]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Completion block should be called"]; + + [self.mapView flyToCamera:camera + withDuration:0.3 + completionHandler:^{ + [expectation fulfill]; + }]; + + [self waitForExpectations:@[expectation] timeout:0.5]; +} +@end + +#pragma mark - camera transitions with NaN values + +@interface MGLMapView (MGLCameraTransitionFinishNaNTests) +- (mbgl::CameraOptions)cameraOptionsObjectForAnimatingToCamera:(MGLMapCamera *)camera edgePadding:(UIEdgeInsets)insets; +@end + +@interface MGLCameraTransitionNaNZoomMapView: MGLMapView +@end + +@implementation MGLCameraTransitionNaNZoomMapView +- (mbgl::CameraOptions)cameraOptionsObjectForAnimatingToCamera:(MGLMapCamera *)camera edgePadding:(UIEdgeInsets)insets { + mbgl::CameraOptions options = [super cameraOptionsObjectForAnimatingToCamera:camera edgePadding:insets]; + options.zoom = NAN; + return options; +} +@end + +// Subclass the entire test suite, but with a different MGLMapView subclass +@interface MGLCameraTransitionFinishNaNTests : MGLCameraTransitionFinishTests +@end + +@implementation MGLCameraTransitionFinishNaNTests +- (MGLMapView *)mapViewForTestWithFrame:(CGRect)rect styleURL:(NSURL *)styleURL { + return [[MGLCameraTransitionNaNZoomMapView alloc] initWithFrame:rect styleURL:styleURL]; +} +@end + diff --git a/platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionTests.mm b/platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionTests.mm new file mode 100644 index 0000000000..2965882372 --- /dev/null +++ b/platform/ios/Integration Tests/Camera Tests/MGLCameraTransitionTests.mm @@ -0,0 +1,394 @@ +#import "MGLMapViewIntegrationTest.h" +#import "MGLTestUtility.h" +#import "../../darwin/src/MGLGeometry_Private.h" + +@interface MGLCameraTransitionTests : MGLMapViewIntegrationTest +@end + +@implementation MGLCameraTransitionTests + +- (void)testSetAndResetNorthWithDispatchAsyncInDelegateMethod { + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + expectation.expectedFulfillmentCount = 2; + expectation.assertForOverFulfill = YES; + + __weak typeof(self) weakself = self; + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + + MGLCameraTransitionTests *strongSelf = weakself; + + if (!strongSelf) return; + + [expectation fulfill]; + + MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); + if (mapView.direction != 0.0) { + dispatch_async(dispatch_get_main_queue(), ^{ + [mapView resetNorth]; + }); + } + }; + + [self.mapView setDirection:90 animated:YES]; + + // loop, render, and wait + [self waitForExpectations:@[expectation] timeout:10]; +} + + +- (void)testSetAndResetNorthInDelegateMethod { + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + expectation.expectedFulfillmentCount = 2; + expectation.assertForOverFulfill = YES; + + __weak typeof(self) weakself = self; + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + + MGLCameraTransitionTests *strongSelf = weakself; + + if (!strongSelf) return; + + [expectation fulfill]; + + MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); + if (mapView.direction != 0.0) { + NSLog(@"Reset to north"); + [mapView resetNorth]; + } + }; + + [self.mapView setDirection:90 animated:YES]; + [self waitForExpectations:@[expectation] timeout:10]; +} + +- (void)testInterruptingAndResetNorthOnlyOnceInIsChanging { + + // Reset to non-zero, prior to testing + [self.mapView setDirection:45 animated:NO]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + expectation.expectedFulfillmentCount = 1; + expectation.assertForOverFulfill = YES; + + __weak typeof(self) weakself = self; + __block BOOL startedReset = NO; + __block BOOL finishedReset = NO; + + self.regionIsChanging = ^(MGLMapView *mapView) { + MGLCameraTransitionTests *strongSelf = weakself; + if (!strongSelf) return; + + if (!startedReset) { + NSLog(@"Reset to north, interrupting the previous transition"); + startedReset = YES; + [mapView resetNorth]; + finishedReset = YES; + } + }; + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + MGLCameraTransitionTests *strongSelf = weakself; + if (!strongSelf) return; + + MGLTestAssert(strongSelf, startedReset); + + if (finishedReset) { + MGLTestAssert(strongSelf, !(reason & MGLCameraChangeReasonTransitionCancelled)); + [expectation fulfill]; + } + else { + MGLTestAssert(strongSelf, reason & MGLCameraChangeReasonTransitionCancelled); + } + }; + + [self.mapView setDirection:90 animated:YES]; + [self waitForExpectations:@[expectation] timeout:10]; + + XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction); +} + +- (void)testSetCenterCancelsTransitions { + XCTestExpectation *cameraIsInDCExpectation = [self expectationWithDescription:@"camera reset to DC"]; + + CLLocationCoordinate2D dc = CLLocationCoordinate2DMake(38.894368, -77.036487); + CLLocationCoordinate2D dc_west = CLLocationCoordinate2DMake(38.894368, -77.076487); + + double zoomLevel = 15.0; + + [self.mapView setCenterCoordinate:dc zoomLevel:zoomLevel animated:NO]; + [self.mapView setCenterCoordinate:dc_west zoomLevel:zoomLevel animated:YES]; + + __weak typeof(self) weakself = self; + + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.15 * NSEC_PER_SEC), + dispatch_get_main_queue(), + ^{ + MGLCameraTransitionTests *strongSelf = weakself; + + [strongSelf.mapView setCenterCoordinate:dc zoomLevel:zoomLevel animated:NO]; + MGLTestAssertEqualWithAccuracy(strongSelf, + dc.latitude, + strongSelf.mapView.centerCoordinate.latitude, + 0.0005, + @"setting center coordinate should cancel transitions"); + MGLTestAssertEqualWithAccuracy(strongSelf, + dc.longitude, + strongSelf.mapView.centerCoordinate.longitude, + 0.0005, + @"setting center coordinate should cancel transitions"); + [cameraIsInDCExpectation fulfill]; + }); + + [self waitForExpectations:@[cameraIsInDCExpectation] timeout:10.0]; +} + +- (void)testSetCenterCoordinateInDelegateMethod { + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + expectation.expectedFulfillmentCount = 2; + expectation.assertForOverFulfill = YES; + + __weak typeof(self) weakself = self; + __block NSInteger delegateCallCount = 0; + + CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0); + CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(-40.0, -40.0); + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + + MGLCameraTransitionTests *strongSelf = weakself; + + if (!strongSelf) return; + + MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); + + CLLocationCoordinate2D center = mapView.centerCoordinate; + + switch(delegateCallCount) { + case 0: + { + // Our center coordinate should match our target (assuming we're not + // constrained by zoom level) + MGLTestAssertEqualWithAccuracy(strongSelf, + target.longitude, + center.longitude, + 0.0005, + @"center coordinate longitude should be at target"); + + MGLTestAssertEqualWithAccuracy(strongSelf, + target.latitude, + center.latitude, + 0.0005, + @"center coordinate latitude should be at target"); + + // Now set another coordinate. + // Should take MGLAnimationDuration seconds (0.3s) + [mapView setCenterCoordinate:target2 animated:YES]; + break; + } + + case 1: + { + // Our center coordinate should match our target (assuming we're not + // constrained by zoom level) + MGLTestAssertEqualWithAccuracy(strongSelf, + target2.longitude, + center.longitude, + 0.0005, + @"center coordinate longitude should be at target2"); + + MGLTestAssertEqualWithAccuracy(strongSelf, + target2.latitude, + center.latitude, + 0.0005, + @"center coordinate latitude should be at target2"); + break; + + } + + default: + MGLTestFail(strongSelf); + break; + } + + delegateCallCount++; + + [expectation fulfill]; + }; + + // Should take MGLAnimationDuration seconds (0.3) + [self.mapView setCenterCoordinate:target zoomLevel:15.0 animated:YES]; + [self waitForExpectations:@[expectation] timeout:10]; +} + +- (void)testFlyToCameraInDelegateMethod { + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + + __weak typeof(self) weakself = self; + __block NSInteger delegateCallCount = 0; + expectation.expectedFulfillmentCount = 3; + expectation.assertForOverFulfill = YES; + + CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0); + CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(30.0, 30.0); + + __block BOOL runloop = YES; + + NSTimeInterval stop0 = CACurrentMediaTime(); + __block NSTimeInterval stop1 = 0.0; + __block NSTimeInterval stop2 = 0.0; + + double zoomLevel = 5.0; + double altitude = MGLAltitudeForZoomLevel(zoomLevel, 0.0, target.latitude, self.mapView.frame.size); + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + + MGLCameraTransitionTests *strongSelf = weakself; + + if (!strongSelf) return; + + MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); + + CLLocationCoordinate2D center = mapView.centerCoordinate; + + switch(delegateCallCount) { + case 0: + { + stop1 = CACurrentMediaTime(); + + // Our center coordinate should match our target (assuming we're not + // constrained by zoom level) + MGLTestAssertEqualWithAccuracy(strongSelf, + target.longitude, + center.longitude, + 0.0005, + @"center coordinate longitude should be at target"); + + MGLTestAssertEqualWithAccuracy(strongSelf, + target.latitude, + center.latitude, + 0.0005, + @"center coordinate latitude should be at target"); + + // Now set another coordinate. + MGLMapCamera *camera = [MGLMapCamera cameraLookingAtCenterCoordinate:target2 + altitude:altitude + pitch:0.0 + heading:0.0]; + + // flyToCamera can take a while... + [mapView flyToCamera:camera completionHandler:^{ + MGLTestAssert(strongSelf, !runloop, @"Completion block should be called after delegate method"); + [expectation fulfill]; + stop2 = CACurrentMediaTime(); + }]; + break; + } + + case 1: + { + // Our center coordinate should match our target (assuming we're not + // constrained by zoom level) + MGLTestAssertEqualWithAccuracy(strongSelf, + target2.longitude, + center.longitude, + 0.0005, + @"center coordinate longitude should be at target2"); + + MGLTestAssertEqualWithAccuracy(strongSelf, + target2.latitude, + center.latitude, + 0.0005, + @"center coordinate latitude should be at target2"); + + runloop = NO; + break; + } + + default: + MGLTestFail(strongSelf); + break; + } + + delegateCallCount++; + + [expectation fulfill]; + }; + + // Should take MGLAnimationDuration + [self.mapView setCenterCoordinate:target zoomLevel:zoomLevel animated:YES]; + + [self waitForExpectations:@[expectation] timeout:10]; + + NSLog(@"setCenterCoordinate: %0.4fs", stop1 - stop0); + NSLog(@"flyToCamera: %0.4fs", stop2 - stop1); + + XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", (long)delegateCallCount); // Once for the setDirection and once for the reset north +} + +#pragma mark - Pending tests + +- (void)testContinuallyResettingNorthInIsChangingPENDING { + // See https://github.com/mapbox/mapbox-gl-native/pull/11614 + // This test currently fails, unsurprisingly, since we're continually + // setting the camera to the same parameters during its update. + // + // Possible solutions/expectations: + // - If you set camera parameters that match the *current* target parameters + // then the transition could be a no-op. We'd need to consider any completion + // block + // - Ideally we would detect this case and disallow it. + + // Reset to non-zero, prior to testing + [self.mapView setDirection:45 animated:NO]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + expectation.expectedFulfillmentCount = 2; + expectation.assertForOverFulfill = YES; + + self.regionIsChanging = ^(MGLMapView *mapView) { + [mapView resetNorth]; + }; + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + [expectation fulfill]; + }; + + [self.mapView setDirection:90 animated:YES]; + [self waitForExpectations:@[expectation] timeout:10]; + + XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction); +} + +- (void)testContinuallySettingCoordinateInIsChangingPENDING { + // See above comment in `-testContinuallyResettingNorthInIsChangingPENDING` + + // Reset to non-zero, prior to testing + [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(0.0, 0.0) animated:NO]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; + expectation.expectedFulfillmentCount = 2; + expectation.assertForOverFulfill = YES; + + __weak typeof(self) weakself = self; + + self.regionIsChanging = ^(MGLMapView *mapView) { + [weakself.mapView setCenterCoordinate:CLLocationCoordinate2DMake(-40.0, -40.0) animated:YES]; + }; + + self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { + [expectation fulfill]; + }; + + [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(40.0, 40.0) animated:YES]; + [self waitForExpectations:@[expectation] timeout:10]; + + XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction); +} + +@end diff --git a/platform/ios/Integration Tests/MGLCameraTransitionTests.mm b/platform/ios/Integration Tests/MGLCameraTransitionTests.mm deleted file mode 100644 index 9679c4c11f..0000000000 --- a/platform/ios/Integration Tests/MGLCameraTransitionTests.mm +++ /dev/null @@ -1,394 +0,0 @@ -#import "MGLMapViewIntegrationTest.h" -#import "MGLTestUtility.h" -#import "../../darwin/src/MGLGeometry_Private.h" - -@interface MBCameraTransitionTests : MGLMapViewIntegrationTest -@end - -@implementation MBCameraTransitionTests - -- (void)testSetAndResetNorthWithDispatchAsyncInDelegateMethod { - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - expectation.expectedFulfillmentCount = 2; - expectation.assertForOverFulfill = YES; - - __weak typeof(self) weakself = self; - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - - MBCameraTransitionTests *strongSelf = weakself; - - if (!strongSelf) return; - - [expectation fulfill]; - - MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); - if (mapView.direction != 0.0) { - dispatch_async(dispatch_get_main_queue(), ^{ - [mapView resetNorth]; - }); - } - }; - - [self.mapView setDirection:90 animated:YES]; - - // loop, render, and wait - [self waitForExpectations:@[expectation] timeout:10]; -} - - -- (void)testSetAndResetNorthInDelegateMethod { - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - expectation.expectedFulfillmentCount = 2; - expectation.assertForOverFulfill = YES; - - __weak typeof(self) weakself = self; - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - - MBCameraTransitionTests *strongSelf = weakself; - - if (!strongSelf) return; - - [expectation fulfill]; - - MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); - if (mapView.direction != 0.0) { - NSLog(@"Reset to north"); - [mapView resetNorth]; - } - }; - - [self.mapView setDirection:90 animated:YES]; - [self waitForExpectations:@[expectation] timeout:10]; -} - -- (void)testInterruptingAndResetNorthOnlyOnceInIsChanging { - - // Reset to non-zero, prior to testing - [self.mapView setDirection:45 animated:NO]; - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - expectation.expectedFulfillmentCount = 1; - expectation.assertForOverFulfill = YES; - - __weak typeof(self) weakself = self; - __block BOOL startedReset = NO; - __block BOOL finishedReset = NO; - - self.regionIsChanging = ^(MGLMapView *mapView) { - MBCameraTransitionTests *strongSelf = weakself; - if (!strongSelf) return; - - if (!startedReset) { - NSLog(@"Reset to north, interrupting the previous transition"); - startedReset = YES; - [mapView resetNorth]; - finishedReset = YES; - } - }; - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - MBCameraTransitionTests *strongSelf = weakself; - if (!strongSelf) return; - - MGLTestAssert(strongSelf, startedReset); - - if (finishedReset) { - MGLTestAssert(strongSelf, !(reason & MGLCameraChangeReasonTransitionCancelled)); - [expectation fulfill]; - } - else { - MGLTestAssert(strongSelf, reason & MGLCameraChangeReasonTransitionCancelled); - } - }; - - [self.mapView setDirection:90 animated:YES]; - [self waitForExpectations:@[expectation] timeout:10]; - - XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction); -} - -- (void)testSetCenterCancelsTransitions { - XCTestExpectation *cameraIsInDCExpectation = [self expectationWithDescription:@"camera reset to DC"]; - - CLLocationCoordinate2D dc = CLLocationCoordinate2DMake(38.894368, -77.036487); - CLLocationCoordinate2D dc_west = CLLocationCoordinate2DMake(38.894368, -77.076487); - - double zoomLevel = 15.0; - - [self.mapView setCenterCoordinate:dc zoomLevel:zoomLevel animated:NO]; - [self.mapView setCenterCoordinate:dc_west zoomLevel:zoomLevel animated:YES]; - - __weak typeof(self) weakself = self; - - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.15 * NSEC_PER_SEC), - dispatch_get_main_queue(), - ^{ - MBCameraTransitionTests *strongSelf = weakself; - - [strongSelf.mapView setCenterCoordinate:dc zoomLevel:zoomLevel animated:NO]; - MGLTestAssertEqualWithAccuracy(strongSelf, - dc.latitude, - strongSelf.mapView.centerCoordinate.latitude, - 0.0005, - @"setting center coordinate should cancel transitions"); - MGLTestAssertEqualWithAccuracy(strongSelf, - dc.longitude, - strongSelf.mapView.centerCoordinate.longitude, - 0.0005, - @"setting center coordinate should cancel transitions"); - [cameraIsInDCExpectation fulfill]; - }); - - [self waitForExpectations:@[cameraIsInDCExpectation] timeout:10.0]; -} - -- (void)testSetCenterCoordinateInDelegateMethod { - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - expectation.expectedFulfillmentCount = 2; - expectation.assertForOverFulfill = YES; - - __weak typeof(self) weakself = self; - __block NSInteger delegateCallCount = 0; - - CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0); - CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(-40.0, -40.0); - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - - MBCameraTransitionTests *strongSelf = weakself; - - if (!strongSelf) return; - - MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); - - CLLocationCoordinate2D center = mapView.centerCoordinate; - - switch(delegateCallCount) { - case 0: - { - // Our center coordinate should match our target (assuming we're not - // constrained by zoom level) - MGLTestAssertEqualWithAccuracy(strongSelf, - target.longitude, - center.longitude, - 0.0005, - @"center coordinate longitude should be at target"); - - MGLTestAssertEqualWithAccuracy(strongSelf, - target.latitude, - center.latitude, - 0.0005, - @"center coordinate latitude should be at target"); - - // Now set another coordinate. - // Should take MGLAnimationDuration seconds (0.3s) - [mapView setCenterCoordinate:target2 animated:YES]; - break; - } - - case 1: - { - // Our center coordinate should match our target (assuming we're not - // constrained by zoom level) - MGLTestAssertEqualWithAccuracy(strongSelf, - target2.longitude, - center.longitude, - 0.0005, - @"center coordinate longitude should be at target2"); - - MGLTestAssertEqualWithAccuracy(strongSelf, - target2.latitude, - center.latitude, - 0.0005, - @"center coordinate latitude should be at target2"); - break; - - } - - default: - MGLTestFail(strongSelf); - break; - } - - delegateCallCount++; - - [expectation fulfill]; - }; - - // Should take MGLAnimationDuration seconds (0.3) - [self.mapView setCenterCoordinate:target zoomLevel:15.0 animated:YES]; - [self waitForExpectations:@[expectation] timeout:10]; -} - -- (void)testFlyToCameraInDelegateMethod { - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - - __weak typeof(self) weakself = self; - __block NSInteger delegateCallCount = 0; - expectation.expectedFulfillmentCount = 3; - expectation.assertForOverFulfill = YES; - - CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0); - CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(30.0, 30.0); - - __block BOOL runloop = YES; - - NSTimeInterval stop0 = CACurrentMediaTime(); - __block NSTimeInterval stop1 = 0.0; - __block NSTimeInterval stop2 = 0.0; - - double zoomLevel = 5.0; - double altitude = MGLAltitudeForZoomLevel(zoomLevel, 0.0, target.latitude, self.mapView.frame.size); - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - - MBCameraTransitionTests *strongSelf = weakself; - - if (!strongSelf) return; - - MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading); - - CLLocationCoordinate2D center = mapView.centerCoordinate; - - switch(delegateCallCount) { - case 0: - { - stop1 = CACurrentMediaTime(); - - // Our center coordinate should match our target (assuming we're not - // constrained by zoom level) - MGLTestAssertEqualWithAccuracy(strongSelf, - target.longitude, - center.longitude, - 0.0005, - @"center coordinate longitude should be at target"); - - MGLTestAssertEqualWithAccuracy(strongSelf, - target.latitude, - center.latitude, - 0.0005, - @"center coordinate latitude should be at target"); - - // Now set another coordinate. - MGLMapCamera *camera = [MGLMapCamera cameraLookingAtCenterCoordinate:target2 - altitude:altitude - pitch:0.0 - heading:0.0]; - - // flyToCamera can take a while... - [mapView flyToCamera:camera completionHandler:^{ - MGLTestAssert(strongSelf, !runloop, @"Completion block should be called after delegate method"); - [expectation fulfill]; - stop2 = CACurrentMediaTime(); - }]; - break; - } - - case 1: - { - // Our center coordinate should match our target (assuming we're not - // constrained by zoom level) - MGLTestAssertEqualWithAccuracy(strongSelf, - target2.longitude, - center.longitude, - 0.0005, - @"center coordinate longitude should be at target2"); - - MGLTestAssertEqualWithAccuracy(strongSelf, - target2.latitude, - center.latitude, - 0.0005, - @"center coordinate latitude should be at target2"); - - runloop = NO; - break; - } - - default: - MGLTestFail(strongSelf); - break; - } - - delegateCallCount++; - - [expectation fulfill]; - }; - - // Should take MGLAnimationDuration - [self.mapView setCenterCoordinate:target zoomLevel:zoomLevel animated:YES]; - - [self waitForExpectations:@[expectation] timeout:10]; - - NSLog(@"setCenterCoordinate: %0.4fs", stop1 - stop0); - NSLog(@"flyToCamera: %0.4fs", stop2 - stop1); - - XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", (long)delegateCallCount); // Once for the setDirection and once for the reset north -} - -#pragma mark - Pending tests - -- (void)testContinuallyResettingNorthInIsChangingPENDING { - // See https://github.com/mapbox/mapbox-gl-native/pull/11614 - // This test currently fails, unsurprisingly, since we're continually - // setting the camera to the same parameters during its update. - // - // Possible solutions/expectations: - // - If you set camera parameters that match the *current* target parameters - // then the transition could be a no-op. We'd need to consider any completion - // block - // - Ideally we would detect this case and disallow it. - - // Reset to non-zero, prior to testing - [self.mapView setDirection:45 animated:NO]; - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - expectation.expectedFulfillmentCount = 2; - expectation.assertForOverFulfill = YES; - - self.regionIsChanging = ^(MGLMapView *mapView) { - [mapView resetNorth]; - }; - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - [expectation fulfill]; - }; - - [self.mapView setDirection:90 animated:YES]; - [self waitForExpectations:@[expectation] timeout:10]; - - XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction); -} - -- (void)testContinuallySettingCoordinateInIsChangingPENDING { - // See above comment in `-testContinuallyResettingNorthInIsChangingPENDING` - - // Reset to non-zero, prior to testing - [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(0.0, 0.0) animated:NO]; - - XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"]; - expectation.expectedFulfillmentCount = 2; - expectation.assertForOverFulfill = YES; - - __weak typeof(self) weakself = self; - - self.regionIsChanging = ^(MGLMapView *mapView) { - [weakself.mapView setCenterCoordinate:CLLocationCoordinate2DMake(-40.0, -40.0) animated:YES]; - }; - - self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) { - [expectation fulfill]; - }; - - [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(40.0, 40.0) animated:YES]; - [self waitForExpectations:@[expectation] timeout:10]; - - XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction); -} - -@end diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h index 07d8698f38..9cebb08ab6 100644 --- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h +++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h @@ -42,4 +42,5 @@ - (NSString*)validAccessToken; - (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout; - (void)waitForMapViewToBeRenderedWithTimeout:(NSTimeInterval)timeout; +- (MGLMapView *)mapViewForTestWithFrame:(CGRect)rect styleURL:(NSURL *)styleURL; @end diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m index 3fa191dfcd..1538c09516 100644 --- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m +++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m @@ -33,13 +33,17 @@ return accessToken; } +- (MGLMapView *)mapViewForTestWithFrame:(CGRect)rect styleURL:(NSURL *)styleURL { + return [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; +} + - (void)setUp { [super setUp]; [MGLAccountManager setAccessToken:@"pk.feedcafedeadbeefbadebede"]; NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"]; - self.mapView = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; + self.mapView = [self mapViewForTestWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL]; self.mapView.delegate = self; UIView *superView = [[UIView alloc] initWithFrame:UIScreen.mainScreen.bounds]; diff --git a/platform/ios/Integration Tests/MGLMapViewPendingBlockTests.m b/platform/ios/Integration Tests/MGLMapViewPendingBlockTests.m index ad71dafd48..c7925d7896 100644 --- a/platform/ios/Integration Tests/MGLMapViewPendingBlockTests.m +++ b/platform/ios/Integration Tests/MGLMapViewPendingBlockTests.m @@ -149,8 +149,7 @@ addToPendingCallback:nil]; } -// Marked as pending due to https://github.com/mapbox/mapbox-gl-native/issues/15471 -- (void)testFlyToCameraPENDING { +- (void)testFlyToCamera { __typeof__(self) weakSelf = self; void (^transition)(dispatch_block_t) = ^(dispatch_block_t completion) { diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj index 018a5f9368..f7d5b35e08 100644 --- a/platform/ios/ios.xcodeproj/project.pbxproj +++ b/platform/ios/ios.xcodeproj/project.pbxproj @@ -506,9 +506,10 @@ CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */; }; CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */; }; CA1B4A512099FB2200EDD491 /* MGLMapSnapshotterTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA1B4A502099FB2200EDD491 /* MGLMapSnapshotterTest.m */; }; - CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */; }; CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */; }; CA4F3BDE230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m */; }; + 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, ); }; }; CA55CD42202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; }; CA65C4F821E9BB080068B0D4 /* MGLCluster.h in Headers */ = {isa = PBXBuildFile; fileRef = CA65C4F721E9BB080068B0D4 /* MGLCluster.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -1197,9 +1198,10 @@ CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLMapViewIntegrationTest.m; sourceTree = ""; wrapsLines = 0; }; CA0C27952076CA50001CE5B7 /* MGLMapViewIntegrationTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLMapViewIntegrationTest.h; sourceTree = ""; }; CA1B4A502099FB2200EDD491 /* MGLMapSnapshotterTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLMapSnapshotterTest.m; sourceTree = ""; }; - CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionTests.mm; sourceTree = ""; }; CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleLayerIntegrationTests.m; sourceTree = ""; }; CA4F3BDD230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MGLMapViewPendingBlockTests.m; sourceTree = ""; }; + CA4F3BE123107793008BAFEA /* MGLCameraTransitionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionTests.mm; sourceTree = ""; }; + CA4F3BE3231077B9008BAFEA /* MGLCameraTransitionFinishTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionFinishTests.mm; sourceTree = ""; }; CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = ""; }; CA5E5042209BDC5F001A8A81 /* MGLTestUtility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLTestUtility.h; path = ../../darwin/test/MGLTestUtility.h; sourceTree = ""; }; CA65C4F721E9BB080068B0D4 /* MGLCluster.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCluster.h; sourceTree = ""; }; @@ -1543,12 +1545,12 @@ 16376B081FFD9DAF0000563E /* Integration Tests */ = { isa = PBXGroup; children = ( + CA4F3BE023107793008BAFEA /* Camera Tests */, CA6914B320E67F07002DB0EE /* Annotations */, CAE7AD5320F46EF5003B6782 /* integration-Bridging-Header.h */, CA1B4A4F2099FA2800EDD491 /* Snapshotter Tests */, 16376B091FFD9DAF0000563E /* MBGLIntegrationTests.mm */, 16376B0B1FFD9DAF0000563E /* Info.plist */, - CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */, CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */, CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */, CA0C27952076CA50001CE5B7 /* MGLMapViewIntegrationTest.h */, @@ -1926,6 +1928,15 @@ path = "Snapshotter Tests"; sourceTree = ""; }; + CA4F3BE023107793008BAFEA /* Camera Tests */ = { + isa = PBXGroup; + children = ( + CA4F3BE123107793008BAFEA /* MGLCameraTransitionTests.mm */, + CA4F3BE3231077B9008BAFEA /* MGLCameraTransitionFinishTests.mm */, + ); + path = "Camera Tests"; + sourceTree = ""; + }; CA6914B320E67F07002DB0EE /* Annotations */ = { isa = PBXGroup; children = ( @@ -3197,7 +3208,6 @@ files = ( CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */, CA7766842229C11A0008DE9E /* SMCalloutView.m in Sources */, - CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */, 16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.mm in Sources */, CA4F3BDE230F74C3008BAFEA /* MGLMapViewPendingBlockTests.m in Sources */, CA88DC3021C85D900059ED5A /* MGLStyleURLIntegrationTest.m in Sources */, @@ -3207,7 +3217,9 @@ CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */, 077061DA215DA00E000FEF62 /* MGLTestLocationManager.m in Sources */, CA6914B520E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.mm in Sources */, + CA4F3BE223107793008BAFEA /* MGLCameraTransitionTests.mm in Sources */, CA1B4A512099FB2200EDD491 /* MGLMapSnapshotterTest.m in Sources */, + CA4F3BE4231077B9008BAFEA /* MGLCameraTransitionFinishTests.mm in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp index 925893b2d6..7ec41be37a 100644 --- a/src/mbgl/map/transform.cpp +++ b/src/mbgl/map/transform.cpp @@ -96,6 +96,9 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim double pitch = camera.pitch ? *camera.pitch * util::DEG2RAD : getPitch(); if (std::isnan(zoom) || std::isnan(bearing) || std::isnan(pitch)) { + if (animation.transitionFinishFn) { + animation.transitionFinishFn(); + } return; } @@ -172,6 +175,9 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima double pitch = camera.pitch ? *camera.pitch * util::DEG2RAD : getPitch(); if (std::isnan(zoom) || std::isnan(bearing) || std::isnan(pitch) || state.size.isEmpty()) { + if (animation.transitionFinishFn) { + animation.transitionFinishFn(); + } return; } @@ -274,6 +280,9 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima if (duration == Duration::zero()) { // Perform an instantaneous transition. jumpTo(camera); + if (animation.transitionFinishFn) { + animation.transitionFinishFn(); + } return; } -- cgit v1.2.1