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

Commit

Permalink
[ios, macos] Short-circuit redundant camera changes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
1ec5 committed Feb 18, 2017
1 parent 28ad35f commit d790132
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 53 deletions.
14 changes: 14 additions & 0 deletions platform/darwin/src/MGLMapCamera.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
19 changes: 19 additions & 0 deletions platform/darwin/src/MGLMapCamera.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

#include <mbgl/util/projection.hpp>

BOOL MGLEqualFloatWithAccuracy(CGFloat left, CGFloat right, CGFloat accuracy)
{
return MAX(left, right) - MIN(left, right) <= accuracy;
}

@implementation MGLMapCamera

+ (BOOL)supportsSecureCoding
Expand Down Expand Up @@ -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
Expand Down
44 changes: 22 additions & 22 deletions platform/darwin/test/MGLGeometryTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,42 @@ - (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 {
CGSize tallSize = CGSizeMake(600, 1200);
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 {
Expand Down
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
74 changes: 55 additions & 19 deletions platform/ios/src/MGLMapView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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<mbgl::LatLng> latLngs;
Expand Down Expand Up @@ -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"];
}
Expand Down Expand Up @@ -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)
{
Expand All @@ -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"];
}
Expand All @@ -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)
{
Expand All @@ -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"];
}
Expand Down
17 changes: 17 additions & 0 deletions platform/ios/uitest/MapViewTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
1 change: 1 addition & 0 deletions platform/macos/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
39 changes: 27 additions & 12 deletions platform/macos/src/MGLMapView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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"];
}
Expand All @@ -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);
Expand All @@ -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"];
}
Expand Down Expand Up @@ -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 = ^() {
Expand Down

0 comments on commit d790132

Please sign in to comment.