From d790132eda070c652b1d3a6ac44dc5d7e66335f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Sun, 20 Nov 2016 01:28:01 -0800 Subject: [PATCH] [ios, macos] Short-circuit redundant camera changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid canceling transitions (and triggering preexisting completion handlers) until we get a chance to ensure that a new transition really does have to begin. Consistently avoid mbgl transitions for redundant camera changes. Upon bailing, schedule the completion handler to run asynchronously on a delay equivalent to the requested animation duration. Added a “functional” equality method to MGLMapCamera that avoids trivial differences. Fixed invocations of XCTAssertEqualWithAccuracy() that incorrectly expressed the accuracy as a number of digits rather than a scalar tolerance. --- platform/darwin/src/MGLMapCamera.h | 14 +++++ platform/darwin/src/MGLMapCamera.mm | 19 ++++++ platform/darwin/test/MGLGeometryTests.mm | 44 +++++++------- platform/ios/CHANGELOG.md | 1 + platform/ios/src/MGLMapView.mm | 74 ++++++++++++++++++------ platform/ios/uitest/MapViewTests.m | 17 ++++++ platform/macos/CHANGELOG.md | 1 + platform/macos/src/MGLMapView.mm | 39 +++++++++---- 8 files changed, 156 insertions(+), 53 deletions(-) diff --git a/platform/darwin/src/MGLMapCamera.h b/platform/darwin/src/MGLMapCamera.h index abb43d9aea2..7d263724001 100644 --- a/platform/darwin/src/MGLMapCamera.h +++ b/platform/darwin/src/MGLMapCamera.h @@ -64,6 +64,20 @@ NS_ASSUME_NONNULL_BEGIN pitch:(CGFloat)pitch heading:(CLLocationDirection)heading; +/** + Returns a Boolean value indicating whether the given camera is functionally + equivalent to the receiver. + + Unlike `-isEqual:`, this method returns `YES` if the difference between the + coordinates, altitudes, pitches, or headings of the two camera objects is + negligible. + + @param otherCamera The camera with which to compare the receiver. + @return A Boolean value indicating whether the two cameras are functionally + equivalent. + */ +- (BOOL)isEqualToMapCamera:(MGLMapCamera *)otherCamera; + @end NS_ASSUME_NONNULL_END diff --git a/platform/darwin/src/MGLMapCamera.mm b/platform/darwin/src/MGLMapCamera.mm index fafbefd17a7..897abab7805 100644 --- a/platform/darwin/src/MGLMapCamera.mm +++ b/platform/darwin/src/MGLMapCamera.mm @@ -2,6 +2,11 @@ #include +BOOL MGLEqualFloatWithAccuracy(CGFloat left, CGFloat right, CGFloat accuracy) +{ + return MAX(left, right) - MIN(left, right) <= accuracy; +} + @implementation MGLMapCamera + (BOOL)supportsSecureCoding @@ -116,6 +121,20 @@ - (BOOL)isEqual:(id)other && _pitch == otherCamera.pitch && _heading == otherCamera.heading); } +- (BOOL)isEqualToMapCamera:(MGLMapCamera *)otherCamera +{ + if (otherCamera == self) + { + return YES; + } + + return (MGLEqualFloatWithAccuracy(_centerCoordinate.latitude, otherCamera.centerCoordinate.latitude, 1e-6) + && MGLEqualFloatWithAccuracy(_centerCoordinate.longitude, otherCamera.centerCoordinate.longitude, 1e-6) + && MGLEqualFloatWithAccuracy(_altitude, otherCamera.altitude, 1e-6) + && MGLEqualFloatWithAccuracy(_pitch, otherCamera.pitch, 1) + && MGLEqualFloatWithAccuracy(_heading, otherCamera.heading, 1)); +} + - (NSUInteger)hash { return (@(_centerCoordinate.latitude).hash + @(_centerCoordinate.longitude).hash diff --git a/platform/darwin/test/MGLGeometryTests.mm b/platform/darwin/test/MGLGeometryTests.mm index 0ffc27b29e1..0dd5efec4dd 100644 --- a/platform/darwin/test/MGLGeometryTests.mm +++ b/platform/darwin/test/MGLGeometryTests.mm @@ -15,23 +15,23 @@ - (void)testCoordinateBoundsIsEmpty { } - (void)testAngleConversions { - XCTAssertEqualWithAccuracy(-180, MGLDegreesFromRadians(-M_PI), 5); + XCTAssertEqualWithAccuracy(-180, MGLDegreesFromRadians(-M_PI), 1e-5); XCTAssertEqual(0, MGLDegreesFromRadians(0)); - XCTAssertEqualWithAccuracy(45, MGLDegreesFromRadians(M_PI_4), 5); - XCTAssertEqualWithAccuracy(90, MGLDegreesFromRadians(M_PI_2), 5); - XCTAssertEqualWithAccuracy(180, MGLDegreesFromRadians(M_PI), 5); - XCTAssertEqualWithAccuracy(360, MGLDegreesFromRadians(2 * M_PI), 5); - XCTAssertEqualWithAccuracy(720, MGLDegreesFromRadians(4 * M_PI), 5); + XCTAssertEqualWithAccuracy(45, MGLDegreesFromRadians(M_PI_4), 1e-5); + XCTAssertEqualWithAccuracy(90, MGLDegreesFromRadians(M_PI_2), 1e-5); + XCTAssertEqualWithAccuracy(180, MGLDegreesFromRadians(M_PI), 1e-5); + XCTAssertEqualWithAccuracy(360, MGLDegreesFromRadians(2 * M_PI), 1e-5); + XCTAssertEqualWithAccuracy(720, MGLDegreesFromRadians(4 * M_PI), 1e-5); - XCTAssertEqualWithAccuracy(-360, MGLDegreesFromRadians(MGLRadiansFromDegrees(-360)), 4); - XCTAssertEqualWithAccuracy(-180, MGLDegreesFromRadians(MGLRadiansFromDegrees(-180)), 5); - XCTAssertEqualWithAccuracy(-90, MGLDegreesFromRadians(MGLRadiansFromDegrees(-90)), 5); - XCTAssertEqualWithAccuracy(-45, MGLDegreesFromRadians(MGLRadiansFromDegrees(-45)), 5); - XCTAssertEqualWithAccuracy(0, MGLDegreesFromRadians(MGLRadiansFromDegrees(0)), 5); - XCTAssertEqualWithAccuracy(45, MGLDegreesFromRadians(MGLRadiansFromDegrees(45)), 5); - XCTAssertEqualWithAccuracy(90, MGLDegreesFromRadians(MGLRadiansFromDegrees(90)), 5); - XCTAssertEqualWithAccuracy(180, MGLDegreesFromRadians(MGLRadiansFromDegrees(180)), 5); - XCTAssertEqualWithAccuracy(360, MGLDegreesFromRadians(MGLRadiansFromDegrees(360)), 4); + XCTAssertEqualWithAccuracy(-360, MGLDegreesFromRadians(MGLRadiansFromDegrees(-360)), 1e-4); + XCTAssertEqualWithAccuracy(-180, MGLDegreesFromRadians(MGLRadiansFromDegrees(-180)), 1e-5); + XCTAssertEqualWithAccuracy(-90, MGLDegreesFromRadians(MGLRadiansFromDegrees(-90)), 1e-5); + XCTAssertEqualWithAccuracy(-45, MGLDegreesFromRadians(MGLRadiansFromDegrees(-45)), 1e-5); + XCTAssertEqualWithAccuracy(0, MGLDegreesFromRadians(MGLRadiansFromDegrees(0)), 1e-5); + XCTAssertEqualWithAccuracy(45, MGLDegreesFromRadians(MGLRadiansFromDegrees(45)), 1e-5); + XCTAssertEqualWithAccuracy(90, MGLDegreesFromRadians(MGLRadiansFromDegrees(90)), 1e-5); + XCTAssertEqualWithAccuracy(180, MGLDegreesFromRadians(MGLRadiansFromDegrees(180)), 1e-5); + XCTAssertEqualWithAccuracy(360, MGLDegreesFromRadians(MGLRadiansFromDegrees(360)), 1e-4); } - (void)testAltitudeConversions { @@ -39,18 +39,18 @@ - (void)testAltitudeConversions { CGSize midSize = CGSizeMake(600, 800); CGSize shortSize = CGSizeMake(600, 400); - XCTAssertEqualWithAccuracy(1800, MGLAltitudeForZoomLevel(MGLZoomLevelForAltitude(1800, 0, 0, midSize), 0, 0, midSize), 1); + XCTAssertEqualWithAccuracy(1800, MGLAltitudeForZoomLevel(MGLZoomLevelForAltitude(1800, 0, 0, midSize), 0, 0, midSize), 1e-8); XCTAssertLessThan(MGLZoomLevelForAltitude(1800, 0, 0, midSize), MGLZoomLevelForAltitude(1800, 0, 0, tallSize)); XCTAssertGreaterThan(MGLZoomLevelForAltitude(1800, 0, 0, midSize), MGLZoomLevelForAltitude(1800, 0, 0, shortSize)); - XCTAssertEqualWithAccuracy(0, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(0, 0, 0, midSize), 0, 0, midSize), 3); - XCTAssertEqualWithAccuracy(18, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(18, 0, 0, midSize), 0, 0, midSize), 3); + XCTAssertEqualWithAccuracy(0, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(0, 0, 0, midSize), 0, 0, midSize), 1e-8); + XCTAssertEqualWithAccuracy(18, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(18, 0, 0, midSize), 0, 0, midSize), 1e-8); - XCTAssertEqualWithAccuracy(0, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(0, 0, 40, midSize), 0, 40, midSize), 3); - XCTAssertEqualWithAccuracy(18, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(18, 0, 40, midSize), 0, 40, midSize), 3); + XCTAssertEqualWithAccuracy(0, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(0, 0, 40, midSize), 0, 40, midSize), 1e-8); + XCTAssertEqualWithAccuracy(18, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(18, 0, 40, midSize), 0, 40, midSize), 1e-8); - XCTAssertEqualWithAccuracy(0, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(0, 60, 40, midSize), 60, 40, midSize), 3); - XCTAssertEqualWithAccuracy(18, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(18, 60, 40, midSize), 60, 40, midSize), 3); + XCTAssertEqualWithAccuracy(0, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(0, 60, 40, midSize), 60, 40, midSize), 1e-8); + XCTAssertEqualWithAccuracy(18, MGLZoomLevelForAltitude(MGLAltitudeForZoomLevel(18, 60, 40, midSize), 60, 40, midSize), 1e-8); } - (void)testGeometryBoxing { diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 7ac04fb916a..55902ba93b3 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -92,6 +92,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Raster tiles such as those from Mapbox Satellite are now cached, eliminating flashing while panning back and forth. ([#7091](https://github.com/mapbox/mapbox-gl-native/pull/7091)) * Improved the performance of symbol style layers. ([#7025](https://github.com/mapbox/mapbox-gl-native/pull/7025)) +* Improved the performance of trivial camera animations. ([#7125](https://github.com/mapbox/mapbox-gl-native/pull/7125)) * As the user zooms in, tiles from lower zoom levels are scaled up until tiles for higher zoom levels are loaded. ([#5143](https://github.com/mapbox/mapbox-gl-native/pull/5143)) * Notification names and user info keys are now string enumeration values for ease of use in Swift. ([#6794](https://github.com/mapbox/mapbox-gl-native/pull/6794)) * MGLMapDebugOverdrawVisualizationMask no longer has any effect in Release builds of the SDK. This debug mask has been disabled for performance reasons. ([#5555](https://github.com/mapbox/mapbox-gl-native/pull/5555)) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index fb056490ffc..24d9fa0d8de 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -2269,8 +2269,6 @@ - (void)_setCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate animated:( - (void)_setCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate edgePadding:(UIEdgeInsets)insets zoomLevel:(double)zoomLevel direction:(CLLocationDirection)direction duration:(NSTimeInterval)duration animationTimingFunction:(nullable CAMediaTimingFunction *)function completionHandler:(nullable void (^)(void))completion { - _mbglMap->cancelTransitions(); - mbgl::CameraOptions cameraOptions; cameraOptions.center = MGLLatLngFromLocationCoordinate2D(centerCoordinate); cameraOptions.padding = MGLEdgeInsetsFromNSEdgeInsets(insets); @@ -2297,6 +2295,20 @@ - (void)_setCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate edgePaddin }); }; } + + MGLMapCamera *camera = [self cameraForCameraOptions:cameraOptions]; + if ([self.camera isEqualToMapCamera:camera]) + { + if (completion) + { + [self animateWithDelay:duration animations:^{ + completion(); + }]; + } + return; + } + + _mbglMap->cancelTransitions(); _mbglMap->easeTo(cameraOptions, animationOptions); } @@ -2415,9 +2427,6 @@ - (void)setVisibleCoordinates:(const CLLocationCoordinate2D *)coordinates count: - (void)_setVisibleCoordinates:(const CLLocationCoordinate2D *)coordinates count:(NSUInteger)count edgePadding:(UIEdgeInsets)insets direction:(CLLocationDirection)direction duration:(NSTimeInterval)duration animationTimingFunction:(nullable CAMediaTimingFunction *)function completionHandler:(nullable void (^)(void))completion { - _mbglMap->cancelTransitions(); - - [self willChangeValueForKey:@"visibleCoordinateBounds"]; mbgl::EdgeInsets padding = MGLEdgeInsetsFromNSEdgeInsets(insets); padding += MGLEdgeInsetsFromNSEdgeInsets(self.contentInset); std::vector latLngs; @@ -2447,6 +2456,21 @@ - (void)_setVisibleCoordinates:(const CLLocationCoordinate2D *)coordinates count }); }; } + + MGLMapCamera *camera = [self cameraForCameraOptions:cameraOptions]; + if ([self.camera isEqualToMapCamera:camera]) + { + if (completion) + { + [self animateWithDelay:duration animations:^{ + completion(); + }]; + } + return; + } + + [self willChangeValueForKey:@"visibleCoordinateBounds"]; + _mbglMap->cancelTransitions(); _mbglMap->easeTo(cameraOptions, animationOptions); [self didChangeValueForKey:@"visibleCoordinateBounds"]; } @@ -2532,13 +2556,6 @@ - (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration a - (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration animationTimingFunction:(nullable CAMediaTimingFunction *)function completionHandler:(nullable void (^)(void))completion { - _mbglMap->cancelTransitions(); - if ([self.camera isEqual:camera]) - { - return; - } - - mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera edgePadding:self.contentInset]; mbgl::AnimationOptions animationOptions; if (duration > 0) { @@ -2553,8 +2570,21 @@ - (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration a }); }; } + + if ([self.camera isEqualToMapCamera:camera]) + { + if (completion) + { + [self animateWithDelay:duration animations:^{ + completion(); + }]; + } + return; + } [self willChangeValueForKey:@"camera"]; + _mbglMap->cancelTransitions(); + mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera edgePadding:self.contentInset]; _mbglMap->easeTo(cameraOptions, animationOptions); [self didChangeValueForKey:@"camera"]; } @@ -2576,13 +2606,6 @@ - (void)flyToCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration - (void)_flyToCamera:(MGLMapCamera *)camera edgePadding:(UIEdgeInsets)insets withDuration:(NSTimeInterval)duration peakAltitude:(CLLocationDistance)peakAltitude completionHandler:(nullable void (^)(void))completion { - _mbglMap->cancelTransitions(); - if ([self.camera isEqual:camera]) - { - return; - } - - mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera edgePadding:insets]; mbgl::AnimationOptions animationOptions; if (duration >= 0) { @@ -2603,8 +2626,21 @@ - (void)_flyToCamera:(MGLMapCamera *)camera edgePadding:(UIEdgeInsets)insets wit }); }; } + + if ([self.camera isEqualToMapCamera:camera]) + { + if (completion) + { + [self animateWithDelay:duration animations:^{ + completion(); + }]; + } + return; + } [self willChangeValueForKey:@"camera"]; + _mbglMap->cancelTransitions(); + mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera edgePadding:insets]; _mbglMap->flyTo(cameraOptions, animationOptions); [self didChangeValueForKey:@"camera"]; } diff --git a/platform/ios/uitest/MapViewTests.m b/platform/ios/uitest/MapViewTests.m index 21310b47a60..779107bc01b 100644 --- a/platform/ios/uitest/MapViewTests.m +++ b/platform/ios/uitest/MapViewTests.m @@ -320,6 +320,23 @@ - (void)testZoomSet { @"setting zoom should take effect"); } +- (void)testCameraDebouncing { + MGLMapCamera *camera = [MGLMapCamera cameraLookingAtCenterCoordinate:CLLocationCoordinate2DMake(45, -122) + fromDistance:100 + pitch:30 + heading:45]; + tester.mapView.camera = camera; + XCTAssertEqualObjects(tester.mapView.camera, camera); + + [tester.mapView setCamera:camera withDuration:10 animationTimingFunction:nil completionHandler:^{ + XCTAssert(NO, @"Camera animation should not be canceled by redundantly setting the camera to the current camera."); + }]; + XCTAssertEqualObjects(tester.mapView.camera, camera); + + tester.mapView.camera = camera; + XCTAssertEqualObjects(tester.mapView.camera, camera); +} + - (void)testMarkerSelection { CGPoint point = CGPointMake(100, 100); MGLPointAnnotation *marker = [MGLPointAnnotation new]; diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md index 523c9368d87..38e77460c05 100644 --- a/platform/macos/CHANGELOG.md +++ b/platform/macos/CHANGELOG.md @@ -70,6 +70,7 @@ This version of the Mapbox macOS SDK corresponds to version 3.4.0 of the Mapbox * Raster tiles such as those from Mapbox Satellite are now cached, eliminating flashing while panning back and forth. ([#7091](https://github.com/mapbox/mapbox-gl-native/pull/7091)) * Fixed an issue where the map view’s center would always be calculated as if the view occupied the entire window. ([#6102](https://github.com/mapbox/mapbox-gl-native/pull/6102)) +* Improved the performance of trivial camera animations. ([#7125](https://github.com/mapbox/mapbox-gl-native/pull/7125)) * Notification names and user info keys are now string enumeration values for ease of use in Swift. ([#6794](https://github.com/mapbox/mapbox-gl-native/pull/6794)) * Fixed a typo in the documentation for the MGLCompassDirectionFormatter class. ([#5879](https://github.com/mapbox/mapbox-gl-native/pull/5879)) * The NSClickGestureRecognizer on MGLMapView that is used for selecting annotations now fails if a click does not select an annotation. ([#7246](https://github.com/mapbox/mapbox-gl-native/pull/7246)) diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index 9cbc9232711..e4a0c364da2 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -1116,12 +1116,6 @@ - (void)setCamera:(MGLMapCamera *)camera animated:(BOOL)animated { } - (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration animationTimingFunction:(nullable CAMediaTimingFunction *)function completionHandler:(nullable void (^)(void))completion { - _mbglMap->cancelTransitions(); - if ([self.camera isEqual:camera]) { - return; - } - - mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera]; mbgl::AnimationOptions animationOptions; if (duration > 0) { animationOptions.duration.emplace(MGLDurationInSeconds(duration)); @@ -1137,8 +1131,19 @@ - (void)setCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration a }); }; } + + if ([self.camera isEqualToMapCamera:camera]) { + if (completion) { + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(duration * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + completion(); + }); + } + return; + } [self willChangeValueForKey:@"camera"]; + _mbglMap->cancelTransitions(); + mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera]; _mbglMap->easeTo(cameraOptions, animationOptions); [self didChangeValueForKey:@"camera"]; } @@ -1152,12 +1157,6 @@ - (void)flyToCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration } - (void)flyToCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration peakAltitude:(CLLocationDistance)peakAltitude completionHandler:(nullable void (^)(void))completion { - _mbglMap->cancelTransitions(); - if ([self.camera isEqual:camera]) { - return; - } - - mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera]; mbgl::AnimationOptions animationOptions; if (duration >= 0) { animationOptions.duration = MGLDurationInSeconds(duration); @@ -1178,8 +1177,19 @@ - (void)flyToCamera:(MGLMapCamera *)camera withDuration:(NSTimeInterval)duration }); }; } + + if ([self.camera isEqualToMapCamera:camera]) { + if (completion) { + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(duration * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + completion(); + }); + } + return; + } [self willChangeValueForKey:@"camera"]; + _mbglMap->cancelTransitions(); + mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera]; _mbglMap->flyTo(cameraOptions, animationOptions); [self didChangeValueForKey:@"camera"]; } @@ -1228,6 +1238,11 @@ - (void)setVisibleCoordinateBounds:(MGLCoordinateBounds)bounds edgePadding:(NSEd if (animated) { animationOptions.duration = MGLDurationInSeconds(MGLAnimationDuration); } + + MGLMapCamera *camera = [self cameraForCameraOptions:cameraOptions]; + if ([self.camera isEqualToMapCamera:camera]) { + return; + } [self willChangeValueForKey:@"visibleCoordinateBounds"]; animationOptions.transitionFinishFn = ^() {