From 98d005792b68d0b299f123c1d31e50c72ba91ba8 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 4 Apr 2017 15:01:39 -0700 Subject: [darwin] Simplify MGLSource initialization and pointer management Introduce `-[MGLSource initWithPendingSource:]`, allowing the base class to track the owned `_pendingSource` pointer and implement `-addToMapView:` and `-removeFromMapView:` without any casts. --- platform/darwin/src/MGLRasterSource.mm | 80 ++++++--------------------- platform/darwin/src/MGLRasterSource_Private.h | 9 --- platform/darwin/src/MGLShapeSource.mm | 64 +++------------------ platform/darwin/src/MGLShapeSource_Private.h | 4 -- platform/darwin/src/MGLSource.mm | 34 ++++++++---- platform/darwin/src/MGLSource_Private.h | 13 ++++- platform/darwin/src/MGLVectorSource.mm | 67 +++------------------- platform/darwin/src/MGLVectorSource_Private.h | 9 --- 8 files changed, 67 insertions(+), 213 deletions(-) diff --git a/platform/darwin/src/MGLRasterSource.mm b/platform/darwin/src/MGLRasterSource.mm index a3d29c7c7f..8fc18ba69d 100644 --- a/platform/darwin/src/MGLRasterSource.mm +++ b/platform/darwin/src/MGLRasterSource.mm @@ -15,15 +15,11 @@ static const CGFloat MGLRasterSourceRetinaTileSize = 512; @interface MGLRasterSource () -- (instancetype)initWithRawSource:(mbgl::style::RasterSource *)rawSource NS_DESIGNATED_INITIALIZER; - -@property (nonatomic) mbgl::style::RasterSource *rawSource; +@property (nonatomic, readonly) mbgl::style::RasterSource *rawSource; @end -@implementation MGLRasterSource { - std::unique_ptr _pendingSource; -} +@implementation MGLRasterSource - (instancetype)initWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL { // The style specification default is 512, but 256 is the expected value for @@ -37,76 +33,32 @@ static const CGFloat MGLRasterSourceRetinaTileSize = 512; } - (instancetype)initWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL tileSize:(CGFloat)tileSize { - if (self = [super initWithIdentifier:identifier]) { - auto source = std::make_unique(identifier.UTF8String, - configurationURL.mgl_URLByStandardizingScheme.absoluteString.UTF8String, - uint16_t(round(tileSize))); - _pendingSource = std::move(source); - self.rawSource = _pendingSource.get(); - } - return self; + auto source = std::make_unique(identifier.UTF8String, + configurationURL.mgl_URLByStandardizingScheme.absoluteString.UTF8String, + uint16_t(round(tileSize))); + return self = [super initWithPendingSource:std::move(source)]; } - (instancetype)initWithIdentifier:(NSString *)identifier tileURLTemplates:(NS_ARRAY_OF(NSString *) *)tileURLTemplates options:(nullable NS_DICTIONARY_OF(MGLTileSourceOption, id) *)options { - if (self = [super initWithIdentifier:identifier]) { - mbgl::Tileset tileSet = MGLTileSetFromTileURLTemplates(tileURLTemplates, options); - - uint16_t tileSize = MGLRasterSourceRetinaTileSize; - if (NSNumber *tileSizeNumber = options[MGLTileSourceOptionTileSize]) { - if (![tileSizeNumber isKindOfClass:[NSNumber class]]) { - [NSException raise:NSInvalidArgumentException - format:@"MGLTileSourceOptionTileSize must be set to an NSNumber."]; - } - tileSize = static_cast(round(tileSizeNumber.doubleValue)); - } - - auto source = std::make_unique(identifier.UTF8String, tileSet, tileSize); - _pendingSource = std::move(source); - self.rawSource = _pendingSource.get(); - } - return self; -} - -- (instancetype)initWithRawSource:(mbgl::style::RasterSource *)rawSource { - return [super initWithRawSource:rawSource]; -} - -- (void)addToMapView:(MGLMapView *)mapView { - if (_pendingSource == nullptr) { - [NSException raise:@"MGLRedundantSourceException" - format:@"This instance %@ was already added to %@. Adding the same source instance " \ - @"to the style more than once is invalid.", self, mapView.style]; - } - - mapView.mbglMap->addSource(std::move(_pendingSource)); -} - -- (void)removeFromMapView:(MGLMapView *)mapView { - if (self.rawSource != mapView.mbglMap->getSource(self.identifier.UTF8String)) { - return; - } + mbgl::Tileset tileSet = MGLTileSetFromTileURLTemplates(tileURLTemplates, options); - auto removedSource = mapView.mbglMap->removeSource(self.identifier.UTF8String); - - mbgl::style::RasterSource *source = dynamic_cast(removedSource.get()); - if (!source) { - return; + uint16_t tileSize = MGLRasterSourceRetinaTileSize; + if (NSNumber *tileSizeNumber = options[MGLTileSourceOptionTileSize]) { + if (![tileSizeNumber isKindOfClass:[NSNumber class]]) { + [NSException raise:NSInvalidArgumentException + format:@"MGLTileSourceOptionTileSize must be set to an NSNumber."]; + } + tileSize = static_cast(round(tileSizeNumber.doubleValue)); } - removedSource.release(); - - _pendingSource = std::unique_ptr(source); - self.rawSource = _pendingSource.get(); + auto source = std::make_unique(identifier.UTF8String, tileSet, tileSize); + return self = [super initWithPendingSource:std::move(source)]; } - (mbgl::style::RasterSource *)rawSource { return (mbgl::style::RasterSource *)super.rawSource; } -- (void)setRawSource:(mbgl::style::RasterSource *)rawSource { - super.rawSource = rawSource; -} - - (NSURL *)configurationURL { auto url = self.rawSource->getURL(); return url ? [NSURL URLWithString:@(url->c_str())] : nil; diff --git a/platform/darwin/src/MGLRasterSource_Private.h b/platform/darwin/src/MGLRasterSource_Private.h index 47b1c13517..76790bd053 100644 --- a/platform/darwin/src/MGLRasterSource_Private.h +++ b/platform/darwin/src/MGLRasterSource_Private.h @@ -2,16 +2,7 @@ NS_ASSUME_NONNULL_BEGIN -namespace mbgl { - namespace style { - class RasterSource; - } -} - @interface MGLRasterSource (Private) - -- (instancetype)initWithRawSource:(mbgl::style::RasterSource *)rawSource; - @end NS_ASSUME_NONNULL_END diff --git a/platform/darwin/src/MGLShapeSource.mm b/platform/darwin/src/MGLShapeSource.mm index 7de2d69af3..15437ebedb 100644 --- a/platform/darwin/src/MGLShapeSource.mm +++ b/platform/darwin/src/MGLShapeSource.mm @@ -20,38 +20,26 @@ const MGLShapeSourceOption MGLShapeSourceOptionSimplificationTolerance = @"MGLSh @interface MGLShapeSource () -- (instancetype)initWithRawSource:(mbgl::style::GeoJSONSource *)rawSource NS_DESIGNATED_INITIALIZER; - @property (nonatomic, readwrite) NSDictionary *options; -@property (nonatomic) mbgl::style::GeoJSONSource *rawSource; +@property (nonatomic, readonly) mbgl::style::GeoJSONSource *rawSource; @end -@implementation MGLShapeSource { - std::unique_ptr _pendingSource; -} +@implementation MGLShapeSource - (instancetype)initWithIdentifier:(NSString *)identifier URL:(NSURL *)url options:(NS_DICTIONARY_OF(NSString *, id) *)options { - if (self = [super initWithIdentifier:identifier]) { - auto geoJSONOptions = MGLGeoJSONOptionsFromDictionary(options); - auto source = std::make_unique(identifier.UTF8String, geoJSONOptions); - - _pendingSource = std::move(source); - self.rawSource = _pendingSource.get(); - + auto geoJSONOptions = MGLGeoJSONOptionsFromDictionary(options); + auto source = std::make_unique(identifier.UTF8String, geoJSONOptions); + if (self = [super initWithPendingSource:std::move(source)]) { self.URL = url; } return self; } - (instancetype)initWithIdentifier:(NSString *)identifier shape:(nullable MGLShape *)shape options:(NS_DICTIONARY_OF(MGLShapeSourceOption, id) *)options { - if (self = [super initWithIdentifier:identifier]) { - auto geoJSONOptions = MGLGeoJSONOptionsFromDictionary(options); - auto source = std::make_unique(identifier.UTF8String, geoJSONOptions); - - _pendingSource = std::move(source); - self.rawSource = _pendingSource.get(); - + auto geoJSONOptions = MGLGeoJSONOptionsFromDictionary(options); + auto source = std::make_unique(identifier.UTF8String, geoJSONOptions); + if (self = [super initWithPendingSource:std::move(source)]) { self.shape = shape; } return self; @@ -72,46 +60,10 @@ const MGLShapeSourceOption MGLShapeSourceOptionSimplificationTolerance = @"MGLSh return [self initWithIdentifier:identifier shape:shapeCollection options:options]; } -- (instancetype)initWithRawSource:(mbgl::style::GeoJSONSource *)rawSource { - return [super initWithRawSource:rawSource]; -} - -- (void)addToMapView:(MGLMapView *)mapView { - if (_pendingSource == nullptr) { - [NSException raise:@"MGLRedundantSourceException" - format:@"This instance %@ was already added to %@. Adding the same source instance " \ - "to the style more than once is invalid.", self, mapView.style]; - } - - mapView.mbglMap->addSource(std::move(_pendingSource)); -} - -- (void)removeFromMapView:(MGLMapView *)mapView { - if (self.rawSource != mapView.mbglMap->getSource(self.identifier.UTF8String)) { - return; - } - - auto removedSource = mapView.mbglMap->removeSource(self.identifier.UTF8String); - - mbgl::style::GeoJSONSource *source = dynamic_cast(removedSource.get()); - if (!source) { - return; - } - - removedSource.release(); - - _pendingSource = std::unique_ptr(source); - self.rawSource = _pendingSource.get(); -} - - (mbgl::style::GeoJSONSource *)rawSource { return (mbgl::style::GeoJSONSource *)super.rawSource; } -- (void)setRawSource:(mbgl::style::GeoJSONSource *)rawSource { - super.rawSource = rawSource; -} - - (NSURL *)URL { auto url = self.rawSource->getURL(); return url ? [NSURL URLWithString:@(url->c_str())] : nil; diff --git a/platform/darwin/src/MGLShapeSource_Private.h b/platform/darwin/src/MGLShapeSource_Private.h index c14f4fbb59..84eb5deed4 100644 --- a/platform/darwin/src/MGLShapeSource_Private.h +++ b/platform/darwin/src/MGLShapeSource_Private.h @@ -6,14 +6,10 @@ NS_ASSUME_NONNULL_BEGIN namespace mbgl { namespace style { class GeoJSONOptions; - struct GeoJSONSource; } } @interface MGLShapeSource (Private) - -- (instancetype)initWithRawSource:(mbgl::style::GeoJSONSource *)rawSource; - @end MGL_EXPORT diff --git a/platform/darwin/src/MGLSource.mm b/platform/darwin/src/MGLSource.mm index c96b6c41c6..1940db688c 100644 --- a/platform/darwin/src/MGLSource.mm +++ b/platform/darwin/src/MGLSource.mm @@ -1,16 +1,21 @@ #import "MGLSource_Private.h" +#import "MGLMapView_Private.h" +#include #include @interface MGLSource () // Even though this class is abstract, MGLStyle uses it to represent some // special internal source types like mbgl::AnnotationSource. -@property (nonatomic) mbgl::style::Source *rawSource; +@property (nonatomic, readonly) mbgl::style::Source *rawSource; @end -@implementation MGLSource +@implementation MGLSource { + std::unique_ptr _pendingSource; +} + - (instancetype)initWithIdentifier:(NSString *)identifier { @@ -28,18 +33,27 @@ return self; } +- (instancetype)initWithPendingSource:(std::unique_ptr)pendingSource { + if (self = [self initWithRawSource:pendingSource.get()]) { + _pendingSource = std::move(pendingSource); + } + return self; +} + - (void)addToMapView:(MGLMapView *)mapView { - [NSException raise:NSInvalidArgumentException format: - @"The source %@ cannot be added to the style. " - @"Make sure the source was created as a member of a concrete subclass of MGLSource.", - self]; + if (_pendingSource == nullptr) { + [NSException raise:@"MGLRedundantSourceException" + format:@"This instance %@ was already added to %@. Adding the same source instance " \ + "to the style more than once is invalid.", self, mapView.style]; + } + + mapView.mbglMap->addSource(std::move(_pendingSource)); } - (void)removeFromMapView:(MGLMapView *)mapView { - [NSException raise:NSInvalidArgumentException format: - @"The source %@ cannot be removed from the style. " - @"Make sure the source was created as a member of a concrete subclass of MGLSource.", - self]; + if (self.rawSource == mapView.mbglMap->getSource(self.identifier.UTF8String)) { + _pendingSource = mapView.mbglMap->removeSource(self.identifier.UTF8String); + } } - (NSString *)description { diff --git a/platform/darwin/src/MGLSource_Private.h b/platform/darwin/src/MGLSource_Private.h index acb325e2f3..2fc170b86b 100644 --- a/platform/darwin/src/MGLSource_Private.h +++ b/platform/darwin/src/MGLSource_Private.h @@ -1,5 +1,7 @@ #import "MGLSource.h" +#include + NS_ASSUME_NONNULL_BEGIN namespace mbgl { @@ -13,10 +15,17 @@ namespace mbgl { @interface MGLSource (Private) /** - Initializes and returns a source with a raw pointer to the backing store. + Initializes and returns a source with a raw pointer to the backing store, + associated with a style. */ - (instancetype)initWithRawSource:(mbgl::style::Source *)rawSource; +/** + Initializes and returns a source with an owning pointer to the backing store, + unassociated from a style. + */ +- (instancetype)initWithPendingSource:(std::unique_ptr)pendingSource; + /** A raw pointer to the mbgl object, which is always initialized, either to the value returned by `mbgl::Map getSource`, or for independently created objects, @@ -24,7 +33,7 @@ namespace mbgl { pointer value stays even after ownership of the object is transferred via `mbgl::Map addSource`. */ -@property (nonatomic) mbgl::style::Source *rawSource; +@property (nonatomic, readonly) mbgl::style::Source *rawSource; /** Adds the mbgl source that this object represents to the mbgl map. diff --git a/platform/darwin/src/MGLVectorSource.mm b/platform/darwin/src/MGLVectorSource.mm index 5404f170e9..afce3bad46 100644 --- a/platform/darwin/src/MGLVectorSource.mm +++ b/platform/darwin/src/MGLVectorSource.mm @@ -1,88 +1,37 @@ #import "MGLVectorSource_Private.h" -#import "MGLMapView_Private.h" #import "MGLFeature_Private.h" #import "MGLSource_Private.h" #import "MGLTileSource_Private.h" #import "NSPredicate+MGLAdditions.h" #import "NSURL+MGLAdditions.h" -#include #include @interface MGLVectorSource () -- (instancetype)initWithRawSource:(mbgl::style::VectorSource *)rawSource NS_DESIGNATED_INITIALIZER; - -@property (nonatomic) mbgl::style::VectorSource *rawSource; +@property (nonatomic, readonly) mbgl::style::VectorSource *rawSource; @end -@implementation MGLVectorSource { - std::unique_ptr _pendingSource; -} +@implementation MGLVectorSource - (instancetype)initWithIdentifier:(NSString *)identifier configurationURL:(NSURL *)configurationURL { - if (self = [super initWithIdentifier:identifier]) { - auto source = std::make_unique(identifier.UTF8String, - configurationURL.mgl_URLByStandardizingScheme.absoluteString.UTF8String); - _pendingSource = std::move(source); - self.rawSource = _pendingSource.get(); - } - return self; + auto source = std::make_unique(identifier.UTF8String, + configurationURL.mgl_URLByStandardizingScheme.absoluteString.UTF8String); + return self = [super initWithPendingSource:std::move(source)]; } - (instancetype)initWithIdentifier:(NSString *)identifier tileURLTemplates:(NS_ARRAY_OF(NSString *) *)tileURLTemplates options:(nullable NS_DICTIONARY_OF(MGLTileSourceOption, id) *)options { - if (self = [super initWithIdentifier:identifier]) { - mbgl::Tileset tileSet = MGLTileSetFromTileURLTemplates(tileURLTemplates, options); - - auto source = std::make_unique(identifier.UTF8String, tileSet); - _pendingSource = std::move(source); - self.rawSource = _pendingSource.get(); - } - return self; -} - -- (instancetype)initWithRawSource:(mbgl::style::VectorSource *)rawSource { - return [super initWithRawSource:rawSource]; -} - -- (void)addToMapView:(MGLMapView *)mapView { - if (_pendingSource == nullptr) { - [NSException raise:@"MGLRedundantSourceException" - format:@"This instance %@ was already added to %@. Adding the same source instance " \ - "to the style more than once is invalid.", self, mapView.style]; - } - - mapView.mbglMap->addSource(std::move(_pendingSource)); -} - -- (void)removeFromMapView:(MGLMapView *)mapView { - if (self.rawSource != mapView.mbglMap->getSource(self.identifier.UTF8String)) { - return; - } - - auto removedSource = mapView.mbglMap->removeSource(self.identifier.UTF8String); - - mbgl::style::VectorSource *source = dynamic_cast(removedSource.get()); - if (!source) { - return; - } - - removedSource.release(); - - _pendingSource = std::unique_ptr(source); - self.rawSource = _pendingSource.get(); + mbgl::Tileset tileSet = MGLTileSetFromTileURLTemplates(tileURLTemplates, options); + auto source = std::make_unique(identifier.UTF8String, tileSet); + return self = [super initWithPendingSource:std::move(source)]; } - (mbgl::style::VectorSource *)rawSource { return (mbgl::style::VectorSource *)super.rawSource; } -- (void)setRawSource:(mbgl::style::VectorSource *)rawSource { - super.rawSource = rawSource; -} - - (NSURL *)configurationURL { auto url = self.rawSource->getURL(); return url ? [NSURL URLWithString:@(url->c_str())] : nil; diff --git a/platform/darwin/src/MGLVectorSource_Private.h b/platform/darwin/src/MGLVectorSource_Private.h index 12fcd82012..335743173e 100644 --- a/platform/darwin/src/MGLVectorSource_Private.h +++ b/platform/darwin/src/MGLVectorSource_Private.h @@ -2,16 +2,7 @@ NS_ASSUME_NONNULL_BEGIN -namespace mbgl { - namespace style { - class VectorSource; - } -} - @interface MGLVectorSource (Private) - -- (instancetype)initWithRawSource:(mbgl::style::VectorSource *)rawSource; - @end NS_ASSUME_NONNULL_END -- cgit v1.2.1