[Android] Couple a sequenced scheduler to the lifetime of a GeoJSON

source
mwilsnd · Mar 7, 2024 · cc854fb · cc854fb
1 parent 9614b56
commit cc854fb
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 25 deletions.
diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp
@@ -41,8 +41,8 @@ class GeoJSONData {
     using TileFeatures = mapbox::feature::feature_collection<int16_t>;
     using Features = mapbox::feature::feature_collection<double>;
     static std::shared_ptr<GeoJSONData> create(const GeoJSON&,
-                                               const Immutable<GeoJSONOptions>& = GeoJSONOptions::defaultOptions(),
-                                               std::shared_ptr<Scheduler> scheduler = nullptr);
+                                               std::shared_ptr<Scheduler> scheduler,
+                                               const Immutable<GeoJSONOptions>& = GeoJSONOptions::defaultOptions());
 
     virtual ~GeoJSONData() = default;
     virtual void getTile(const CanonicalTileID&, const std::function<void(TileFeatures)>&) = 0;
@@ -51,8 +51,6 @@ class GeoJSONData {
     virtual Features getChildren(std::uint32_t) = 0;
     virtual Features getLeaves(std::uint32_t, std::uint32_t limit, std::uint32_t offset) = 0;
     virtual std::uint8_t getClusterExpansionZoom(std::uint32_t) = 0;
-
-    virtual std::shared_ptr<Scheduler> getScheduler() { return nullptr; }
 };
 
 class GeoJSONSource final : public Source {
@@ -83,6 +81,7 @@ class GeoJSONSource final : public Source {
     std::optional<std::string> url;
     std::unique_ptr<AsyncRequest> req;
     std::shared_ptr<Scheduler> threadPool;
+    std::shared_ptr<Scheduler> sequencedScheduler;
     mapbox::base::WeakPtrFactory<Source> weakFactory{this};
 };
 

diff --git a/src/mbgl/style/sources/geojson_source.cpp b/src/mbgl/style/sources/geojson_source.cpp
@@ -21,7 +21,8 @@ Immutable<GeoJSONOptions> GeoJSONOptions::defaultOptions() {
 
 GeoJSONSource::GeoJSONSource(std::string id, Immutable<GeoJSONOptions> options)
     : Source(makeMutable<Impl>(std::move(id), std::move(options))),
-      threadPool(Scheduler::GetBackground()) {}
+      threadPool(Scheduler::GetBackground()),
+      sequencedScheduler(Scheduler::GetSequenced()) {}
 
 GeoJSONSource::~GeoJSONSource() = default;
 
@@ -40,20 +41,8 @@ void GeoJSONSource::setURL(const std::string& url_) {
     }
 }
 
-namespace {
-
-inline std::shared_ptr<GeoJSONData> createGeoJSONData(const mapbox::geojson::geojson& geoJSON,
-                                                      const GeoJSONSource::Impl& impl) {
-    if (auto data = impl.getData().lock()) {
-        return GeoJSONData::create(geoJSON, impl.getOptions(), data->getScheduler());
-    }
-    return GeoJSONData::create(geoJSON, impl.getOptions());
-}
-
-} // namespace
-
 void GeoJSONSource::setGeoJSON(const mapbox::geojson::geojson& geoJSON) {
-    setGeoJSONData(createGeoJSONData(geoJSON, impl()));
+    setGeoJSONData(GeoJSONData::create(geoJSON, sequencedScheduler, impl().getOptions()));
 }
 
 void GeoJSONSource::setGeoJSONData(std::shared_ptr<GeoJSONData> geoJSONData) {
@@ -88,13 +77,13 @@ void GeoJSONSource::loadDescription(FileSource& fileSource) {
         } else if (res.noContent) {
             observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty GeoJSON")));
         } else {
-            auto makeImplInBackground = [currentImpl = baseImpl, data = res.data]() -> Immutable<Source::Impl> {
+            auto makeImplInBackground = [currentImpl = baseImpl, data = res.data, seqScheduler{sequencedScheduler}]() -> Immutable<Source::Impl> {
                 assert(data);
                 auto& current = static_cast<const Impl&>(*currentImpl);
                 conversion::Error error;
                 std::shared_ptr<GeoJSONData> geoJSONData;
                 if (std::optional<GeoJSON> geoJSON = conversion::convertJSON<GeoJSON>(*data, error)) {
-                    geoJSONData = createGeoJSONData(*geoJSON, current);
+                    geoJSONData = GeoJSONData::create(*geoJSON, std::move(seqScheduler), current.getOptions());
                 } else {
                     // Create an empty GeoJSON VT object to make sure we're not
                     // infinitely waiting for tiles to load.

diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp
@@ -38,8 +38,6 @@ class GeoJSONVTData final : public GeoJSONData {
 
     std::uint8_t getClusterExpansionZoom(std::uint32_t) final { return 0; }
 
-    std::shared_ptr<Scheduler> getScheduler() final { return scheduler; }
-
     friend GeoJSONData;
     GeoJSONVTData(const GeoJSON& geoJSON,
                   const mapbox::geojsonvt::Options& options,
@@ -91,8 +89,8 @@ T evaluateFeature(const mapbox::feature::feature<double>& f,
 
 // static
 std::shared_ptr<GeoJSONData> GeoJSONData::create(const GeoJSON& geoJSON,
-                                                 const Immutable<GeoJSONOptions>& options,
-                                                 std::shared_ptr<Scheduler> scheduler) {
+                                                 std::shared_ptr<Scheduler> scheduler,
+                                                 const Immutable<GeoJSONOptions>& options) {
     constexpr double scale = util::EXTENT / util::tileSize_D;
     if (options->cluster && geoJSON.is<Features>() && !geoJSON.get<Features>().empty()) {
         mapbox::supercluster::Options clusterOptions;
@@ -128,7 +126,6 @@ std::shared_ptr<GeoJSONData> GeoJSONData::create(const GeoJSON& geoJSON,
     vtOptions.buffer = static_cast<uint16_t>(::round(scale * options->buffer));
     vtOptions.tolerance = scale * options->tolerance;
     vtOptions.lineMetrics = options->lineMetrics;
-    if (!scheduler) scheduler = Scheduler::GetSequenced();
     return std::shared_ptr<GeoJSONData>(new GeoJSONVTData(geoJSON, vtOptions, std::move(scheduler)));
 }