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

Short-circuit redundant camera changes #7125

Merged
merged 1 commit into from
Feb 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the best I could come up with for a test of the redundancy check. It essentially only verifies that cancelTransitions() isn’t called in the case of a redundant camera change. The test in #6060 is infeasible because the completion handler is eventually called even for redundant camera changes.

I verified via breakpoints that we do short-circuit mbgl for redundant camera changes and that significant camera changes still work.

}];
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