move sprite to a use future loading

4 files changed, 81 insertions, 54 deletions

diff --git a/src/map/map.cpp b/src/map/map.cpp
index ca43662e09..269d174c8c 100644
--- a/src/map/map.cpp
+++ b/src/map/map.cpp
@@ -201,6 +201,17 @@ std::string Map::getAccessToken() const {
     return accessToken;
 }
 
+std::shared_ptr<Sprite> Map::getSprite() {
+    float pixelRatio = state.getPixelRatio();
+    const std::string &sprite_url = style->getSpriteURL();
+    if (!sprite || sprite->pixelRatio != pixelRatio || sprite->url != sprite_url) {
+        sprite = Sprite::Create(sprite_url, pixelRatio);
+    }
+
+    return sprite;
+}
+
+
 #pragma mark - Size
 
 // Note: This function is called from another thread. Make sure you only call threadsafe functions!
@@ -502,14 +513,6 @@ void Map::prepare() {
     bool dimensionsChanged = oldState.getFramebufferWidth() != state.getFramebufferWidth() ||
                              oldState.getFramebufferHeight() != state.getFramebufferHeight();
 
-    if (pixelRatioChanged) {
-        style->sprite = std::make_shared<Sprite>(*this, state.getPixelRatio());
-        if (style->sprite_url.size()) {
-            style->sprite->load(style->sprite_url);
-        }
-        spriteAtlas->resize(state.getPixelRatio());
-    }
-
     // Create a new glyph store object in case the glyph URL changed.
     // TODO: Move this to a less frequently called place; we only need to do this when the
     // stylesheet changes.
@@ -528,11 +531,13 @@ void Map::prepare() {
     updateSources();
     style->updateProperties(state.getNormalizedZoom(), animationTime);
 
-    // Allow the sprite atlas to potentially pull new sprite images if needed.
-    if (style->sprite && style->sprite->isLoaded()) {
-        spriteAtlas->update(*style->sprite);
+    if (pixelRatioChanged) {
+        spriteAtlas->resize(state.getPixelRatio());
     }
 
+    // Allow the sprite atlas to potentially pull new sprite images if needed.
+    spriteAtlas->update(*getSprite());
+
     updateTiles();
 }
 
diff --git a/src/map/sprite.cpp b/src/map/sprite.cpp
index 60f19ab006..6cf40f23f9 100644
--- a/src/map/sprite.cpp
+++ b/src/map/sprite.cpp
@@ -20,48 +20,75 @@ SpritePosition::SpritePosition(uint16_t x, uint16_t y, uint16_t width, uint16_t
       pixelRatio(pixelRatio) {
 }
 
-Sprite::Sprite(Map &map, float pixelRatio)
+std::shared_ptr<Sprite> Sprite::Create(const std::string& base_url, float pixelRatio) {
+    std::shared_ptr<Sprite> sprite(std::make_shared<Sprite>(Key(), base_url, pixelRatio));
+    sprite->load();
+    return sprite;
+}
+
+Sprite::Sprite(const Key &, const std::string& base_url, float pixelRatio)
     : pixelRatio(pixelRatio),
+      url(base_url),
       raster(),
-      map(map),
       loadedImage(false),
-      loadedJSON(false) {
+      loadedJSON(false),
+      future(promise.get_future()) {
+}
+
+void Sprite::waitUntilLoaded() const {
+    future.wait();
 }
 
-void Sprite::load(const std::string& base_url) {
-    loadedImage = false;
-    loadedJSON = false;
+Sprite::operator bool() const {
+    return isLoaded() && !pos.empty();
+}
+
+
+// Note: This is a separate function that must be called exactly once after creation
+// The reason this isn't part of the constructor is that calling shared_from_this() in
+// the constructor fails.
+void Sprite::load() {
+    if (url.length() == 0) {
+        // Treat a non-existent sprite as an empty sprite.
+        loadedImage = true;
+        loadedJSON = true;
+        promise.set_value();
+        return;
+    }
 
-    url = base_url;
     std::shared_ptr<Sprite> sprite = shared_from_this();
 
-    std::string suffix = (pixelRatio > 1 ? "@2x" : "");
+    const std::string suffix = (pixelRatio > 1 ? "@2x" : "");
 
-    platform::request_http(base_url + suffix + ".json", [sprite](platform::Response *res) {
+    platform::request_http(url + suffix + ".json", [sprite](platform::Response *res) {
         if (res->code == 200) {
             sprite->body.swap(res->body);
-            sprite->asyncParseJSON();
+            sprite->parseJSON();
+            sprite->complete();
         } else {
             fprintf(stderr, "failed to load sprite info\n");
             fprintf(stderr, "error %d: %s\n", res->code, res->error_message.c_str());
+            sprite->promise.set_exception(std::make_exception_ptr(std::runtime_error(res->error_message)));
         }
-    }, map.getLoop());
+    });
 
-    platform::request_http(base_url + suffix + ".png", [sprite](platform::Response *res) {
+    platform::request_http(url + suffix + ".png", [sprite](platform::Response *res) {
         if (res->code == 200) {
-             sprite->image.swap(res->body);
-             sprite->asyncParseImage();
+            sprite->image.swap(res->body);
+            sprite->parseImage();
+            sprite->complete();
         } else {
             fprintf(stderr, "failed to load sprite image\n");
             fprintf(stderr, "error %d: %s\n", res->code, res->error_message.c_str());
+            sprite->promise.set_exception(std::make_exception_ptr(std::runtime_error(res->error_message)));
         }
-    }, map.getLoop());
+    });
 }
 
-void Sprite::complete(std::shared_ptr<Sprite> &sprite) {
-    if (sprite->loadedImage && sprite->loadedJSON) {
-        sprite->map.update();
-        Log::Info(Event::Sprite, "loaded %s", sprite->url.c_str());
+void Sprite::complete() {
+    if (loadedImage && loadedJSON) {
+        Log::Info(Event::Sprite, "loaded %s", url.c_str());
+        promise.set_value();
     }
 }
 
@@ -69,28 +96,20 @@ bool Sprite::isLoaded() const {
     return loadedImage && loadedJSON;
 }
 
-void Sprite::asyncParseImage() {
-    new uv::work<std::shared_ptr<Sprite>>(map.getLoop(), parseImage, complete, shared_from_this());
+void Sprite::parseImage() {
+    raster = std::make_unique<util::Image>(image);
+    image.clear();
+    loadedImage = true;
 }
 
-void Sprite::asyncParseJSON() {
-    new uv::work<std::shared_ptr<Sprite>>(map.getLoop(), parseJSON, complete, shared_from_this());
-}
-
-void Sprite::parseImage(std::shared_ptr<Sprite> &sprite) {
-    sprite->raster = std::make_unique<util::Image>(sprite->image);
-    sprite->image.clear();
-    sprite->loadedImage = true;
-}
-
-void Sprite::parseJSON(std::shared_ptr<Sprite> &sprite) {
+void Sprite::parseJSON() {
     rapidjson::Document d;
-    d.Parse<0>(sprite->body.c_str());
-    sprite->body.clear();
-
-    if (d.IsObject()) {
-        std::unordered_map<std::string, SpritePosition> pos;
+    d.Parse<0>(body.c_str());
+    body.clear();
 
+    if (d.HasParseError()) {
+        Log::Warning(Event::Sprite, "sprite JSON is invalid");
+    } else if (d.IsObject()) {
         for (rapidjson::Value::ConstMemberIterator itr = d.MemberBegin(); itr != d.MemberEnd(); ++itr) {
             const std::string& name = itr->name.GetString();
             const rapidjson::Value& value = itr->value;
@@ -110,10 +129,11 @@ void Sprite::parseJSON(std::shared_ptr<Sprite> &sprite) {
                 pos.emplace(name, SpritePosition { x, y, width, height, pixelRatio });
             }
         }
-
-        sprite->pos.swap(pos);
-        sprite->loadedJSON = true;
+    } else {
+        Log::Warning(Event::Sprite, "sprite JSON root is not an object");
     }
+
+    loadedJSON = true;
 }
 
 const SpritePosition &Sprite::getSpritePosition(const std::string& name) const {
diff --git a/src/map/tile_parser.cpp b/src/map/tile_parser.cpp
index 6be9bcb32c..99ee676237 100644
--- a/src/map/tile_parser.cpp
+++ b/src/map/tile_parser.cpp
@@ -34,13 +34,15 @@ TileParser::TileParser(const std::string &data, VectorTileData &tile,
                        const std::shared_ptr<const Style> &style,
                        const std::shared_ptr<GlyphAtlas> &glyphAtlas,
                        const std::shared_ptr<GlyphStore> &glyphStore,
-                       const std::shared_ptr<SpriteAtlas> &spriteAtlas)
+                       const std::shared_ptr<SpriteAtlas> &spriteAtlas,
+                       const std::shared_ptr<Sprite> &sprite)
     : vector_data(pbf((const uint8_t *)data.data(), data.size())),
       tile(tile),
       style(style),
       glyphAtlas(glyphAtlas),
       glyphStore(glyphStore),
       spriteAtlas(spriteAtlas),
+      sprite(sprite),
       placement(tile.id.z, tile.id.z >= tile.source.max_zoom ? tile.source.max_zoom - tile.id.z : 1) {
 }
 
@@ -153,6 +155,6 @@ std::unique_ptr<Bucket> TileParser::createLineBucket(const VectorTileLayer& laye
 
 std::unique_ptr<Bucket> TileParser::createSymbolBucket(const VectorTileLayer& layer, const FilterExpression &filter, const StyleBucketSymbol &symbol) {
     std::unique_ptr<SymbolBucket> bucket = std::make_unique<SymbolBucket>(tile.textVertexBuffer, tile.iconVertexBuffer, tile.triangleElementsBuffer, symbol, placement);
-    bucket->addFeatures(layer, filter, tile.id, *spriteAtlas, *glyphAtlas, *glyphStore);
+    bucket->addFeatures(layer, filter, tile.id, *spriteAtlas, *sprite, *glyphAtlas, *glyphStore);
     return obsolete() ? nullptr : std::move(bucket);
 }
diff --git a/src/map/vector_tile_data.cpp b/src/map/vector_tile_data.cpp
index fd3cd6ec8c..99c521b8ab 100644
--- a/src/map/vector_tile_data.cpp
+++ b/src/map/vector_tile_data.cpp
@@ -20,7 +20,7 @@ VectorTileData::~VectorTileData() {
 }
 
 void VectorTileData::beforeParse() {
-    parser = std::make_unique<TileParser>(data, *this, map.getStyle(), map.getGlyphAtlas(), map.getGlyphStore(), map.getSpriteAtlas());
+    parser = std::make_unique<TileParser>(data, *this, map.getStyle(), map.getGlyphAtlas(), map.getGlyphStore(), map.getSpriteAtlas(), map.getSprite());
 }
 
 void VectorTileData::parse() {