From 48f034cecd584ef53d14b62a75381ce668ec1dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Fri, 13 Mar 2020 00:18:24 -0700 Subject: [PATCH 1/6] mbgl v1.4.0 --- vendor/mapbox-gl-native | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/mapbox-gl-native b/vendor/mapbox-gl-native index 90461930e4..09d307db14 160000 --- a/vendor/mapbox-gl-native +++ b/vendor/mapbox-gl-native @@ -1 +1 @@ -Subproject commit 90461930e4e7b8ce9901f9c41be47de8ab40d3f5 +Subproject commit 09d307db14514479a8f2fbbd6e6a98496b1f23c7 From 5be63a0d4a1847a22a8129182a8077308f769da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Fri, 13 Mar 2020 00:55:23 -0700 Subject: [PATCH 2/6] [ios, macos] Updated MGLSymbolStyleLayer.symbolSortKey documentation --- platform/darwin/scripts/generate-style-code.js | 3 +++ platform/darwin/src/MGLSymbolStyleLayer.h | 10 ++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/platform/darwin/scripts/generate-style-code.js b/platform/darwin/scripts/generate-style-code.js index 9b164f2f4d..ab0513d333 100755 --- a/platform/darwin/scripts/generate-style-code.js +++ b/platform/darwin/scripts/generate-style-code.js @@ -327,6 +327,9 @@ global.propertyDoc = function (propertyName, property, layerType, kind) { let doc = property.doc.replace(/`([^`]+?)` is set to `([^`]+?)`(?: or `([^`]+?)`)?/g, function (m, peerPropertyName, propertyValue, secondPropertyValue, offset, str) { let otherProperty = camelizeWithLeadingLowercase(peerPropertyName); let otherValue = objCType(layerType, peerPropertyName) + camelize(propertyValue); + if (propertyValue === 'true' || propertyValue === 'false') { + otherValue = propertyValue === 'true' ? 'YES' : 'NO'; + } if (property.type == 'array' && kind == 'light') { otherValue = propertyValue; } diff --git a/platform/darwin/src/MGLSymbolStyleLayer.h b/platform/darwin/src/MGLSymbolStyleLayer.h index 712ed55b84..f5e2b490dd 100644 --- a/platform/darwin/src/MGLSymbolStyleLayer.h +++ b/platform/darwin/src/MGLSymbolStyleLayer.h @@ -1057,10 +1057,12 @@ MGL_EXPORT @property (nonatomic, null_resettable) NSExpression *symbolPlacement; /** - Sorts features in ascending order based on this value. Features with a higher - sort key will appear above features with a lower sort key when they overlap. - Features with a lower sort key will have priority over other features when - doing placement. + Sorts features in ascending order based on this value. Features with lower sort + keys are drawn and placed first. When `iconAllowsOverlap` or + `textAllowsOverlap` is `false`, features with a lower sort key will have + priority during placement. When `iconAllowsOverlap` or `textAllowsOverlap` is + set to `YES`, features with a higher sort key will overlap over features with a + lower sort key. You can set this property to an expression containing any of the following: From 98e8af96d3793aad5f17599195d51c56a4e97a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Fri, 13 Mar 2020 00:20:48 -0700 Subject: [PATCH 3/6] [ios, macos] Streamlined MGLMapSnapshotter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upgraded MGLMapSnapshotter to be compatible with the latest mbgl. Converted as many of MGLMapSnapshotter’s instance variables and properties as possible into local variables close to the point of use. The completion handler of -startWithCompletionHandler: and its variants is no longer called as part of -cancel, when the snapshotter is deallocated, or when the application is terminating. -loading once again works and is used to avoid concurrent requests, now that the completion handler is no longer memoized. Snapshotter options are copied and converted into a snapshotter on demand as late as possible. Made the decoration step synchronous while continuing to perform it on a worker thread. Only access self on user-facing queues. Ensure MGLMapSnapshotter.loading is reset after snapshotting. Pushed some drawing and completion code up from inner completion blocks to outer completion blocks to avoid excessively passing state around. Converted a few class methods into standalone C functions to emphasize the avoidance of captured state. --- platform/darwin/src/MGLMapSnapshotter.h | 4 +- platform/darwin/src/MGLMapSnapshotter.mm | 324 +++++++----------- platform/ios/CHANGELOG.md | 2 + .../Snapshotter Tests/MGLMapSnapshotterTest.m | 50 +-- platform/macos/CHANGELOG.md | 7 +- 5 files changed, 158 insertions(+), 229 deletions(-) diff --git a/platform/darwin/src/MGLMapSnapshotter.h b/platform/darwin/src/MGLMapSnapshotter.h index 98c8e8a375..ed7a8c2e5d 100644 --- a/platform/darwin/src/MGLMapSnapshotter.h +++ b/platform/darwin/src/MGLMapSnapshotter.h @@ -58,7 +58,7 @@ typedef void (^MGLMapSnapshotOverlayHandler)(MGLMapSnapshotOverlay * snapshotOve The options to use when creating images with the `MGLMapSnapshotter`. */ MGL_EXPORT -@interface MGLMapSnapshotOptions : NSObject +@interface MGLMapSnapshotOptions : NSObject /** Creates a set of options with the minimum required information. @@ -252,7 +252,7 @@ MGL_EXPORT /** Starts the snapshot creation and executes the specified blocks with the result on the specified queue. Use this option if you want to add custom drawing on top of the - resulting `MGLMapSnapShot`. + resulting `MGLMapSnapshot`. @param overlayHandler The block to handle manipulation of the `MGLMapSnapshotter`'s `CGContext`. @param completionHandler The block to handle the result in. */ diff --git a/platform/darwin/src/MGLMapSnapshotter.mm b/platform/darwin/src/MGLMapSnapshotter.mm index 7c2385a5ff..908181ef73 100644 --- a/platform/darwin/src/MGLMapSnapshotter.mm +++ b/platform/darwin/src/MGLMapSnapshotter.mm @@ -33,6 +33,9 @@ const CGPoint MGLLogoImagePosition = CGPointMake(8, 8); const CGFloat MGLSnapshotterMinimumPixelSize = 64; +MGLImage *MGLAttributedSnapshot(mbgl::MapSnapshotter::Attributions attributions, MGLImage *mglImage, mbgl::MapSnapshotter::PointForFn pointForFn, mbgl::MapSnapshotter::LatLngForFn latLngForFn, MGLMapSnapshotOptions *options, MGLMapSnapshotOverlayHandler overlayHandler); +MGLMapSnapshot *MGLSnapshotWithDecoratedImage(MGLImage *mglImage, MGLMapSnapshotOptions *options, mbgl::MapSnapshotter::Attributions attributions, mbgl::MapSnapshotter::PointForFn pointForFn, mbgl::MapSnapshotter::LatLngForFn latLngForFn, MGLMapSnapshotOverlayHandler overlayHandler, NSError * _Nullable *outError); +NSArray *MGLAttributionInfosFromAttributions(mbgl::MapSnapshotter::Attributions attributions); @interface MGLMapSnapshotOverlay() @property (nonatomic, assign) CGFloat scale; @@ -131,6 +134,15 @@ - (instancetype _Nonnull)initWithStyleURL:(nullable NSURL *)styleURL camera:(MGL return self; } +- (nonnull id)copyWithZone:(nullable NSZone *)zone { + __typeof__(self) copy = [[[self class] alloc] initWithStyleURL:_styleURL camera:_camera size:_size]; + copy.zoomLevel = _zoomLevel; + copy.coordinateBounds = _coordinateBounds; + copy.scale = _scale; + copy.showsLogo = _showsLogo; + return copy; +} + @end @interface MGLMapSnapshot() @@ -190,32 +202,19 @@ - (CLLocationCoordinate2D)coordinateForPoint:(NSPoint)point @end @interface MGLMapSnapshotter() -@property (nonatomic) BOOL cancelled; +@property (nonatomic) BOOL loading; @property (nonatomic) BOOL terminated; -@property (nonatomic) dispatch_queue_t resultQueue; -@property (nonatomic, copy) MGLMapSnapshotCompletionHandler completion; -+ (void)completeWithErrorCode:(MGLErrorCode)errorCode description:(nonnull NSString*)description onQueue:(dispatch_queue_t)queue completion:(MGLMapSnapshotCompletionHandler)completion; @end @implementation MGLMapSnapshotter { std::unique_ptr _mbglMapSnapshotter; - std::unique_ptr> _snapshotCallback; } - (void)dealloc { [[NSNotificationCenter defaultCenter] removeObserver:self]; - - if (_completion) { - MGLAssert(_snapshotCallback, @"Snapshot in progress - there should be a valid callback"); - - [MGLMapSnapshotter completeWithErrorCode:MGLErrorCodeSnapshotFailed - description:@"MGLMapSnapshotter deallocated prior to snapshot completion." - onQueue:_resultQueue - completion:_completion]; - } + [self cancel]; } - - (instancetype)init { NSAssert(NO, @"Please use -[MGLMapSnapshotter initWithOptions:]"); [super doesNotRecognizeSelector:_cmd]; @@ -227,7 +226,7 @@ - (instancetype)initWithOptions:(MGLMapSnapshotOptions *)options MGLLogDebug(@"Initializing withOptions: %@", options); self = [super init]; if (self) { - [self setOptions:options]; + self.options = options; #if TARGET_OS_IPHONE [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(applicationWillTerminate:) name:UIApplicationWillTerminateNotification object:nil]; #else @@ -242,13 +241,7 @@ - (instancetype)initWithOptions:(MGLMapSnapshotOptions *)options - (void)applicationWillTerminate:(NSNotification *)notification { - if (self.completion) { - [self cancel]; - } - - _mbglMapSnapshotter.reset(); - _snapshotCallback.reset(); - + [self cancel]; self.terminated = YES; } @@ -273,36 +266,25 @@ - (void)startWithQueue:(dispatch_queue_t)queue overlayHandler:(MGLMapSnapshotOve format:@"startWithQueue:completionHandler: must be called from a thread with an active run loop."]; } - if (self.completion) { + if (self.loading) { // Consider replacing this exception with an error passed to the completion block. [NSException raise:NSInternalInconsistencyException format:@"Already started this snapshotter."]; } + self.loading = YES; if (self.terminated) { [NSException raise:NSInternalInconsistencyException format:@"Starting a snapshotter after application termination is not supported."]; } + + MGLMapSnapshotOptions *options = [self.options copy]; + [self configureWithOptions:options]; + MGLLogDebug(@"Starting with options: %@", self.options); - self.completion = completion; - self.resultQueue = queue; - self.cancelled = NO; - + // Capture weakSelf to avoid retain cycle if callback is never called (i.e., snapshot canceled). __weak __typeof__(self) weakSelf = self; - // mbgl::Scheduler::GetCurrent() scheduler means "run callback on current (ie UI/main) thread" - // capture weakSelf to avoid retain cycle if callback is never called (ie snapshot cancelled) - - _snapshotCallback = std::make_unique>( - *mbgl::Scheduler::GetCurrent(), - [=](std::exception_ptr mbglError, mbgl::PremultipliedImage image, mbgl::MapSnapshotter::Attributions attributions, mbgl::MapSnapshotter::PointForFn pointForFn, mbgl::MapSnapshotter::LatLngForFn latLngForFn) { - - __typeof__(self) strongSelf = weakSelf; - // If self had died, _snapshotCallback would have been destroyed and this block would not be executed - MGLCAssert(strongSelf, @"Snapshot callback executed after being destroyed."); - - if (!strongSelf.completion) - return; - + _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}; @@ -312,34 +294,41 @@ - (void)startWithQueue:(dispatch_queue_t)queue overlayHandler:(MGLMapSnapshotOve #endif // Dispatch to result queue dispatch_async(queue, ^{ - strongSelf.completion(nil, error); - strongSelf.completion = nil; + weakSelf.loading = NO; + completion(nil, error); }); - } else { #if TARGET_OS_IPHONE - MGLImage *mglImage = [[MGLImage alloc] initWithMGLPremultipliedImage:std::move(image) scale:strongSelf.options.scale]; + MGLImage *mglImage = [[MGLImage alloc] initWithMGLPremultipliedImage:std::move(image) scale:options.scale]; #else MGLImage *mglImage = [[MGLImage alloc] initWithMGLPremultipliedImage:std::move(image)]; - mglImage.size = NSMakeSize(mglImage.size.width / strongSelf.options.scale, - mglImage.size.height / strongSelf.options.scale); + mglImage.size = NSMakeSize(mglImage.size.width / options.scale, + mglImage.size.height / options.scale); #endif - - [strongSelf drawAttributedSnapshot:attributions snapshotImage:mglImage pointForFn:pointForFn latLngForFn:latLngForFn overlayHandler:overlayHandler]; + // Process image watermark in a work queue + dispatch_queue_t workQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + + dispatch_async(workQueue, ^{ + // Call a function that cannot accidentally capture self. + NSError *error; + MGLMapSnapshot *snapshot = MGLSnapshotWithDecoratedImage(mglImage, options, attributions, pointForFn, latLngForFn, overlayHandler, &error); + + // Dispatch result to result queue + dispatch_async(queue, ^{ + __typeof__(self) strongSelf = weakSelf; + strongSelf.loading = NO; + if (completion) { + completion(snapshot, error); + } + }); + }); } - strongSelf->_snapshotCallback = NULL; - - }); - - // Launches snapshot on background Thread owned by mbglMapSnapshotter - // _snapshotCallback->self() is an ActorRef: if the callback is destroyed, further messages - // to the callback are just no-ops - _mbglMapSnapshotter->snapshot(_snapshotCallback->self()); + }); } -+ (MGLImage*)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)attributions snapshotImage:(MGLImage *)mglImage pointForFn:(mbgl::MapSnapshotter::PointForFn)pointForFn latLngForFn:(mbgl::MapSnapshotter::LatLngForFn)latLngForFn scale:(CGFloat)scale size:(CGSize)size showsLogo:(BOOL)showsLogo overlayHandler:(MGLMapSnapshotOverlayHandler)overlayHandler { +MGLImage *MGLAttributedSnapshot(mbgl::MapSnapshotter::Attributions attributions, MGLImage *mglImage, MGLMapSnapshotOptions *options, void (^overlayHandler)()) { - NSArray* attributionInfo = [MGLMapSnapshotter generateAttributionInfos:attributions]; + NSArray *attributionInfo = MGLAttributionInfosFromAttributions(attributions); #if TARGET_OS_IPHONE MGLAttributionInfoStyle attributionInfoStyle = MGLAttributionInfoStyleLong; @@ -375,31 +364,20 @@ + (MGLImage*)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)at attributionBackgroundSize.width * mglImage.scale, attributionBackgroundSize.height * mglImage.scale); - - UIGraphicsBeginImageContextWithOptions(mglImage.size, NO, scale); + UIGraphicsBeginImageContextWithOptions(mglImage.size, NO, options.scale); [mglImage drawInRect:CGRectMake(0, 0, mglImage.size.width, mglImage.size.height)]; + overlayHandler(); CGContextRef currentContext = UIGraphicsGetCurrentContext(); - if (currentContext && overlayHandler) { - MGLMapSnapshotOverlay *snapshotOverlay = [[MGLMapSnapshotOverlay alloc] initWithContext:currentContext - scale:scale - pointForFn:pointForFn - latLngForFn:latLngForFn]; - CGContextSaveGState(snapshotOverlay.context); - overlayHandler(snapshotOverlay); - CGContextRestoreGState(snapshotOverlay.context); - currentContext = UIGraphicsGetCurrentContext(); - } - - if (!currentContext && overlayHandler) { + if (!currentContext) { // If the current context has been corrupted by the user, // return nil so we can generate an error later. return nil; } - if (showsLogo) { + if (options.showsLogo) { [logoImage drawInRect:logoImageRect]; } @@ -423,9 +401,7 @@ + (MGLImage*)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)at return compositedImage; #else - - NSSize targetSize = NSMakeSize(size.width, size.height); - NSRect targetFrame = NSMakeRect(0, 0, targetSize.width, targetSize.height); + NSRect targetFrame = { .origin = NSZeroPoint, .size = options.size }; MGLAttributionInfoStyle attributionInfoStyle = MGLAttributionInfoStyleLong; for (NSUInteger styleValue = MGLAttributionInfoStyleLong; styleValue >= MGLAttributionInfoStyleShort; styleValue--) { @@ -461,25 +437,14 @@ + (MGLImage*)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)at NSImageRep *sourceImageRep = [sourceImage bestRepresentationForRect:targetFrame context:nil hints:nil]; - compositedImage = [[NSImage alloc] initWithSize:targetSize]; + compositedImage = [[NSImage alloc] initWithSize:targetFrame.size]; [compositedImage lockFocus]; [sourceImageRep drawInRect: targetFrame]; NSGraphicsContext *currentContext = [NSGraphicsContext currentContext]; - if (currentContext && overlayHandler) { - MGLMapSnapshotOverlay *snapshotOverlay = [[MGLMapSnapshotOverlay alloc] initWithContext:currentContext.CGContext - scale:scale - pointForFn:pointForFn - latLngForFn:latLngForFn]; - [currentContext saveGraphicsState]; - overlayHandler(snapshotOverlay); - [currentContext restoreGraphicsState]; - currentContext = [NSGraphicsContext currentContext]; - } - - if (!currentContext && overlayHandler) { + if (!currentContext) { // If the current context has been corrupted by the user, // return nil so we can generate an error later. return nil; @@ -505,59 +470,55 @@ + (MGLImage*)drawAttributedSnapshotWorker:(mbgl::MapSnapshotter::Attributions)at #endif } -- (void)drawAttributedSnapshot:(mbgl::MapSnapshotter::Attributions)attributions snapshotImage:(MGLImage *)mglImage pointForFn:(mbgl::MapSnapshotter::PointForFn)pointForFn latLngForFn:(mbgl::MapSnapshotter::LatLngForFn)latLngForFn overlayHandler:(MGLMapSnapshotOverlayHandler)overlayHandler { - - // Process image watermark in a work queue - dispatch_queue_t workQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); - dispatch_queue_t resultQueue = self.resultQueue; - - // Capture scale and size by value to avoid accessing self from another thread - CGFloat scale = self.options.scale; - CGSize size = self.options.size; - BOOL showsLogo = self.options.showsLogo; - - // pointForFn is a copyable std::function that captures state by value: see MapSnapshotter::Impl::snapshot - __weak __typeof__(self) weakself = self; - - dispatch_async(workQueue, ^{ - // Call a class method to ensure we're not accidentally capturing self - MGLImage *compositedImage = [MGLMapSnapshotter drawAttributedSnapshotWorker:attributions - snapshotImage:mglImage - pointForFn:pointForFn - latLngForFn:latLngForFn - scale:scale - size:size - showsLogo:showsLogo - overlayHandler:overlayHandler]; - - // Dispatch result to origin queue - dispatch_async(resultQueue, ^{ - __typeof__(self) strongself = weakself; - - if (strongself.completion) { - - if (!compositedImage) { - NSDictionary *userInfo = @{NSLocalizedDescriptionKey: @"Failed to generate composited snapshot."}; - NSError *error = [NSError errorWithDomain:MGLErrorDomain - code:MGLErrorCodeSnapshotFailed - userInfo:userInfo]; - - strongself.completion(nil, error); - strongself.completion = nil; - } else { - MGLMapSnapshot* snapshot = [[MGLMapSnapshot alloc] initWithImage:compositedImage - scale:scale - pointForFn:pointForFn - latLngForFn:latLngForFn]; - strongself.completion(snapshot, nil); - strongself.completion = nil; - } - } - }); +MGLMapSnapshot *MGLSnapshotWithDecoratedImage(MGLImage *mglImage, MGLMapSnapshotOptions *options, mbgl::MapSnapshotter::Attributions attributions, mbgl::MapSnapshotter::PointForFn pointForFn, mbgl::MapSnapshotter::LatLngForFn latLngForFn, MGLMapSnapshotOverlayHandler overlayHandler, NSError * _Nullable *outError) { + MGLImage *compositedImage = MGLAttributedSnapshot(attributions, mglImage, options, ^{ + if (!overlayHandler) { + return; + } +#if TARGET_OS_IPHONE + CGContextRef context = UIGraphicsGetCurrentContext(); + if (!context) { + return; + } + MGLMapSnapshotOverlay *snapshotOverlay = [[MGLMapSnapshotOverlay alloc] initWithContext:context + scale:options.scale + pointForFn:pointForFn + latLngForFn:latLngForFn]; + CGContextSaveGState(context); + overlayHandler(snapshotOverlay); + CGContextRestoreGState(context); +#else + NSGraphicsContext *context = [NSGraphicsContext currentContext]; + if (!context) { + return; + } + MGLMapSnapshotOverlay *snapshotOverlay = [[MGLMapSnapshotOverlay alloc] initWithContext:context.CGContext + scale:options.scale + pointForFn:pointForFn + latLngForFn:latLngForFn]; + [context saveGraphicsState]; + overlayHandler(snapshotOverlay); + [context restoreGraphicsState]; +#endif }); + + if (compositedImage) { + return [[MGLMapSnapshot alloc] initWithImage:compositedImage + scale:options.scale + pointForFn:pointForFn + latLngForFn:latLngForFn]; + } else { + if (outError) { + NSDictionary *userInfo = @{NSLocalizedDescriptionKey: @"Failed to generate composited snapshot."}; + *outError = [NSError errorWithDomain:MGLErrorDomain + code:MGLErrorCodeSnapshotFailed + userInfo:userInfo]; + } + return nil; + } } -+ (NSArray*) generateAttributionInfos:(mbgl::MapSnapshotter::Attributions)attributions { +NSArray *MGLAttributionInfosFromAttributions(mbgl::MapSnapshotter::Attributions attributions) { NSMutableArray *infos = [NSMutableArray array]; #if TARGET_OS_IPHONE @@ -567,8 +528,8 @@ - (void)drawAttributedSnapshot:(mbgl::MapSnapshotter::Attributions)attributions CGFloat fontSize = [NSFont systemFontSizeForControlSize:NSMiniControlSize]; NSColor *attributeFontColor = [NSColor blackColor]; #endif - for (auto attribution = attributions.begin(); attribution != attributions.end(); ++attribution) { - NSString *attributionHTMLString = @(attribution->c_str()); + for (auto attribution : attributions) { + NSString *attributionHTMLString = @(attribution.c_str()); NSArray *tileSetInfos = [MGLAttributionInfo attributionInfosFromHTMLString:attributionHTMLString fontSize:fontSize linkColor:attributeFontColor]; @@ -688,57 +649,16 @@ + (CGSize)attributionTextSizeWithStyle:(MGLAttributionInfoStyle)attributionStyle - (void)cancel { MGLLogInfo(@"Cancelling snapshotter."); - self.cancelled = YES; - - if (_snapshotCallback) { - [MGLMapSnapshotter completeWithErrorCode:MGLErrorCodeSnapshotFailed - description:[NSString stringWithFormat:@"MGLMapSnapshotter cancelled from %s", __PRETTY_FUNCTION__] - onQueue:self.resultQueue - completion:self.completion]; - self.completion = nil; + + if (_mbglMapSnapshotter) { + _mbglMapSnapshotter->cancel(); } - - _snapshotCallback.reset(); _mbglMapSnapshotter.reset(); } -+ (void)completeWithErrorCode:(MGLErrorCode)errorCode description:(nonnull NSString*)description onQueue:(dispatch_queue_t)queue completion:(MGLMapSnapshotCompletionHandler)completion { - // The snapshot hasn't completed, so we should alert the caller - if (completion && queue) { - dispatch_async(queue, ^{ - NSDictionary *userInfo = @{NSLocalizedDescriptionKey: description}; - NSError *error = [NSError errorWithDomain:MGLErrorDomain - code:errorCode - userInfo:userInfo]; -#if TARGET_OS_IPHONE || TARGET_OS_SIMULATOR - [[MMEEventsManager sharedManager] reportError:error]; -#endif - completion(NULL, error); - }); - } -} - -- (void)setOptions:(MGLMapSnapshotOptions *)options -{ - if (_terminated) { - [NSException raise:NSInternalInconsistencyException - format:@"Calling MGLMapSnapshotter.options after application termination is not supported."]; - } - - MGLLogDebug(@"Setting options: %@", options); - - if (_completion) { - [self cancel]; - } - - _cancelled = NO; - _options = options; - +- (void)configureWithOptions:(MGLMapSnapshotOptions *)options { auto mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource]; - std::string styleURL = std::string([options.styleURL.absoluteString UTF8String]); - std::pair style = std::make_pair(false, styleURL); - // Size; taking into account the minimum texture size for OpenGL ES // For non retina screens the ratio is 1:1 MGLSnapshotterMinimumPixelSize mbgl::Size size = { @@ -748,6 +668,19 @@ - (void)setOptions:(MGLMapSnapshotOptions *)options float pixelRatio = MAX(options.scale, 1); + // App-global configuration + MGLRendererConfiguration *config = [MGLRendererConfiguration currentConfiguration]; + + mbgl::ResourceOptions resourceOptions; + resourceOptions.withCachePath(MGLOfflineStorage.sharedOfflineStorage.mbglCachePath) + .withAssetPath(NSBundle.mainBundle.resourceURL.path.UTF8String); + + // Create the snapshotter + _mbglMapSnapshotter = std::make_unique( + size, pixelRatio, resourceOptions, mbgl::MapSnapshotterObserver::nullObserver(), config.localFontFamilyName); + + _mbglMapSnapshotter->setStyleURL(std::string(options.styleURL.absoluteString.UTF8String)); + // Camera options mbgl::CameraOptions cameraOptions; if (CLLocationCoordinate2DIsValid(options.camera.centerCoordinate)) { @@ -756,23 +689,12 @@ - (void)setOptions:(MGLMapSnapshotOptions *)options cameraOptions.bearing = MAX(0, options.camera.heading); cameraOptions.zoom = MAX(0, options.zoomLevel); cameraOptions.pitch = MAX(0, options.camera.pitch); + _mbglMapSnapshotter->setCameraOptions(cameraOptions); // Region - mbgl::optional coordinateBounds; if (!MGLCoordinateBoundsIsEmpty(options.coordinateBounds)) { - coordinateBounds = MGLLatLngBoundsFromCoordinateBounds(options.coordinateBounds); + _mbglMapSnapshotter->setRegion(MGLLatLngBoundsFromCoordinateBounds(options.coordinateBounds)); } - - // App-global configuration - MGLRendererConfiguration* config = [MGLRendererConfiguration currentConfiguration]; - - mbgl::ResourceOptions resourceOptions; - resourceOptions.withCachePath([[MGLOfflineStorage sharedOfflineStorage] mbglCachePath]) - .withAssetPath([NSBundle mainBundle].resourceURL.path.UTF8String); - - // Create the snapshotter - _mbglMapSnapshotter = std::make_unique( - style, size, pixelRatio, cameraOptions, coordinateBounds, config.localFontFamilyName, resourceOptions); } @end diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index bd3df88dc7..b4552f8e44 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -19,6 +19,8 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208)) * Fixed issues where an offline pack would stop downloading before completion. ([#16230](https://github.com/mapbox/mapbox-gl-native/pull/16230), [#16240](https://github.com/mapbox/mapbox-gl-native/pull/16240)) * When an offline pack encounters an HTTP 404 error, the `MGLOfflinePackUserInfoKeyError` user info key of the `MGLOfflinePackErrorNotification` now indicates the resource that could not be downloaded. ([#16240](https://github.com/mapbox/mapbox-gl-native/pull/16240)) +* The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#209](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. ([#209](https://github.com/mapbox/mapbox-gl-native-ios/pull/210)) * Fixed a memory leak when zooming with any options enabled in the `MGLMapView.debugMask` property. ([#15179](https://github.com/mapbox/mapbox-gl-native/issues/15179)) * Added the `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:completionHandler:]` method for determining when the data is ready to retrieve from the cache. ([#188](https://github.com/mapbox/mapbox-gl-native-ios/pull/188)) diff --git a/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m b/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m index 9ef2054dff..e016651def 100644 --- a/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m +++ b/platform/ios/Integration Tests/Snapshotter Tests/MGLMapSnapshotterTest.m @@ -72,6 +72,27 @@ - (void)testMultipleSnapshotsWithASingleSnapshotter🔒 { [self waitForExpectations:@[expectation] timeout:10.0]; } +- (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; + + CGSize size = self.mapView.bounds.size; + CLLocationCoordinate2D coordinates = CLLocationCoordinate2DMake(30.0, 30.0); + __weak MGLMapSnapshotter *weakSnapshotter; + @autoreleasepool { + MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coordinates, size); + weakSnapshotter = snapshotter; + [snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) { + // This completion block should not 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."); + + [self waitForExpectations:@[expectation] timeout:5]; +} + - (void)testDeallocatingSnapshotterDuringSnapshot🔒 { // See also https://github.com/mapbox/mapbox-gl-native/issues/12336 @@ -93,20 +114,12 @@ - (void)testDeallocatingSnapshotterDuringSnapshot🔒 { dispatch_async(dispatch_get_main_queue(), ^{ MGLMapSnapshotter *snapshotter = snapshotterWithCoordinates(coord, size); - __weak MGLMapSnapshotter *weakSnapshotter = snapshotter; - [snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) { - // We expect this completion block to be called with an error + // This completion block should not be called. __typeof__(self) strongself = weakself; - - MGLTestAssertNil(strongself, snapshot); - MGLTestAssert(strongself, - ([error.domain isEqualToString:MGLErrorDomain] && error.code == MGLErrorCodeSnapshotFailed), - @"Should have errored"); - MGLTestAssertNil(strongself, weakSnapshotter, @"Snapshotter should have been deallocated"); - - dispatch_group_leave(dg); + MGLTestFail(strongself, @"Completion handler should not be called after snapshotter is deallocated."); }]; + dispatch_group_leave(dg); }); dispatch_group_notify(dg, dispatch_get_main_queue(), ^{ @@ -162,10 +175,6 @@ - (void)testSnapshotterUsingNestedDispatchQueues🔒 { } - (void)testCancellingSnapshot🔒 { - XCTestExpectation *expectation = [self expectationWithDescription:@"snapshots"]; - expectation.assertForOverFulfill = YES; - expectation.expectedFulfillmentCount = 1; - CGSize size = self.mapView.bounds.size; CLLocationCoordinate2D coord = CLLocationCoordinate2DMake(30.0, 30.0); @@ -174,20 +183,11 @@ - (void)testCancellingSnapshot🔒 { __weak __typeof__(self) weakself = self; [snapshotter startWithCompletionHandler:^(MGLMapSnapshot * _Nullable snapshot, NSError * _Nullable error) { - // We expect this completion block to be called with an error __typeof__(self) strongself = weakself; - - MGLTestAssertNil(strongself, snapshot); - MGLTestAssert(strongself, - ([error.domain isEqualToString:MGLErrorDomain] && error.code == MGLErrorCodeSnapshotFailed), - @"Should have been cancelled"); - MGLTestAssert(strongself, snapshotter.cancelled, @"Should have been cancelled"); - [expectation fulfill]; + MGLTestFail(strongself, @"Completion handler should not be called when canceling."); }]; [snapshotter cancel]; - - [self waitForExpectations:@[expectation] timeout:5.0]; } - (void)testAllocatingSnapshotOnBackgroundQueue🔒 { diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md index 504c49d73e..a7625208aa 100644 --- a/platform/macos/CHANGELOG.md +++ b/platform/macos/CHANGELOG.md @@ -43,6 +43,12 @@ * Fixed an issue where `-[MGLMapView visibleFeaturesInRect:]` and `-[MGLShapeSource featuresMatchingPredicate:]` could return incorrect coordinates at zoom levels 20 and higher. ([#15560](https://github.com/mapbox/mapbox-gl-native/pull/15560)) * Improved feature querying performance. ([#14930](https://github.com/mapbox/mapbox-gl-native/pull/14930)) +### 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)) +* The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#209](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. ([#209](https://github.com/mapbox/mapbox-gl-native-ios/pull/210)) + ### Networking and storage * Ideographic glyphs from Chinese, Japanese, and Korean are no longer downloaded by default as part of offline packs; they are instead rendered on-device, saving bandwidth and storage while improving performance. ([#14176](https://github.com/mapbox/mapbox-gl-native/pull/14176)) @@ -55,7 +61,6 @@ ### Other changes -* 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)) * Fixed a memory leak when zooming with any options enabled in the `MGLMapView.debugMask` property. ([#15179](https://github.com/mapbox/mapbox-gl-native/issues/15179)) * `MGLLoggingLevel` has been updated to better match core log levels. You can now use `MGLLoggingConfiguration.loggingLevel` to filter logs from core. ([#15120](https://github.com/mapbox/mapbox-gl-native/pull/15120)) From 8998f7a314848745be02ed5f7f2477cd921ec19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Fri, 13 Mar 2020 13:55:21 -0700 Subject: [PATCH 4/6] [ios, macos] Documented performance and caching improvements in mbgl v1.4.0 --- platform/darwin/src/MGLOfflineStorage.h | 8 ++++---- platform/ios/CHANGELOG.md | 9 +++++++-- platform/macos/CHANGELOG.md | 2 ++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/platform/darwin/src/MGLOfflineStorage.h b/platform/darwin/src/MGLOfflineStorage.h index 3621e16243..50709ba66e 100644 --- a/platform/darwin/src/MGLOfflineStorage.h +++ b/platform/darwin/src/MGLOfflineStorage.h @@ -376,10 +376,10 @@ MGL_EXPORT to `0`. Setting the maximum ambient cache size does not impact the maximum size of offline packs. - While this method does not limit the space available to offline packs, - data in offline packs count towards this limit. If the maximum ambient - cache size is set to 30 MB and 20 MB of offline packs are downloaded, - there may be only 10 MB reserved for the ambient cache. + This method does not limit the space available to offline packs, and data in + offline packs does not count towards this limit. If you set the maximum ambient + cache size to 30 MB then download 20 MB of offline packs, 30 MB will remain + available for the ambient cache. This method should be called before the map and map style have been loaded. diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index b4552f8e44..f79ed04d8f 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -14,13 +14,18 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Fixed an issue where an `MGLSymbolStyleLayer.lineDashPattern` value of `{1, 0}` resulted in hairline gaps. ([#16202](https://github.com/mapbox/mapbox-gl-native/pull/16202)) * Improved the performance of loading a style that has many style images. ([#16187](https://github.com/mapbox/mapbox-gl-native/pull/16187)) -### Other changes +### Networking and storage -* Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208)) +* Downloaded offline packs no longer reduce the storage space available for ambient caching of tiles and other resources. ([#15622](https://github.com/mapbox/mapbox-gl-native/pull/15622)) * Fixed issues where an offline pack would stop downloading before completion. ([#16230](https://github.com/mapbox/mapbox-gl-native/pull/16230), [#16240](https://github.com/mapbox/mapbox-gl-native/pull/16240)) * When an offline pack encounters an HTTP 404 error, the `MGLOfflinePackUserInfoKeyError` user info key of the `MGLOfflinePackErrorNotification` now indicates the resource that could not be downloaded. ([#16240](https://github.com/mapbox/mapbox-gl-native/pull/16240)) + +### Other changes + +* Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208)) * The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#209](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. ([#209](https://github.com/mapbox/mapbox-gl-native-ios/pull/210)) +* Improved performance when continuously animating a tilted map. ([#16287](https://github.com/mapbox/mapbox-gl-native/pull/16287)) * Fixed a memory leak when zooming with any options enabled in the `MGLMapView.debugMask` property. ([#15179](https://github.com/mapbox/mapbox-gl-native/issues/15179)) * Added the `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:completionHandler:]` method for determining when the data is ready to retrieve from the cache. ([#188](https://github.com/mapbox/mapbox-gl-native-ios/pull/188)) diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md index a7625208aa..f4d5ca87d4 100644 --- a/platform/macos/CHANGELOG.md +++ b/platform/macos/CHANGELOG.md @@ -36,6 +36,7 @@ * Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208)) * Fixed an issue where it was possible to set the map’s content insets then tilt the map enough to see the horizon, causing performance issues. ([#15195](https://github.com/mapbox/mapbox-gl-native/pull/15195)) * Fixed an issue where animated camera transitions zoomed in or out too dramatically. ([#15281](https://github.com/mapbox/mapbox-gl-native/pull/15281)) +* Improved performance when continuously animating a tilted map. ([#16287](https://github.com/mapbox/mapbox-gl-native/pull/16287)) ### Feature querying @@ -52,6 +53,7 @@ ### Networking and storage * Ideographic glyphs from Chinese, Japanese, and Korean are no longer downloaded by default as part of offline packs; they are instead rendered on-device, saving bandwidth and storage while improving performance. ([#14176](https://github.com/mapbox/mapbox-gl-native/pull/14176)) +* Downloaded offline packs no longer reduce the storage space available for ambient caching of tiles and other resources. ([#15622](https://github.com/mapbox/mapbox-gl-native/pull/15622)) * Added the `MGLMapView.prefetchesTiles` property to configure lower-resolution tile prefetching behavior. ([#14816](https://github.com/mapbox/mapbox-gl-native/pull/14816)) * Fixed a crash when `-[MGLOfflinePack invalidate]` is called on different threads. ([#15582](https://github.com/mapbox/mapbox-gl-native/pull/15582)) * Fixed issues where an offline pack would stop downloading before completion. ([#16230](https://github.com/mapbox/mapbox-gl-native/pull/16230), [#16240](https://github.com/mapbox/mapbox-gl-native/pull/16240)) From 8148cb7ff99bbefde7be68a234cacabd7a87447f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Fri, 13 Mar 2020 15:41:05 -0700 Subject: [PATCH 5/6] [ios, macos] Document changes to snapshotter memory management --- platform/darwin/src/MGLMapSnapshotter.h | 24 ++++++++++++++++++- .../test/MGLDocumentationExampleTests.swift | 4 +++- platform/ios/CHANGELOG.md | 8 +++++-- platform/macos/CHANGELOG.md | 5 ++-- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/platform/darwin/src/MGLMapSnapshotter.h b/platform/darwin/src/MGLMapSnapshotter.h index ed7a8c2e5d..d8668b28c9 100644 --- a/platform/darwin/src/MGLMapSnapshotter.h +++ b/platform/darwin/src/MGLMapSnapshotter.h @@ -185,6 +185,11 @@ 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. @@ -203,7 +208,8 @@ typedef void (^MGLMapSnapshotCompletionHandler)(MGLMapSnapshot* _Nullable snapsh let options = MGLMapSnapshotOptions(styleURL: MGLStyle.satelliteStreetsStyleURL, camera: camera, size: CGSize(width: 320, height: 480)) options.zoomLevel = 10 - let snapshotter = MGLMapSnapshotter(options: options) + // The containing class should hold a strong reference to this object. + snapshotter = MGLMapSnapshotter(options: options) snapshotter.start { (snapshot, error) in if let error = error { fatalError(error.localizedDescription) @@ -236,6 +242,11 @@ 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 handle the result in. */ - (void)startWithCompletionHandler:(MGLMapSnapshotCompletionHandler)completionHandler; @@ -244,6 +255,11 @@ 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 to handle the result on. @param completionHandler The block to handle the result in. */ @@ -253,6 +269,12 @@ MGL_EXPORT Starts the snapshot creation and executes the specified blocks with the result 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 handle manipulation of the `MGLMapSnapshotter`'s `CGContext`. @param completionHandler The block to handle the result in. */ diff --git a/platform/darwin/test/MGLDocumentationExampleTests.swift b/platform/darwin/test/MGLDocumentationExampleTests.swift index 7d6bdbed54..0c12561115 100644 --- a/platform/darwin/test/MGLDocumentationExampleTests.swift +++ b/platform/darwin/test/MGLDocumentationExampleTests.swift @@ -463,13 +463,15 @@ 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 - let snapshotter = MGLMapSnapshotter(options: options) + // The containing class should hold a strong reference to this object. + snapshotter = MGLMapSnapshotter(options: options) snapshotter.start { (snapshot, error) in if let error = error { fatalError(error.localizedDescription) diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index f79ed04d8f..4a315a2a4e 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -14,6 +14,12 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Fixed an issue where an `MGLSymbolStyleLayer.lineDashPattern` value of `{1, 0}` resulted in hairline gaps. ([#16202](https://github.com/mapbox/mapbox-gl-native/pull/16202)) * Improved the performance of loading a style that has many style images. ([#16187](https://github.com/mapbox/mapbox-gl-native/pull/16187)) +### 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)) +* 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)) + ### Networking and storage * Downloaded offline packs no longer reduce the storage space available for ambient caching of tiles and other resources. ([#15622](https://github.com/mapbox/mapbox-gl-native/pull/15622)) @@ -23,8 +29,6 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT ### Other changes * Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208)) -* The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#209](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. ([#209](https://github.com/mapbox/mapbox-gl-native-ios/pull/210)) * Improved performance when continuously animating a tilted map. ([#16287](https://github.com/mapbox/mapbox-gl-native/pull/16287)) * Fixed a memory leak when zooming with any options enabled in the `MGLMapView.debugMask` property. ([#15179](https://github.com/mapbox/mapbox-gl-native/issues/15179)) * Added the `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:completionHandler:]` method for determining when the data is ready to retrieve from the cache. ([#188](https://github.com/mapbox/mapbox-gl-native-ios/pull/188)) diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md index f4d5ca87d4..61acbcb424 100644 --- a/platform/macos/CHANGELOG.md +++ b/platform/macos/CHANGELOG.md @@ -47,8 +47,9 @@ ### 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)) -* The `-[MGLMapSnapshotter cancel]` method no longer calls the completion handler passed into `-[MGLMapSnapshotter startWithCompletionHandler:]`. ([#209](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. ([#209](https://github.com/mapbox/mapbox-gl-native-ios/pull/210)) +* `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)) +* 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)) ### Networking and storage From d357f8c511ff30689def233bf7638d9c101dca50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Mon, 16 Mar 2020 11:29:01 -0700 Subject: [PATCH 6/6] [ios, macos] Clarify threading in snapshotter documentation --- platform/darwin/src/MGLMapSnapshotter.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/platform/darwin/src/MGLMapSnapshotter.h b/platform/darwin/src/MGLMapSnapshotter.h index d8668b28c9..e30830ebc1 100644 --- a/platform/darwin/src/MGLMapSnapshotter.h +++ b/platform/darwin/src/MGLMapSnapshotter.h @@ -247,7 +247,8 @@ MGL_EXPORT 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 handle the result in. + @param completionHandler The block to call with a finished snapshot. The block + is executed on the main queue. */ - (void)startWithCompletionHandler:(MGLMapSnapshotCompletionHandler)completionHandler; @@ -260,23 +261,29 @@ MGL_EXPORT reference to it, for example by storing it in an instance variable of the class where you call this method. - @param queue The queue to handle the result on. - @param completionHandler The block to handle the result in. + @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 + is executed on the queue specified in the `queue` parameter. */ - (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshotCompletionHandler)completionHandler; /** Starts the snapshot creation and executes the specified blocks with the result - on the specified queue. Use this option if you want to add custom drawing on top of the - resulting `MGLMapSnapshot`. + 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 handle manipulation of the `MGLMapSnapshotter`'s `CGContext`. - @param completionHandler The block to handle the result in. + @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 + background queue. + @param completionHandler The block to call with a finished snapshot. The block + is executed on the main queue. */ - (void)startWithOverlayHandler:(MGLMapSnapshotOverlayHandler)overlayHandler completionHandler:(MGLMapSnapshotCompletionHandler)completionHandler;