Move the Sprite handling to ResourceLoader

This is the first concrete step to remove promises from the parsing
pipeline. Buckets depending on the Sprite (i.e. SymbolBucket) won't
be created if the Sprite is not loaded yet. When the Sprite finally gets
loaded, it will fire a signal that will ultimately trigger a reparse.

We still have to expose the Sprite to the Map object as it is needed
for getting offset pixels for annotation symbols, but we can probably
improve that.

9 files changed, 69 insertions, 64 deletions

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index 50225b4297..2352e09c2b 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -3,7 +3,6 @@
 #include <mbgl/map/view.hpp>
 #include <mbgl/map/environment.hpp>
 #include <mbgl/map/source.hpp>
-#include <mbgl/map/sprite.hpp>
 #include <mbgl/map/still_image.hpp>
 
 #include <mbgl/platform/log.hpp>
@@ -64,7 +63,6 @@ MapContext::~MapContext() {
     // cleaned up by env.performCleanup();
     resourceLoader.reset();
     style.reset();
-    sprite.reset();
     painter.reset();
     texturePool.reset();
     lineAtlas.reset();
@@ -125,25 +123,10 @@ void MapContext::setStyleJSON(const std::string& json, const std::string& base)
     loadStyleJSON(json, base);
 }
 
-util::ptr<Sprite> MapContext::getSprite() {
-    assert(Environment::currentlyOn(ThreadType::Map));
-    const float pixelRatio = transformState.getPixelRatio();
-    const std::string &sprite_url = style->getSpriteURL();
-    if (!sprite || !sprite->hasPixelRatio(pixelRatio)) {
-        sprite = std::make_shared<Sprite>(sprite_url, pixelRatio, env, [this] {
-            assert(Environment::currentlyOn(ThreadType::Map));
-            triggerUpdate();
-        });
-    }
-
-    return sprite;
-}
-
 void MapContext::loadStyleJSON(const std::string& json, const std::string& base) {
     assert(Environment::currentlyOn(ThreadType::Map));
 
     resourceLoader.reset();
-    sprite.reset();
     style.reset();
 
     style = util::make_unique<Style>();
@@ -167,7 +150,7 @@ void MapContext::updateTiles() {
     assert(Environment::currentlyOn(ThreadType::Map));
 
     resourceLoader->update(data, transformState, *glyphAtlas, *glyphStore,
-                           *spriteAtlas, getSprite(), *texturePool);
+                           *spriteAtlas, *texturePool);
 }
 
 void MapContext::updateAnnotationTiles(const std::vector<TileID>& ids) {
@@ -209,13 +192,9 @@ void MapContext::update() {
             style->recalculate(transformState.getNormalizedZoom(), now);
         }
 
-        // Allow the sprite atlas to potentially pull new sprite images if needed.
-        spriteAtlas->resize(transformState.getPixelRatio());
-        spriteAtlas->setSprite(getSprite());
-
         updateTiles();
 
-        if (sprite->isLoaded() && style->isLoaded()) {
+        if (style->isLoaded()) {
             if (!data.getFullyLoaded()) {
                 data.setFullyLoaded(true);
             }
@@ -255,7 +234,7 @@ void MapContext::render() {
 
     painter->render(*style, transformState, data.getAnimationTime());
 
-    if (data.mode == MapMode::Still && callback && style->isLoaded() && getSprite()->isLoaded()) {
+    if (data.mode == MapMode::Still && callback && style->isLoaded() && resourceLoader->getSprite()->isLoaded()) {
         callback(view.readStillImage());
         callback = nullptr;
     }
@@ -268,8 +247,7 @@ void MapContext::render() {
 
 double MapContext::getTopOffsetPixelsForAnnotationSymbol(const std::string& symbol) {
     assert(Environment::currentlyOn(ThreadType::Map));
-    assert(sprite);
-    const SpritePosition pos = sprite->getSpritePosition(symbol);
+    const SpritePosition pos = resourceLoader->getSprite()->getSpritePosition(symbol);
     return -pos.height / pos.pixelRatio / 2;
 }
 
diff --git a/src/mbgl/map/map_context.hpp b/src/mbgl/map/map_context.hpp
index 6941c3040d..0463d076b6 100644
--- a/src/mbgl/map/map_context.hpp
+++ b/src/mbgl/map/map_context.hpp
@@ -90,8 +90,6 @@ private:
     std::unique_ptr<Style> style;
     std::unique_ptr<ResourceLoader> resourceLoader;
 
-    util::ptr<Sprite> sprite;
-
     std::string styleURL;
     std::string styleJSON;
 
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 3d4a6ea5bb..00688911a9 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -1,7 +1,10 @@
 #include <mbgl/map/resource_loader.hpp>
 
+#include <mbgl/geometry/sprite_atlas.hpp>
 #include <mbgl/map/environment.hpp>
 #include <mbgl/map/source.hpp>
+#include <mbgl/map/sprite.hpp>
+#include <mbgl/map/transform.hpp>
 #include <mbgl/style/style.hpp>
 
 #include <cassert>
@@ -18,6 +21,8 @@ ResourceLoader::~ResourceLoader() {
     for (const auto& source : style_->sources) {
         source->setObserver(nullptr);
     }
+
+    sprite_->setObserver(nullptr);
 }
 
 void ResourceLoader::setObserver(Observer* observer) {
@@ -28,6 +33,8 @@ void ResourceLoader::setObserver(Observer* observer) {
 }
 
 void ResourceLoader::setStyle(Style* style) {
+    assert(style);
+
     style_ = style;
 
     Environment& env = Environment::Get();
@@ -46,15 +53,23 @@ void ResourceLoader::update(MapData& data,
                             GlyphAtlas& glyphAtlas,
                             GlyphStore& glyphStore,
                             SpriteAtlas& spriteAtlas,
-                            util::ptr<Sprite> sprite,
                             TexturePool& texturePool) {
     if (!style_) {
         return;
     }
 
+    const float pixelRatio = transform.getPixelRatio();
+    if (!sprite_ || !sprite_->hasPixelRatio(pixelRatio)) {
+        sprite_ = util::make_unique<Sprite>(style_->getSpriteURL(), pixelRatio);
+        sprite_->setObserver(this);
+
+        spriteAtlas.resize(pixelRatio);
+        spriteAtlas.setSprite(sprite_);
+    }
+
     for (const auto& source : style_->sources) {
         source->update(
-            data, transform, *style_, glyphAtlas, glyphStore, spriteAtlas, sprite, texturePool);
+            data, transform, *style_, glyphAtlas, glyphStore, spriteAtlas, sprite_, texturePool);
     }
 }
 
@@ -66,6 +81,10 @@ void ResourceLoader::onTileLoaded() {
     emitTileDataChanged();
 }
 
+void ResourceLoader::onSpriteLoaded() {
+    emitTileDataChanged();
+}
+
 void ResourceLoader::emitTileDataChanged() {
     assert(Environment::currentlyOn(ThreadType::Map));
 
diff --git a/src/mbgl/map/resource_loader.hpp b/src/mbgl/map/resource_loader.hpp
index 391c460595..af518a48a7 100644
--- a/src/mbgl/map/resource_loader.hpp
+++ b/src/mbgl/map/resource_loader.hpp
@@ -2,6 +2,7 @@
 #define MBGL_MAP_RESOURCE_LOADER
 
 #include <mbgl/map/source.hpp>
+#include <mbgl/map/sprite.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <string>
@@ -20,7 +21,7 @@ class TransformState;
 // by the Style. The Source object currently owns all the tiles, thus this
 // class will notify its observers of any change on these tiles which will
 // ultimately cause a new rendering to be triggered.
-class ResourceLoader : public Source::Observer, private util::noncopyable {
+class ResourceLoader : public Source::Observer, public Sprite::Observer, private util::noncopyable {
 public:
     class Observer {
     public:
@@ -44,16 +45,25 @@ public:
     // Fetch the tiles needed by the current viewport and emit a signal when
     // a tile is ready so observers can render the tile.
     void update(MapData&, const TransformState&, GlyphAtlas&, GlyphStore&,
-                SpriteAtlas&, util::ptr<Sprite>, TexturePool&);
+                SpriteAtlas&, TexturePool&);
+
+    // FIXME: There is probably a better place for this.
+    inline util::ptr<Sprite> getSprite() const {
+        return sprite_;
+    }
 
     // Source::Observer implementation.
     void onSourceLoaded() override;
     void onTileLoaded() override;
 
+    // Sprite::Observer implementation.
+    void onSpriteLoaded() override;
+
 private:
     void emitTileDataChanged();
 
     std::string accessToken_;
+    util::ptr<Sprite> sprite_;
     Style* style_ = nullptr;
     Observer* observer_ = nullptr;
 };
diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp
index 75e5f845c4..cfc0974571 100644
--- a/src/mbgl/map/sprite.cpp
+++ b/src/mbgl/map/sprite.cpp
@@ -23,20 +23,16 @@ SpritePosition::SpritePosition(uint16_t x_, uint16_t y_, uint16_t width_, uint16
       sdf(sdf_) {
 }
 
-Sprite::Sprite(const std::string& baseUrl, float pixelRatio_, Environment& env_, std::function<void ()> callback_)
+Sprite::Sprite(const std::string& baseUrl, float pixelRatio_)
     : pixelRatio(pixelRatio_ > 1 ? 2 : 1),
       raster(),
       loadedImage(false),
       loadedJSON(false),
-      future(promise.get_future()),
-      callback(callback_),
-      env(env_) {
-
+      env(Environment::Get()) {
     if (baseUrl.empty()) {
         // Treat a non-existent sprite as a successfully loaded empty sprite.
         loadedImage = true;
         loadedJSON = true;
-        promise.set_value();
         return;
     }
 
@@ -52,7 +48,7 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_, Environment& env_,
             Log::Warning(Event::Sprite, "Failed to load sprite info: %s", res.message.c_str());
         }
         loadedJSON = true;
-        complete();
+        emitSpriteLoadedIfComplete();
     });
 
     spriteRequest = env.request({ Resource::Kind::Image, spriteURL }, [this](const Response &res) {
@@ -64,7 +60,7 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_, Environment& env_,
             Log::Warning(Event::Sprite, "Failed to load sprite image: %s", res.message.c_str());
         }
         loadedImage = true;
-        complete();
+        emitSpriteLoadedIfComplete();
     });
 }
 
@@ -78,10 +74,9 @@ Sprite::~Sprite() {
     }
 }
 
-void Sprite::complete() {
-    if (loadedImage && loadedJSON) {
-        promise.set_value();
-        callback();
+void Sprite::emitSpriteLoadedIfComplete() {
+    if (isLoaded() && observer) {
+        observer->onSpriteLoaded();
     }
 }
 
@@ -89,10 +84,6 @@ bool Sprite::isLoaded() const {
     return loadedImage && loadedJSON;
 }
 
-void Sprite::waitUntilLoaded() const {
-    future.wait();
-}
-
 bool Sprite::hasPixelRatio(float ratio) const {
     return pixelRatio == (ratio > 1 ? 2 : 1);
 }
@@ -144,3 +135,7 @@ const SpritePosition &Sprite::getSpritePosition(const std::string& name) const {
     auto it = pos.find(name);
     return it == pos.end() ? empty : it->second;
 }
+
+void Sprite::setObserver(Observer* observer_) {
+    observer = observer_;
+}
diff --git a/src/mbgl/map/sprite.hpp b/src/mbgl/map/sprite.hpp
index bd6ae89bc6..ccf2561618 100644
--- a/src/mbgl/map/sprite.hpp
+++ b/src/mbgl/map/sprite.hpp
@@ -11,7 +11,6 @@
 #include <iosfwd>
 #include <string>
 #include <unordered_map>
-#include <future>
 
 namespace mbgl {
 
@@ -35,23 +34,31 @@ public:
 
 class Sprite : private util::noncopyable {
 public:
-    Sprite(const std::string& baseUrl, float pixelRatio, Environment&, std::function<void()> callback);
+    class Observer {
+    public:
+        virtual ~Observer() = default;
+
+        virtual void onSpriteLoaded() = 0;
+    };
+
+    Sprite(const std::string& baseUrl, float pixelRatio);
     ~Sprite();
 
     const SpritePosition &getSpritePosition(const std::string& name) const;
 
     bool hasPixelRatio(float ratio) const;
 
-    void waitUntilLoaded() const;
     bool isLoaded() const;
 
     const float pixelRatio;
     std::unique_ptr<util::Image> raster;
 
+    void setObserver(Observer* observer);
 private:
+    void emitSpriteLoadedIfComplete();
+
     void parseJSON();
     void parseImage();
-    void complete();
 
     std::string body;
     std::string image;
@@ -60,13 +67,10 @@ private:
     std::unordered_map<std::string, SpritePosition> pos;
     const SpritePosition empty;
 
-    std::promise<void> promise;
-    std::future<void> future;
-    std::function<void ()> callback;
-
     Environment& env;
     Request* jsonRequest = nullptr;
     Request* spriteRequest = nullptr;
+    Observer* observer = nullptr;
 };
 
 }
diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp
index 1438bcddaa..ad66aa1544 100644
--- a/src/mbgl/map/tile_parser.cpp
+++ b/src/mbgl/map/tile_parser.cpp
@@ -3,6 +3,7 @@
 #include <mbgl/platform/log.hpp>
 #include <mbgl/style/style_layer.hpp>
 #include <mbgl/map/source.hpp>
+#include <mbgl/map/sprite.hpp>
 #include <mbgl/renderer/fill_bucket.hpp>
 #include <mbgl/renderer/line_bucket.hpp>
 #include <mbgl/renderer/symbol_bucket.hpp>
@@ -117,7 +118,11 @@ std::unique_ptr<Bucket> TileParser::createBucket(const StyleBucket &bucketDesc)
         } else if (bucketDesc.type == StyleLayerType::Line) {
             return createLineBucket(*layer, bucketDesc);
         } else if (bucketDesc.type == StyleLayerType::Symbol) {
-            return createSymbolBucket(*layer, bucketDesc);
+            if (sprite->isLoaded()) {
+                return createSymbolBucket(*layer, bucketDesc);
+            } else {
+                return nullptr;
+            }
         } else if (bucketDesc.type == StyleLayerType::Raster) {
             return nullptr;
         } else {
diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp
index 769feb67a4..d6ae6c397f 100644
--- a/src/mbgl/renderer/symbol_bucket.cpp
+++ b/src/mbgl/renderer/symbol_bucket.cpp
@@ -64,8 +64,7 @@ bool SymbolBucket::hasIconData() const { return !icon.groups.empty(); }
 
 std::vector<SymbolFeature> SymbolBucket::processFeatures(const GeometryTileLayer& layer,
                                                          const FilterExpression& filter,
-                                                         GlyphStore &glyphStore,
-                                                         const Sprite &sprite) {
+                                                         GlyphStore &glyphStore) {
     const bool has_text = !layout.text.field.empty() && !layout.text.font.empty();
     const bool has_icon = !layout.icon.image.empty();
 
@@ -136,7 +135,6 @@ std::vector<SymbolFeature> SymbolBucket::processFeatures(const GeometryTileLayer
     }
 
     glyphStore.waitForGlyphRanges(layout.text.font, ranges);
-    sprite.waitUntilLoaded();
 
     return features;
 }
@@ -148,7 +146,7 @@ void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
                                Sprite& sprite,
                                GlyphAtlas& glyphAtlas,
                                GlyphStore& glyphStore) {
-    const std::vector<SymbolFeature> features = processFeatures(layer, filter, glyphStore, sprite);
+    const std::vector<SymbolFeature> features = processFeatures(layer, filter, glyphStore);
 
     float horizontalAlign = 0.5;
     float verticalAlign = 0.5;
@@ -221,7 +219,6 @@ void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
 
         // if feature has icon, get sprite atlas position
         if (feature.sprite.length()) {
-            sprite.waitUntilLoaded();
             image = spriteAtlas.getImage(feature.sprite, false);
 
             if (sprite.getSpritePosition(feature.sprite).sdf) {
diff --git a/src/mbgl/renderer/symbol_bucket.hpp b/src/mbgl/renderer/symbol_bucket.hpp
index b1dc44a113..54a77b5099 100644
--- a/src/mbgl/renderer/symbol_bucket.hpp
+++ b/src/mbgl/renderer/symbol_bucket.hpp
@@ -76,8 +76,7 @@ public:
 private:
     std::vector<SymbolFeature> processFeatures(const GeometryTileLayer&,
                                                const FilterExpression&,
-                                               GlyphStore&,
-                                               const Sprite&);
+                                               GlyphStore&);
 
     void addFeature(const std::vector<Coordinate> &line, const Shaping &shaping, const GlyphPositions &face, const Rect<uint16_t> &image);