Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions platform/darwin/src/MGLMapSnapshotter.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#import <Foundation/Foundation.h>
#import "MGLTypes.h"
#import "MGLGeometry.h"
#import "MGLMapCamera.h"

NS_ASSUME_NONNULL_BEGIN

MGL_EXPORT
/**
The options to use when creating images with the `MGLMapsnapshotter`.
*/
@interface MGLMapSnapshotOptions : NSObject

/**
Creates a set of options with the minimum required information
@param styleURL the style url to use
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Start with a capital letter and end with a period. This makes things more consistent in the event that we have to cram more than one sentence in a parameter description, which isn’t uncommon.

@param camera the camera settings
@param size the image size
*/
- (instancetype)initWithStyleURL:(NSURL*)styleURL camera:(MGLMapCamera*)camera size:(CGSize)size;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to allow the style URL to be nil and default to Mapbox Streets, as MGLMapView and MGLOfflineStorage do?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@1ec5 could you please help me understand why defaulting to Mapbox streets is a good option.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When you create an MGLMapView or MGLOfflineRegion with a nil style URL, we default to the current version of Mapbox Streets:

styleURL = [MGLStyle streetsStyleURLWithVersion:MGLStyleDefaultVersion];

styleURL = [MGLStyle streetsStyleURLWithVersion:MGLStyleDefaultVersion];

For consistency with these APIs, I think we should treat a nil style URL here the same way.


#pragma mark - Configuring the map
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For our public headers, we use Title Case in marks, since jazzy uses the marks to produce section headings in the generated documentation. We could move to sentence case for that, but it would be nice to do it consistently across all the headers when we make that decision.


/**
The style URL for these options.
*/
@property (nonatomic, readonly) NSURL* styleURL;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI, the predominant style in our Objective-C code is to place the space before the asterisk. I realize this is different than the predominant style for our C++ code.


/**
The zoom. Default is 0.
*/
@property (nonatomic) double zoom;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The iOS/macOS APIs consistently use the term “zoom level” rather than “zoom” for this concept. The main reason is that “zoom” is also a verb; in Objective-C, [mapView zoom] looks like a call to an action method rather than a reference to a property getter.


/**
The `MGLMapcamera` options to use.
*/
@property (nonatomic) MGLMapCamera* camera;

/**
A region to capture. Overrides the center coordinate
in the mapCamera options if set
*/
@property (nonatomic) MGLCoordinateBounds region;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of the API uses the term “coordinate bounds” instead of “region”, although there is some holdover from MapKit in the delegate methods. It’s a confusing situation overall, but I think it would be slightly less confusing to use coordinateBounds here, to align with MGLMapView.visibleCoordinateBounds.


#pragma mark - Configuring the image

/**
The size of the output image. Minimum is 64x64
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this size measured in points or pixels?

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

/**
The scale of the output image. Defaults to the main screen scale.
Minimum is 1.
*/
@property (nonatomic) CGFloat scale;

@end

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

The result will be either an `MGLImage` or a `NSError`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MGLImage is an unfamiliar typedef that we don’t document anywhere. Instead, create duplicate declarations, one using NSImage and the other using UIImage, and have the compiler switch between them using conditional compilation. See MGLStyle.h for an example.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There’s nothing technically wrong with using MGLImage as the return value, since the compiler (and thus jazzy) will automatically substitute NSImage or UIImage as appropriate. But neither Quick Help nor jazzy will replace MGLImage inside a documentation comment. The same goes for MGLColor; that’s why we use conditional compilation whenever we need to refer specifically to the underlying type in documentation.

Besides, this sentence isn’t accurate: the block doesn’t return anything (void), so there’s no result to speak of. Rather, I think the intention was to say, “Either snapshot or error will be non-nil,” but that’s already suitably captured in the following two @param lines. So I think this line can simply be removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What I meant is since MGLStyle.h is not using conditional compilation I should go ahead and add it there too.


@param snapshot The image 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);

/**
A utility object for capturing map-based images.
*/
MGL_EXPORT
@interface MGLMapSnapshotter : NSObject

- (instancetype)initWithOptions:(MGLMapSnapshotOptions*)options;

/**
Starts the snapshot creation and executes the specified block with the result.

@param completionHandler The block to handle the result in.
*/
- (void)startWithCompletionHandler:(MGLMapSnapshotCompletionHandler)completionHandler;

/**
Starts the snapshot creation and executes the specified block with the result on the specified queue.

@param queue The queue to handle the result on.
@param completionHandler The block to handle the result in.
*/
- (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshotCompletionHandler)completionHandler;

/**
Cancels the snapshot creation request, if any.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this method be called more than once on the same snapshotter object?

*/
- (void)cancel;

/**
Indicates whether as snapshot is currently being generated.
*/
@property (nonatomic, readonly, getter=isLoading) BOOL loading;

@end

NS_ASSUME_NONNULL_END
163 changes: 163 additions & 0 deletions platform/darwin/src/MGLMapSnapshotter.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
#import "MGLMapSnapshotter.h"

#import <mbgl/actor/actor.hpp>
#import <mbgl/actor/scheduler.hpp>
#import <mbgl/util/geo.hpp>
#import <mbgl/map/map_snapshotter.hpp>
#import <mbgl/map/camera.hpp>
#import <mbgl/storage/default_file_source.hpp>
#import <mbgl/util/default_thread_pool.hpp>
#import <mbgl/util/string.hpp>
#import <mbgl/util/shared_thread_pool.hpp>

#import "MGLOfflineStorage_Private.h"
#import "MGLGeometry_Private.h"
#import "NSBundle+MGLAdditions.h"

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

@implementation MGLMapSnapshotOptions

- (instancetype _Nonnull)initWithStyleURL:(NSURL* _Nonnull)styleURL camera:(MGLMapCamera*)camera size:(CGSize) size;
{
self = [super init];
if (self) {
_styleURL = styleURL;
_size = size;
_camera = camera;
#if TARGET_OS_IPHONE
_scale = [UIScreen mainScreen].scale;
#else
_scale = [NSScreen mainScreen].backingScaleFactor;
#endif

}
return self;
}

@end

@implementation MGLMapSnapshotter {

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) {
_loading = false;

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

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

// Size; taking into account the minimum texture size for OpenGL ES
mbgl::Size size = {
static_cast<uint32_t>(MAX(options.size.width, 64)),
static_cast<uint32_t>(MAX(options.size.height, 64))
};

float pixelRatio = MAX(options.scale, 1);

// Camera options
mbgl::CameraOptions cameraOptions;
if (CLLocationCoordinate2DIsValid(options.camera.centerCoordinate)) {
cameraOptions.center = MGLLatLngFromLocationCoordinate2D(options.camera.centerCoordinate);
}
cameraOptions.angle = MAX(0, options.camera.heading) * mbgl::util::DEG2RAD;
cameraOptions.zoom = MAX(0, options.zoom);
cameraOptions.pitch = MAX(0, options.camera.pitch);

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

// Create the snapshotter
mbglMapSnapshotter = std::make_unique<mbgl::MapSnapshotter>(*mbglFileSource, *mbglThreadPool, styleURL, size, pixelRatio, cameraOptions, region);
}
return self;
}

- (void)startWithCompletionHandler:(MGLMapSnapshotCompletionHandler)completion;
{
[self startWithQueue:dispatch_get_main_queue() completionHandler:completion];
}

- (void)startWithQueue:(dispatch_queue_t)queue completionHandler:(MGLMapSnapshotCompletionHandler)completion;
{
if ([self isLoading]) {
NSDictionary *userInfo = @{NSLocalizedDescriptionKey: @"Already started this snapshotter"};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This string should be localizable, since it can be presented to the user.

NSError *error = [NSError errorWithDomain:MGLErrorDomain code:1 userInfo:userInfo];
dispatch_async(queue, ^{
completion(nil, error);
});
return;
}

_loading = true;

dispatch_async(queue, ^{
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];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This arbitrary code should be replaced with a new value of MGLErrorCode defined in MGLTypes.h.


// Dispatch result to origin queue
dispatch_async(queue, ^{
completion(nil, error);
});
} else {
MGLImage *mglImage = [[MGLImage alloc] initWithMGLPremultipliedImage:std::move(image)];

// 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);

[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)];
UIImage *compositedImage = UIGraphicsGetImageFromCurrentImageContext();

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

[compositedImage lockFocus];
[logoImage drawInRect:CGRectMake(8, 8, logoImage.size.width,logoImage.size.height)];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For maintainability, we should move these eights into a constant CGPoint.

[compositedImage unlockFocus];
#endif

// Dispatch result to origin queue
dispatch_async(queue, ^{
completion(compositedImage, nil);
});
});
}
});
mbglMapSnapshotter->snapshot(snapshotCallback->self());
});
}

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

@end
5 changes: 5 additions & 0 deletions platform/ios/app/MBXSnapshotsViewController.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#import <UIKit/UIKit.h>

@interface MBXSnapshotsViewController : UIViewController

@end
66 changes: 66 additions & 0 deletions platform/ios/app/MBXSnapshotsViewController.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#import "MBXSnapshotsViewController.h"

#import <Mapbox/Mapbox.h>

@interface MBXSnapshotsViewController ()

// Top row
@property (weak, nonatomic) IBOutlet UIImageView *snapshotImageViewTL;
@property (weak, nonatomic) IBOutlet UIImageView *snapshotImageViewTM;
@property (weak, nonatomic) IBOutlet UIImageView *snapshotImageViewTR;

// Bottom row
@property (weak, nonatomic) IBOutlet UIImageView *snapshotImageViewBL;
@property (weak, nonatomic) IBOutlet UIImageView *snapshotImageViewBM;
@property (weak, nonatomic) IBOutlet UIImageView *snapshotImageViewBR;

@end

@implementation MBXSnapshotsViewController {
// Top row
MGLMapSnapshotter* snapshotterTL;
MGLMapSnapshotter* snapshotterTM;
MGLMapSnapshotter* snapshotterTR;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI, the convention in Objective-C is to use a naming scheme like topCenterSnapshotter.


// Bottom row
MGLMapSnapshotter* snapshotterBL;
MGLMapSnapshotter* snapshotterBM;
MGLMapSnapshotter* snapshotterBR;
}

- (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)];

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

- (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)];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MGLStyle has factory methods for style URLs. We use them whenever possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In this case the traffic style is deprecated, even tho it has not yet merged into master. Our recommendation is create an URL with this string mapbox://styles/mapbox/traffic-day-v2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In this case, the demo doesn’t depend on the traffic congestion information displayed by the Traffic Day style, so I’d recommend choosing a non-deprecated method like +[MGLStyle satelliteStreets].

options.zoom = 10;

// Create and start the snapshotter
MGLMapSnapshotter* snapshotter = [[MGLMapSnapshotter alloc] initWithOptions:options];
[snapshotter startWithCompletionHandler: ^(UIImage *image, NSError *error) {
if (error) {
NSLog(@"Could not load snapshot: %@", [error localizedDescription]);
} else {
imageView.image = image;
}
}];

return snapshotter;
}


@end
7 changes: 7 additions & 0 deletions platform/ios/app/MBXViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ typedef NS_ENUM(NSInteger, MBXSettingsMiscellaneousRows) {
MBXSettingsMiscellaneousScrollView,
MBXSettingsMiscellaneousToggleTwoMaps,
MBXSettingsMiscellaneousCountryLabels,
MBXSettingsMiscellaneousShowSnapshots,
MBXSettingsMiscellaneousPrintLogFile,
MBXSettingsMiscellaneousDeleteLogFile,
};
Expand Down Expand Up @@ -358,6 +359,7 @@ - (void)dismissSettings:(__unused id)sender
@"Embedded Map View",
[NSString stringWithFormat:@"%@ Second Map", ([self.view viewWithTag:2] == nil ? @"Show" : @"Hide")],
[NSString stringWithFormat:@"Show Labels in %@", (_usingLocaleBasedCountryLabels ? @"Default Language" : [[NSLocale currentLocale] displayNameForKey:NSLocaleIdentifier value:[self bestLanguageForUser]])],
@"Show Snapshots"
]];

if (self.debugLoggingEnabled)
Expand Down Expand Up @@ -639,6 +641,11 @@ - (void)performActionForSettingAtIndexPath:(NSIndexPath *)indexPath
constant:0]];
}
break;
case MBXSettingsMiscellaneousShowSnapshots:
{
[self performSegueWithIdentifier:@"ShowSnapshots" sender:nil];
break;
}
default:
NSAssert(NO, @"All miscellaneous setting rows should be implemented");
break;
Expand Down
Loading