From aa216d00254c18f7b5903026ab1a489ae7797dd8 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Mon, 22 May 2017 13:28:09 -0700
Subject: [core] Don't use a separate SpriteAtlas for annotation images

Instead, just add them to the Style as needed. Includes changes from #8905 and takes care to avoid regressing #3817.
---
 cmake/core-files.cmake                        |  2 --
 cmake/test-files.cmake                        |  1 -
 include/mbgl/style/image.hpp                  |  1 +
 src/mbgl/annotation/annotation_manager.cpp    | 47 +++++++++++++++++----------
 src/mbgl/annotation/annotation_manager.hpp    | 10 +++---
 src/mbgl/annotation/annotation_tile.cpp       |  2 +-
 src/mbgl/map/map.cpp                          |  8 ++---
 src/mbgl/renderer/buckets/symbol_bucket.hpp   |  2 --
 src/mbgl/renderer/painter.cpp                 |  3 +-
 src/mbgl/renderer/painter.hpp                 |  3 +-
 src/mbgl/renderer/painters/painter_symbol.cpp |  5 ++-
 src/mbgl/sprite/sprite_atlas.cpp              | 18 ++++++++--
 src/mbgl/sprite/sprite_atlas.hpp              |  8 ++---
 src/mbgl/sprite/sprite_image_collection.cpp   | 40 -----------------------
 src/mbgl/sprite/sprite_image_collection.hpp   | 24 --------------
 src/mbgl/style/image.cpp                      |  2 ++
 src/mbgl/style/style.cpp                      | 28 ++++++++--------
 src/mbgl/style/style.hpp                      |  2 +-
 src/mbgl/tile/geometry_tile.cpp               |  7 ++--
 test/sprite/sprite_atlas.test.cpp             | 40 +++++++++++++++++++++++
 test/sprite/sprite_image_collection.test.cpp  | 45 -------------------------
 21 files changed, 121 insertions(+), 177 deletions(-)
 delete mode 100644 src/mbgl/sprite/sprite_image_collection.cpp
 delete mode 100644 src/mbgl/sprite/sprite_image_collection.hpp
 delete mode 100644 test/sprite/sprite_image_collection.test.cpp

diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake
index dfe11b82b8..e56f2fdc37 100644
--- a/cmake/core-files.cmake
+++ b/cmake/core-files.cmake
@@ -288,8 +288,6 @@ set(MBGL_CORE_FILES
     # sprite
     src/mbgl/sprite/sprite_atlas.cpp
     src/mbgl/sprite/sprite_atlas.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
diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake
index 83b935d63f..c0673bedef 100644
--- a/cmake/test-files.cmake
+++ b/cmake/test-files.cmake
@@ -46,7 +46,6 @@ 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
 
diff --git a/include/mbgl/style/image.hpp b/include/mbgl/style/image.hpp
index 53cab23c2f..55760ca503 100644
--- a/include/mbgl/style/image.hpp
+++ b/include/mbgl/style/image.hpp
@@ -11,6 +11,7 @@ namespace style {
 class Image {
 public:
     Image(std::string id, PremultipliedImage&&, float pixelRatio, bool sdf = false);
+    Image(const Image&);
 
     std::string getID() const;
 
diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index f04a0bb3dc..9ea47acadb 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -4,9 +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/image_impl.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
 #include <mbgl/storage/file_source.hpp>
@@ -20,12 +18,7 @@ using namespace style;
 const std::string AnnotationManager::SourceID = "com.mapbox.annotations";
 const std::string AnnotationManager::PointLayerID = "com.mapbox.annotations.points";
 
-AnnotationManager::AnnotationManager() {
-    // This is a special atlas, holding only images added via addIcon, so we always treat it as
-    // loaded.
-    spriteAtlas.markAsLoaded();
-}
-
+AnnotationManager::AnnotationManager() = default;
 AnnotationManager::~AnnotationManager() = default;
 
 AnnotationID AnnotationManager::addAnnotation(const Annotation& annotation, const uint8_t maxZoom) {
@@ -155,7 +148,7 @@ void AnnotationManager::updateStyle(Style& style) {
         std::unique_ptr<SymbolLayer> layer = std::make_unique<SymbolLayer>(PointLayerID, SourceID);
 
         layer->setSourceLayer(PointLayerID);
-        layer->setIconImage({"{sprite}"});
+        layer->setIconImage({SourceID + ".{sprite}"});
         layer->setIconAllowOverlap(true);
         layer->setIconIgnorePlacement(true);
 
@@ -166,13 +159,30 @@ void AnnotationManager::updateStyle(Style& style) {
         shape.second->updateStyle(style);
     }
 
+    for (const auto& image : images) {
+        // Call addImage even for images we may have previously added, because we must support
+        // addAnnotationImage being used to update an existing image. Creating a new image is
+        // relatively cheap, as it copies only the Immutable reference. (We can't keep track
+        // of which images need to be added because we don't know if the style is the same
+        // instance as in the last updateStyle call. If it's a new style, we need to add all
+        // images.)
+        style.addImage(std::make_unique<style::Image>(image.second));
+    }
+
     for (const auto& layer : obsoleteShapeAnnotationLayers) {
         if (style.getLayer(layer)) {
             style.removeLayer(layer);
         }
     }
 
+    for (const auto& image : obsoleteImages) {
+        if (style.getImage(image)) {
+            style.removeImage(image);
+        }
+    }
+
     obsoleteShapeAnnotationLayers.clear();
+    obsoleteImages.clear();
 }
 
 void AnnotationManager::updateData() {
@@ -191,20 +201,23 @@ void AnnotationManager::removeTile(AnnotationTile& tile) {
 }
 
 void AnnotationManager::addImage(std::unique_ptr<style::Image> image) {
-    addSpriteImage(spriteImages, std::move(image), [&](style::Image& added) {
-        spriteAtlas.addImage(added.impl);
-    });
+    // To ensure that annotation images do not collide with images from the style,
+    // create a new image with the input ID prefixed by "com.mapbox.annotations".
+    std::string id = SourceID + "." + image->getID();
+    images.erase(id);
+    images.emplace(id,
+        style::Image(id, image->getImage().clone(), image->getPixelRatio(), image->isSdf()));
+    obsoleteImages.erase(id);
 }
 
 void AnnotationManager::removeImage(const std::string& id) {
-    removeSpriteImage(spriteImages, id, [&] () {
-        spriteAtlas.removeImage(id);
-    });
+    images.erase(id);
+    obsoleteImages.insert(id);
 }
 
 double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id) {
-    const style::Image::Impl* impl = spriteAtlas.getImage(id);
-    return impl ? -(impl->image.size.height / impl->pixelRatio) / 2 : 0;
+    auto it = images.find(id);
+    return it == images.end() ? -(it->second.getImage().size.height / it->second.getPixelRatio()) / 2 : 0;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp
index 4d22ac81f6..36f18e1aae 100644
--- a/src/mbgl/annotation/annotation_manager.hpp
+++ b/src/mbgl/annotation/annotation_manager.hpp
@@ -2,7 +2,7 @@
 
 #include <mbgl/annotation/annotation.hpp>
 #include <mbgl/annotation/symbol_annotation_impl.hpp>
-#include <mbgl/sprite/sprite_atlas.hpp>
+#include <mbgl/style/image.hpp>
 #include <mbgl/map/update.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
@@ -21,7 +21,6 @@ class ShapeAnnotationImpl;
 
 namespace style {
 class Style;
-class Image;
 } // namespace style
 
 class AnnotationManager : private util::noncopyable {
@@ -36,7 +35,6 @@ public:
     void addImage(std::unique_ptr<style::Image>);
     void removeImage(const std::string&);
     double getTopOffsetPixelsForImage(const std::string&);
-    SpriteAtlas& getSpriteAtlas() { return spriteAtlas; }
 
     void updateStyle(style::Style&);
     void updateData();
@@ -67,16 +65,16 @@ private:
     // <https://github.com/mapbox/mapbox-gl-native/issues/5691>
     using SymbolAnnotationMap = std::map<AnnotationID, std::shared_ptr<SymbolAnnotationImpl>>;
     using ShapeAnnotationMap = std::map<AnnotationID, std::unique_ptr<ShapeAnnotationImpl>>;
+    using ImageMap = std::unordered_map<std::string, style::Image>;
 
     SymbolAnnotationTree symbolTree;
     SymbolAnnotationMap symbolAnnotations;
     ShapeAnnotationMap shapeAnnotations;
+    ImageMap images;
     std::unordered_set<std::string> obsoleteShapeAnnotationLayers;
+    std::unordered_set<std::string> obsoleteImages;
     std::unordered_set<AnnotationTile*> tiles;
 
-    std::unordered_map<std::string, std::unique_ptr<style::Image>> spriteImages;
-    SpriteAtlas spriteAtlas;
-
     friend class AnnotationTile;
 };
 
diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp
index 1253681414..1b34026f74 100644
--- a/src/mbgl/annotation/annotation_tile.cpp
+++ b/src/mbgl/annotation/annotation_tile.cpp
@@ -13,7 +13,7 @@ AnnotationTile::AnnotationTile(const OverscaledTileID& overscaledTileID,
                                const TileParameters& parameters)
     : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters,
                    *parameters.style.glyphAtlas,
-                   parameters.annotationManager.spriteAtlas),
+                   *parameters.style.spriteAtlas),
       annotationManager(parameters.annotationManager) {
     annotationManager.addTile(*this);
 }
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 14a2401ed5..a24e798fb9 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -278,8 +278,7 @@ void Map::Impl::render(View& view) {
 
         painter->render(*style,
                         frameData,
-                        view,
-                        annotationManager->getSpriteAtlas());
+                        view);
 
         painter->cleanup();
 
@@ -312,8 +311,7 @@ void Map::Impl::render(View& view) {
 
         painter->render(*style,
                         frameData,
-                        view,
-                        annotationManager->getSpriteAtlas());
+                        view);
 
         auto request = std::move(stillImageRequest);
         request->callback(nullptr);
@@ -790,10 +788,12 @@ LatLng Map::latLngForPixel(const ScreenCoordinate& pixel) const {
 
 void Map::addAnnotationImage(std::unique_ptr<style::Image> image) {
     impl->annotationManager->addImage(std::move(image));
+    impl->onUpdate(Update::AnnotationStyle);
 }
 
 void Map::removeAnnotationImage(const std::string& id) {
     impl->annotationManager->removeImage(id);
+    impl->onUpdate(Update::AnnotationStyle);
 }
 
 double Map::getTopOffsetPixelsForAnnotationImage(const std::string& id) {
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
index f30c9cf10e..652f2ea8e3 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -70,8 +70,6 @@ public:
         optional<gl::VertexBuffer<CollisionBoxVertex>> vertexBuffer;
         optional<gl::IndexBuffer<gl::Lines>> indexBuffer;
     } collisionBox;
-
-    SpriteAtlas* spriteAtlas = nullptr;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp
index 7649f86ca6..3832403cce 100644
--- a/src/mbgl/renderer/painter.cpp
+++ b/src/mbgl/renderer/painter.cpp
@@ -125,7 +125,7 @@ void Painter::cleanup() {
     context.performCleanup();
 }
 
-void Painter::render(const Style& style, const FrameData& frame_, View& view, SpriteAtlas& annotationSpriteAtlas) {
+void Painter::render(const Style& style, const FrameData& frame_, View& view) {
     frame = frame_;
     if (frame.contextMode == GLContextMode::Shared) {
         context.setDirtyState();
@@ -176,7 +176,6 @@ void Painter::render(const Style& style, const FrameData& frame_, View& view, Sp
         lineAtlas->upload(context, 0);
         glyphAtlas->upload(context, 0);
         frameHistory.upload(context, 0);
-        annotationSpriteAtlas.upload(context, 0);
 
         for (const auto& item : order) {
             for (const auto& tileRef : item.tiles) {
diff --git a/src/mbgl/renderer/painter.hpp b/src/mbgl/renderer/painter.hpp
index 47b469d971..e2777134d0 100644
--- a/src/mbgl/renderer/painter.hpp
+++ b/src/mbgl/renderer/painter.hpp
@@ -79,8 +79,7 @@ public:
 
     void render(const style::Style&,
                 const FrameData&,
-                View&,
-                SpriteAtlas& annotationSpriteAtlas);
+                View&);
 
     void cleanup();
 
diff --git a/src/mbgl/renderer/painters/painter_symbol.cpp b/src/mbgl/renderer/painters/painter_symbol.cpp
index a89e7db28d..563489987c 100644
--- a/src/mbgl/renderer/painters/painter_symbol.cpp
+++ b/src/mbgl/renderer/painters/painter_symbol.cpp
@@ -67,12 +67,11 @@ void Painter::renderSymbol(PaintParameters& parameters,
         auto values = layer.iconPropertyValues(layout);
         auto paintPropertyValues = layer.iconPaintProperties();
 
-        SpriteAtlas& atlas = *bucket.spriteAtlas;
         const bool iconScaled = layout.get<IconSize>().constantOr(1.0) != 1.0 || bucket.iconsNeedLinear;
         const bool iconTransformed = values.rotationAlignment == AlignmentType::Map || state.getPitch() != 0;
-        atlas.bind(bucket.sdfIcons || state.isChanging() || iconScaled || iconTransformed, context, 0);
+        spriteAtlas->bind(bucket.sdfIcons || state.isChanging() || iconScaled || iconTransformed, context, 0);
 
-        const Size texsize = atlas.getPixelSize();
+        const Size texsize = spriteAtlas->getPixelSize();
 
         if (bucket.sdfIcons) {
             if (values.hasHalo) {
diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp
index d3f0072b20..97bf9edcf9 100644
--- a/src/mbgl/sprite/sprite_atlas.cpp
+++ b/src/mbgl/sprite/sprite_atlas.cpp
@@ -44,7 +44,7 @@ SpriteAtlas::SpriteAtlas()
 SpriteAtlas::~SpriteAtlas() = default;
 
 void SpriteAtlas::onSpriteLoaded() {
-    markAsLoaded();
+    loaded = true;
     for (auto requestor : requestors) {
         requestor->onIconsAvailable(buildIconMap());
     }
@@ -106,8 +106,20 @@ const style::Image::Impl* SpriteAtlas::getImage(const std::string& id) const {
     return nullptr;
 }
 
-void SpriteAtlas::getIcons(IconRequestor& requestor) {
-    if (isLoaded()) {
+void SpriteAtlas::getIcons(IconRequestor& requestor, IconDependencies dependencies) {
+    // If the sprite has been loaded, or if all the icon dependencies are already present
+    // (i.e. if they've been addeded via runtime styling), then notify the requestor immediately.
+    // Otherwise, delay notification until the sprite is loaded. At that point, if any of the
+    // dependencies are still unavailable, we'll just assume they are permanently missing.
+    bool hasAllDependencies = true;
+    if (!isLoaded()) {
+        for (const auto& dependency : dependencies) {
+            if (entries.find(dependency) == entries.end()) {
+                hasAllDependencies = false;
+            }
+        }
+    }
+    if (isLoaded() || hasAllDependencies) {
         requestor.onIconsAvailable(buildIconMap());
     } else {
         requestors.insert(&requestor);
diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp
index f5c7fd114a..807c871731 100644
--- a/src/mbgl/sprite/sprite_atlas.hpp
+++ b/src/mbgl/sprite/sprite_atlas.hpp
@@ -66,10 +66,6 @@ public:
 
     void onSpriteLoaded();
 
-    void markAsLoaded() {
-        loaded = true;
-    }
-
     bool isLoaded() const {
         return loaded;
     }
@@ -80,8 +76,8 @@ public:
     void addImage(Immutable<style::Image::Impl>);
     void removeImage(const std::string&);
 
-    void getIcons(IconRequestor& requestor);
-    void removeRequestor(IconRequestor& requestor);
+    void getIcons(IconRequestor&, IconDependencies);
+    void removeRequestor(IconRequestor&);
 
     // Ensure that the atlas contains the named image suitable for rendering as an icon, and
     // return its metrics. The image will be padded on each side with a one pixel wide transparent
diff --git a/src/mbgl/sprite/sprite_image_collection.cpp b/src/mbgl/sprite/sprite_image_collection.cpp
deleted file mode 100644
index ae00a6b146..0000000000
--- a/src/mbgl/sprite/sprite_image_collection.cpp
+++ /dev/null
@@ -1,40 +0,0 @@
-#include <mbgl/sprite/sprite_image_collection.hpp>
-#include <mbgl/util/logging.hpp>
-
-namespace mbgl {
-
-void addSpriteImage(Images& images,
-                           std::unique_ptr<style::Image> image_,
-                           std::function<void (style::Image&)> onAdded) {
-    std::string id = image_->getID();
-    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
deleted file mode 100644
index 44c7bcd411..0000000000
--- a/src/mbgl/sprite/sprite_image_collection.hpp
+++ /dev/null
@@ -1,24 +0,0 @@
-#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&,
-                    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/style/image.cpp b/src/mbgl/style/image.cpp
index 0cce32ab98..6039c0458c 100644
--- a/src/mbgl/style/image.cpp
+++ b/src/mbgl/style/image.cpp
@@ -16,6 +16,8 @@ std::string Image::getID() const {
     return impl->id;
 }
 
+Image::Image(const Image&) = default;
+
 const PremultipliedImage& Image::getImage() const {
     return impl->image;
 }
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index 1a4cf0ca10..9445772a66 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -16,7 +16,6 @@
 #include <mbgl/style/parser.hpp>
 #include <mbgl/style/transition_options.hpp>
 #include <mbgl/sprite/sprite_atlas.hpp>
-#include <mbgl/sprite/sprite_image_collection.hpp>
 #include <mbgl/sprite/sprite_loader.hpp>
 #include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/geometry/line_atlas.hpp>
@@ -36,6 +35,7 @@
 #include <mbgl/renderer/layers/render_raster_layer.hpp>
 #include <mbgl/renderer/layers/render_symbol_layer.hpp>
 #include <mbgl/renderer/style_diff.hpp>
+#include <mbgl/sprite/sprite_atlas.hpp>
 #include <mbgl/tile/tile.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/exception.hpp>
@@ -506,22 +506,24 @@ bool Style::isLoaded() const {
 }
 
 void Style::addImage(std::unique_ptr<style::Image> image) {
-    addSpriteImage(spriteImages, std::move(image), [&](style::Image& added) {
-        spriteAtlas->addImage(added.impl);
-        observer->onUpdate(Update::Repaint);
-    });
+    std::string id = image->getID();
+    auto it = images.find(id);
+    if (it != images.end() && it->second->getImage().size != image->getImage().size) {
+        Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", id.c_str());
+        return;
+    }
+    spriteAtlas->addImage(image->impl);
+    images[id] = std::move(image);
 }
 
 void Style::removeImage(const std::string& id) {
-    removeSpriteImage(spriteImages, id, [&] () {
-        spriteAtlas->removeImage(id);
-        observer->onUpdate(Update::Repaint);
-    });
+    images.erase(id);
+    spriteAtlas->removeImage(id);
 }
 
 const style::Image* Style::getImage(const std::string& id) const {
-    auto it = spriteImages.find(id);
-    return it == spriteImages.end() ? nullptr : it->second.get();
+    auto it = images.find(id);
+    return it == images.end() ? nullptr : it->second.get();
 }
 
 RenderData Style::getRenderData(MapDebugOptions debugOptions, float angle) const {
@@ -735,8 +737,8 @@ void Style::onTileError(RenderSource& source, const OverscaledTileID& tileID, st
     observer->onResourceError(error);
 }
 
-void Style::onSpriteLoaded(std::vector<std::unique_ptr<Image>>&& images) {
-    for (auto& image : images) {
+void Style::onSpriteLoaded(std::vector<std::unique_ptr<Image>>&& images_) {
+    for (auto& image : images_) {
         addImage(std::move(image));
     }
     spriteAtlas->onSpriteLoaded();
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index bc1d52eed8..5d3c2899fd 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -125,6 +125,7 @@ public:
     RenderSource* getRenderSource(const std::string& id) const;
 
 private:
+    std::unordered_map<std::string, std::unique_ptr<style::Image>> images;
     std::vector<std::unique_ptr<Source>> sources;
     std::vector<std::unique_ptr<Layer>> layers;
     TransitionOptions transitionOptions;
@@ -150,7 +151,6 @@ private:
     void onGlyphsError(const FontStack&, const GlyphRange&, std::exception_ptr) override;
 
     // SpriteLoaderObserver implementation.
-    std::unordered_map<std::string, std::unique_ptr<style::Image>> spriteImages;
     void onSpriteLoaded(std::vector<std::unique_ptr<Image>>&&) override;
     void onSpriteError(std::exception_ptr) override;
 
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index e3c4e7bf3b..d25d93d6b8 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -139,9 +139,6 @@ void GeometryTile::onPlacement(PlacementResult result) {
         pending = false;
     }
     symbolBuckets = std::move(result.symbolBuckets);
-    for (auto& entry : symbolBuckets) {
-        dynamic_cast<SymbolBucket*>(entry.second.get())->spriteAtlas = &spriteAtlas;
-    }
     collisionTile = std::move(result.collisionTile);
     observer->onTileChanged(*this);
 }
@@ -165,8 +162,8 @@ void GeometryTile::onIconsAvailable(IconMap icons) {
     worker.invoke(&GeometryTileWorker::onIconsAvailable, std::move(icons));
 }
 
-void GeometryTile::getIcons(IconDependencies) {
-    spriteAtlas.getIcons(*this);
+void GeometryTile::getIcons(IconDependencies iconDependencies) {
+    spriteAtlas.getIcons(*this, std::move(iconDependencies));
 }
 
 Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const {
diff --git a/test/sprite/sprite_atlas.test.cpp b/test/sprite/sprite_atlas.test.cpp
index 7a638a9ec5..4291fe9902 100644
--- a/test/sprite/sprite_atlas.test.cpp
+++ b/test/sprite/sprite_atlas.test.cpp
@@ -145,3 +145,43 @@ TEST(SpriteAtlas, RemoveReleasesBinPackRect) {
     EXPECT_TRUE(atlas.getIcon("big"));
     EXPECT_TRUE(log.empty());
 }
+
+class StubIconRequestor : public IconRequestor {
+public:
+    void onIconsAvailable(IconMap icons) final {
+        if (iconsAvailable) iconsAvailable(icons);
+    }
+
+    std::function<void (IconMap)> iconsAvailable;
+};
+
+TEST(SpriteAtlas, NotifiesRequestorWhenSpriteIsLoaded) {
+    SpriteAtlas atlas;
+    StubIconRequestor requestor;
+    bool notified = false;
+
+    requestor.iconsAvailable = [&] (IconMap) {
+        notified = true;
+    };
+
+    atlas.getIcons(requestor, {"one"});
+    ASSERT_FALSE(notified);
+
+    atlas.onSpriteLoaded();
+    ASSERT_TRUE(notified);
+}
+
+TEST(SpriteAtlas, NotifiesRequestorImmediatelyIfDependenciesAreSatisfied) {
+    SpriteAtlas atlas;
+    StubIconRequestor requestor;
+    bool notified = false;
+
+    requestor.iconsAvailable = [&] (IconMap) {
+        notified = true;
+    };
+
+    atlas.addImage(makeMutable<style::Image::Impl>("one", PremultipliedImage({ 16, 16 }), 2));
+    atlas.getIcons(requestor, {"one"});
+
+    ASSERT_TRUE(notified);
+}
diff --git a/test/sprite/sprite_image_collection.test.cpp b/test/sprite/sprite_image_collection.test.cpp
deleted file mode 100644
index 788d7a59d5..0000000000
--- a/test/sprite/sprite_image_collection.test.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-#include <mbgl/test/util.hpp>
-#include <mbgl/test/fixture_log_observer.hpp>
-
-#include <mbgl/sprite/sprite_image_collection.hpp>
-#include <mbgl/util/image.hpp>
-#include <mbgl/util/string.hpp>
-
-#include <utility>
-
-using namespace mbgl;
-
-TEST(SpriteImageCollection, OtherPixelRatio) {
-    FixtureLog log;
-    Images images;
-
-    // Adding mismatched sprite image
-    addSpriteImage(images, std::make_unique<style::Image>("one", PremultipliedImage({ 8, 8 }), 2));
-}
-
-TEST(SpriteImageCollection, Replace) {
-    FixtureLog log;
-    Images images;
-
-    addSpriteImage(images, std::make_unique<style::Image>("sprite", PremultipliedImage({ 16, 16 }), 2));
-    auto image = images.find("sprite")->second.get();
-    addSpriteImage(images, std::make_unique<style::Image>("sprite", PremultipliedImage({ 16, 16 }), 2));
-    EXPECT_NE(image, images.find("sprite")->second.get());
-}
-
-TEST(SpriteImageCollection, ReplaceWithDifferentDimensions) {
-    FixtureLog log;
-    Images images;
-
-    addSpriteImage(images, std::make_unique<style::Image>("sprite", PremultipliedImage({ 16, 16 }), 2));
-    addSpriteImage(images, std::make_unique<style::Image>("sprite", PremultipliedImage({ 18, 18 }), 2));
-
-    EXPECT_EQ(1u, log.count({
-                                    EventSeverity::Warning,
-                                    Event::Sprite,
-                                    int64_t(-1),
-                                    "Can't change sprite dimensions for 'sprite'",
-                            }));
-}
-
-
-- 
cgit v1.2.1