[core] Split style image collection from SpriteAtlas

11 files changed, 272 insertions, 152 deletions

diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp
index fed8451aed..bef74b7ce5 100644
--- a/src/mbgl/sprite/sprite_atlas.cpp
+++ b/src/mbgl/sprite/sprite_atlas.cpp
@@ -1,7 +1,4 @@
 #include <mbgl/sprite/sprite_atlas.hpp>
-#include <mbgl/sprite/sprite_atlas_worker.hpp>
-#include <mbgl/sprite/sprite_atlas_observer.hpp>
-#include <mbgl/sprite/sprite_parser.hpp>
 #include <mbgl/gl/context.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/platform.hpp>
@@ -9,11 +6,6 @@
 #include <mbgl/util/std.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/exception.hpp>
-#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>
 #include <cmath>
@@ -21,28 +13,12 @@
 
 namespace mbgl {
 
-static SpriteAtlasObserver nullObserver;
-
-struct SpriteAtlas::Loader {
-    Loader(Scheduler& scheduler, SpriteAtlas& spriteAtlas)
-        : mailbox(std::make_shared<Mailbox>(*util::RunLoop::Get())),
-          worker(scheduler, ActorRef<SpriteAtlas>(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<SpriteAtlasWorker> worker;
-};
-
 SpriteAtlasElement::SpriteAtlasElement(Rect<uint16_t> rect_,
                                        const style::Image& image,
                                        Size size_, float pixelRatio)
     : pos(std::move(rect_)),
-      sdf(image.sdf),
-      relativePixelRatio(image.pixelRatio / pixelRatio),
+      sdf(image.isSdf()),
+      relativePixelRatio(image.getPixelRatio() / pixelRatio),
       width(image.getWidth()),
       height(image.getHeight()) {
 
@@ -59,87 +35,25 @@ SpriteAtlasElement::SpriteAtlasElement(Rect<uint16_t> rect_,
 SpriteAtlas::SpriteAtlas(Size size_, float pixelRatio_)
     : size(std::move(size_)),
       pixelRatio(pixelRatio_),
-      observer(&nullObserver),
       bin(size.width, size.height),
       dirty(true) {
 }
 
 SpriteAtlas::~SpriteAtlas() = default;
 
-void SpriteAtlas::load(const std::string& url, Scheduler& scheduler, FileSource& fileSource) {
-    if (url.empty()) {
-        // Treat a non-existent sprite as a successfully loaded empty sprite.
-        markAsLoaded();
-        return;
-    }
-
-    loader = std::make_unique<Loader>(scheduler, *this);
-
-    loader->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)));
-        } else if (res.notModified) {
-            return;
-        } else if (res.noContent) {
-            loader->json = std::make_shared<const std::string>();
-            emitSpriteLoadedIfComplete();
-        } else {
-            // Only trigger a sprite loaded event we got new data.
-            loader->json = res.data;
-            emitSpriteLoadedIfComplete();
-        }
-    });
-
-    loader->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)));
-        } else if (res.notModified) {
-            return;
-        } else if (res.noContent) {
-            loader->image = std::make_shared<const std::string>();
-            emitSpriteLoadedIfComplete();
-        } else {
-            loader->image = res.data;
-            emitSpriteLoadedIfComplete();
-        }
-    });
-}
-
-void SpriteAtlas::emitSpriteLoadedIfComplete() {
-    assert(loader);
-
-    if (!loader->image || !loader->json) {
-        return;
-    }
-
-    loader->worker.invoke(&SpriteAtlasWorker::parse, loader->image, loader->json);
-    // TODO: delete the loader?
-}
-
-void SpriteAtlas::onParsed(Images&& result) {
+void SpriteAtlas::onSpriteLoaded(Images&& result) {
     markAsLoaded();
+
     for (auto& pair : result) {
         addImage(pair.first, std::move(pair.second));
     }
-    observer->onSpriteLoaded();
+
     for (auto requestor : requestors) {
         requestor->onIconsAvailable(buildIconMap());
     }
     requestors.clear();
 }
 
-void SpriteAtlas::onError(std::exception_ptr err) {
-    observer->onSpriteError(err);
-}
-
-void SpriteAtlas::setObserver(SpriteAtlasObserver* observer_) {
-    observer = observer_;
-}
-
-void SpriteAtlas::dumpDebugLogs() const {
-    Log::Info(Event::General, "SpriteAtlas::loaded: %d", loaded);
-}
-
 void SpriteAtlas::addImage(const std::string& id, std::unique_ptr<style::Image> image_) {
     icons.clear();
 
@@ -152,10 +66,7 @@ void SpriteAtlas::addImage(const std::string& id, std::unique_ptr<style::Image>
     Entry& entry = it->second;
 
     // There is already a sprite with that name in our store.
-    if (entry.image->image.size != image_->image.size) {
-        Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", id.c_str());
-        return;
-    }
+    assert(entry.image->getImage().size == image_->getImage().size);
 
     entry.image = std::move(image_);
 
@@ -172,9 +83,7 @@ void SpriteAtlas::removeImage(const std::string& id) {
     icons.clear();
 
     auto it = entries.find(id);
-    if (it == entries.end()) {
-        return;
-    }
+    assert(it != entries.end());
 
     Entry& entry = it->second;
 
@@ -243,8 +152,8 @@ optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& id,
         };
     }
 
-    const uint16_t pixelWidth = std::ceil(entry.image->image.size.width / pixelRatio);
-    const uint16_t pixelHeight = std::ceil(entry.image->image.size.height / pixelRatio);
+    const uint16_t pixelWidth = std::ceil(entry.image->getImage().size.width / pixelRatio);
+    const uint16_t pixelHeight = std::ceil(entry.image->getImage().size.height / pixelRatio);
 
     // Increase to next number divisible by 4, but at least 1.
     // This is so we can scale down the texture coordinates and pack them
@@ -279,7 +188,7 @@ void SpriteAtlas::copy(const Entry& entry, optional<Rect<uint16_t>> Entry::*entr
         image.fill(0);
     }
 
-    const PremultipliedImage& src = entry.image->image;
+    const PremultipliedImage& src = entry.image->getImage();
     const Rect<uint16_t>& rect = *(entry.*entryRect);
 
     const uint32_t padding = 1;
@@ -337,4 +246,8 @@ void SpriteAtlas::bind(bool linear, gl::Context& context, gl::TextureUnit unit)
                         linear ? gl::TextureFilter::Linear : gl::TextureFilter::Nearest);
 }
 
+void SpriteAtlas::dumpDebugLogs() const {
+    Log::Info(Event::General, "SpriteAtlas::loaded: %d", loaded);
+}
+
 } // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp
index 5d99677726..7ae583dcba 100644
--- a/src/mbgl/sprite/sprite_atlas.hpp
+++ b/src/mbgl/sprite/sprite_atlas.hpp
@@ -7,7 +7,6 @@
 #include <mbgl/style/image.hpp>
 
 #include <string>
-#include <map>
 #include <set>
 #include <unordered_map>
 #include <array>
@@ -15,10 +14,6 @@
 
 namespace mbgl {
 
-class Scheduler;
-class FileSource;
-class SpriteAtlasObserver;
-
 namespace gl {
 class Context;
 } // namespace gl
@@ -38,7 +33,7 @@ public:
     float height;
 };
 
-using IconMap = std::map<std::string, SpriteAtlasElement>;
+using IconMap = std::unordered_map<std::string, SpriteAtlasElement>;
 using IconDependencies = std::set<std::string>;
 
 class IconRequestor {
@@ -48,12 +43,12 @@ public:
 
 class SpriteAtlas : public util::noncopyable {
 public:
-    using Images = std::map<std::string, std::unique_ptr<style::Image>>;
+    using Images = std::unordered_map<std::string, std::unique_ptr<style::Image>>;
 
     SpriteAtlas(Size, float pixelRatio);
     ~SpriteAtlas();
 
-    void load(const std::string& url, Scheduler&, FileSource&);
+    void onSpriteLoaded(Images&&);
 
     void markAsLoaded() {
         loaded = true;
@@ -65,8 +60,6 @@ public:
 
     void dumpDebugLogs() const;
 
-    void setObserver(SpriteAtlasObserver*);
-
     const style::Image* getImage(const std::string&) const;
     void addImage(const std::string&, std::unique_ptr<style::Image>);
     void removeImage(const std::string&);
@@ -93,25 +86,12 @@ public:
     }
 
 private:
-    void emitSpriteLoadedIfComplete();
-
-    // Invoked by SpriteAtlasWorker
-    friend class SpriteAtlasWorker;
-    void onParsed(Images&& result);
-    void onError(std::exception_ptr);
-
     const Size size;
     const float pixelRatio;
-
-    struct Loader;
-    std::unique_ptr<Loader> loader;
-
     bool loaded = false;
 
-    SpriteAtlasObserver* observer = nullptr;
-
     struct Entry {
-        std::unique_ptr<style::Image> image;
+        std::unique_ptr<const style::Image> image;
 
         // One sprite image might be used as both an icon image and a pattern image. If so,
         // it must have two distinct entries in the texture. The one for the icon image has
diff --git a/src/mbgl/sprite/sprite_atlas_observer.hpp b/src/mbgl/sprite/sprite_atlas_observer.hpp
deleted file mode 100644
index 263c95b0e8..0000000000
--- a/src/mbgl/sprite/sprite_atlas_observer.hpp
+++ /dev/null
@@ -1,15 +0,0 @@
-#pragma once
-
-#include <exception>
-
-namespace mbgl {
-
-class SpriteAtlasObserver {
-public:
-    virtual ~SpriteAtlasObserver() = default;
-
-    virtual void onSpriteLoaded() {}
-    virtual void onSpriteError(std::exception_ptr) {}
-};
-
-} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_image_collection.cpp b/src/mbgl/sprite/sprite_image_collection.cpp
new file mode 100644
index 0000000000..787ba83db2
--- /dev/null
+++ b/src/mbgl/sprite/sprite_image_collection.cpp
@@ -0,0 +1,40 @@
+#include <mbgl/sprite/sprite_image_collection.hpp>
+#include <mbgl/util/logging.hpp>
+
+namespace mbgl {
+
+void addSpriteImage(Images& images, const std::string& id,
+                           std::unique_ptr<style::Image> image_,
+                           std::function<void (style::Image&)> onAdded) {
+
+    auto it = images.find(id);
+    if (it == images.end()) {
+        // Add new
+        it = images.emplace(id, std::move(image_)).first;
+        onAdded(*it->second.get());
+        return;
+    }
+
+    std::unique_ptr<style::Image>& image = it->second;
+
+    // There is already a sprite with that name in our store.
+    if (image->getImage().size != image_->getImage().size) {
+        Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", id.c_str());
+    }
+
+    // Update existing
+    image = std::move(image_);
+    onAdded(*it->second.get());
+}
+
+void removeSpriteImage(Images& images,
+                              const std::string& id,
+                              std::function<void ()> onRemoved) {
+    if (images.erase(id) > 0) {
+        onRemoved();
+    }
+}
+
+
+
+} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_image_collection.hpp b/src/mbgl/sprite/sprite_image_collection.hpp
new file mode 100644
index 0000000000..00e252f74f
--- /dev/null
+++ b/src/mbgl/sprite/sprite_image_collection.hpp
@@ -0,0 +1,25 @@
+#pragma once
+
+#include <mbgl/style/image.hpp>
+
+#include <functional>
+#include <memory>
+#include <string>
+#include <unordered_map>
+
+namespace mbgl {
+
+using Images = std::unordered_map<std::string, std::unique_ptr<style::Image>>;
+
+void addSpriteImage(Images&,
+                    const std::string&,
+                    std::unique_ptr<style::Image>,
+                    std::function<void (style::Image&)> onAdded = [] (style::Image&){});
+
+void removeSpriteImage(Images&,
+                       const std::string&,
+                       std::function<void ()> onRemoved = [] (){});
+
+
+
+} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader.cpp b/src/mbgl/sprite/sprite_loader.cpp
new file mode 100644
index 0000000000..86325bba35
--- /dev/null
+++ b/src/mbgl/sprite/sprite_loader.cpp
@@ -0,0 +1,107 @@
+#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>
+#include <mbgl/util/exception.hpp>
+#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() = default;
+
+void SpriteLoader::load(const std::string& url, Scheduler& scheduler, FileSource& fileSource) {
+    if (url.empty()) {
+        // Treat a non-existent sprite as a successfully loaded empty sprite.
+        observer->onSpriteLoaded({});
+        return;
+    }
+
+    loader = std::make_unique<Loader>(scheduler, *this);
+
+    loader->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)));
+        } else if (res.notModified) {
+            return;
+        } else if (res.noContent) {
+            loader->json = std::make_shared<const std::string>();
+            emitSpriteLoadedIfComplete();
+        } else {
+            // Only trigger a sprite loaded event we got new data.
+            loader->json = res.data;
+            emitSpriteLoadedIfComplete();
+        }
+    });
+
+    loader->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)));
+        } else if (res.notModified) {
+            return;
+        } else if (res.noContent) {
+            loader->image = std::make_shared<const std::string>();
+            emitSpriteLoadedIfComplete();
+        } else {
+            loader->image = res.data;
+            emitSpriteLoadedIfComplete();
+        }
+    });
+}
+
+void SpriteLoader::emitSpriteLoadedIfComplete() {
+    assert(loader);
+
+    if (!loader->image || !loader->json) {
+        return;
+    }
+
+    loader->worker.invoke(&SpriteLoaderWorker::parse, loader->image, loader->json);
+}
+
+void SpriteLoader::onParsed(Images&& result) {
+
+
+
+    observer->onSpriteLoaded(std::move(result));
+}
+
+void SpriteLoader::onError(std::exception_ptr err) {
+    observer->onSpriteError(err);
+}
+
+void SpriteLoader::setObserver(SpriteLoaderObserver* observer_) {
+    observer = observer_;
+}
+
+} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp
new file mode 100644
index 0000000000..dbebede03b
--- /dev/null
+++ b/src/mbgl/sprite/sprite_loader.hpp
@@ -0,0 +1,46 @@
+#pragma once
+
+#include <mbgl/util/noncopyable.hpp>
+#include <mbgl/style/image.hpp>
+
+#include <string>
+#include <map>
+#include <set>
+#include <unordered_map>
+#include <array>
+#include <memory>
+
+namespace mbgl {
+
+class Scheduler;
+class FileSource;
+class SpriteLoaderObserver;
+
+class SpriteLoader : public util::noncopyable {
+public:
+    using Images = std::unordered_map<std::string, std::unique_ptr<style::Image>>;
+
+    SpriteLoader(float pixelRatio);
+    ~SpriteLoader();
+
+    void load(const std::string& url, Scheduler&, FileSource&);
+
+    void setObserver(SpriteLoaderObserver*);
+
+private:
+    void emitSpriteLoadedIfComplete();
+
+    // Invoked by SpriteAtlasWorker
+    friend class SpriteLoaderWorker;
+    void onParsed(Images&& result);
+    void onError(std::exception_ptr);
+
+    const float pixelRatio;
+
+    struct Loader;
+    std::unique_ptr<Loader> loader;
+
+    SpriteLoaderObserver* observer = nullptr;
+};
+
+} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_loader_observer.hpp b/src/mbgl/sprite/sprite_loader_observer.hpp
new file mode 100644
index 0000000000..632c48ea56
--- /dev/null
+++ b/src/mbgl/sprite/sprite_loader_observer.hpp
@@ -0,0 +1,24 @@
+#pragma once
+
+#include <exception>
+#include <memory>
+#include <unordered_map>
+#include <string>
+
+namespace mbgl {
+
+namespace style {
+class Image;
+} // namespace style
+
+class SpriteLoaderObserver {
+public:
+    using Images = std::unordered_map<std::string, std::unique_ptr<style::Image>>;
+
+    virtual ~SpriteLoaderObserver() = default;
+
+    virtual void onSpriteLoaded(Images&&) {}
+    virtual void onSpriteError(std::exception_ptr) {}
+};
+
+} // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_atlas_worker.cpp b/src/mbgl/sprite/sprite_loader_worker.cpp
index da507aabab..4bded33d53 100644
--- a/src/mbgl/sprite/sprite_atlas_worker.cpp
+++ b/src/mbgl/sprite/sprite_loader_worker.cpp
@@ -1,14 +1,14 @@
-#include <mbgl/sprite/sprite_atlas_worker.hpp>
-#include <mbgl/sprite/sprite_atlas.hpp>
+#include <mbgl/sprite/sprite_loader_worker.hpp>
+#include <mbgl/sprite/sprite_loader.hpp>
 #include <mbgl/sprite/sprite_parser.hpp>
 
 namespace mbgl {
 
-SpriteAtlasWorker::SpriteAtlasWorker(ActorRef<SpriteAtlasWorker>, ActorRef<SpriteAtlas> parent_)
+SpriteLoaderWorker::SpriteLoaderWorker(ActorRef<SpriteLoaderWorker>, ActorRef<SpriteLoader> parent_)
     : parent(std::move(parent_)) {
 }
 
-void SpriteAtlasWorker::parse(std::shared_ptr<const std::string> image,
+void SpriteLoaderWorker::parse(std::shared_ptr<const std::string> image,
                               std::shared_ptr<const std::string> json) {
     try {
         if (!image) {
@@ -20,9 +20,9 @@ void SpriteAtlasWorker::parse(std::shared_ptr<const std::string> image,
             throw std::runtime_error("missing sprite metadata");
         }
 
-        parent.invoke(&SpriteAtlas::onParsed, parseSprite(*image, *json));
+        parent.invoke(&SpriteLoader::onParsed, parseSprite(*image, *json));
     } catch (...) {
-        parent.invoke(&SpriteAtlas::onError, std::current_exception());
+        parent.invoke(&SpriteLoader::onError, std::current_exception());
     }
 }
 
diff --git a/src/mbgl/sprite/sprite_atlas_worker.hpp b/src/mbgl/sprite/sprite_loader_worker.hpp
index 70ca0d52e5..d61e07d14f 100644
--- a/src/mbgl/sprite/sprite_atlas_worker.hpp
+++ b/src/mbgl/sprite/sprite_loader_worker.hpp
@@ -8,16 +8,16 @@
 
 namespace mbgl {
 
-class SpriteAtlas;
+class SpriteLoader;
 
-class SpriteAtlasWorker {
+class SpriteLoaderWorker {
 public:
-    SpriteAtlasWorker(ActorRef<SpriteAtlasWorker>, ActorRef<SpriteAtlas>);
+    SpriteLoaderWorker(ActorRef<SpriteLoaderWorker>, ActorRef<SpriteLoader>);
 
     void parse(std::shared_ptr<const std::string> image, std::shared_ptr<const std::string> json);
 
 private:
-    ActorRef<SpriteAtlas> parent;
+    ActorRef<SpriteLoader> parent;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/sprite/sprite_parser.hpp b/src/mbgl/sprite/sprite_parser.hpp
index 5be8435ebb..175ec8a883 100644
--- a/src/mbgl/sprite/sprite_parser.hpp
+++ b/src/mbgl/sprite/sprite_parser.hpp
@@ -7,7 +7,7 @@
 
 #include <string>
 #include <memory>
-#include <map>
+#include <unordered_map>
 
 namespace mbgl {
 
@@ -24,7 +24,7 @@ std::unique_ptr<style::Image> createStyleImage(const PremultipliedImage&,
                                  double ratio,
                                  bool sdf);
 
-using Images = std::map<std::string, std::unique_ptr<style::Image>>;
+using Images = std::unordered_map<std::string, std::unique_ptr<style::Image>>;
 
 // Parses an image and an associated JSON file and returns the sprite objects.
 Images parseSprite(const std::string& image, const std::string& json);