diff options
author | Mikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com> | 2019-11-28 18:19:17 +0200 |
---|---|---|
committer | Mikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com> | 2019-11-29 09:53:28 +0200 |
commit | cd4136ed4b4fc5d9a44fe17aa98dfd590c9c0bfb (patch) | |
tree | 42e601f7b16a83a62a8418511b795b6dfcf9eeff /platform | |
parent | e3d1daaf6687467ee5c064d2577d2903c508c2dc (diff) | |
download | qtlocation-mapboxgl-cd4136ed4b4fc5d9a44fe17aa98dfd590c9c0bfb.tar.gz |
[core][android][darwin] Fix GeoJSONOptions handling
- share the `GeoJSONOptions` instances using `Immutable<GeoJSONOptions>` - avoid extra copying
- fix wrapping of the `GeoJSONOptions` instances in supercluster map/reduce lambdas.
Previously, local variables were wrapped by reference.
Diffstat (limited to 'platform')
-rw-r--r-- | platform/android/src/style/sources/geojson_source.cpp | 62 | ||||
-rw-r--r-- | platform/android/src/style/sources/geojson_source.hpp | 4 | ||||
-rw-r--r-- | platform/darwin/src/MGLShapeSource.mm | 26 | ||||
-rw-r--r-- | platform/darwin/src/MGLShapeSource_Private.h | 4 | ||||
-rw-r--r-- | platform/darwin/test/MGLShapeSourceTests.mm | 16 |
5 files changed, 57 insertions, 55 deletions
diff --git a/platform/android/src/style/sources/geojson_source.cpp b/platform/android/src/style/sources/geojson_source.cpp index 0eece4b1ad..f68c9d8f42 100644 --- a/platform/android/src/style/sources/geojson_source.cpp +++ b/platform/android/src/style/sources/geojson_source.cpp @@ -1,4 +1,5 @@ #include "geojson_source.hpp" +#include <mbgl/style/sources/geojson_source_impl.hpp> #include "../../attach_env.hpp" #include <mbgl/renderer/query.hpp> @@ -29,44 +30,42 @@ namespace android { // This conversion is expected not to fail because it's used only in contexts where // the value was originally a GeoJsonOptions object on the Java side. If it fails // to convert, it's a bug in our serialization or Java-side static typing. - static style::GeoJSONOptions convertGeoJSONOptions(jni::JNIEnv& env, const jni::Object<>& options) { - using namespace mbgl::style::conversion; - if (!options) { - return style::GeoJSONOptions(); - } - Error error; - optional<style::GeoJSONOptions> result = convert<style::GeoJSONOptions>( - mbgl::android::Value(env, options), error); - if (!result) { - throw std::logic_error(error.message); - } - return *result; +static Immutable<style::GeoJSONOptions> convertGeoJSONOptions(jni::JNIEnv& env, const jni::Object<>& options) { + using namespace mbgl::style::conversion; + if (!options) { + return style::GeoJSONOptions::defaultOptions(); } + Error error; + optional<style::GeoJSONOptions> result = convert<style::GeoJSONOptions>(mbgl::android::Value(env, options), error); + if (!result) { + throw std::logic_error(error.message); + } + return makeMutable<style::GeoJSONOptions>(std::move(*result)); +} - GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, const jni::String& sourceId, const jni::Object<>& options) - : Source(env, - std::make_unique<mbgl::style::GeoJSONSource>(jni::Make<std::string>(env, sourceId), - convertGeoJSONOptions(env, options))), - converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(), - source.as<style::GeoJSONSource>()->getOptions())) {} - - GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend& frontend) - : Source(env, coreSource, createJavaPeer(env), frontend), - converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(), - source.as<style::GeoJSONSource>()->getOptions())) {} +GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, const jni::String& sourceId, const jni::Object<>& options) + : Source(env, + std::make_unique<mbgl::style::GeoJSONSource>(jni::Make<std::string>(env, sourceId), + convertGeoJSONOptions(env, options))), + converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(), + source.as<style::GeoJSONSource>()->impl().getOptions())) {} - GeoJSONSource::~GeoJSONSource() = default; +GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend& frontend) + : Source(env, coreSource, createJavaPeer(env), frontend), + converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(), + source.as<style::GeoJSONSource>()->impl().getOptions())) {} - void GeoJSONSource::setGeoJSONString(jni::JNIEnv& env, const jni::String& jString) { +GeoJSONSource::~GeoJSONSource() = default; - std::shared_ptr<std::string> json = std::make_shared<std::string>(jni::Make<std::string>(env, jString)); +void GeoJSONSource::setGeoJSONString(jni::JNIEnv& env, const jni::String& jString) { + std::shared_ptr<std::string> json = std::make_shared<std::string>(jni::Make<std::string>(env, jString)); - Update::Converter converterFn = [this, json](ActorRef<GeoJSONDataCallback> _callback) { - converter->self().invoke(&FeatureConverter::convertJson, json, _callback); - }; + Update::Converter converterFn = [this, json](ActorRef<GeoJSONDataCallback> _callback) { + converter->self().invoke(&FeatureConverter::convertJson, json, _callback); + }; - setAsync(converterFn); - } + setAsync(converterFn); +} void GeoJSONSource::setFeatureCollection(jni::JNIEnv& env, const jni::Object<geojson::FeatureCollection>& jFeatures) { setCollectionAsync(env, jFeatures); @@ -237,6 +236,7 @@ namespace android { mbgl::Log::Error(mbgl::Event::JNI, "Error setting geo json: " + error.message); return; } + callback.invoke(&GeoJSONDataCallback::operator(), style::GeoJSONData::create(*converted, options)); } diff --git a/platform/android/src/style/sources/geojson_source.hpp b/platform/android/src/style/sources/geojson_source.hpp index e506191ceb..668e944e1c 100644 --- a/platform/android/src/style/sources/geojson_source.hpp +++ b/platform/android/src/style/sources/geojson_source.hpp @@ -15,7 +15,7 @@ using GeoJSONDataCallback = std::function<void(std::shared_ptr<style::GeoJSONDat class FeatureConverter { public: - explicit FeatureConverter(style::GeoJSONOptions options_) : options(std::move(options_)) {} + explicit FeatureConverter(Immutable<style::GeoJSONOptions> options_) : options(std::move(options_)) {} void convertJson(std::shared_ptr<std::string>, ActorRef<GeoJSONDataCallback>); template <class JNIType> @@ -23,7 +23,7 @@ public: ActorRef<GeoJSONDataCallback>); private: - style::GeoJSONOptions options; + Immutable<style::GeoJSONOptions> options; }; struct Update { diff --git a/platform/darwin/src/MGLShapeSource.mm b/platform/darwin/src/MGLShapeSource.mm index a4a100aaa2..2590865ac2 100644 --- a/platform/darwin/src/MGLShapeSource.mm +++ b/platform/darwin/src/MGLShapeSource.mm @@ -27,15 +27,15 @@ const MGLShapeSourceOption MGLShapeSourceOptionMinimumZoomLevel = @"MGLShapeSour const MGLShapeSourceOption MGLShapeSourceOptionSimplificationTolerance = @"MGLShapeSourceOptionSimplificationTolerance"; const MGLShapeSourceOption MGLShapeSourceOptionLineDistanceMetrics = @"MGLShapeSourceOptionLineDistanceMetrics"; -mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShapeSourceOption, id> *options) { - auto geoJSONOptions = mbgl::style::GeoJSONOptions(); +mbgl::Immutable<mbgl::style::GeoJSONOptions> MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShapeSourceOption, id> *options) { + auto geoJSONOptions = mbgl::makeMutable<mbgl::style::GeoJSONOptions>(); if (NSNumber *value = options[MGLShapeSourceOptionMinimumZoomLevel]) { if (![value isKindOfClass:[NSNumber class]]) { [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionMaximumZoomLevel must be an NSNumber."]; } - geoJSONOptions.minzoom = value.integerValue; + geoJSONOptions->minzoom = value.integerValue; } if (NSNumber *value = options[MGLShapeSourceOptionMaximumZoomLevel]) { @@ -43,7 +43,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionMaximumZoomLevel must be an NSNumber."]; } - geoJSONOptions.maxzoom = value.integerValue; + geoJSONOptions->maxzoom = value.integerValue; } if (NSNumber *value = options[MGLShapeSourceOptionBuffer]) { @@ -51,7 +51,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionBuffer must be an NSNumber."]; } - geoJSONOptions.buffer = value.integerValue; + geoJSONOptions->buffer = value.integerValue; } if (NSNumber *value = options[MGLShapeSourceOptionSimplificationTolerance]) { @@ -59,7 +59,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionSimplificationTolerance must be an NSNumber."]; } - geoJSONOptions.tolerance = value.doubleValue; + geoJSONOptions->tolerance = value.doubleValue; } if (NSNumber *value = options[MGLShapeSourceOptionClusterRadius]) { @@ -67,7 +67,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionClusterRadius must be an NSNumber."]; } - geoJSONOptions.clusterRadius = value.integerValue; + geoJSONOptions->clusterRadius = value.integerValue; } if (NSNumber *value = options[MGLShapeSourceOptionMaximumZoomLevelForClustering]) { @@ -75,7 +75,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionMaximumZoomLevelForClustering must be an NSNumber."]; } - geoJSONOptions.clusterMaxZoom = value.integerValue; + geoJSONOptions->clusterMaxZoom = value.integerValue; } if (NSNumber *value = options[MGLShapeSourceOptionClustered]) { @@ -83,7 +83,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionClustered must be an NSNumber."]; } - geoJSONOptions.cluster = value.boolValue; + geoJSONOptions->cluster = value.boolValue; } if (NSDictionary *value = options[MGLShapeSourceOptionClusterProperties]) { @@ -133,7 +133,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap std::string keyString = std::string([key UTF8String]); - geoJSONOptions.clusterProperties.emplace(keyString, std::make_pair(std::move(map), std::move(reduce))); + geoJSONOptions->clusterProperties.emplace(keyString, std::make_pair(std::move(map), std::move(reduce))); } } @@ -142,7 +142,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap [NSException raise:NSInvalidArgumentException format:@"MGLShapeSourceOptionLineDistanceMetrics must be an NSNumber."]; } - geoJSONOptions.lineMetrics = value.boolValue; + geoJSONOptions->lineMetrics = value.boolValue; } return geoJSONOptions; @@ -159,7 +159,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap - (instancetype)initWithIdentifier:(NSString *)identifier URL:(NSURL *)url options:(NSDictionary<NSString *, id> *)options { auto geoJSONOptions = MGLGeoJSONOptionsFromDictionary(options); - auto source = std::make_unique<mbgl::style::GeoJSONSource>(identifier.UTF8String, geoJSONOptions); + auto source = std::make_unique<mbgl::style::GeoJSONSource>(identifier.UTF8String, std::move(geoJSONOptions)); if (self = [super initWithPendingSource:std::move(source)]) { self.URL = url; } @@ -168,7 +168,7 @@ mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShap - (instancetype)initWithIdentifier:(NSString *)identifier shape:(nullable MGLShape *)shape options:(NSDictionary<MGLShapeSourceOption, id> *)options { auto geoJSONOptions = MGLGeoJSONOptionsFromDictionary(options); - auto source = std::make_unique<mbgl::style::GeoJSONSource>(identifier.UTF8String, geoJSONOptions); + auto source = std::make_unique<mbgl::style::GeoJSONSource>(identifier.UTF8String, std::move(geoJSONOptions)); if (self = [super initWithPendingSource:std::move(source)]) { if ([shape isMemberOfClass:[MGLShapeCollection class]]) { static dispatch_once_t onceToken; diff --git a/platform/darwin/src/MGLShapeSource_Private.h b/platform/darwin/src/MGLShapeSource_Private.h index c7eaf3d0a8..940c80a17d 100644 --- a/platform/darwin/src/MGLShapeSource_Private.h +++ b/platform/darwin/src/MGLShapeSource_Private.h @@ -1,6 +1,8 @@ #import "MGLFoundation.h" #import "MGLShapeSource.h" +#include <mbgl/util/immutable.hpp> + NS_ASSUME_NONNULL_BEGIN namespace mbgl { @@ -10,7 +12,7 @@ namespace mbgl { } MGL_EXPORT -mbgl::style::GeoJSONOptions MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShapeSourceOption, id> *options); +mbgl::Immutable<mbgl::style::GeoJSONOptions> MGLGeoJSONOptionsFromDictionary(NSDictionary<MGLShapeSourceOption, id> *options); @interface MGLShapeSource (Private) diff --git a/platform/darwin/test/MGLShapeSourceTests.mm b/platform/darwin/test/MGLShapeSourceTests.mm index 3bf3ef04bd..9046607010 100644 --- a/platform/darwin/test/MGLShapeSourceTests.mm +++ b/platform/darwin/test/MGLShapeSourceTests.mm @@ -26,14 +26,14 @@ MGLShapeSourceOptionLineDistanceMetrics: @YES}; auto mbglOptions = MGLGeoJSONOptionsFromDictionary(options); - XCTAssertTrue(mbglOptions.cluster); - XCTAssertEqual(mbglOptions.clusterRadius, 42); - XCTAssertEqual(mbglOptions.clusterMaxZoom, 98); - XCTAssertEqual(mbglOptions.maxzoom, 99); - XCTAssertEqual(mbglOptions.buffer, 1976); - XCTAssertEqual(mbglOptions.tolerance, 0.42); - XCTAssertTrue(mbglOptions.lineMetrics); - XCTAssertTrue(!mbglOptions.clusterProperties.empty()); + XCTAssertTrue(mbglOptions->cluster); + XCTAssertEqual(mbglOptions->clusterRadius, 42); + XCTAssertEqual(mbglOptions->clusterMaxZoom, 98); + XCTAssertEqual(mbglOptions->maxzoom, 99); + XCTAssertEqual(mbglOptions->buffer, 1976); + XCTAssertEqual(mbglOptions->tolerance, 0.42); + XCTAssertTrue(mbglOptions->lineMetrics); + XCTAssertTrue(!mbglOptions->clusterProperties.empty()); options = @{MGLShapeSourceOptionClustered: @"number 1"}; XCTAssertThrows(MGLGeoJSONOptionsFromDictionary(options)); |