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

[ios, macos] Keep MGLMapSnapshotter alive through completion

Browse Source
This commit is contained in:
Minh Nguyễn
2020-03-14 01:12:24 -07:00
committed by Minh Nguyễn
parent d357f8c511
commit 3318a0a383
6 changed files with 74 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
platform/darwin/src/MGLMapSnapshotter.h
View File

@@ -185,11 +185,6 @@ typedef void (^MGLMapSnapshotCompletionHandler)(MGLMapSnapshot* _Nullable snapsh
snapshot at a time. If you need to generate multiple snapshots concurrently,
create multiple snapshotter objects.
An `MGLMapSnapshotter` object may be deallocated before the completion handler
is called. To prevent this object from being deallocated prematurely, maintain
a strong reference to it, for example by storing it in an instance variable of
the class where you initialize and start the snapshotter.
For an interactive map, use the `MGLMapView` class. Both `MGLMapSnapshotter`
and `MGLMapView` are compatible with offline packs managed by the
`MGLOfflineStorage` class.
@@ -208,8 +203,7 @@ typedef void (^MGLMapSnapshotCompletionHandler)(MGLMapSnapshot* _Nullable snapsh
let options = MGLMapSnapshotOptions(styleURL: MGLStyle.satelliteStreetsStyleURL, camera: camera, size: CGSize(width: 320, height: 480))
options.zoomLevel = 10
// The containing class should hold a strong reference to this object.
snapshotter = MGLMapSnapshotter(options: options)
let snapshotter = MGLMapSnapshotter(options: options)
snapshotter.start { (snapshot, error) in
if let error = error {
fatalError(error.localizedDescription)
@@ -242,11 +236,6 @@ MGL_EXPORT
/**
Starts the snapshot creation and executes the specified block with the result.
The snapshotter may be deallocated before the completion handler is called. To
prevent the snapshotter from being deallocated prematurely, maintain a strong
reference to it, for example by storing it in an instance variable of the class
where you call this method.
@param completionHandler The block to call with a finished snapshot. The block
is executed on the main queue.
*/
@@ -256,11 +245,6 @@ MGL_EXPORT
Starts the snapshot creation and executes the specified block with the result
on the specified queue.
The snapshotter may be deallocated before the completion handler is called. To
prevent the snapshotter from being deallocated prematurely, maintain a strong
reference to it, for example by storing it in an instance variable of the class
where you call this method.
@param queue The queue on which to call the block specified in the
`completionHandler` parameter.
@param completionHandler The block to call with a finished snapshot. The block
@@ -273,11 +257,6 @@ MGL_EXPORT
on the specified queue. Use this option if you want to add custom drawing on
top of the resulting `MGLMapSnapshot`.
The snapshotter may be deallocated before the completion handler is called. To
prevent the snapshotter from being deallocated prematurely, maintain a strong
reference to it, for example by storing it in an instance variable of the class
where you call this method.
@param overlayHandler The block to call after the base map finishes drawing but
before certain built-in overlays draw. The block can use Core Graphics to
draw custom content directly over the base map. The block is executed on a

20
platform/darwin/src/MGLMapSnapshotter.mm
View File

@@ -261,6 +261,9 @@ NSArray<MGLAttributionInfo *> *MGLAttributionInfosFromAttributions(mbgl::MapSnap
- (void)startWithQueue:(dispatch_queue_t)queue overlayHandler:(MGLMapSnapshotOverlayHandler)overlayHandler completionHandler:(MGLMapSnapshotCompletionHandler)completion
{
if (!completion) {
return;
}
if (!mbgl::Scheduler::GetCurrent()) {
[NSException raise:NSInvalidArgumentException
format:@"startWithQueue:completionHandler: must be called from a thread with an active run loop."];
@@ -282,9 +285,11 @@ NSArray<MGLAttributionInfo *> *MGLAttributionInfosFromAttributions(mbgl::MapSnap
[self configureWithOptions:options];
MGLLogDebug(@"Starting with options: %@", self.options);
// Capture weakSelf to avoid retain cycle if callback is never called (i.e., snapshot canceled).
__weak __typeof__(self) weakSelf = self;
_mbglMapSnapshotter->snapshot([=](std::exception_ptr mbglError, mbgl::PremultipliedImage image, mbgl::MapSnapshotter::Attributions attributions, mbgl::MapSnapshotter::PointForFn pointForFn, mbgl::MapSnapshotter::LatLngForFn latLngForFn) {
// Temporarily capture the snapshotter until the completion handler finishes executing, to keep standalone local usage of the snapshotter from becoming a no-op.
// POSTCONDITION: Only refer to this variable in the final result queue.
// POSTCONDITION: It is important to nil out this variable at some point in the future to avoid a leak. In cases where the completion handler gets called, the variable should be nilled out explicitly. If -cancel is called, mbgl releases the snapshot block below, causing the only remaining references to the snapshotter to go out of scope.
__block MGLMapSnapshotter *strongSelf = self;
_mbglMapSnapshotter->snapshot(^(std::exception_ptr mbglError, mbgl::PremultipliedImage image, mbgl::MapSnapshotter::Attributions attributions, mbgl::MapSnapshotter::PointForFn pointForFn, mbgl::MapSnapshotter::LatLngForFn latLngForFn) {
if (mbglError) {
NSString *description = @(mbgl::util::toString(mbglError).c_str());
NSDictionary *userInfo = @{NSLocalizedDescriptionKey: description};
@@ -294,8 +299,9 @@ NSArray<MGLAttributionInfo *> *MGLAttributionInfosFromAttributions(mbgl::MapSnap
#endif
// Dispatch to result queue
dispatch_async(queue, ^{
weakSelf.loading = NO;
strongSelf.loading = NO;
completion(nil, error);
strongSelf = nil;
});
} else {
#if TARGET_OS_IPHONE
@@ -315,11 +321,9 @@ NSArray<MGLAttributionInfo *> *MGLAttributionInfosFromAttributions(mbgl::MapSnap
// Dispatch result to result queue
dispatch_async(queue, ^{
__typeof__(self) strongSelf = weakSelf;
strongSelf.loading = NO;
if (completion) {
completion(snapshot, error);
}
completion(snapshot, error);
strongSelf = nil;
});
});
}

4
platform/darwin/test/MGLDocumentationExampleTests.swift
View File

@@ -463,15 +463,13 @@ class MGLDocumentationExampleTests: XCTestCase, MGLMapViewDelegate {
}
}
let snapshotter: MGLMapSnapshotter
//#-example-code
let camera = MGLMapCamera(lookingAtCenter: CLLocationCoordinate2D(latitude: 37.7184, longitude: -122.4365), altitude: 100, pitch: 20, heading: 0)
let options = MGLMapSnapshotOptions(styleURL: MGLStyle.satelliteStreetsStyleURL, camera: camera, size: CGSize(width: 320, height: 480))
options.zoomLevel = 10
// The containing class should hold a strong reference to this object.
snapshotter = MGLMapSnapshotter(options: options)
let snapshotter = MGLMapSnapshotter(options: options)
snapshotter.start { (snapshot, error) in
if let error = error {
fatalError(error.localizedDescription)

2
platform/ios/CHANGELOG.md
View File

@@ -16,7 +16,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
### Snapshots
* `MGLMapSnapshotter` no longer keeps itself from being deallocated before its completion handler is called. To ensure that the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]` is called, maintain a strong reference to the snapshotter from a longer-lived class, such as the class where you call `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))
* You no longer need to explicitly capture the `MGLMapSnapshotter` object in the completion handler that you specify in `-[MGLMapSnapshotter startWithCompletionHandler:]`. Even if you declare the snapshotter locally without holding a strong reference to it, the snapshotter is only deallocated after the completion handler finishes, and the completion handler generally receives a valid snapshot. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))
* The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))
* Fixed an issue where the `MGLMapSnapshotter.loading` property always returned `NO`, even while loading a snapshot. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))

112
platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m
View File

@@ -73,8 +73,7 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
}
- (void)testSnapshotterWithoutStrongReference🔒 {
XCTestExpectation *expectation = [self expectationWithDescription:@"Completion handler shouldn’t be called if there’s no strong reference to the snapshotter and the snapshotter goes away."];
expectation.inverted = YES;
XCTestExpectation *expectation = [self expectationWithDescription:@"Completion handler should be called even if there’s no strong reference to the snapshotter."];
CGSize size = self.mapView.bounds.size;
CLLocationCoordinate2D coordinates = CLLocationCoordinate2DMake(30.0, 30.0);
@@ -82,18 +81,21 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
@autoreleasepool {
MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coordinates, size);
weakSnapshotter = snapshotter;
__weak __typeof__(self) weakSelf = self;
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
// This completion block should not be called.
MGLTestAssertNotNil(weakSelf, weakSnapshotter, @"Snapshotter should not go away until the completion handler finishes executing.");
// This completion block should be called.
[expectation fulfill];
}];
MGLTestAssertNotNil(self, weakSnapshotter, @"Locally scoped snapshotter should not go away while in scope.");
}
MGLTestAssertNil(self, weakSnapshotter, @"Snapshotter should go away on its own if there’s no strong reference to it.");
MGLTestAssertNotNil(self, weakSnapshotter, @"Snapshotter should remain until it finishes even there’s no strong reference to it.");
[self waitForExpectations:@[expectation] timeout:5];
[self waitForExpectations:@[expectation] timeout:10.0];
MGLTestAssertNil(self, weakSnapshotter, @"Completion handler should not leak snapshotter.");
}
- (void)testDeallocatingSnapshotterDuringSnapshot🔒 {
- (void)testSnapshotterInBackgroundWithoutStrongReference🔒 {
// See also https://github.com/mapbox/mapbox-gl-native/issues/12336
NSTimeInterval timeout = 10.0;
@@ -106,6 +108,7 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
// dispatch_queue_t backgroundQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
__weak __typeof__(self) weakself = self;
__block __weak MGLMapSnapshotter *weakSnapshotter;
dispatch_async(backgroundQueue, ^{
@@ -113,13 +116,21 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
dispatch_group_enter(dg);
dispatch_async(dispatch_get_main_queue(), ^{
MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
// This completion block should not be called.
__typeof__(self) strongself = weakself;
MGLTestFail(strongself, @"Completion handler should not be called after snapshotter is deallocated.");
}];
dispatch_group_leave(dg);
@autoreleasepool {
MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
weakSnapshotter = snapshotter;
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
// We expect this completion block to be called with an error
__typeof__(self) strongself = weakself;
MGLTestAssertNotNil(strongself, snapshot);
MGLTestAssertNil(strongself, error, @"Snapshotter should have completed");
MGLTestAssertNotNil(strongself, weakSnapshotter, @"Snapshotter should not have been deallocated yet.");
dispatch_group_leave(dg);
}];
}
});
dispatch_group_notify(dg, dispatch_get_main_queue(), ^{
@@ -128,6 +139,7 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
});
[self waitForExpectations:@[expectation] timeout:timeout];
MGLTestAssertNil(self, weakSnapshotter, @"Snapshotter should not leak.");
}
- (void)testSnapshotterUsingNestedDispatchQueues🔒 {
@@ -157,7 +169,7 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
__block MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
// We expect this completion block to be called with an error
// We expect this completion block to be called with no error
__typeof__(self) strongself = weakself;
MGLTestAssertNotNil(strongself, snapshot);
MGLTestAssertNil(strongself, error, @"Snapshotter should have completed");
@@ -178,16 +190,20 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
CGSize size = self.mapView.bounds.size;
CLLocationCoordinate2D coord = CLLocationCoordinate2DMake(30.0, 30.0);
MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
__block __weak MGLMapSnapshotter *weakSnapshotter;
@autoreleasepool {
MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
weakSnapshotter = snapshotter;
__weak __typeof__(self) weakself = self;
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
__typeof__(self) strongself = weakself;
MGLTestFail(strongself, @"Completion handler should not be called when canceling.");
}];
}
__weak __typeof__(self) weakself = self;
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
__typeof__(self) strongself = weakself;
MGLTestFail(strongself, @"Completion handler should not be called when canceling.");
}];
[snapshotter cancel];
MGLTestAssertNotNil(self, weakSnapshotter);
[weakSnapshotter cancel];
MGLTestAssertNil(self, weakSnapshotter, @"Snapshotter should not leak after being canceled.");
}
- (void)testAllocatingSnapshotOnBackgroundQueue🔒 {
@@ -290,39 +306,27 @@ MGLMapSnapshotter* snapshotterWithBounds(MGLCoordinateBounds bounds, CGSize size
float lon = (ratio*120.0) + ((1.0-ratio)*54.0);
CLLocationCoordinate2D coord = CLLocationCoordinate2DMake(57.0, lon);
__block MGLMapSnapshotter *snapshotter;
// Allocate from an autorelease pool here, to avoid having
// snapshotter retained for longer than we'd like to test.
@autoreleasepool {
snapshotter = snapshotterWithCoordinates(coord, size);
XCTAssertNotNil(snapshotter);
MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size);
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
// This should be the main queue
__typeof__(self) strongself = weakself;
MGLTestAssertNotNil(strongself, strongself);
MGLTestAssertNotNil(strongself, snapshot);
MGLTestAssertNotNil(strongself, snapshot.image);
MGLTestAssertNil(strongself, error, @"Snapshot should not error with: %@", error);
// Change this to XCTAttachmentLifetimeKeepAlways to be able to look at the snapshots after running
XCTAttachment *attachment = [XCTAttachment attachmentWithImage:snapshot.image];
attachment.lifetime = XCTAttachmentLifetimeDeleteOnSuccess;
[strongself addAttachment:attachment];
[expectation fulfill];
}];
}
[snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) {
// This should be the main queue
__typeof__(self) strongself = weakself;
MGLTestAssertNotNil(strongself, strongself);
MGLTestAssertNotNil(strongself, snapshot);
MGLTestAssertNotNil(strongself, snapshot.image);
MGLTestAssertNil(strongself, error, @"Snapshot should not error with: %@", error);
// Change this to XCTAttachmentLifetimeKeepAlways to be able to look at the snapshots after running
XCTAttachment *attachment = [XCTAttachment attachmentWithImage:snapshot.image];
attachment.lifetime = XCTAttachmentLifetimeDeleteOnSuccess;
[strongself addAttachment:attachment];
// Dealloc the snapshotter (by having this line in the block, we
// also retained the snapshotter. Setting to nil should release, as
// this block should be the only thing retaining it (since it was
// allocated from the above autorelease pool)
snapshotter = nil;
[expectation fulfill];
}];
} // end for loop
[self waitForExpectations:@[expectation] timeout:60.0];

2
platform/macos/CHANGELOG.md
View File

@@ -47,7 +47,7 @@
### Snapshots
* Added an `-[MGLMapSnapshotter startWithOverlayHandler:completionHandler:]` method to provide the snapshot's current `CGContext` in order to perform custom drawing on `MGLMapSnapshot` objects. ([#15530](https://github.com/mapbox/mapbox-gl-native/pull/15530))
* `MGLMapSnapshotter` no longer keeps itself from being deallocated before its completion handler is called. To ensure that the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]` is called, maintain a strong reference to the snapshotter from a longer-lived class, such as the class where you call `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))
* You no longer need to explicitly capture the `MGLMapSnapshotter` object in the completion handler that you specify in `-[MGLMapSnapshotter startWithCompletionHandler:]`. Even if you declare the snapshotter locally without holding a strong reference to it, the snapshotter is only deallocated after the completion handler finishes, and the completion handler generally receives a valid snapshot. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))
* The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))
* Fixed an issue where the `MGLMapSnapshotter.loading` property always returned `NO`, even while loading a snapshot. ([#210](https://github.com/mapbox/mapbox-gl-native-ios/pull/210))