tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[ios,macos] Fix for infinite loop crash when setting shape to nil in MGLMapViewDelegate methods (#11614)

Browse Source
This commit is contained in:
Julian Rex
2018-05-21 14:14:58 -04:00
committed by GitHub
parent a84ac4c8d7
commit bef2a7ccb6
11 changed files with 630 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
platform/ios/CHANGELOG.md
View File

@@ -15,6 +15,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
### Other changes
* Unknown tokens in URLs are now preserved, rather than replaced with an empty string. ([#11787](https://github.com/mapbox/mapbox-gl-native/issues/11787))
* Adjusted when and how the camera transition update and finish callbacks are called, fixing recursion bugs. ([#11614](https://github.com/mapbox/mapbox-gl-native/pull/11614))
## 4.0.1 - May 14, 2018

398
platform/ios/Integration Tests/MGLCameraTransitionTests.mm Normal file
View File

@@ -0,0 +1,398 @@
#import "MGLMapViewIntegrationTest.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:1.5];
}
- (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:1.5];
}
- (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:1.5];
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:1.5];
}
- (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
fromDistance: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];
// Run the loop, so the camera can fly to the new camera
while (runloop) {
[[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantFuture]];
}
[self waitForExpectations:@[expectation] timeout:0.5];
NSLog(@"setCenterCoordinate: %0.4fs", stop1 - stop0);
NSLog(@"flyToCamera: %0.4fs", stop2 - stop1);
XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", delegateCallCount); // Once for the setDirection and once for the reset north
}
#pragma mark - Pending tests
- (void)disabled_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:1.5];
XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction);
}
- (void)disabled_testContinuallySettingCoordinateInIsChangingPENDING {
// See above comment in `-disabled_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:1.5];
XCTAssertEqualWithAccuracy(self.mapView.direction, 0.0, 0.001, @"Camera should have reset to north. %0.3f", self.mapView.direction);
}
@end

13
platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
View File

@@ -1,18 +1,23 @@
#import <XCTest/XCTest.h>
#import <Mapbox/Mapbox.h>
#define TestFailWithSelf(myself, ...) \
#define MGLTestFail(myself, ...) \
_XCTPrimitiveFail(myself, __VA_ARGS__)
#define MGLTestAssert(myself, expression, ...) \
_XCTPrimitiveAssertTrue(myself, expression, @#expression, __VA_ARGS__)
#define MGLTestAssertEqualWithAccuracy(myself, expression1, expression2, accuracy, ...) \
_XCTPrimitiveAssertEqualWithAccuracy(myself, expression1, @#expression1, expression2, @#expression2, accuracy, @#accuracy, __VA_ARGS__)
@interface MGLMapViewIntegrationTest : XCTestCase <MGLMapViewDelegate>
@property (nonatomic) MGLMapView *mapView;
@property (nonatomic) MGLStyle *style;
@property (nonatomic) XCTestExpectation *styleLoadingExpectation;
@property (nonatomic) XCTestExpectation *renderFinishedExpectation;
@property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, BOOL animated);
@property (nonatomic) void (^regionWillChange)(MGLMapView *mapView, BOOL animated);
@property (nonatomic) void (^regionIsChanging)(MGLMapView *mapView);
@property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated);
// Utility methods
- (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout;

12
platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
View File

@@ -45,9 +45,9 @@
self.renderFinishedExpectation = nil;
}
- (void)mapView:(MGLMapView *)mapView regionDidChangeAnimated:(BOOL)animated {
if (self.regionDidChange) {
self.regionDidChange(mapView, animated);
- (void)mapView:(MGLMapView *)mapView regionWillChangeAnimated:(BOOL)animated {
if (self.regionWillChange) {
self.regionWillChange(mapView, animated);
}
}
@@ -57,6 +57,12 @@
}
}
- (void)mapView:(MGLMapView *)mapView regionDidChangeWithReason:(MGLCameraChangeReason)reason animated:(BOOL)animated {
if (self.regionDidChange) {
self.regionDidChange(mapView, reason, animated);
}
}
#pragma mark - Utilities
- (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout {

126
platform/ios/Integration Tests/MGLShapeSourceTests.m Normal file
View File

@@ -0,0 +1,126 @@
//
// MBShapeSourceTests.m
// integration
//
// Created by Julian Rex on 4/5/18.
// Copyright © 2018 Mapbox. All rights reserved.
//
#import "MGLMapViewIntegrationTest.h"
@interface MGLShapeSourceTests : MGLMapViewIntegrationTest
@end
@implementation MGLShapeSourceTests
- (void)testSettingShapeSourceToNilInRegionDidChange {
NSMutableArray *features = [[NSMutableArray alloc] init];
for (NSUInteger i = 0; i <= 180; i+=5) {
CLLocationCoordinate2D coord[4] = {
CLLocationCoordinate2DMake(round(0), round(i)),
CLLocationCoordinate2DMake(round(20), round(i)),
CLLocationCoordinate2DMake(round(0), round(i / 2 )),
CLLocationCoordinate2DMake(round(20), round(i / 2))};
MGLPolygonFeature *feature = [MGLPolygonFeature polygonWithCoordinates:coord count:4];
[features addObject:feature];
}
MGLShapeSource *shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"source" features:features options:nil];
[self.style addSource:shapeSource];
MGLFillStyleLayer *layer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"layer" source:shapeSource];
layer.fillOpacity = [NSExpression expressionForConstantValue:@0.5];
[self.style addLayer:layer];
XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
expectation.expectedFulfillmentCount = 1;
expectation.assertForOverFulfill = YES;
__weak typeof(self) weakself = self;
__block NSInteger delegateCallCount = 0;
self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
MGLShapeSourceTests *strongSelf = weakself;
if (!strongSelf)
return;
delegateCallCount++;
// Setting the shapeSource.shape = nil, was causing an infinite loop, so here
// we check for a runaway call. 10 here is arbitrary. We could argue that this
// should check that the call count is only 1, however in this case we particularly
// want to check for the infinite loop.
// See https://github.com/mapbox/mapbox-gl-native/issues/11180
if (delegateCallCount > 10) {
MGLTestFail(strongSelf);
}
else {
shapeSource.shape = nil;
}
[expectation fulfill];
};
// setCenterCoordinate is NOT animated here.
[self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(10.0, 10.0)];
[self waitForExpectations:@[expectation] timeout:5.0];
}
- (void)testSettingShapeSourceToNilInRegionIsChanging {
NSMutableArray *features = [[NSMutableArray alloc] init];
for (NSUInteger i = 0; i <= 180; i+=5) {
CLLocationCoordinate2D coord[4] = {
CLLocationCoordinate2DMake(round(0), round(i)),
CLLocationCoordinate2DMake(round(20), round(i)),
CLLocationCoordinate2DMake(round(0), round(i / 2 )),
CLLocationCoordinate2DMake(round(20), round(i / 2))};
MGLPolygonFeature *feature = [MGLPolygonFeature polygonWithCoordinates:coord count:4];
[features addObject:feature];
}
MGLShapeSource *shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"source" features:features options:nil];
[self.style addSource:shapeSource];
MGLFillStyleLayer *layer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"layer" source:shapeSource];
layer.fillOpacity = [NSExpression expressionForConstantValue:@0.5];
[self.style addLayer:layer];
XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
expectation.expectedFulfillmentCount = 1;
expectation.assertForOverFulfill = YES;
__block NSInteger delegateCallCount = 0;
__weak typeof(self) weakself = self;
self.regionIsChanging = ^(MGLMapView *mapView) {
// See https://github.com/mapbox/mapbox-gl-native/issues/11180
shapeSource.shape = nil;
};
self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
delegateCallCount++;
if (delegateCallCount > 1) {
MGLTestFail(weakself);
}
[expectation fulfill];
};
// Should take MGLAnimationDuration seconds (0.3)
[self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(10.0, 10.0) animated:YES];
[self waitForExpectations:@[expectation] timeout:1.0];
}
@end

12
platform/ios/ios.xcodeproj/project.pbxproj
View File

@@ -363,8 +363,10 @@
AC518E00201BB55A00EBC820 /* MGLTelemetryConfig.h in Headers */ = {isa = PBXBuildFile; fileRef = AC518DFD201BB55A00EBC820 /* MGLTelemetryConfig.h */; };
AC518E03201BB56000EBC820 /* MGLTelemetryConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */; };
AC518E04201BB56100EBC820 /* MGLTelemetryConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */; };
CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */; };
CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */; };
CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */; };
CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.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, ); }; };
CAA69DA4206DCD0E007279CD /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; };
@@ -996,9 +998,11 @@
96F3F73B1F5711F1003E2D2C /* MGLUserLocationHeadingIndicator.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLUserLocationHeadingIndicator.h; sourceTree = "<group>"; };
AC518DFD201BB55A00EBC820 /* MGLTelemetryConfig.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLTelemetryConfig.h; sourceTree = "<group>"; };
AC518DFE201BB55A00EBC820 /* MGLTelemetryConfig.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLTelemetryConfig.m; sourceTree = "<group>"; };
CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLShapeSourceTests.m; sourceTree = "<group>"; };
CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLMapViewIntegrationTest.m; sourceTree = "<group>"; };
CA0C27952076CA50001CE5B7 /* MGLMapViewIntegrationTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MGLMapViewIntegrationTest.h; sourceTree = "<group>"; };
CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleLayerIntegrationTests.m; sourceTree = "<group>"; };
CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLCameraTransitionTests.mm; sourceTree = "<group>"; };
CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCameraChangeReason.h; sourceTree = "<group>"; };
DA00FC8C1D5EEB0D009AABC8 /* MGLAttributionInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAttributionInfo.h; sourceTree = "<group>"; };
DA00FC8D1D5EEB0D009AABC8 /* MGLAttributionInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAttributionInfo.mm; sourceTree = "<group>"; };
@@ -1353,6 +1357,8 @@
children = (
16376B091FFD9DAF0000563E /* MBGLIntegrationTests.m */,
16376B0B1FFD9DAF0000563E /* Info.plist */,
CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */,
CA0C27912076C804001CE5B7 /* MGLShapeSourceTests.m */,
CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */,
CA0C27952076CA50001CE5B7 /* MGLMapViewIntegrationTest.h */,
CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */,
@@ -2809,8 +2815,10 @@
buildActionMask = 2147483647;
files = (
CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */,
CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */,
16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */,
CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */,
CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
@@ -3379,6 +3387,7 @@
/* Begin XCBuildConfiguration section */
16376B0E1FFD9DAF0000563E /* Debug */ = {
isa = XCBuildConfiguration;
baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */;
buildSettings = {
BUNDLE_LOADER = "$(TEST_HOST)";
CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
@@ -3393,6 +3402,7 @@
CODE_SIGN_IDENTITY = "iPhone Developer";
CODE_SIGN_STYLE = Automatic;
GCC_C_LANGUAGE_STANDARD = gnu11;
HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)";
INFOPLIST_FILE = "Integration Tests/Info.plist";
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests";
@@ -3404,6 +3414,7 @@
};
16376B0F1FFD9DAF0000563E /* Release */ = {
isa = XCBuildConfiguration;
baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */;
buildSettings = {
BUNDLE_LOADER = "$(TEST_HOST)";
CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
@@ -3418,6 +3429,7 @@
CODE_SIGN_IDENTITY = "iPhone Developer";
CODE_SIGN_STYLE = Automatic;
GCC_C_LANGUAGE_STANDARD = gnu11;
HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)";
INFOPLIST_FILE = "Integration Tests/Info.plist";
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks";
PRODUCT_BUNDLE_IDENTIFIER = "com.mapbox.integration-tests";

6
platform/ios/src/MGLCameraChangeReason.h
View File

@@ -57,5 +57,9 @@ typedef NS_OPTIONS(NSUInteger, MGLCameraChangeReason)
MGLCameraChangeReasonGestureOneFingerZoom = 1 << 7,
// :nodoc: The user panned with two fingers to tilt the map (two finger drag).
MGLCameraChangeReasonGestureTilt = 1 << 8
MGLCameraChangeReasonGestureTilt = 1 << 8,
// :nodoc: Cancelled
MGLCameraChangeReasonTransitionCancelled = 1 << 16
};

34
platform/ios/src/MGLMapView.mm
View File

@@ -1271,7 +1271,7 @@ public:
{
[self setUserTrackingMode:MGLUserTrackingModeNone animated:NO];
}
_mbglMap->cancelTransitions();
[self cancelTransitions];
}
- (void)notifyGestureDidBegin {
@@ -1321,7 +1321,7 @@ public:
{
if ( ! self.isScrollEnabled) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
MGLMapCamera *oldCamera = self.camera;
@@ -1390,7 +1390,7 @@ public:
{
if ( ! self.isZoomEnabled) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
CGPoint centerPoint = [self anchorPointForGesture:pinch];
MGLMapCamera *oldCamera = self.camera;
@@ -1491,7 +1491,7 @@ public:
{
if ( ! self.isRotateEnabled) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
CGPoint centerPoint = [self anchorPointForGesture:rotate];
MGLMapCamera *oldCamera = self.camera;
@@ -1696,7 +1696,7 @@ public:
{
if ( ! self.isZoomEnabled) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
if (doubleTap.state == UIGestureRecognizerStateEnded)
{
@@ -1737,7 +1737,7 @@ public:
if (_mbglMap->getZoom() == _mbglMap->getMinZoom()) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonGestureZoomOut;
@@ -1776,7 +1776,7 @@ public:
{
if ( ! self.isZoomEnabled) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonGestureOneFingerZoom;
@@ -1821,7 +1821,7 @@ public:
{
if ( ! self.isPitchEnabled) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonGestureTilt;
@@ -2981,7 +2981,7 @@ public:
return;
}
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
@@ -3006,7 +3006,7 @@ public:
- (void)setZoomLevel:(double)zoomLevel animated:(BOOL)animated
{
if (zoomLevel == self.zoomLevel) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
@@ -3149,7 +3149,7 @@ public:
}
[self willChangeValueForKey:@"visibleCoordinateBounds"];
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
@@ -3182,7 +3182,7 @@ public:
- (void)_setDirection:(CLLocationDirection)direction animated:(BOOL)animated
{
if (direction == self.direction) return;
_mbglMap->cancelTransitions();
[self cancelTransitions];
CGFloat duration = animated ? MGLAnimationDuration : 0;
@@ -3271,7 +3271,7 @@ public:
}
[self willChangeValueForKey:@"camera"];
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
@@ -3330,7 +3330,7 @@ public:
}
[self willChangeValueForKey:@"camera"];
_mbglMap->cancelTransitions();
[self cancelTransitions];
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonProgrammatic;
@@ -3339,6 +3339,12 @@ public:
[self didChangeValueForKey:@"camera"];
}
- (void)cancelTransitions {
self.cameraChangeReasonBitmask |= MGLCameraChangeReasonTransitionCancelled;
_mbglMap->cancelTransitions();
self.cameraChangeReasonBitmask &= ~MGLCameraChangeReasonTransitionCancelled;
}
- (MGLMapCamera *)cameraThatFitsCoordinateBounds:(MGLCoordinateBounds)bounds
{
return [self cameraThatFitsCoordinateBounds:bounds edgePadding:UIEdgeInsetsZero];

1
platform/macos/CHANGELOG.md
View File

@@ -13,6 +13,7 @@
### Other changes
* Unknown tokens in URLs are now preserved, rather than replaced with an empty string. ([#11787](https://github.com/mapbox/mapbox-gl-native/issues/11787))
* Adjusted when and how the camera transition update and finish callbacks are called, fixing recursion bugs. ([#11614](https://github.com/mapbox/mapbox-gl-native/pull/11614))
## 0.7.1

57
src/mbgl/map/transform.cpp
View File

@@ -594,13 +594,10 @@ void Transform::startTransition(const CameraOptions& camera,
animation.transitionFrameFn(t);
}
observer.onCameraIsChanging();
return false;
} else {
transitionFinishFn();
transitionFinishFn = nullptr;
// This callback gets destroyed here,
// we can only return after this point.
transitionFrameFn = nullptr;
// Indicate that we need to terminate this transition
return true;
}
};
@@ -615,7 +612,14 @@ void Transform::startTransition(const CameraOptions& camera,
};
if (!isAnimated) {
transitionFrameFn(Clock::now());
auto update = std::move(transitionFrameFn);
auto finish = std::move(transitionFinishFn);
transitionFrameFn = nullptr;
transitionFinishFn = nullptr;
update(Clock::now());
finish();
}
}
@@ -624,8 +628,43 @@ bool Transform::inTransition() const {
}
void Transform::updateTransitions(const TimePoint& now) {
if (transitionFrameFn) {
transitionFrameFn(now);
// Use a temporary function to ensure that the transitionFrameFn lambda is
// called only once per update.
// This addresses the symptoms of https://github.com/mapbox/mapbox-gl-native/issues/11180
// where setting a shape source to nil (or similar) in the `onCameraIsChanging`
// observer function causes `Map::Impl::onUpdate()` to be called which
// in turn calls this function (before the current iteration has completed),
// leading to an infinite loop. See https://github.com/mapbox/mapbox-gl-native/issues/5833
// for a similar, related, issue.
//
// By temporarily nulling the `transitionFrameFn` (and then restoring it
// after the temporary has been called) we stop this recursion.
//
// It's important to note that the scope of this change is stop the above
// crashes. It doesn't address any potential deeper issue (for example
// user error, how often and when transition callbacks are called).
auto transition = std::move(transitionFrameFn);
transitionFrameFn = nullptr;
if (transition && transition(now)) {
// If the transition indicates that it is complete, then we should call
// the finish lambda (going via a temporary as above)
auto finish = std::move(transitionFinishFn);
transitionFinishFn = nullptr;
transitionFrameFn = nullptr;
if (finish) {
finish();
}
} else if (!transitionFrameFn) {
// We have to check `transitionFrameFn` is nil here, since a new transition
// may have been triggered in a user callback (from the transition call
// above)
transitionFrameFn = std::move(transition);
}
}

2
src/mbgl/map/transform.hpp
View File

@@ -165,7 +165,7 @@ private:
TimePoint transitionStart;
Duration transitionDuration;
std::function<void(const TimePoint)> transitionFrameFn;
std::function<bool(const TimePoint)> transitionFrameFn;
std::function<void()> transitionFinishFn;
};