[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.

4 files changed, 31 insertions, 27 deletions

diff --git a/src/mbgl/style/conversion/source.cpp b/src/mbgl/style/conversion/source.cpp
index 980a1a5772..b4a3d74720 100644
--- a/src/mbgl/style/conversion/source.cpp
+++ b/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);
diff --git a/src/mbgl/style/sources/geojson_source.cpp b/src/mbgl/style/sources/geojson_source.cpp
index 5523336f12..fc8a563fd4 100644
--- a/src/mbgl/style/sources/geojson_source.cpp
+++ b/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) {
diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp
index 468deb6134..f716b81c5b 100644
--- a/src/mbgl/style/sources/geojson_source_impl.cpp
+++ b/src/mbgl/style/sources/geojson_source_impl.cpp
@@ -83,26 +83,26 @@ T evaluateFeature(const mapbox::feature::feature<double>& f,
 }
 
 // static
-std::shared_ptr<GeoJSONData> GeoJSONData::create(const GeoJSON& geoJSON, const GeoJSONOptions& options) {
+std::shared_ptr<GeoJSONData> GeoJSONData::create(const GeoJSON& geoJSON, Immutable<GeoJSONOptions> options) {
     constexpr double scale = util::EXTENT / util::tileSize;
-    if (options.cluster && geoJSON.is<mapbox::feature::feature_collection<double>>() &&
+    if (options->cluster && geoJSON.is<mapbox::feature::feature_collection<double>>() &&
         !geoJSON.get<mapbox::feature::feature_collection<double>>().empty()) {
         mapbox::supercluster::Options clusterOptions;
-        clusterOptions.maxZoom = options.clusterMaxZoom;
+        clusterOptions.maxZoom = options->clusterMaxZoom;
         clusterOptions.extent = util::EXTENT;
-        clusterOptions.radius = ::round(scale * options.clusterRadius);
+        clusterOptions.radius = ::round(scale * options->clusterRadius);
         Feature feature;
-        clusterOptions.map = [&](const PropertyMap& properties) -> PropertyMap {
+        clusterOptions.map = [&, options](const PropertyMap& properties) -> PropertyMap {
             PropertyMap ret{};
             if (properties.empty()) return ret;
-            for (const auto& p : options.clusterProperties) {
+            for (const auto& p : options->clusterProperties) {
                 feature.properties = properties;
                 ret[p.first] = evaluateFeature<Value>(feature, p.second.first);
             }
             return ret;
         };
-        clusterOptions.reduce = [&](PropertyMap& toReturn, const PropertyMap& toFill) {
-            for (const auto& p : options.clusterProperties) {
+        clusterOptions.reduce = [&, options](PropertyMap& toReturn, const PropertyMap& toFill) {
+            for (const auto& p : options->clusterProperties) {
                 if (toFill.count(p.first) == 0) {
                     continue;
                 }
@@ -116,18 +116,16 @@ std::shared_ptr<GeoJSONData> GeoJSONData::create(const GeoJSON& geoJSON, const G
     }
 
     mapbox::geojsonvt::Options vtOptions;
-    vtOptions.maxZoom = options.maxzoom;
+    vtOptions.maxZoom = options->maxzoom;
     vtOptions.extent = util::EXTENT;
-    vtOptions.buffer = ::round(scale * options.buffer);
-    vtOptions.tolerance = scale * options.tolerance;
-    vtOptions.lineMetrics = options.lineMetrics;
+    vtOptions.buffer = ::round(scale * options->buffer);
+    vtOptions.tolerance = scale * options->tolerance;
+    vtOptions.lineMetrics = options->lineMetrics;
     return std::make_shared<GeoJSONVTData>(geoJSON, vtOptions);
 }
 
-GeoJSONSource::Impl::Impl(std::string id_, optional<GeoJSONOptions> options_)
-    : Source::Impl(SourceType::GeoJSON, std::move(id_)) {
-    options = options_ ? std::move(*options_) : GeoJSONOptions{};
-}
+GeoJSONSource::Impl::Impl(std::string id_, Immutable<GeoJSONOptions> options_)
+    : Source::Impl(SourceType::GeoJSON, std::move(id_)), options(std::move(options_)) {}
 
 GeoJSONSource::Impl::Impl(const GeoJSONSource::Impl& other, std::shared_ptr<GeoJSONData> data_)
     : Source::Impl(other), options(other.options), data(std::move(data_)) {}
@@ -135,7 +133,7 @@ GeoJSONSource::Impl::Impl(const GeoJSONSource::Impl& other, std::shared_ptr<GeoJ
 GeoJSONSource::Impl::~Impl() = default;
 
 Range<uint8_t> GeoJSONSource::Impl::getZoomRange() const {
-    return { options.minzoom, options.maxzoom };
+    return {options->minzoom, options->maxzoom};
 }
 
 std::weak_ptr<GeoJSONData> GeoJSONSource::Impl::getData() const {
diff --git a/src/mbgl/style/sources/geojson_source_impl.hpp b/src/mbgl/style/sources/geojson_source_impl.hpp
index da2673a38c..3b106e3c00 100644
--- a/src/mbgl/style/sources/geojson_source_impl.hpp
+++ b/src/mbgl/style/sources/geojson_source_impl.hpp
@@ -13,18 +13,18 @@ namespace style {
 
 class GeoJSONSource::Impl : public Source::Impl {
 public:
-    Impl(std::string id, optional<GeoJSONOptions>);
+    Impl(std::string id, Immutable<GeoJSONOptions>);
     Impl(const GeoJSONSource::Impl&, std::shared_ptr<GeoJSONData>);
     ~Impl() final;
 
     Range<uint8_t> getZoomRange() const;
     std::weak_ptr<GeoJSONData> getData() const;
-    const GeoJSONOptions& getOptions() const { return options; }
+    const Immutable<GeoJSONOptions>& getOptions() const { return options; }
 
     optional<std::string> getAttribution() const final;
 
 private:
-    GeoJSONOptions options;
+    Immutable<GeoJSONOptions> options;
     std::shared_ptr<GeoJSONData> data;
 };