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

[ios, macos] Improve snap shotter documentation. #10020

Merged
merged 11 commits into from
Oct 4, 2017
65 changes: 44 additions & 21 deletions platform/darwin/src/MGLMapSnapshotter.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,52 @@ MGL_EXPORT
@interface MGLMapSnapshotOptions : NSObject

/**
Creates a set of options with the minimum required information
@param styleURL the style url to use
@param camera the camera settings
@param size the image size
Creates a set of options with the minimum required information.

@param styleURL URL of the map style to snapshot. The URL may be a full HTTP or HTTPS URL,
a Mapbox URL indicating the style’s map ID (`mapbox://styles/{user}/{style`}), or a path
to a local file relative to the application’s resource path. Specify `nil` for the default style.
@param size The image size.
*/
- (instancetype)initWithStyleURL:(NSURL*)styleURL camera:(MGLMapCamera*)camera size:(CGSize)size;
- (instancetype)initWithStyleURL:(nullable NSURL *)styleURL camera:(MGLMapCamera *)camera size:(CGSize)size;

#pragma mark - Configuring the map
#pragma mark - Configuring the Map

/**
The style URL for these options.
URL of the map style to snapshot.
*/
@property (nonatomic, readonly) NSURL* styleURL;
@property (nonatomic, readonly) NSURL *styleURL;

/**
The zoom. Default is 0.
The zoom level.

The default zoom level is 0. If this property is non-zero and the camera property
is non-nil, the camera’s altitude is ignored in favor of this property’s value.
*/
@property (nonatomic) double zoom;
@property (nonatomic) double zoomLevel;
Copy link
Contributor

Choose a reason for hiding this comment

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

It sure would be nice to implement #5583 instead of offering three different ways to set the viewport on a snapshot that interact in unintuitive ways.


/**
The `MGLMapcamera` options to use.
A camera representing the viewport visible in the snapshot.
Copy link
Contributor

Choose a reason for hiding this comment

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

A camera representing the viewport visible in the snapshot.

If this property is non-nil and the coordinateBounds property is set to a non-empty coordinate bounds, the camera’s center coordinate and altitude are ignored in favor of the coordinateBounds property.


If this property is non-nil and the `coordinateBounds` property is set to a non-empty
coordinate bounds, the camera’s center coordinate and altitude are ignored in favor
of the `coordinateBounds` property.
*/
@property (nonatomic) MGLMapCamera* camera;
@property (nonatomic) MGLMapCamera *camera;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is styleURL read-only but camera read-write? Are there situations in which it’s unsafe to change the camera? If so, we should document or fix them.


/**
A region to capture. Overrides the center coordinate
in the mapCamera options if set
The cooordinate rectangle that encompasses the bounds to capture.

If this property is non-empty and the camera property is non-nil, the camera’s
center coordinate and altitude are ignored in favor of this property’s value.
*/
@property (nonatomic) MGLCoordinateBounds region;
@property (nonatomic) MGLCoordinateBounds coordinateBounds;

#pragma mark - Configuring the image
#pragma mark - Configuring the Image

/**
The size of the output image. Minimum is 64x64
The size of the output image, measured in points.

*/
@property (nonatomic, readonly) CGSize size;

Expand All @@ -57,18 +69,26 @@ MGL_EXPORT

@end

#if TARGET_OS_IPHONE
/**
A block to processes the result or error of a snapshot request.

The result will be either an `MGLImage` or a `NSError`
@param snapshot The `UIImage` that was generated or `nil` if an error occurred.
@param error The error that occured or `nil` when successful.
*/
typedef void (^MGLMapSnapshotCompletionHandler)(UIImage* _Nullable snapshot, NSError* _Nullable error);
#else
/**
A block to processes the result or error of a snapshot request.

@param snapshot The image that was generated or `nil` if an error occurred.
@param snapshot The `NSImage` that was generated or `nil` if an error occurred.
@param error The eror that occured or `nil` when succesful.
*/
typedef void (^MGLMapSnapshotCompletionHandler)(MGLImage* _Nullable snapshot, NSError* _Nullable error);
typedef void (^MGLMapSnapshotCompletionHandler)(NSImage* _Nullable snapshot, NSError* _Nullable error);
#endif

/**
A utility object for capturing map-based images.
An immutable utility object for capturing map-based images.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

As tail work, it would be helpful to add a small code example of working with MGLMapSnapshotter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tail work ticket in our ios-sdk repo https://github.com/mapbox/ios-sdk/issues/332

Copy link
Contributor

Choose a reason for hiding this comment

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

I was referring to a short code example here inside the header, similar to what we do for classes like MGLShapeSource and MGLMapView, so that the example shows up at the beginning of the MGLMapSnapshotter class reference.

MGL_EXPORT
@interface MGLMapSnapshotter : NSObject
Expand All @@ -92,6 +112,9 @@ MGL_EXPORT

/**
Cancels the snapshot creation request, if any.

Once you call this method, you cannot resume the snapshot. In order to obtain the
snapshot, create a new `MGLMapSnapshotter` object.
*/
- (void)cancel;

Expand Down
78 changes: 51 additions & 27 deletions platform/darwin/src/MGLMapSnapshotter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,27 @@
#import "MGLOfflineStorage_Private.h"
#import "MGLGeometry_Private.h"
#import "NSBundle+MGLAdditions.h"
#import "MGLStyle.h"

#if TARGET_OS_IPHONE
#import "UIImage+MGLAdditions.h"
#else
#import "NSImage+MGLAdditions.h"
#endif

const CGPoint MGLLogoImagePosition = CGPointMake(8, 8);
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be interesting to make this position configurable as tail work.

const CGFloat MGLSnapshotterMinimumPixelSize = 64;

@implementation MGLMapSnapshotOptions

- (instancetype _Nonnull)initWithStyleURL:(NSURL* _Nonnull)styleURL camera:(MGLMapCamera*)camera size:(CGSize) size;
- (instancetype _Nonnull)initWithStyleURL:(nullable NSURL*)styleURL camera:(MGLMapCamera*)camera size:(CGSize) size;
{
self = [super init];
if (self) {
if ( !styleURL)
{
styleURL = [MGLStyle streetsStyleURLWithVersion:MGLStyleDefaultVersion];
}
_styleURL = styleURL;
_size = size;
_camera = camera;
Expand All @@ -41,28 +49,34 @@ - (instancetype _Nonnull)initWithStyleURL:(NSURL* _Nonnull)styleURL camera:(MGLM

@end

@interface MGLMapSnapshotter()
@property (nonatomic) MGLMapSnapshotOptions *options;
@end

@implementation MGLMapSnapshotter {

std::shared_ptr<mbgl::ThreadPool> mbglThreadPool;
std::unique_ptr<mbgl::MapSnapshotter> mbglMapSnapshotter;
std::unique_ptr<mbgl::Actor<mbgl::MapSnapshotter::Callback>> snapshotCallback;
std::shared_ptr<mbgl::ThreadPool> _mbglThreadPool;
std::unique_ptr<mbgl::MapSnapshotter> _mbglMapSnapshotter;
std::unique_ptr<mbgl::Actor<mbgl::MapSnapshotter::Callback>> _snapshotCallback;
}

- (instancetype)initWithOptions:(MGLMapSnapshotOptions*)options;
{
self = [super init];
if (self) {
_options = options;
_loading = false;

mbgl::DefaultFileSource *mbglFileSource = [MGLOfflineStorage sharedOfflineStorage].mbglFileSource;
mbglThreadPool = mbgl::sharedThreadPool();
_mbglThreadPool = mbgl::sharedThreadPool();

std::string styleURL = std::string([options.styleURL.absoluteString UTF8String]);

// Size; taking into account the minimum texture size for OpenGL ES
// For non retina screens the ratio is 1:1 MGLSnapshotterMinimumPixelSize
mbgl::Size size = {
static_cast<uint32_t>(MAX(options.size.width, 64)),
static_cast<uint32_t>(MAX(options.size.height, 64))
static_cast<uint32_t>(MAX(options.size.width, MGLSnapshotterMinimumPixelSize)),
static_cast<uint32_t>(MAX(options.size.height, MGLSnapshotterMinimumPixelSize))
};

float pixelRatio = MAX(options.scale, 1);
Expand All @@ -73,17 +87,17 @@ - (instancetype)initWithOptions:(MGLMapSnapshotOptions*)options;
cameraOptions.center = MGLLatLngFromLocationCoordinate2D(options.camera.centerCoordinate);
}
cameraOptions.angle = MAX(0, options.camera.heading) * mbgl::util::DEG2RAD;
cameraOptions.zoom = MAX(0, options.zoom);
cameraOptions.zoom = MAX(0, options.zoomLevel);
cameraOptions.pitch = MAX(0, options.camera.pitch);

// Region
mbgl::optional<mbgl::LatLngBounds> region;
if (!MGLCoordinateBoundsIsEmpty(options.region)) {
region = MGLLatLngBoundsFromCoordinateBounds(options.region);
mbgl::optional<mbgl::LatLngBounds> coordinateBounds;
if (!MGLCoordinateBoundsIsEmpty(options.coordinateBounds)) {
coordinateBounds = MGLLatLngBoundsFromCoordinateBounds(options.coordinateBounds);
}

// Create the snapshotter
mbglMapSnapshotter = std::make_unique<mbgl::MapSnapshotter>(*mbglFileSource, *mbglThreadPool, styleURL, size, pixelRatio, cameraOptions, region);
_mbglMapSnapshotter = std::make_unique<mbgl::MapSnapshotter>(*mbglFileSource, *_mbglThreadPool, styleURL, size, pixelRatio, cameraOptions, coordinateBounds);
}
return self;
}
Expand All @@ -96,51 +110,61 @@ - (void)startWithCompletionHandler:(MGLMapSnapshotCompletionHandler)completion;
- (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshotCompletionHandler)completion;
{
if ([self isLoading]) {
NSDictionary *userInfo = @{NSLocalizedDescriptionKey: @"Already started this snapshotter"};
NSError *error = [NSError errorWithDomain:MGLErrorDomain code:1 userInfo:userInfo];
dispatch_async(queue, ^{
completion(nil, error);
});
return;
[NSException raise:@"MGLAlreadyStartedSnapshotterException"
Copy link
Contributor

Choose a reason for hiding this comment

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

Per #7258, use one of Foundation's predefined exception names or pull this string out into a public constant.

format:@"Already started this snapshotter."];
}

_loading = true;

dispatch_async(queue, ^{
snapshotCallback = std::make_unique<mbgl::Actor<mbgl::MapSnapshotter::Callback>>(*mbgl::Scheduler::GetCurrent(), [=](std::exception_ptr mbglError, mbgl::PremultipliedImage image) {
_snapshotCallback = std::make_unique<mbgl::Actor<mbgl::MapSnapshotter::Callback>>(*mbgl::Scheduler::GetCurrent(), [=](std::exception_ptr mbglError, mbgl::PremultipliedImage image) {
_loading = false;
if (mbglError) {
NSString *description = @(mbgl::util::toString(mbglError).c_str());
NSDictionary *userInfo = @{NSLocalizedDescriptionKey: description};
NSError *error = [NSError errorWithDomain:MGLErrorDomain code:1 userInfo:userInfo];
NSError *error = [NSError errorWithDomain:MGLErrorDomain code:MGLErrorCodeSnapshotFailed userInfo:userInfo];
Copy link
Contributor

Choose a reason for hiding this comment

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

Per #10020 (comment), should we remove this error or replace it with an NSException?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense let the developer know that it has a snapshotter task running it's easy to forget this. A NSException is too drastic, I would like to continue with the NSError for the time being and evaluate this again once we made the snapshotter reusable.

Copy link
Contributor

Choose a reason for hiding this comment

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

To elaborate, NSError is intended to problems outside the developer’s control, for issues that can’t be fixed in the developer’s own code. If the developer is reusing a snapshotter in their own code, I think that would be an error on their part and an appropriate use of NSException. But if MapKit simply sets the NSError in this case, then I guess it’s OK to follow their lead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you NSError is intended to problems outside the developer's control. NSException is a better choice. Making the changes.


// Dispatch result to origin queue
dispatch_async(queue, ^{
completion(nil, error);
});
} else {
#if TARGET_OS_IPHONE
MGLImage *mglImage = [[MGLImage alloc] initWithMGLPremultipliedImage:std::move(image) scale:self.options.scale];
#else
MGLImage *mglImage = [[MGLImage alloc] initWithMGLPremultipliedImage:std::move(image)];
#endif

// Process image watermark in a work queue
dispatch_queue_t workQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
dispatch_async(workQueue, ^{
#if TARGET_OS_IPHONE
UIImage *logoImage = [UIImage imageNamed:@"mapbox" inBundle:[NSBundle mgl_frameworkBundle] compatibleWithTraitCollection:nil];

UIGraphicsBeginImageContext(mglImage.size);
UIGraphicsBeginImageContextWithOptions(mglImage.size, NO, self.options.scale);

[mglImage drawInRect:CGRectMake(0, 0, mglImage.size.width, mglImage.size.height)];
[logoImage drawInRect:CGRectMake(8, mglImage.size.height - (8 + logoImage.size.height), logoImage.size.width,logoImage.size.height)];
[logoImage drawInRect:CGRectMake(MGLLogoImagePosition.x, mglImage.size.height - (MGLLogoImagePosition.y + logoImage.size.height), logoImage.size.width,logoImage.size.height)];
UIImage *compositedImage = UIGraphicsGetImageFromCurrentImageContext();

UIGraphicsEndImageContext();
#else
NSImage *logoImage = [[NSImage alloc] initWithContentsOfFile:[[NSBundle mgl_frameworkBundle] pathForResource:@"mapbox" ofType:@"pdf"]];
NSImage *compositedImage = mglImage;
NSImage *sourceImage = mglImage;

NSSize targetSize = NSMakeSize(self.options.size.width, self.options.size.height);
NSRect targetFrame = NSMakeRect(0, 0, targetSize.width, targetSize.height);
NSImage *compositedImage = nil;
NSImageRep *sourceImageRep = [sourceImage bestRepresentationForRect:targetFrame
context:nil
hints:nil];
compositedImage = [[NSImage alloc] initWithSize:targetSize];

[compositedImage lockFocus];
[logoImage drawInRect:CGRectMake(8, 8, logoImage.size.width,logoImage.size.height)];
[sourceImageRep drawInRect: targetFrame];
[logoImage drawInRect:CGRectMake(MGLLogoImagePosition.x, MGLLogoImagePosition.y, logoImage.size.width,logoImage.size.height)];
[compositedImage unlockFocus];

#endif

// Dispatch result to origin queue
Expand All @@ -150,14 +174,14 @@ - (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshot
});
}
});
mbglMapSnapshotter->snapshot(snapshotCallback->self());
_mbglMapSnapshotter->snapshot(_snapshotCallback->self());
});
}

- (void)cancel;
{
snapshotCallback.reset();
mbglMapSnapshotter.reset();
_snapshotCallback.reset();
_mbglMapSnapshotter.reset();
}

@end
2 changes: 2 additions & 0 deletions platform/darwin/src/MGLTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ typedef NS_ENUM(NSInteger, MGLErrorCode) {
MGLErrorCodeParseStyleFailed = 4,
/** An attempt to load the style failed. */
MGLErrorCodeLoadStyleFailed = 5,
/** An error occurred while snapshotting the map. */
MGLErrorCodeSnapshotFailed = 6,
};

/** Options for enabling debugging features in an `MGLMapView` instance. */
Expand Down
28 changes: 14 additions & 14 deletions platform/ios/app/MBXSnapshotsViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,36 @@ @interface MBXSnapshotsViewController ()

@implementation MBXSnapshotsViewController {
// Top row
MGLMapSnapshotter* snapshotterTL;
MGLMapSnapshotter* snapshotterTM;
MGLMapSnapshotter* snapshotterTR;
MGLMapSnapshotter* topLeftSnapshotter;
MGLMapSnapshotter* topCenterSnapshotter;
MGLMapSnapshotter* topRightSnapshotter;

// Bottom row
MGLMapSnapshotter* snapshotterBL;
MGLMapSnapshotter* snapshotterBM;
MGLMapSnapshotter* snapshotterBR;
MGLMapSnapshotter* bottomLeftSnapshotter;
MGLMapSnapshotter* bottomCenterSnapshotter;
MGLMapSnapshotter* bottomRightSnapshotter;
}

- (void)viewDidLoad {
[super viewDidLoad];

// Start snapshotters
snapshotterTL = [self startSnapshotterForImageView:_snapshotImageViewTL coordinates:CLLocationCoordinate2DMake(37.7184, -122.4365)];
snapshotterTM = [self startSnapshotterForImageView:_snapshotImageViewTM coordinates:CLLocationCoordinate2DMake(38.8936, -77.0146)];
snapshotterTR = [self startSnapshotterForImageView:_snapshotImageViewTR coordinates:CLLocationCoordinate2DMake(-13.1356, -74.2442)];
topLeftSnapshotter = [self startSnapshotterForImageView:_snapshotImageViewTL coordinates:CLLocationCoordinate2DMake(37.7184, -122.4365)];
topCenterSnapshotter = [self startSnapshotterForImageView:_snapshotImageViewTM coordinates:CLLocationCoordinate2DMake(38.8936, -77.0146)];
topRightSnapshotter = [self startSnapshotterForImageView:_snapshotImageViewTR coordinates:CLLocationCoordinate2DMake(-13.1356, -74.2442)];

snapshotterBL = [self startSnapshotterForImageView:_snapshotImageViewBL coordinates:CLLocationCoordinate2DMake(52.5072, 13.4247)];
snapshotterBM = [self startSnapshotterForImageView:_snapshotImageViewBM coordinates:CLLocationCoordinate2DMake(60.2118, 24.6754)];
snapshotterBR = [self startSnapshotterForImageView:_snapshotImageViewBR coordinates:CLLocationCoordinate2DMake(31.2780, 121.4286)];
bottomLeftSnapshotter = [self startSnapshotterForImageView:_snapshotImageViewBL coordinates:CLLocationCoordinate2DMake(52.5072, 13.4247)];
bottomCenterSnapshotter = [self startSnapshotterForImageView:_snapshotImageViewBM coordinates:CLLocationCoordinate2DMake(60.2118, 24.6754)];
bottomRightSnapshotter = [self startSnapshotterForImageView:_snapshotImageViewBR coordinates:CLLocationCoordinate2DMake(31.2780, 121.4286)];
}

- (MGLMapSnapshotter*) startSnapshotterForImageView:(UIImageView*) imageView coordinates:(CLLocationCoordinate2D) coordinates {
// Create snapshot options
MGLMapCamera* mapCamera = [[MGLMapCamera alloc] init];
mapCamera.pitch = 20;
mapCamera.centerCoordinate = coordinates;
MGLMapSnapshotOptions* options = [[MGLMapSnapshotOptions alloc] initWithStyleURL:[NSURL URLWithString:@"mapbox://styles/mapbox/traffic-day-v2"] camera:mapCamera size:CGSizeMake(imageView.frame.size.width, imageView.frame.size.height)];
options.zoom = 10;
MGLMapSnapshotOptions* options = [[MGLMapSnapshotOptions alloc] initWithStyleURL:[MGLStyle satelliteStreetsStyleURL] camera:mapCamera size:CGSizeMake(imageView.frame.size.width, imageView.frame.size.height)];
options.zoomLevel = 10;

// Create and start the snapshotter
MGLMapSnapshotter* snapshotter = [[MGLMapSnapshotter alloc] initWithOptions:options];
Expand Down
2 changes: 1 addition & 1 deletion platform/ios/src/UIImage+MGLAdditions.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ NS_ASSUME_NONNULL_BEGIN

- (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)styleImage;

- (nullable instancetype)initWithMGLPremultipliedImage:(const mbgl::PremultipliedImage&&)mbglImage;
- (nullable instancetype)initWithMGLPremultipliedImage:(const mbgl::PremultipliedImage&&)mbglImage scale:(CGFloat)scale;

- (std::unique_ptr<mbgl::style::Image>)mgl_styleImageWithIdentifier:(NSString *)identifier;

Expand Down
4 changes: 2 additions & 2 deletions platform/ios/src/UIImage+MGLAdditions.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ - (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)style
return self;
}

- (nullable instancetype)initWithMGLPremultipliedImage:(const mbgl::PremultipliedImage&&)mbglImage
- (nullable instancetype)initWithMGLPremultipliedImage:(const mbgl::PremultipliedImage&&)mbglImage scale:(CGFloat)scale
{
CGImageRef image = CGImageFromMGLPremultipliedImage(mbglImage.clone());
if (!image) {
return nil;
}

self = [self initWithCGImage:image scale:1.0 orientation:UIImageOrientationUp];
self = [self initWithCGImage:image scale:scale orientation:UIImageOrientationUp];

CGImageRelease(image);
return self;
Expand Down
2 changes: 1 addition & 1 deletion platform/macos/app/Base.lproj/MainMenu.xib
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
<action selector="revertDocumentToSaved:" target="-1" id="iJ3-Pv-kwq"/>
</connections>
</menuItem>
<menuItem title="Save snapshot" id="vjX-0E-kLO">
<menuItem title="Export Image…" id="vjX-0E-kLO">
<modifierMask key="keyEquivalentModifierMask"/>
<connections>
<action selector="takeSnapshot:" target="-1" id="H06-sU-n4U"/>
Expand Down
Loading