[core] Fix supercluster lambdas capturing

include/mbgl/style/sources/geojson_source.hpp

@@ -34,10 +34,13 @@ struct GeoJSONOptions {
                                         std::shared_ptr<mbgl::style::expression::Expression>>;
     using ClusterProperties = std::unordered_map<std::string, ClusterExpression>;
     ClusterProperties clusterProperties;
+
+    static Immutable<GeoJSONOptions> defaultOptions();
 };
 class GeoJSONData {
 public:
-    static std::shared_ptr<GeoJSONData> create(const GeoJSON&, const GeoJSONOptions&);
+    static std::shared_ptr<GeoJSONData> create(const GeoJSON&,
+                                               Immutable<GeoJSONOptions> = GeoJSONOptions::defaultOptions());
 
     virtual ~GeoJSONData() = default;
     virtual mapbox::feature::feature_collection<int16_t> getTile(const CanonicalTileID&) = 0;
@@ -52,7 +55,7 @@ class GeoJSONData {
 
 class GeoJSONSource final : public Source {
 public:
-    GeoJSONSource(const std::string& id, optional<GeoJSONOptions> = nullopt);
+    GeoJSONSource(std::string id, Immutable<GeoJSONOptions> = GeoJSONOptions::defaultOptions());
     ~GeoJSONSource() final;
 
     void setURL(const std::string& url);

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

platform/android/src/style/sources/geojson_source.hpp

@@ -15,15 +15,15 @@ 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>
     void convertObject(std::shared_ptr<jni::Global<jni::Object<JNIType>, jni::EnvAttachingDeleter>>,
                        ActorRef<GeoJSONDataCallback>);
 
 private:
-    style::GeoJSONOptions options;
+    Immutable<style::GeoJSONOptions> options;
 };
 
 struct Update {

platform/darwin/src/MGLShapeSource.mm

@@ -27,63 +27,63 @@
 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]) {
         if (![value isKindOfClass:[NSNumber class]]) {
             [NSException raise:NSInvalidArgumentException
                         format:@"MGLShapeSourceOptionMaximumZoomLevel must be an NSNumber."];
         }
-        geoJSONOptions.maxzoom = value.integerValue;
+        geoJSONOptions->maxzoom = value.integerValue;
     }
 
     if (NSNumber *value = options[MGLShapeSourceOptionBuffer]) {
         if (![value isKindOfClass:[NSNumber class]]) {
             [NSException raise:NSInvalidArgumentException
                         format:@"MGLShapeSourceOptionBuffer must be an NSNumber."];
         }
-        geoJSONOptions.buffer = value.integerValue;
+        geoJSONOptions->buffer = value.integerValue;
     }
 
     if (NSNumber *value = options[MGLShapeSourceOptionSimplificationTolerance]) {
         if (![value isKindOfClass:[NSNumber class]]) {
             [NSException raise:NSInvalidArgumentException
                         format:@"MGLShapeSourceOptionSimplificationTolerance must be an NSNumber."];
         }
-        geoJSONOptions.tolerance = value.doubleValue;
+        geoJSONOptions->tolerance = value.doubleValue;
     }
 
     if (NSNumber *value = options[MGLShapeSourceOptionClusterRadius]) {
         if (![value isKindOfClass:[NSNumber class]]) {
             [NSException raise:NSInvalidArgumentException
                         format:@"MGLShapeSourceOptionClusterRadius must be an NSNumber."];
         }
-        geoJSONOptions.clusterRadius = value.integerValue;
+        geoJSONOptions->clusterRadius = value.integerValue;
     }
 
     if (NSNumber *value = options[MGLShapeSourceOptionMaximumZoomLevelForClustering]) {
         if (![value isKindOfClass:[NSNumber class]]) {
             [NSException raise:NSInvalidArgumentException
                         format:@"MGLShapeSourceOptionMaximumZoomLevelForClustering must be an NSNumber."];
         }
-        geoJSONOptions.clusterMaxZoom = value.integerValue;
+        geoJSONOptions->clusterMaxZoom = value.integerValue;
     }
 
     if (NSNumber *value = options[MGLShapeSourceOptionClustered]) {
         if (![value isKindOfClass:[NSNumber class]]) {
             [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 @@
 
             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 @@
             [NSException raise:NSInvalidArgumentException
                         format:@"MGLShapeSourceOptionLineDistanceMetrics must be an NSNumber."];
         }
-        geoJSONOptions.lineMetrics = value.boolValue;
+        geoJSONOptions->lineMetrics = value.boolValue;
     }
 
     return geoJSONOptions;
@@ -159,7 +159,7 @@ @implementation MGLShapeSource
 
 - (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 @@ - (instancetype)initWithIdentifier:(NSString *)identifier URL:(NSURL *)url optio
 
 - (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;

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)
 

platform/darwin/test/MGLShapeSourceTests.mm

@@ -26,14 +26,14 @@ - (void)testGeoJSONOptionsFromDictionary {
                               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));

src/mbgl/style/conversion/source.cpp

@@ -116,12 +116,12 @@ static optional<std::unique_ptr<Source>> convertGeoJSONSource(const std::string&
         return nullopt;
     }
 
-    optional<GeoJSONOptions> options = convert<GeoJSONOptions>(value, error);
-    if (!options) {
-        return nullopt;
+    Immutable<GeoJSONOptions> options = GeoJSONOptions::defaultOptions();
+    if (optional<GeoJSONOptions> converted = convert<GeoJSONOptions>(value, error)) {
+        options = makeMutable<GeoJSONOptions>(std::move(*converted));
     }
 
-    auto result = std::make_unique<GeoJSONSource>(id, *options);
+    auto result = std::make_unique<GeoJSONSource>(id, std::move(options));
 
     if (isObject(*dataValue)) {
         optional<GeoJSON> geoJSON = convert<GeoJSON>(*dataValue, error);

src/mbgl/style/sources/geojson_source.cpp

@@ -12,8 +12,14 @@
 namespace mbgl {
 namespace style {
 
-GeoJSONSource::GeoJSONSource(const std::string& id, optional<GeoJSONOptions> options)
-    : Source(makeMutable<Impl>(id, options)) {}
+// static
+Immutable<GeoJSONOptions> GeoJSONOptions::defaultOptions() {
+    static Immutable<GeoJSONOptions> options = makeMutable<GeoJSONOptions>();
+    return options;
+}
+
+GeoJSONSource::GeoJSONSource(std::string id, Immutable<GeoJSONOptions> options)
+    : Source(makeMutable<Impl>(std::move(id), std::move(options))) {}
 
 GeoJSONSource::~GeoJSONSource() = default;
 
@@ -47,7 +53,7 @@ optional<std::string> GeoJSONSource::getURL() const {
 }
 
 const GeoJSONOptions& GeoJSONSource::getOptions() const {
-    return impl().getOptions();
+    return *impl().getOptions();
 }
 
 void GeoJSONSource::loadDescription(FileSource& fileSource) {