[core] Split style image collection from SpriteAtlas

cmake/core-files.cmake

@@ -283,9 +283,13 @@ set(MBGL_CORE_FILES
     # sprite
     src/mbgl/sprite/sprite_atlas.cpp
     src/mbgl/sprite/sprite_atlas.hpp
-    src/mbgl/sprite/sprite_atlas_observer.hpp
-    src/mbgl/sprite/sprite_atlas_worker.cpp
-    src/mbgl/sprite/sprite_atlas_worker.hpp
+    src/mbgl/sprite/sprite_image_collection.cpp
+    src/mbgl/sprite/sprite_image_collection.hpp
+    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
 
@@ -322,6 +326,8 @@ set(MBGL_CORE_FILES
     src/mbgl/style/class_dictionary.cpp
     src/mbgl/style/class_dictionary.hpp
     src/mbgl/style/image.cpp
+    src/mbgl/style/image_impl.cpp
+    src/mbgl/style/image_impl.hpp
     src/mbgl/style/layer.cpp
     src/mbgl/style/layer_impl.cpp
     src/mbgl/style/layer_impl.hpp
@@ -363,6 +369,7 @@ set(MBGL_CORE_FILES
     include/mbgl/style/conversion/tileset.hpp
     include/mbgl/style/conversion/transition_options.hpp
     src/mbgl/style/conversion/geojson.cpp
+    src/mbgl/style/conversion/json.hpp
     src/mbgl/style/conversion/stringify.hpp
 
     # style/function

cmake/test-files.cmake

@@ -46,6 +46,8 @@ set(MBGL_TEST_FILES
 
     # sprite
     test/sprite/sprite_atlas.test.cpp
+    test/sprite/sprite_image_collection.test.cpp
+    test/sprite/sprite_loader.test.cpp
     test/sprite/sprite_parser.test.cpp
 
     # src/mbgl/test

include/mbgl/style/image.hpp

@@ -2,23 +2,30 @@
 
 #include <mbgl/util/image.hpp>
 
+#include <memory>
+
 namespace mbgl {
 namespace style {
 
 class Image {
 public:
     Image(PremultipliedImage&&, float pixelRatio, bool sdf = false);
 
-    PremultipliedImage image;
+    PremultipliedImage& getImage() const;
 
     // Pixel ratio of the sprite image.
-    const float pixelRatio;
+    float getPixelRatio() const;
 
     // Whether this image should be interpreted as a signed distance field icon.
-    const bool sdf;
+    bool isSdf() const;
+
+    float getWidth() const;
+    float getHeight() const;
+
+    class Impl;
 
-    float getWidth() const { return image.size.width / pixelRatio; }
-    float getHeight() const { return image.size.height / pixelRatio; }
+private:
+    const std::shared_ptr<Impl> impl;
 };
 
 } // namespace style

platform/ios/src/UIImage+MGLAdditions.mm

@@ -6,14 +6,14 @@ @implementation UIImage (MGLAdditions)
 
 - (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)styleImage
 {
-    CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->image.clone());
+    CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->getImage().clone());
     if (!image) {
         return nil;
     }
 
-    if (self = [self initWithCGImage:image scale:styleImage->pixelRatio orientation:UIImageOrientationUp])
+    if (self = [self initWithCGImage:image scale:styleImage->getPixelRatio() orientation:UIImageOrientationUp])
     {
-        if (styleImage->sdf)
+        if (styleImage->isSdf())
         {
             self = [self imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate];
         }

platform/macos/src/NSImage+MGLAdditions.mm

@@ -16,7 +16,7 @@ - (nullable instancetype)initWithMGLPremultipliedImage:(mbgl::PremultipliedImage
 }
 
 - (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)styleImage {
-    CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->image.clone());
+    CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->getImage().clone());
     if (!image) {
         return nil;
     }
@@ -25,7 +25,7 @@ - (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)style
     CGImageRelease(image);
     if (self = [self initWithSize:NSMakeSize(styleImage->getWidth(), styleImage->getHeight())]) {
         [self addRepresentation:rep];
-        [self setTemplate:styleImage->sdf];
+        [self setTemplate:styleImage->isSdf()];
     }
     return self;
 }

src/mbgl/annotation/annotation_manager.cpp

@@ -4,6 +4,7 @@
 #include <mbgl/annotation/symbol_annotation_impl.hpp>
 #include <mbgl/annotation/line_annotation_impl.hpp>
 #include <mbgl/annotation/fill_annotation_impl.hpp>
+#include <mbgl/sprite/sprite_image_collection.hpp>
 #include <mbgl/style/style.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
@@ -19,7 +20,7 @@ const std::string AnnotationManager::SourceID = "com.mapbox.annotations";
 const std::string AnnotationManager::PointLayerID = "com.mapbox.annotations.points";
 
 AnnotationManager::AnnotationManager(float pixelRatio)
-    : spriteAtlas({ 1024, 1024 }, pixelRatio) {
+        : spriteAtlas({ 1024, 1024 }, pixelRatio){
     // This is a special atlas, holding only images added via addIcon, so we always treat it as
     // loaded.
     spriteAtlas.markAsLoaded();
@@ -190,16 +191,20 @@ void AnnotationManager::removeTile(AnnotationTile& tile) {
 }
 
 void AnnotationManager::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
-    spriteAtlas.addImage(id, std::move(image));
+    addSpriteImage(spriteImages, id, std::move(image), [&](style::Image& added) {
+        spriteAtlas.addImage(id, std::make_unique<style::Image>(added));
+    });
 }
 
 void AnnotationManager::removeImage(const std::string& id) {
-    spriteAtlas.removeImage(id);
+    removeSpriteImage(spriteImages, id, [&] () {
+        spriteAtlas.removeImage(id);
+    });
 }
 
 double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id) {
     const style::Image* image = spriteAtlas.getImage(id);
-    return image ? -(image->image.size.height / image->pixelRatio) / 2 : 0;
+    return image ? -(image->getImage().size.height / image->getPixelRatio()) / 2 : 0;
 }
 
 } // namespace mbgl

src/mbgl/annotation/annotation_manager.hpp

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 #include <unordered_set>
+#include <unordered_map>
 
 namespace mbgl {
 
@@ -72,6 +73,8 @@ class AnnotationManager : private util::noncopyable {
     ShapeAnnotationMap shapeAnnotations;
     std::unordered_set<std::string> obsoleteShapeAnnotationLayers;
     std::unordered_set<AnnotationTile*> tiles;
+
+    std::unordered_map<std::string, std::unique_ptr<style::Image>> spriteImages;
     SpriteAtlas spriteAtlas;
 
     friend class AnnotationTile;

src/mbgl/map/map.cpp

@@ -942,8 +942,7 @@ void Map::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
     }
 
     impl->styleMutated = true;
-    impl->style->spriteAtlas->addImage(id, std::move(image));
-    impl->onUpdate(Update::Repaint);
+    impl->style->addImage(id, std::move(image));
 }
 
 void Map::removeImage(const std::string& id) {
@@ -952,13 +951,12 @@ void Map::removeImage(const std::string& id) {
     }
 
     impl->styleMutated = true;
-    impl->style->spriteAtlas->removeImage(id);
-    impl->onUpdate(Update::Repaint);
+    impl->style->removeImage(id);
 }
 
 const style::Image* Map::getImage(const std::string& id) {
     if (impl->style) {
-        return impl->style->spriteAtlas->getImage(id);
+        return impl->style->getImage(id);
     }
     return nullptr;
 }

src/mbgl/sprite/sprite_atlas.cpp

@@ -1,48 +1,24 @@
 #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>
 #include <mbgl/util/math.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>
 #include <cmath>
 #include <algorithm>
 
 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