[core] make spriteloader an actor

9 files changed, 72 insertions, 123 deletions

diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake
index e56f2fdc37..d27bffa83e 100644
--- a/cmake/core-files.cmake
+++ b/cmake/core-files.cmake
@@ -291,8 +291,6 @@ set(MBGL_CORE_FILES
     src/mbgl/sprite/sprite_loader.cpp
     src/mbgl/sprite/sprite_loader.hpp
     src/mbgl/sprite/sprite_loader_observer.hpp
-    src/mbgl/sprite/sprite_loader_worker.cpp
-    src/mbgl/sprite/sprite_loader_worker.hpp
     src/mbgl/sprite/sprite_parser.cpp
     src/mbgl/sprite/sprite_parser.hpp
 
diff --git a/src/mbgl/sprite/sprite_loader.cpp b/src/mbgl/sprite/sprite_loader.cpp
index 7c5fe40e05..8824b53411 100644
--- a/src/mbgl/sprite/sprite_loader.cpp
+++ b/src/mbgl/sprite/sprite_loader.cpp
@@ -1,8 +1,6 @@
 #include <mbgl/sprite/sprite_loader.hpp>
-#include <mbgl/sprite/sprite_loader_worker.hpp>
 #include <mbgl/sprite/sprite_loader_observer.hpp>
 #include <mbgl/sprite/sprite_parser.hpp>
-#include <mbgl/util/logging.hpp>
 #include <mbgl/util/platform.hpp>
 #include <mbgl/util/std.hpp>
 #include <mbgl/util/constants.hpp>
@@ -10,95 +8,89 @@
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/storage/resource.hpp>
 #include <mbgl/storage/response.hpp>
-#include <mbgl/util/run_loop.hpp>
 #include <mbgl/actor/actor.hpp>
 
 #include <cassert>
 
 namespace mbgl {
 
-static SpriteLoaderObserver nullObserver;
-
-struct SpriteLoader::Loader {
-    Loader(Scheduler& scheduler, SpriteLoader& spriteAtlas)
-        : mailbox(std::make_shared<Mailbox>(*util::RunLoop::Get())),
-          worker(scheduler, ActorRef<SpriteLoader>(spriteAtlas, mailbox)) {
-    }
-
-    std::shared_ptr<const std::string> image;
-    std::shared_ptr<const std::string> json;
-    std::unique_ptr<AsyncRequest> jsonRequest;
-    std::unique_ptr<AsyncRequest> spriteRequest;
-    std::shared_ptr<Mailbox> mailbox;
-    Actor<SpriteLoaderWorker> worker;
-};
-
-SpriteLoader::SpriteLoader(float pixelRatio_)
-        : pixelRatio(pixelRatio_)
-        , observer(&nullObserver) {
+SpriteLoader::SpriteLoader(ActorRef<SpriteLoader> self_, ActorRef<SpriteLoaderObserver> observer_, FileSource& fileSource_, float pixelRatio_)
+        : self(self_)
+        , observer(observer_)
+        , fileSource(fileSource_)
+        , pixelRatio(pixelRatio_) {
 }
 
 SpriteLoader::~SpriteLoader() = default;
 
-void SpriteLoader::load(const std::string& url, Scheduler& scheduler, FileSource& fileSource) {
+void SpriteLoader::load(const std::string& url) {
     if (url.empty()) {
         // Treat a non-existent sprite as a successfully loaded empty sprite.
-        observer->onSpriteLoaded({});
+        observer.invoke(&SpriteLoaderObserver::onSpriteLoaded, SpriteLoaderObserver::Images());
         return;
     }
 
-    loader = std::make_unique<Loader>(scheduler, *this);
-
-    loader->jsonRequest = fileSource.request(Resource::spriteJSON(url, pixelRatio), [this](Response res) {
+    jsonRequest = fileSource.request(Resource::spriteJSON(url, pixelRatio), [this](Response res) {
         if (res.error) {
-            observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message)));
+            observer.invoke(&SpriteLoaderObserver::onSpriteError, std::make_exception_ptr(std::runtime_error(res.error->message)));
         } else if (res.notModified) {
             return;
         } else if (res.noContent) {
-            loader->json = std::make_shared<const std::string>();
-            emitSpriteLoadedIfComplete();
+            json = std::make_shared<const std::string>();
+            parseIfComplete();
         } else {
             // Only trigger a sprite loaded event we got new data.
-            loader->json = res.data;
-            emitSpriteLoadedIfComplete();
+            json = res.data;
+            parseIfComplete();
         }
     });
 
-    loader->spriteRequest = fileSource.request(Resource::spriteImage(url, pixelRatio), [this](Response res) {
+    spriteRequest = fileSource.request(Resource::spriteImage(url, pixelRatio), [this](Response res) {
         if (res.error) {
-            observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message)));
+            observer.invoke(&SpriteLoaderObserver::onSpriteError, std::make_exception_ptr(std::runtime_error(res.error->message)));
         } else if (res.notModified) {
             return;
         } else if (res.noContent) {
-            loader->image = std::make_shared<const std::string>();
-            emitSpriteLoadedIfComplete();
+            image = std::make_shared<const std::string>();
+            parseIfComplete();
         } else {
-            loader->image = res.data;
-            emitSpriteLoadedIfComplete();
+            image = res.data;
+            parseIfComplete();
         }
     });
 }
 
-void SpriteLoader::emitSpriteLoadedIfComplete() {
-    assert(loader);
+void SpriteLoader::parse() {
+    try {
+        if (!image) {
+            // This shouldn't happen, since we always invoke it with a non-empty pointer.
+            throw std::runtime_error("missing sprite image");
+        }
+        if (!json) {
+            // This shouldn't happen, since we always invoke it with a non-empty pointer.
+            throw std::runtime_error("missing sprite metadata");
+        }
+
+        self.invoke(&SpriteLoader::onParsed, parseSprite(*image, *json));
+    } catch (...) {
+        self.invoke(&SpriteLoader::onError, std::current_exception());
+    }
+}
 
-    if (!loader->image || !loader->json) {
+void SpriteLoader::parseIfComplete() {
+    if (!image || !json) {
         return;
     }
 
-    loader->worker.invoke(&SpriteLoaderWorker::parse, loader->image, loader->json);
+    self.invoke(&SpriteLoader::parse);
 }
 
 void SpriteLoader::onParsed(std::vector<std::unique_ptr<style::Image>>&& result) {
-    observer->onSpriteLoaded(std::move(result));
+    observer.invoke(&SpriteLoaderObserver::onSpriteLoaded, std::move(result));
 }
 
 void SpriteLoader::onError(std::exception_ptr err) {
-    observer->onSpriteError(err);
-}
-
-void SpriteLoader::setObserver(SpriteLoaderObserver* observer_) {
-    observer = observer_;
+    observer.invoke(&SpriteLoaderObserver::onSpriteError, err);
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp
index 0daf46be9c..d3e65496b8 100644
--- a/src/mbgl/sprite/sprite_loader.hpp
+++ b/src/mbgl/sprite/sprite_loader.hpp
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mbgl/actor/actor.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/style/image.hpp>
 
@@ -12,33 +13,35 @@
 
 namespace mbgl {
 
+class AsyncRequest;
 class Scheduler;
 class FileSource;
 class SpriteLoaderObserver;
 
 class SpriteLoader : public util::noncopyable {
 public:
-    SpriteLoader(float pixelRatio);
+    SpriteLoader(ActorRef<SpriteLoader>, ActorRef<SpriteLoaderObserver>, FileSource&, float pixelRatio);
     ~SpriteLoader();
 
-    void load(const std::string& url, Scheduler&, FileSource&);
-
-    void setObserver(SpriteLoaderObserver*);
+    void load(const std::string& url);
 
 private:
-    void emitSpriteLoadedIfComplete();
+    ActorRef<SpriteLoader> self;
+    ActorRef<SpriteLoaderObserver> observer;
+    FileSource& fileSource;
+    const float pixelRatio;
 
-    // Invoked by SpriteAtlasWorker
-    friend class SpriteLoaderWorker;
-    void onParsed(std::vector<std::unique_ptr<style::Image>>&&);
-    void onError(std::exception_ptr);
+    std::unique_ptr<AsyncRequest> jsonRequest;
+    std::unique_ptr<AsyncRequest> spriteRequest;
 
-    const float pixelRatio;
+    std::shared_ptr<const std::string> image;
+    std::shared_ptr<const std::string> json;
 
-    struct Loader;
-    std::unique_ptr<Loader> loader;
+    void parseIfComplete();
+    void parse();
 
-    SpriteLoaderObserver* observer = nullptr;
+    void onParsed(std::vector<std::unique_ptr<style::Image>>&&);
+    void onError(std::exception_ptr);
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader_observer.hpp b/src/mbgl/sprite/sprite_loader_observer.hpp
index c730549c2c..49cb2fb1fc 100644
--- a/src/mbgl/sprite/sprite_loader_observer.hpp
+++ b/src/mbgl/sprite/sprite_loader_observer.hpp
@@ -14,7 +14,8 @@ class SpriteLoaderObserver {
 public:
     virtual ~SpriteLoaderObserver() = default;
 
-    virtual void onSpriteLoaded(std::vector<std::unique_ptr<style::Image>>&&) {}
+    using Images = std::vector<std::unique_ptr<style::Image>>;
+    virtual void onSpriteLoaded(Images&&) {}
     virtual void onSpriteError(std::exception_ptr) {}
 };
 
diff --git a/src/mbgl/sprite/sprite_loader_worker.cpp b/src/mbgl/sprite/sprite_loader_worker.cpp
deleted file mode 100644
index 4bded33d53..0000000000
--- a/src/mbgl/sprite/sprite_loader_worker.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-#include <mbgl/sprite/sprite_loader_worker.hpp>
-#include <mbgl/sprite/sprite_loader.hpp>
-#include <mbgl/sprite/sprite_parser.hpp>
-
-namespace mbgl {
-
-SpriteLoaderWorker::SpriteLoaderWorker(ActorRef<SpriteLoaderWorker>, ActorRef<SpriteLoader> parent_)
-    : parent(std::move(parent_)) {
-}
-
-void SpriteLoaderWorker::parse(std::shared_ptr<const std::string> image,
-                              std::shared_ptr<const std::string> json) {
-    try {
-        if (!image) {
-            // This shouldn't happen, since we always invoke it with a non-empty pointer.
-            throw std::runtime_error("missing sprite image");
-        }
-        if (!json) {
-            // This shouldn't happen, since we always invoke it with a non-empty pointer.
-            throw std::runtime_error("missing sprite metadata");
-        }
-
-        parent.invoke(&SpriteLoader::onParsed, parseSprite(*image, *json));
-    } catch (...) {
-        parent.invoke(&SpriteLoader::onError, std::current_exception());
-    }
-}
-
-} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader_worker.hpp b/src/mbgl/sprite/sprite_loader_worker.hpp
deleted file mode 100644
index d61e07d14f..0000000000
--- a/src/mbgl/sprite/sprite_loader_worker.hpp
+++ /dev/null
@@ -1,23 +0,0 @@
-#pragma once
-
-#include <mbgl/actor/actor_ref.hpp>
-#include <mbgl/sprite/sprite_parser.hpp>
-
-#include <memory>
-#include <string>
-
-namespace mbgl {
-
-class SpriteLoader;
-
-class SpriteLoaderWorker {
-public:
-    SpriteLoaderWorker(ActorRef<SpriteLoaderWorker>, ActorRef<SpriteLoader>);
-
-    void parse(std::shared_ptr<const std::string> image, std::shared_ptr<const std::string> json);
-
-private:
-    ActorRef<SpriteLoader> parent;
-};
-
-} // namespace mbgl
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index 0a212b977f..fdf4043853 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -43,6 +43,7 @@
 #include <mbgl/util/string.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/math.hpp>
+#include <mbgl/util/run_loop.hpp>
 #include <mbgl/util/std.hpp>
 #include <mbgl/math/minmax.hpp>
 #include <mbgl/map/query.hpp>
@@ -71,16 +72,16 @@ struct QueueSourceReloadVisitor {
 
 Style::Style(Scheduler& scheduler_, FileSource& fileSource_, float pixelRatio)
     : scheduler(scheduler_),
+      mailbox(std::make_shared<Mailbox>(*util::RunLoop::Get())),
       fileSource(fileSource_),
       glyphAtlas(std::make_unique<GlyphAtlas>(Size{ 2048, 2048 }, fileSource)),
-      spriteLoader(std::make_unique<SpriteLoader>(pixelRatio)),
+      spriteLoader(std::make_unique<Actor<SpriteLoader>>(scheduler, ActorRef<SpriteLoaderObserver>(*this, mailbox), fileSource, pixelRatio)),
       spriteAtlas(std::make_unique<SpriteAtlas>()),
       lineAtlas(std::make_unique<LineAtlas>(Size{ 256, 512 })),
       light(std::make_unique<Light>()),
       renderLight(light->impl),
       observer(&nullObserver) {
     glyphAtlas->setObserver(this);
-    spriteLoader->setObserver(this);
     light->setObserver(this);
 }
 
@@ -134,7 +135,7 @@ void Style::setJSON(const std::string& json) {
     setLight(std::make_unique<Light>(parser.light));
 
     glyphAtlas->setURL(parser.glyphURL);
-    spriteLoader->load(parser.spriteURL, scheduler, fileSource);
+    spriteLoader->invoke(&SpriteLoader::load, parser.spriteURL);
 
     loaded = true;
 
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index 5d3c2899fd..cf8f5b8107 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -15,6 +15,7 @@
 #include <mbgl/map/mode.hpp>
 #include <mbgl/map/zoom_history.hpp>
 
+#include <mbgl/actor/actor.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/chrono.hpp>
 #include <mbgl/util/optional.hpp>
@@ -116,9 +117,14 @@ public:
     void dumpDebugLogs() const;
 
     Scheduler& scheduler;
+
+private:
+    std::shared_ptr<Mailbox> mailbox;
+
+public:
     FileSource& fileSource;
     std::unique_ptr<GlyphAtlas> glyphAtlas;
-    std::unique_ptr<SpriteLoader> spriteLoader;
+    std::unique_ptr<Actor<SpriteLoader>> spriteLoader;
     std::unique_ptr<SpriteAtlas> spriteAtlas;
     std::unique_ptr<LineAtlas> lineAtlas;
 
diff --git a/test/sprite/sprite_loader.test.cpp b/test/sprite/sprite_loader.test.cpp
index 3691572265..6b16cce977 100644
--- a/test/sprite/sprite_loader.test.cpp
+++ b/test/sprite/sprite_loader.test.cpp
@@ -36,16 +36,16 @@ public:
 
     util::RunLoop loop;
     StubFileSource fileSource;
+    std::shared_ptr<Mailbox> mailbox = std::make_shared<Mailbox>(loop);
     StubSpriteLoaderObserver observer;
     ThreadPool threadPool { 1 };
-    SpriteLoader spriteLoader{ 1 };
+    Actor<SpriteLoader> spriteLoader {threadPool, ActorRef<SpriteLoaderObserver>(observer, mailbox), fileSource, 1};
 
     void run() {
         // Squelch logging.
         Log::setObserver(std::make_unique<Log::NullObserver>());
 
-        spriteLoader.setObserver(&observer);
-        spriteLoader.load("test/fixtures/resources/sprite", threadPool, fileSource);
+        spriteLoader.invoke(&SpriteLoader::load, "test/fixtures/resources/sprite");
 
         loop.run();
     }