From deca8186147299b72061c1f2e67c13da8f07e0a6 Mon Sep 17 00:00:00 2001
From: Alexander Shalamov <alexander.shalamov@mapbox.com>
Date: Mon, 2 Mar 2020 16:24:46 +0200
Subject: [PATCH] [core] Simplify MapSnapshotter constructor and add observer
 interface

---
 .../include/mbgl/map/map_snapshotter.hpp      | 22 ++++--
 .../default/src/mbgl/map/map_snapshotter.cpp  | 72 +++++++++----------
 2 files changed, 50 insertions(+), 44 deletions(-)

diff --git a/platform/default/include/mbgl/map/map_snapshotter.hpp b/platform/default/include/mbgl/map/map_snapshotter.hpp
index df5711777..3c2df2028 100644
--- a/platform/default/include/mbgl/map/map_snapshotter.hpp
+++ b/platform/default/include/mbgl/map/map_snapshotter.hpp
@@ -19,15 +19,25 @@ namespace style {
 class Style;
 } // namespace style
 
+class MapSnapshotterObserver {
+public:
+    virtual ~MapSnapshotterObserver() = default;
+
+    static MapSnapshotterObserver& nullObserver();
+    virtual void onDidFailLoadingStyle(const std::string&) {}
+    virtual void onDidFinishLoadingStyle() {}
+    virtual void onStyleImageMissing(const std::string&) {}
+};
+
 class MapSnapshotter {
 public:
-    MapSnapshotter(std::pair<bool, std::string> style,
-                   Size size,
+    MapSnapshotter(Size size,
                    float pixelRatio,
-                   optional<CameraOptions> cameraOptions,
-                   optional<LatLngBounds> region,
-                   optional<std::string> localFontFamily,
-                   const ResourceOptions&);
+                   const ResourceOptions&,
+                   MapSnapshotterObserver&,
+                   optional<std::string> localFontFamily = nullopt);
+
+    MapSnapshotter(Size size, float pixelRatio, const ResourceOptions&);
 
     ~MapSnapshotter();
 
diff --git a/platform/default/src/mbgl/map/map_snapshotter.cpp b/platform/default/src/mbgl/map/map_snapshotter.cpp
index 201dd7824..fe051d8ca 100644
--- a/platform/default/src/mbgl/map/map_snapshotter.cpp
+++ b/platform/default/src/mbgl/map/map_snapshotter.cpp
@@ -17,6 +17,12 @@
 
 namespace mbgl {
 
+// static
+MapSnapshotterObserver& MapSnapshotterObserver::nullObserver() {
+    static MapSnapshotterObserver mapSnapshotterObserver;
+    return mapSnapshotterObserver;
+}
+
 class ForwardingRendererObserver final : public RendererObserver {
 public:
     explicit ForwardingRendererObserver(RendererObserver& delegate_)
@@ -32,6 +38,10 @@ public:
         delegate.invoke(&RendererObserver::onDidFinishRenderingFrame, mode, repaintNeeded, placementChanged);
     }
 
+    void onStyleImageMissing(const std::string& image, StyleImageMissingCallback cb) override {
+        delegate.invoke(&RendererObserver::onStyleImageMissing, image, std::move(cb));
+    }
+
 private:
     std::shared_ptr<Mailbox> mailbox;
     ActorRef<RendererObserver> delegate;
@@ -127,35 +137,19 @@ private:
     const std::unique_ptr<util::Thread<SnapshotterRenderer>> renderer;
 };
 
-class MapSnapshotter::Impl {
+class MapSnapshotter::Impl final : public MapObserver {
 public:
-    Impl(std::pair<bool, std::string> style,
-         Size size,
+    Impl(Size size,
          float pixelRatio,
-         optional<CameraOptions> cameraOptions,
-         optional<LatLngBounds> region,
-         optional<std::string> localFontFamily,
-         const ResourceOptions& resourceOptions)
-        : frontend(size, pixelRatio, std::move(localFontFamily)),
+         const ResourceOptions& resourceOptions,
+         MapSnapshotterObserver& observer_,
+         optional<std::string> localFontFamily)
+        : observer(observer_),
+          frontend(size, pixelRatio, std::move(localFontFamily)),
           map(frontend,
-              MapObserver::nullObserver(),
+              *this,
               MapOptions().withMapMode(MapMode::Static).withSize(size).withPixelRatio(pixelRatio),
-              resourceOptions) {
-        if (style.first) {
-            map.getStyle().loadJSON(style.second);
-        } else {
-            map.getStyle().loadURL(style.second);
-        }
-
-        if (cameraOptions) {
-            map.jumpTo(*cameraOptions);
-        }
-
-        // Set region, if specified
-        if (region) {
-            setRegion(*region);
-        }
-    }
+              resourceOptions) {}
 
     void setRegion(const LatLngBounds& region) {
         mbgl::EdgeInsets insets{0, 0, 0, 0};
@@ -229,6 +223,11 @@ public:
         });
     }
 
+    // MapObserver overrides
+    void onDidFailLoadingMap(MapLoadError, const std::string& error) override { observer.onDidFailLoadingStyle(error); }
+    void onDidFinishLoadingStyle() override { observer.onDidFinishLoadingStyle(); }
+    void onStyleImageMissing(const std::string& image) override { observer.onStyleImageMissing(image); }
+
     Map& getMap() { return map; }
     const Map& getMap() const { return map; }
     SnapshotterRendererFrontend& getRenderer() { return frontend; }
@@ -236,24 +235,21 @@ public:
 
 private:
     std::unique_ptr<Actor<MapSnapshotter::Callback>> renderStillCallback;
+    MapSnapshotterObserver& observer;
     SnapshotterRendererFrontend frontend;
     Map map;
 };
 
-MapSnapshotter::MapSnapshotter(std::pair<bool, std::string> style,
-                               Size size,
+MapSnapshotter::MapSnapshotter(Size size,
                                float pixelRatio,
-                               optional<CameraOptions> cameraOptions,
-                               optional<LatLngBounds> region,
-                               optional<std::string> localFontFamily,
-                               const ResourceOptions& resourceOptions)
-    : impl(std::make_unique<MapSnapshotter::Impl>(std::move(style),
-                                                  size,
-                                                  pixelRatio,
-                                                  std::move(cameraOptions),
-                                                  std::move(region),
-                                                  localFontFamily,
-                                                  resourceOptions.clone())) {}
+                               const ResourceOptions& resourceOptions,
+                               MapSnapshotterObserver& observer,
+                               optional<std::string> localFontFamily)
+    : impl(std::make_unique<MapSnapshotter::Impl>(
+          size, pixelRatio, resourceOptions, observer, std::move(localFontFamily))) {}
+
+MapSnapshotter::MapSnapshotter(Size size, float pixelRatio, const ResourceOptions& resourceOptions)
+    : MapSnapshotter(size, pixelRatio, resourceOptions, MapSnapshotterObserver::nullObserver()) {}
 
 MapSnapshotter::~MapSnapshotter() = default;