[core] Refactor Snapshotter

Move renderer to a separate thread while keeping map on a client thread.

2 files changed, 200 insertions, 150 deletions

diff --git a/platform/default/include/mbgl/map/map_snapshotter.hpp b/platform/default/include/mbgl/map/map_snapshotter.hpp
index c1544c36ab..938c9bd4bf 100644
--- a/platform/default/include/mbgl/map/map_snapshotter.hpp
+++ b/platform/default/include/mbgl/map/map_snapshotter.hpp
@@ -1,7 +1,6 @@
 #pragma once
 
 #include <mbgl/util/image.hpp>
-#include <mbgl/util/thread.hpp>
 #include <mbgl/util/optional.hpp>
 #include <mbgl/util/geo.hpp>
 
@@ -26,12 +25,12 @@ class Style;
 
 class MapSnapshotter {
 public:
-    MapSnapshotter(const std::pair<bool, std::string> style,
-                   const Size&,
-                   const float pixelRatio,
-                   const optional<CameraOptions> cameraOptions,
-                   const optional<LatLngBounds> region,
-                   const optional<std::string> localFontFamily,
+    MapSnapshotter(std::pair<bool, std::string> style,
+                   Size size,
+                   float pixelRatio,
+                   optional<CameraOptions> cameraOptions,
+                   optional<LatLngBounds> region,
+                   optional<std::string> localFontFamily,
                    const ResourceOptions&);
 
     ~MapSnapshotter();
@@ -59,7 +58,7 @@ public:
 
 private:
     class Impl;
-    std::unique_ptr<util::Thread<Impl>> impl;
+    std::unique_ptr<Impl> impl;
 };
 
 } // namespace mbgl
diff --git a/platform/default/src/mbgl/map/map_snapshotter.cpp b/platform/default/src/mbgl/map/map_snapshotter.cpp
index 705a791af9..a3029f151a 100644
--- a/platform/default/src/mbgl/map/map_snapshotter.cpp
+++ b/platform/default/src/mbgl/map/map_snapshotter.cpp
@@ -4,218 +4,269 @@
 #include <mbgl/gfx/headless_frontend.hpp>
 #include <mbgl/map/map.hpp>
 #include <mbgl/map/map_options.hpp>
+#include <mbgl/map/transform.hpp>
 #include <mbgl/map/transform_state.hpp>
+#include <mbgl/renderer/renderer_observer.hpp>
+#include <mbgl/renderer/update_parameters.hpp>
 #include <mbgl/storage/resource_options.hpp>
 #include <mbgl/style/style.hpp>
 #include <mbgl/util/event.hpp>
-#include <mbgl/map/transform.hpp>
+#include <mbgl/util/thread.hpp>
 
 namespace mbgl {
 
-class MapSnapshotter::Impl {
+class ForwardingRendererObserver final : public RendererObserver {
 public:
-    Impl(const std::pair<bool, std::string> style,
-         const Size&,
-         const float pixelRatio,
-         const optional<CameraOptions> cameraOptions,
-         const optional<LatLngBounds> region,
-         const optional<std::string> localFontFamily,
-         const ResourceOptions& resourceOptions);
+    explicit ForwardingRendererObserver(RendererObserver& delegate_)
+        : mailbox(std::make_shared<Mailbox>(*Scheduler::GetCurrent())), delegate(delegate_, mailbox) {}
 
-    void setStyleURL(std::string styleURL);
-    std::string getStyleURL() const;
+    ~ForwardingRendererObserver() override { mailbox->close(); }
 
-    void setStyleJSON(std::string styleJSON);
-    std::string getStyleJSON() const;
+    void onInvalidate() override { delegate.invoke(&RendererObserver::onInvalidate); }
 
-    void setSize(Size);
-    Size getSize() const;
+    void onResourceError(std::exception_ptr err) override { delegate.invoke(&RendererObserver::onResourceError, err); }
 
-    void setCameraOptions(CameraOptions);
-    CameraOptions getCameraOptions() const;
+    void onDidFinishRenderingFrame(RenderMode mode, bool repaintNeeded, bool placementChanged) override {
+        delegate.invoke(&RendererObserver::onDidFinishRenderingFrame, mode, repaintNeeded, placementChanged);
+    }
 
-    void setRegion(LatLngBounds);
-    LatLngBounds getRegion() const;
+private:
+    std::shared_ptr<Mailbox> mailbox;
+    ActorRef<RendererObserver> delegate;
+};
 
-    void snapshot(ActorRef<MapSnapshotter::Callback>);
+class SnapshotterRenderer final : public RendererObserver {
+public:
+    SnapshotterRenderer(Size size, float pixelRatio, optional<std::string> localFontFamily)
+        : frontend(size,
+                   pixelRatio,
+                   gfx::HeadlessBackend::SwapBehaviour::NoFlush,
+                   gfx::ContextMode::Unique,
+                   std::move(localFontFamily)) {}
+
+    void reset() {
+        hasPendingStillImageRequest = false;
+        frontend.reset();
+    }
+
+    void onInvalidate() override { rendererObserver->onInvalidate(); }
+
+    void onResourceError(std::exception_ptr err) override {
+        hasPendingStillImageRequest = false;
+        rendererObserver->onResourceError(err);
+    }
+
+    void onDidFinishRenderingFrame(RenderMode mode, bool repaintNeeded, bool placementChanged) override {
+        if (mode == RenderMode::Full && hasPendingStillImageRequest) {
+            stillImage = frontend.readStillImage();
+        }
+        rendererObserver->onDidFinishRenderingFrame(mode, repaintNeeded, placementChanged);
+    }
+
+    void setObserver(std::shared_ptr<RendererObserver> observer) {
+        assert(observer);
+        rendererObserver = std::move(observer);
+        frontend.setObserver(*this);
+    }
+
+    void update(std::shared_ptr<UpdateParameters> params) {
+        assert(params);
+        hasPendingStillImageRequest = params->stillImageRequest;
+        frontend.update(std::move(params));
+    }
+
+    void setSize(Size size) { frontend.setSize(size); }
+
+    PremultipliedImage takeImage() {
+        assert(stillImage.valid());
+        return std::move(stillImage);
+    }
 
 private:
+    PremultipliedImage stillImage;
+    bool hasPendingStillImageRequest = false;
+    std::shared_ptr<RendererObserver> rendererObserver;
     HeadlessFrontend frontend;
-    Map map;
 };
 
-MapSnapshotter::Impl::Impl(const std::pair<bool, std::string> style,
-                           const Size& size,
-                           const float pixelRatio,
-                           const optional<CameraOptions> cameraOptions,
-                           const optional<LatLngBounds> region,
-                           const optional<std::string> localFontFamily,
-                           const ResourceOptions& resourceOptions)
-    : frontend(
-          size, pixelRatio, gfx::HeadlessBackend::SwapBehaviour::NoFlush, gfx::ContextMode::Unique, localFontFamily),
-      map(frontend,
-          MapObserver::nullObserver(),
-          MapOptions().withMapMode(MapMode::Static).withSize(size).withPixelRatio(pixelRatio),
-          resourceOptions) {
-    if (style.first) {
-        map.getStyle().loadJSON(style.second);
-    } else {
-        map.getStyle().loadURL(style.second);
-    }
+class SnapshotterRendererFrontend final : public RendererFrontend {
+public:
+    SnapshotterRendererFrontend(Size size, float pixelRatio, optional<std::string> localFontFamily)
+        : renderer(std::make_unique<util::Thread<SnapshotterRenderer>>(
+              "Snapshotter", size, pixelRatio, std::move(localFontFamily))) {}
+
+    ~SnapshotterRendererFrontend() = default;
+
+    void reset() override { renderer->actor().invoke(&SnapshotterRenderer::reset); }
 
-    if (cameraOptions) {
-        map.jumpTo(*cameraOptions);
+    void setObserver(RendererObserver& observer) override {
+        renderer->actor().invoke(&SnapshotterRenderer::setObserver,
+                                 std::make_unique<ForwardingRendererObserver>(observer));
     }
 
-    // Set region, if specified
-    if (region) {
-        this->setRegion(*region);
+    void update(std::shared_ptr<UpdateParameters> parameters) override {
+        updateParameters = std::move(parameters);
+        renderer->actor().invoke(&SnapshotterRenderer::update, updateParameters);
     }
-}
 
-void MapSnapshotter::Impl::snapshot(ActorRef<MapSnapshotter::Callback> callback) {
-    map.renderStill([this, callback = std::move(callback)] (std::exception_ptr error) {
-
-        // Create lambda that captures the current transform state
-        // and can be used to translate for geographic to screen
-        // coordinates
-        assert (frontend.getTransformState());
-        PointForFn pointForFn { [=, center = *map.getCameraOptions().center, transformState = *frontend.getTransformState()] (const LatLng& latLng) {
-            LatLng unwrappedLatLng = latLng.wrapped();
-            unwrappedLatLng.unwrapForShortestPath(center);
-            Transform transform { transformState };
-            return transform.latLngToScreenCoordinate(unwrappedLatLng);
-        }};
-
-        // Create lambda that captures the current transform state
-        // and can be used to translate for geographic to screen
-        // coordinates
-        assert (frontend.getTransformState());
-        LatLngForFn latLngForFn { [=, transformState = *frontend.getTransformState()] (const ScreenCoordinate& screenCoordinate) {
-            Transform transform { transformState };
-            return transform.screenCoordinateToLatLng(screenCoordinate);
-        }};
-
-        // Collect all source attributions
-        std::vector<std::string> attributions;
-        for (auto source : map.getStyle().getSources()) {
-            auto attribution = source->getAttribution();
-            if (attribution) {
-                attributions.push_back(*attribution);
-            }
-        }
+    void setSize(Size size) { renderer->actor().invoke(&SnapshotterRenderer::setSize, size); }
 
-        // Invoke callback
-        callback.invoke(
-                &MapSnapshotter::Callback::operator(),
-                error,
-                error ? PremultipliedImage() : frontend.readStillImage(),
-                std::move(attributions),
-                std::move(pointForFn),
-                std::move(latLngForFn)
-        );
-    });
-}
+    const TransformState& getTransformState() const {
+        assert(updateParameters);
+        static TransformState defaultTransformState{};
+        if (updateParameters) return updateParameters->transformState;
+        return defaultTransformState;
+    }
 
-void MapSnapshotter::Impl::setStyleURL(std::string styleURL) {
-    map.getStyle().loadURL(styleURL);
-}
+    PremultipliedImage takeImage() { return renderer->actor().ask(&SnapshotterRenderer::takeImage).get(); }
 
-std::string MapSnapshotter::Impl::getStyleURL() const {
-    return map.getStyle().getURL();
-}
+private:
+    std::shared_ptr<UpdateParameters> updateParameters;
+    const std::unique_ptr<util::Thread<SnapshotterRenderer>> renderer;
+};
 
-void MapSnapshotter::Impl::setStyleJSON(std::string styleJSON) {
-    map.getStyle().loadJSON(styleJSON);
-}
+class MapSnapshotter::Impl {
+public:
+    Impl(std::pair<bool, std::string> style,
+         Size size,
+         float pixelRatio,
+         optional<CameraOptions> cameraOptions,
+         optional<LatLngBounds> region,
+         optional<std::string> localFontFamily,
+         const ResourceOptions& resourceOptions)
+        : frontend(size, pixelRatio, std::move(localFontFamily)),
+          map(frontend,
+              MapObserver::nullObserver(),
+              MapOptions().withMapMode(MapMode::Static).withSize(size).withPixelRatio(pixelRatio),
+              resourceOptions) {
+        if (style.first) {
+            map.getStyle().loadJSON(style.second);
+        } else {
+            map.getStyle().loadURL(style.second);
+        }
 
-std::string MapSnapshotter::Impl::getStyleJSON() const {
-   return map.getStyle().getJSON();
-}
+        if (cameraOptions) {
+            map.jumpTo(*cameraOptions);
+        }
 
-void MapSnapshotter::Impl::setSize(Size size) {
-    map.setSize(size);
-    frontend.setSize(size);
-}
+        // Set region, if specified
+        if (region) {
+            setRegion(*region);
+        }
+    }
 
-Size MapSnapshotter::Impl::getSize() const {
-    return map.getMapOptions().size();
-}
+    void setRegion(const LatLngBounds& region) {
+        mbgl::EdgeInsets insets{0, 0, 0, 0};
+        std::vector<LatLng> latLngs = {region.southwest(), region.northeast()};
+        map.jumpTo(map.cameraForLatLngs(latLngs, insets));
+    }
 
-void MapSnapshotter::Impl::setCameraOptions(CameraOptions cameraOptions) {
-    map.jumpTo(cameraOptions);
-}
+    void snapshot(ActorRef<MapSnapshotter::Callback> callback) {
+        map.renderStill([this, callback = std::move(callback)](std::exception_ptr error) {
+            // Create lambda that captures the current transform state
+            // and can be used to translate for geographic to screen
+            // coordinates
+            assert(map.getCameraOptions().center);
+            PointForFn pointForFn = [center = *map.getCameraOptions().center,
+                                     transformState = frontend.getTransformState()](const LatLng& latLng) {
+                LatLng unwrappedLatLng = latLng.wrapped();
+                unwrappedLatLng.unwrapForShortestPath(center);
+                Transform transform{transformState};
+                return transform.latLngToScreenCoordinate(unwrappedLatLng);
+            };
+
+            // Create lambda that captures the current transform state
+            // and can be used to translate for geographic to screen
+            // coordinates
+            LatLngForFn latLngForFn = [transformState =
+                                           frontend.getTransformState()](const ScreenCoordinate& screenCoordinate) {
+                Transform transform{transformState};
+                return transform.screenCoordinateToLatLng(screenCoordinate);
+            };
+
+            // Collect all source attributions
+            std::vector<std::string> attributions;
+            for (auto source : map.getStyle().getSources()) {
+                auto attribution = source->getAttribution();
+                if (attribution) {
+                    attributions.push_back(*attribution);
+                }
+            }
 
-CameraOptions MapSnapshotter::Impl::getCameraOptions() const {
-    EdgeInsets insets;
-    return map.getCameraOptions(insets);
-}
+            // Invoke callback
+            callback.invoke(&MapSnapshotter::Callback::operator(),
+                            error,
+                            error ? PremultipliedImage() : frontend.takeImage(),
+                            std::move(attributions),
+                            std::move(pointForFn),
+                            std::move(latLngForFn));
+        });
+    }
 
-void MapSnapshotter::Impl::setRegion(LatLngBounds region) {
-    mbgl::EdgeInsets insets = { 0, 0, 0, 0 };
-    std::vector<LatLng> latLngs = { region.southwest(), region.northeast() };
-    map.jumpTo(map.cameraForLatLngs(latLngs, insets));
-}
+    Map& getMap() { return map; }
+    SnapshotterRendererFrontend& getRenderer() { return frontend; }
 
-LatLngBounds MapSnapshotter::Impl::getRegion() const {
-    return map.latLngBoundsForCamera(getCameraOptions());
-}
+private:
+    SnapshotterRendererFrontend frontend;
+    Map map;
+};
 
-MapSnapshotter::MapSnapshotter(const std::pair<bool, std::string> style,
-                               const Size& size,
-                               const float pixelRatio,
-                               const optional<CameraOptions> cameraOptions,
-                               const optional<LatLngBounds> region,
-                               const optional<std::string> localFontFamily,
+MapSnapshotter::MapSnapshotter(std::pair<bool, std::string> style,
+                               Size size,
+                               float pixelRatio,
+                               optional<CameraOptions> cameraOptions,
+                               optional<LatLngBounds> region,
+                               optional<std::string> localFontFamily,
                                const ResourceOptions& resourceOptions)
-   : impl(std::make_unique<util::Thread<MapSnapshotter::Impl>>(
-       "Map Snapshotter", style, size, pixelRatio, cameraOptions,
-       region, localFontFamily, resourceOptions.clone())) {}
+    : impl(std::make_unique<MapSnapshotter::Impl>(
+          std::move(style), size, pixelRatio, std::move(cameraOptions), std::move(region), localFontFamily, resourceOptions.clone())) {}
 
 MapSnapshotter::~MapSnapshotter() = default;
 
 void MapSnapshotter::snapshot(ActorRef<MapSnapshotter::Callback> callback) {
-    impl->actor().invoke(&Impl::snapshot, std::move(callback));
+    impl->snapshot(std::move(callback));
 }
 
 void MapSnapshotter::setStyleURL(const std::string& styleURL) {
-    impl->actor().invoke(&Impl::setStyleURL, styleURL);
+    impl->getMap().getStyle().loadURL(styleURL);
 }
 
 std::string MapSnapshotter::getStyleURL() const {
-    return impl->actor().ask(&Impl::getStyleURL).get();
+    return impl->getMap().getStyle().getURL();
 }
 
 void MapSnapshotter::setStyleJSON(const std::string& styleJSON) {
-    impl->actor().invoke(&Impl::setStyleJSON, styleJSON);
+    impl->getMap().getStyle().loadJSON(styleJSON);
 }
 
 std::string MapSnapshotter::getStyleJSON() const {
-    return impl->actor().ask(&Impl::getStyleJSON).get();
+    return impl->getMap().getStyle().getJSON();
 }
 
 void MapSnapshotter::setSize(const Size& size) {
-    impl->actor().invoke(&Impl::setSize, size);
+    impl->getMap().setSize(size);
+    impl->getRenderer().setSize(size);
 }
 
 Size MapSnapshotter::getSize() const {
-    return impl->actor().ask(&Impl::getSize).get();
+    return impl->getMap().getMapOptions().size();
 }
 
 void MapSnapshotter::setCameraOptions(const CameraOptions& options) {
-    impl->actor().invoke(&Impl::setCameraOptions, options);
+    impl->getMap().jumpTo(options);
 }
 
 CameraOptions MapSnapshotter::getCameraOptions() const {
-    return impl->actor().ask(&Impl::getCameraOptions).get();
+    EdgeInsets insets;
+    return impl->getMap().getCameraOptions(insets);
 }
 
-void MapSnapshotter::setRegion(const LatLngBounds& bounds) {
-    impl->actor().invoke(&Impl::setRegion, std::move(bounds));
+void MapSnapshotter::setRegion(const LatLngBounds& region) {
+    impl->setRegion(region);
 }
 
 LatLngBounds MapSnapshotter::getRegion() const {
-    return impl->actor().ask(&Impl::getRegion).get();
+    return impl->getMap().latLngBoundsForCamera(getCameraOptions());
 }
 
 } // namespace mbgl