From 4956da61b9112618cd0ae7e247ca532582648cb7 Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 21 Apr 2015 12:05:37 +0200
Subject: Introduce the ResourceLoader

ResourceLoader will handle all the tile changes and notifications
through the Source object owned by the Style. This object lives
entirely on the Map thread and should be only accessed by it.

The observer will receive notifications every time the tile data changes
and this will probably trigger a new rendering operation.

This first patch will delegate Source loading to the ResourceLoader.
---
 src/mbgl/map/map_context.cpp     | 17 +++++++++-----
 src/mbgl/map/map_context.hpp     |  7 +++++-
 src/mbgl/map/resource_loader.cpp | 48 ++++++++++++++++++++++++++++++++++++++++
 src/mbgl/map/resource_loader.hpp | 47 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 112 insertions(+), 7 deletions(-)
 create mode 100644 src/mbgl/map/resource_loader.cpp
 create mode 100644 src/mbgl/map/resource_loader.hpp

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index 4e0a04472..c14cedb03 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -62,6 +62,7 @@ MapContext::~MapContext() {
 
     // Explicit resets currently necessary because these abandon resources that need to be
     // cleaned up by env.performCleanup();
+    resourceLoader.reset();
     style.reset();
     sprite.reset();
     painter.reset();
@@ -141,6 +142,7 @@ util::ptr<Sprite> MapContext::getSprite() {
 void MapContext::loadStyleJSON(const std::string& json, const std::string& base) {
     assert(Environment::currentlyOn(ThreadType::Map));
 
+    resourceLoader.reset();
     sprite.reset();
     style.reset();
 
@@ -153,12 +155,10 @@ void MapContext::loadStyleJSON(const std::string& json, const std::string& base)
     const std::string glyphURL = util::mapbox::normalizeGlyphsURL(style->glyph_url, data.getAccessToken());
     glyphStore->setURL(glyphURL);
 
-    for (const auto& source : style->sources) {
-        source->load(data.getAccessToken(), env, [this]() {
-            assert(Environment::currentlyOn(ThreadType::Map));
-            triggerUpdate();
-        });
-    }
+    resourceLoader = util::make_unique<ResourceLoader>();
+    resourceLoader->setAccessToken(data.getAccessToken());
+    resourceLoader->setObserver(this);
+    resourceLoader->setStyle(style.get());
 
     triggerUpdate(Update::Zoom);
 }
@@ -299,4 +299,9 @@ void MapContext::onLowMemory() {
     view.invalidate([this] { render(); });
 }
 
+void MapContext::onTileDataChanged() {
+    assert(Environment::currentlyOn(ThreadType::Map));
+    triggerUpdate();
+}
+
 }
diff --git a/src/mbgl/map/map_context.hpp b/src/mbgl/map/map_context.hpp
index 93670fd1f..6941c3040 100644
--- a/src/mbgl/map/map_context.hpp
+++ b/src/mbgl/map/map_context.hpp
@@ -4,6 +4,7 @@
 #include <mbgl/map/tile_id.hpp>
 #include <mbgl/map/update.hpp>
 #include <mbgl/map/environment.hpp>
+#include <mbgl/map/resource_loader.hpp>
 #include <mbgl/map/transform_state.hpp>
 #include <mbgl/util/ptr.hpp>
 
@@ -32,7 +33,7 @@ class StillImage;
 struct LatLng;
 struct LatLngBounds;
 
-class MapContext {
+class MapContext : public ResourceLoader::Observer {
 public:
     MapContext(uv_loop_t*, View&, FileSource&, MapData&, bool startPaused);
     ~MapContext();
@@ -58,6 +59,9 @@ public:
     void setSourceTileCacheSize(size_t size);
     void onLowMemory();
 
+    // ResourceLoader::Observer implementation.
+    void onTileDataChanged() override;
+
 private:
     util::ptr<Sprite> getSprite();
     void updateTiles();
@@ -84,6 +88,7 @@ private:
     std::unique_ptr<TexturePool> texturePool;
     std::unique_ptr<Painter> painter;
     std::unique_ptr<Style> style;
+    std::unique_ptr<ResourceLoader> resourceLoader;
 
     util::ptr<Sprite> sprite;
 
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
new file mode 100644
index 000000000..5186c665f
--- /dev/null
+++ b/src/mbgl/map/resource_loader.cpp
@@ -0,0 +1,48 @@
+#include <mbgl/map/resource_loader.hpp>
+
+#include <mbgl/map/environment.hpp>
+#include <mbgl/map/source.hpp>
+#include <mbgl/style/style.hpp>
+
+#include <cassert>
+#include <functional>
+
+namespace mbgl {
+
+ResourceLoader::ResourceLoader() {
+    assert(Environment::currentlyOn(ThreadType::Map));
+}
+
+ResourceLoader::~ResourceLoader() {
+    assert(Environment::currentlyOn(ThreadType::Map));
+}
+
+void ResourceLoader::setObserver(Observer* observer) {
+    assert(Environment::currentlyOn(ThreadType::Map));
+    assert(!observer_);
+
+    observer_ = observer;
+}
+
+void ResourceLoader::setStyle(Style* style) {
+    style_ = style;
+
+    Environment& env = Environment::Get();
+    for (const auto& source : style->sources) {
+        source->load(accessToken_, env, std::bind(&ResourceLoader::emitTileDataChanged, this));
+    }
+}
+
+void ResourceLoader::setAccessToken(const std::string& accessToken) {
+    accessToken_ = accessToken;
+}
+
+void ResourceLoader::emitTileDataChanged() {
+    assert(Environment::currentlyOn(ThreadType::Map));
+
+    if (observer_) {
+        observer_->onTileDataChanged();
+    }
+}
+
+}
diff --git a/src/mbgl/map/resource_loader.hpp b/src/mbgl/map/resource_loader.hpp
new file mode 100644
index 000000000..6d2089e3a
--- /dev/null
+++ b/src/mbgl/map/resource_loader.hpp
@@ -0,0 +1,47 @@
+#ifndef MBGL_MAP_RESOURCE_LOADER
+#define MBGL_MAP_RESOURCE_LOADER
+
+#include <mbgl/util/noncopyable.hpp>
+
+#include <string>
+
+namespace mbgl {
+
+class Style;
+
+// ResourceLoader is responsible for loading and updating the Source(s) owned
+// 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 : private util::noncopyable {
+public:
+    class Observer {
+    public:
+        virtual ~Observer() = default;
+
+        virtual void onTileDataChanged() = 0;
+    };
+
+    ResourceLoader();
+    ~ResourceLoader();
+
+    void setObserver(Observer* observer);
+
+    // The style object currently owns all the sources. When setting
+    // a new style we will go through all of them and try to load.
+    void setStyle(Style* style);
+
+    // Set the access token to be used for loading the tile data.
+    void setAccessToken(const std::string& accessToken);
+
+private:
+    void emitTileDataChanged();
+
+    std::string accessToken_;
+    Style* style_ = nullptr;
+    Observer* observer_ = nullptr;
+};
+
+}
+
+#endif
-- 
cgit v1.2.3


From f4baa0ba3eaef86b42b6307f37ea7315827f9c2a Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 21 Apr 2015 14:33:14 +0200
Subject: Notify data changes using Observer pattern

This pattern, in contrast with callbacks, allows us to clearly see
the dependency and hierarchy between the objects. It will also help
on writing unit tests as we can count how many times a notification
was triggered.
---
 src/mbgl/map/map_context.cpp     |  5 +----
 src/mbgl/map/resource_loader.cpp | 16 ++++++++++++++--
 src/mbgl/map/resource_loader.hpp |  7 ++++++-
 src/mbgl/map/source.cpp          | 34 ++++++++++++++++++++++++----------
 src/mbgl/map/source.hpp          | 26 +++++++++++++++++++-------
 5 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index c14cedb03..146d7ec4e 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -168,10 +168,7 @@ void MapContext::updateTiles() {
     if (!style) return;
     for (const auto& source : style->sources) {
         source->update(data, transformState, *style, *glyphAtlas, *glyphStore, *spriteAtlas,
-                       getSprite(), *texturePool, [this]() {
-            assert(Environment::currentlyOn(ThreadType::Map));
-            triggerUpdate();
-        });
+                       getSprite(), *texturePool);
     }
 }
 
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 5186c665f..91f183d69 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -5,7 +5,6 @@
 #include <mbgl/style/style.hpp>
 
 #include <cassert>
-#include <functional>
 
 namespace mbgl {
 
@@ -15,6 +14,10 @@ ResourceLoader::ResourceLoader() {
 
 ResourceLoader::~ResourceLoader() {
     assert(Environment::currentlyOn(ThreadType::Map));
+
+    for (const auto& source : style_->sources) {
+        source->setObserver(nullptr);
+    }
 }
 
 void ResourceLoader::setObserver(Observer* observer) {
@@ -29,7 +32,8 @@ void ResourceLoader::setStyle(Style* style) {
 
     Environment& env = Environment::Get();
     for (const auto& source : style->sources) {
-        source->load(accessToken_, env, std::bind(&ResourceLoader::emitTileDataChanged, this));
+        source->setObserver(this);
+        source->load(accessToken_, env);
     }
 }
 
@@ -37,6 +41,14 @@ void ResourceLoader::setAccessToken(const std::string& accessToken) {
     accessToken_ = accessToken;
 }
 
+void ResourceLoader::onSourceLoaded() {
+    emitTileDataChanged();
+}
+
+void ResourceLoader::onTileLoaded() {
+    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 6d2089e3a..f03ba791d 100644
--- a/src/mbgl/map/resource_loader.hpp
+++ b/src/mbgl/map/resource_loader.hpp
@@ -1,6 +1,7 @@
 #ifndef MBGL_MAP_RESOURCE_LOADER
 #define MBGL_MAP_RESOURCE_LOADER
 
+#include <mbgl/map/source.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <string>
@@ -13,7 +14,7 @@ class Style;
 // 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 : private util::noncopyable {
+class ResourceLoader : public Source::Observer, private util::noncopyable {
 public:
     class Observer {
     public:
@@ -34,6 +35,10 @@ public:
     // Set the access token to be used for loading the tile data.
     void setAccessToken(const std::string& accessToken);
 
+    // Source::Observer implementation.
+    void onSourceLoaded() override;
+    void onTileLoaded() override;
+
 private:
     void emitTileDataChanged();
 
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 708ff6b27..c63cd026d 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -136,9 +136,7 @@ bool Source::isLoaded() const {
 // 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 Source::load(const std::string& accessToken,
-                  Environment& env,
-                  std::function<void()> callback) {
+void Source::load(const std::string& accessToken, Environment& env) {
     if (info.url.empty()) {
         loaded = true;
         return;
@@ -147,7 +145,7 @@ void Source::load(const std::string& accessToken,
     util::ptr<Source> source = shared_from_this();
 
     const std::string url = util::mapbox::normalizeSourceURL(info.url, accessToken);
-    env.request({ Resource::Kind::JSON, url }, [source, callback](const Response &res) {
+    env.request({ Resource::Kind::JSON, url }, [source](const Response &res) {
         if (res.status != Response::Successful) {
             Log::Warning(Event::General, "Failed to load source TileJSON: %s", res.message.c_str());
             return;
@@ -164,7 +162,7 @@ void Source::load(const std::string& accessToken,
         source->info.parseTileJSONProperties(d);
         source->loaded = true;
 
-        callback();
+        source->emitSourceLoaded();
     });
 }
 
@@ -226,8 +224,7 @@ TileData::State Source::addTile(MapData& data,
                                 SpriteAtlas& spriteAtlas,
                                 util::ptr<Sprite> sprite,
                                 TexturePool& texturePool,
-                                const TileID& id,
-                                std::function<void()> callback) {
+                                const TileID& id) {
     const TileData::State state = hasTile(id);
 
     if (state != TileData::State::invalid) {
@@ -256,6 +253,8 @@ TileData::State Source::addTile(MapData& data,
         new_tile.data = cache.get(normalized_id.to_uint64());
     }
 
+    auto callback = std::bind(&Source::emitTileLoaded, this);
+
     if (!new_tile.data) {
         // If we don't find working tile data, we're just going to load it.
         if (info.type == SourceType::Vector) {
@@ -366,8 +365,7 @@ void Source::update(MapData& data,
                     GlyphStore& glyphStore,
                     SpriteAtlas& spriteAtlas,
                     util::ptr<Sprite> sprite,
-                    TexturePool& texturePool,
-                    std::function<void()> callback) {
+                    TexturePool& texturePool) {
     if (!loaded || data.getAnimationTime() <= updated) {
         return;
     }
@@ -387,7 +385,7 @@ void Source::update(MapData& data,
     // Add existing child/parent tiles if the actual tile is not yet loaded
     for (const auto& id : required) {
         const TileData::State state = addTile(data, transformState, style, glyphAtlas, glyphStore,
-                                              spriteAtlas, sprite, texturePool, id, callback);
+                                              spriteAtlas, sprite, texturePool, id);
 
         if (state != TileData::State::parsed) {
             // The tile we require is not yet loaded. Try to find a parent or
@@ -477,4 +475,20 @@ void Source::onLowMemory() {
     cache.clear();
 }
 
+void Source::setObserver(Observer* observer) {
+    observer_ = observer;
+}
+
+void Source::emitSourceLoaded() {
+    if (observer_) {
+        observer_->onSourceLoaded();
+    }
+}
+
+void Source::emitTileLoaded() {
+    if (observer_) {
+        observer_->onTileLoaded();
+    }
+}
+
 }
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index b15de0e64..255e97208 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -52,12 +52,18 @@ public:
 
 class Source : public std::enable_shared_from_this<Source>, private util::noncopyable {
 public:
+    class Observer {
+    public:
+        virtual ~Observer() = default;
+
+        virtual void onSourceLoaded() = 0;
+        virtual void onTileLoaded() = 0;
+    };
+
     Source();
     ~Source();
 
-    void load(const std::string& accessToken,
-              Environment&,
-              std::function<void()> callback);
+    void load(const std::string& accessToken, Environment&);
     bool isLoaded() const;
 
     void load(MapData&, Environment&, std::function<void()> callback);
@@ -68,8 +74,7 @@ public:
                 GlyphStore&,
                 SpriteAtlas&,
                 util::ptr<Sprite>,
-                TexturePool&,
-                std::function<void()> callback);
+                TexturePool&);
 
     void invalidateTiles(const std::vector<TileID>&);
 
@@ -83,10 +88,15 @@ public:
     void setCacheSize(size_t);
     void onLowMemory();
 
+    void setObserver(Observer* observer);
+
     SourceInfo info;
     bool enabled;
 
 private:
+    void emitSourceLoaded();
+    void emitTileLoaded();
+
     bool findLoadedChildren(const TileID& id, int32_t maxCoveringZoom, std::forward_list<TileID>& retain);
     bool findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::forward_list<TileID>& retain);
     int32_t coveringZoomLevel(const TransformState&) const;
@@ -100,8 +110,7 @@ private:
                             SpriteAtlas&,
                             util::ptr<Sprite>,
                             TexturePool&,
-                            const TileID&,
-                            std::function<void()> callback);
+                            const TileID&);
 
     TileData::State hasTile(const TileID& id);
     void updateTilePtrs();
@@ -117,6 +126,9 @@ private:
     std::vector<Tile*> tilePtrs;
     std::map<TileID, std::weak_ptr<TileData>> tile_data;
     TileCache cache;
+
+
+    Observer* observer_ = nullptr;
 };
 
 }
-- 
cgit v1.2.3


From d726613f6f29c8395c446ce678f7e51d275d6a60 Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 21 Apr 2015 16:33:58 +0200
Subject: Update the tiles via ResourceLoader

---
 src/mbgl/map/map_context.cpp     |  8 +++-----
 src/mbgl/map/resource_loader.cpp | 17 +++++++++++++++++
 src/mbgl/map/resource_loader.hpp | 11 +++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index 146d7ec4e..50225b429 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -165,11 +165,9 @@ void MapContext::loadStyleJSON(const std::string& json, const std::string& base)
 
 void MapContext::updateTiles() {
     assert(Environment::currentlyOn(ThreadType::Map));
-    if (!style) return;
-    for (const auto& source : style->sources) {
-        source->update(data, transformState, *style, *glyphAtlas, *glyphStore, *spriteAtlas,
-                       getSprite(), *texturePool);
-    }
+
+    resourceLoader->update(data, transformState, *glyphAtlas, *glyphStore,
+                           *spriteAtlas, getSprite(), *texturePool);
 }
 
 void MapContext::updateAnnotationTiles(const std::vector<TileID>& ids) {
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 91f183d69..3d4a6ea5b 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -41,6 +41,23 @@ void ResourceLoader::setAccessToken(const std::string& accessToken) {
     accessToken_ = accessToken;
 }
 
+void ResourceLoader::update(MapData& data,
+                            const TransformState& transform,
+                            GlyphAtlas& glyphAtlas,
+                            GlyphStore& glyphStore,
+                            SpriteAtlas& spriteAtlas,
+                            util::ptr<Sprite> sprite,
+                            TexturePool& texturePool) {
+    if (!style_) {
+        return;
+    }
+
+    for (const auto& source : style_->sources) {
+        source->update(
+            data, transform, *style_, glyphAtlas, glyphStore, spriteAtlas, sprite, texturePool);
+    }
+}
+
 void ResourceLoader::onSourceLoaded() {
     emitTileDataChanged();
 }
diff --git a/src/mbgl/map/resource_loader.hpp b/src/mbgl/map/resource_loader.hpp
index f03ba791d..391c46059 100644
--- a/src/mbgl/map/resource_loader.hpp
+++ b/src/mbgl/map/resource_loader.hpp
@@ -8,7 +8,13 @@
 
 namespace mbgl {
 
+class GlyphAtlas;
+class GlyphStore;
+class MapData;
+class SpriteAtlas;
 class Style;
+class TexturePool;
+class TransformState;
 
 // ResourceLoader is responsible for loading and updating the Source(s) owned
 // by the Style. The Source object currently owns all the tiles, thus this
@@ -35,6 +41,11 @@ public:
     // Set the access token to be used for loading the tile data.
     void setAccessToken(const std::string& accessToken);
 
+    // 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&);
+
     // Source::Observer implementation.
     void onSourceLoaded() override;
     void onTileLoaded() override;
-- 
cgit v1.2.3


From c487c521b44c3a4a3ef13422580be76d466a9c70 Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 21 Apr 2015 17:45:14 +0200
Subject: 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.
---
 src/mbgl/map/map_context.cpp        | 30 ++++--------------------------
 src/mbgl/map/map_context.hpp        |  2 --
 src/mbgl/map/resource_loader.cpp    | 23 +++++++++++++++++++++--
 src/mbgl/map/resource_loader.hpp    | 14 ++++++++++++--
 src/mbgl/map/sprite.cpp             | 27 +++++++++++----------------
 src/mbgl/map/sprite.hpp             | 20 ++++++++++++--------
 src/mbgl/map/tile_parser.cpp        |  7 ++++++-
 src/mbgl/renderer/symbol_bucket.cpp |  7 ++-----
 src/mbgl/renderer/symbol_bucket.hpp |  3 +--
 9 files changed, 69 insertions(+), 64 deletions(-)

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index 50225b429..2352e09c2 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 6941c3040..0463d076b 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 3d4a6ea5b..00688911a 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 391c46059..af518a48a 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 75e5f845c..cfc097457 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 bd6ae89bc..ccf256161 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 1438bcdda..ad66aa154 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 769feb67a..d6ae6c397 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 b1dc44a11..54a77b509 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);
 
-- 
cgit v1.2.3


From 1455e5e53c1795401bec1b88272e93ce326d1753 Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Wed, 22 Apr 2015 15:05:42 +0200
Subject: Add partial tile parsing

This change will add a new state to the tile called "partial" which
is a state where the tile can be rendered but some information might
be missing.

For now we skip the SymbolBucket if the Sprites are not loaded (will
also do the same for the glyphs in the future). When the Sprites are
finally loaded, we trigger another update, this time the Tile will
get completely re-rendered and change the state to "parsed".
---
 src/mbgl/map/source.cpp           | 44 ++++++++++++++++++++++++++++++++------
 src/mbgl/map/source.hpp           |  1 +
 src/mbgl/map/tile_data.cpp        | 15 ++++++++++++-
 src/mbgl/map/tile_data.hpp        | 21 +++++++++++++++++-
 src/mbgl/map/tile_parser.cpp      | 22 ++++++++++---------
 src/mbgl/map/tile_parser.hpp      |  4 ++++
 src/mbgl/map/vector_tile_data.cpp | 45 +++++++++++++++++++++++++++++----------
 src/mbgl/map/vector_tile_data.hpp | 17 +++++++++++----
 8 files changed, 135 insertions(+), 34 deletions(-)

diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index c63cd026d..9b6631da9 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -216,6 +216,23 @@ TileData::State Source::hasTile(const TileID& id) {
     return TileData::State::invalid;
 }
 
+void Source::handlePartialTile(const TileID &id, Worker &worker) {
+    const TileID normalized_id = id.normalized();
+
+    auto it = tile_data.find(normalized_id);
+    if (it == tile_data.end()) {
+        return;
+    }
+
+    util::ptr<TileData> data = it->second.lock();
+    if (!data) {
+        return;
+    }
+
+    auto callback = std::bind(&Source::emitTileLoaded, this);
+    data->reparse(worker, callback);
+}
+
 TileData::State Source::addTile(MapData& data,
                                 const TransformState& transformState,
                                 Style& style,
@@ -254,7 +271,6 @@ TileData::State Source::addTile(MapData& data,
     }
 
     auto callback = std::bind(&Source::emitTileLoaded, this);
-
     if (!new_tile.data) {
         // If we don't find working tile data, we're just going to load it.
         if (info.type == SourceType::Vector) {
@@ -324,7 +340,7 @@ bool Source::findLoadedChildren(const TileID& id, int32_t maxCoveringZoom, std::
     auto ids = id.children(z + 1);
     for (const auto& child_id : ids) {
         const TileData::State state = hasTile(child_id);
-        if (state == TileData::State::parsed) {
+        if (TileData::isReadyState(state)) {
             retain.emplace_front(child_id);
         } else {
             complete = false;
@@ -350,7 +366,7 @@ bool Source::findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::fo
     for (int32_t z = id.z - 1; z >= minCoveringZoom; --z) {
         const TileID parent_id = id.parent(z);
         const TileData::State state = hasTile(parent_id);
-        if (state == TileData::State::parsed) {
+        if (TileData::isReadyState(state)) {
             retain.emplace_front(parent_id);
             return true;
         }
@@ -384,10 +400,21 @@ void Source::update(MapData& data,
 
     // Add existing child/parent tiles if the actual tile is not yet loaded
     for (const auto& id : required) {
-        const TileData::State state = addTile(data, transformState, style, glyphAtlas, glyphStore,
-                                              spriteAtlas, sprite, texturePool, id);
+        TileData::State state = hasTile(id);
+
+        switch (state) {
+        case TileData::State::partial:
+            handlePartialTile(id, style.workers);
+            break;
+        case TileData::State::invalid:
+            state = addTile(data, transformState, style, glyphAtlas, glyphStore,
+                            spriteAtlas, sprite, texturePool, id);
+            break;
+        default:
+            break;
+        }
 
-        if (state != TileData::State::parsed) {
+        if (!TileData::isReadyState(state)) {
             // The tile we require is not yet loaded. Try to find a parent or
             // child tile that we already have.
 
@@ -422,7 +449,10 @@ void Source::update(MapData& data,
         bool obsolete = std::find(retain.begin(), retain.end(), tile.id) == retain.end();
         if (!obsolete) {
             retain_data.insert(tile.data->id);
-        } else if (type != SourceType::Raster && tile.data->ready()) {
+        } else if (type != SourceType::Raster && tile.data->state == TileData::State::parsed) {
+            // Partially parsed tiles are never added to the cache because otherwise
+            // they never get updated if the go out from the viewport and the pending
+            // resources arrive.
             tileCache.add(tile.id.normalized().to_uint64(), tile.data);
         }
         return obsolete;
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index 255e97208..dd02cdc3b 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -97,6 +97,7 @@ private:
     void emitSourceLoaded();
     void emitTileLoaded();
 
+    void handlePartialTile(const TileID &id, Worker &worker);
     bool findLoadedChildren(const TileID& id, int32_t maxCoveringZoom, std::forward_list<TileID>& retain);
     bool findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::forward_list<TileID>& retain);
     int32_t coveringZoomLevel(const TransformState&) const;
diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp
index dd51cb841..4025bacfc 100644
--- a/src/mbgl/map/tile_data.cpp
+++ b/src/mbgl/map/tile_data.cpp
@@ -12,6 +12,7 @@ TileData::TileData(const TileID& id_, const SourceInfo& source_)
     : id(id_),
       name(id),
       state(State::initial),
+      parsing(ATOMIC_FLAG_INIT),
       source(source_),
       env(Environment::Get()),
       debugBucket(debugFontBuffer) {
@@ -60,6 +61,18 @@ void TileData::cancel() {
     }
 }
 
+bool TileData::mayStartParsing() {
+    return !parsing.test_and_set(std::memory_order_acquire);
+}
+
+void TileData::endParsing() {
+    parsing.clear(std::memory_order_release);
+}
+
 void TileData::reparse(Worker& worker, std::function<void()> callback) {
-    workRequest = worker.send([this] { parse(); }, callback);
+    if (!mayStartParsing()) {
+        return;
+    }
+
+    workRequest = worker.send([this] { parse(); endParsing(); }, callback);
 }
diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp
index 14e937eec..3be10dc03 100644
--- a/src/mbgl/map/tile_data.hpp
+++ b/src/mbgl/map/tile_data.hpp
@@ -29,10 +29,18 @@ public:
         initial,
         loading,
         loaded,
+        partial,
         parsed,
         obsolete
     };
 
+    // Tile data considered "Ready" can be used for rendering. Data in
+    // partial state is still waiting for network resources but can also
+    // be rendered, although layers will be missing.
+    inline static bool isReadyState(const State& state) {
+        return state == State::partial || state == State::parsed;
+    }
+
     TileData(const TileID&, const SourceInfo&);
     ~TileData();
 
@@ -42,9 +50,11 @@ public:
     const std::string toString() const;
 
     inline bool ready() const {
-        return state == State::parsed;
+        return isReadyState(state);
     }
 
+    void endParsing();
+
     // Override this in the child class.
     virtual void parse() = 0;
     virtual Bucket* getBucket(StyleLayer const &layer_desc) = 0;
@@ -52,8 +62,17 @@ public:
     const TileID id;
     const std::string name;
     std::atomic<State> state;
+    std::atomic_flag parsing;
 
 protected:
+    // Set the internal parsing state to true so we prevent
+    // multiple workers to parse the same tile in parallel,
+    // which can happen if the tile is in the "partial" state.
+    // It will return true if is possible to start pasing the
+    // tile or false if not (so some other worker is already
+    // parsing the tile).
+    bool mayStartParsing();
+
     const SourceInfo& source;
     Environment& env;
 
diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp
index ad66aa154..18bc3b2ae 100644
--- a/src/mbgl/map/tile_parser.cpp
+++ b/src/mbgl/map/tile_parser.cpp
@@ -35,7 +35,8 @@ TileParser::TileParser(const GeometryTile& geometryTile_,
       glyphStore(glyphStore_),
       spriteAtlas(spriteAtlas_),
       sprite(sprite_),
-      collision(util::make_unique<Collision>(tile.id.z, 4096, tile.source.tile_size, tile.depth)) {
+      collision(util::make_unique<Collision>(tile.id.z, 4096, tile.source.tile_size, tile.depth)),
+      partialParse(false) {
     assert(sprite);
     assert(collision);
 }
@@ -57,15 +58,15 @@ void TileParser::parse() {
         if (layer_desc->bucket) {
             // This is a singular layer. Check if this bucket already exists. If not,
             // parse this bucket.
-            auto bucket_it = tile.buckets.find(layer_desc->bucket->name);
-            if (bucket_it == tile.buckets.end()) {
-                // We need to create this bucket since it doesn't exist yet.
-                std::unique_ptr<Bucket> bucket = createBucket(*layer_desc->bucket);
-                if (bucket) {
-                    // Bucket creation might fail because the data tile may not
-                    // contain any data that falls into this bucket.
-                    tile.buckets[layer_desc->bucket->name] = std::move(bucket);
-                }
+            if (tile.getBucket(*layer_desc)) {
+                continue;
+            }
+
+            std::unique_ptr<Bucket> bucket = createBucket(*layer_desc->bucket);
+            if (bucket) {
+                // Bucket creation might fail because the data tile may not
+                // contain any data that falls into this bucket.
+                tile.setBucket(*layer_desc, std::move(bucket));
             }
         } else {
             Log::Warning(Event::ParseTile, "layer '%s' does not have buckets", layer_desc->id.c_str());
@@ -121,6 +122,7 @@ std::unique_ptr<Bucket> TileParser::createBucket(const StyleBucket &bucketDesc)
             if (sprite->isLoaded()) {
                 return createSymbolBucket(*layer, bucketDesc);
             } else {
+                partialParse = true;
                 return nullptr;
             }
         } else if (bucketDesc.type == StyleLayerType::Raster) {
diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp
index 2dbb8cb17..dcada6fc1 100644
--- a/src/mbgl/map/tile_parser.hpp
+++ b/src/mbgl/map/tile_parser.hpp
@@ -44,6 +44,9 @@ public:
 
 public:
     void parse();
+    inline bool isPartialParse() const {
+        return partialParse;
+    }
 
 private:
     bool obsolete() const;
@@ -67,6 +70,7 @@ private:
     util::ptr<Sprite> sprite;
 
     std::unique_ptr<Collision> collision;
+    bool partialParse;
 };
 
 }
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
index e10e2f826..7560168ed 100644
--- a/src/mbgl/map/vector_tile_data.cpp
+++ b/src/mbgl/map/vector_tile_data.cpp
@@ -35,7 +35,7 @@ VectorTileData::~VectorTileData() {
 }
 
 void VectorTileData::parse() {
-    if (state != State::loaded) {
+    if (state != State::loaded && state != State::partial) {
         return;
     }
 
@@ -47,24 +47,47 @@ void VectorTileData::parse() {
         const VectorTile* vt = &vectorTile;
         TileParser parser(*vt, *this, style, glyphAtlas, glyphStore, spriteAtlas, sprite);
         parser.parse();
+
+        if (state == State::obsolete) {
+            return;
+        }
+
+        if (parser.isPartialParse()) {
+            state = State::partial;
+        } else {
+            state = State::parsed;
+        }
     } catch (const std::exception& ex) {
         Log::Error(Event::ParseTile, "Parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what());
         state = State::obsolete;
         return;
     }
+}
+
+Bucket* VectorTileData::getBucket(StyleLayer const& layer) {
+    if (!ready() || !layer.bucket) {
+        return nullptr;
+    }
 
-    if (state != State::obsolete) {
-        state = State::parsed;
+    std::lock_guard<std::mutex> lock(bucketsMutex);
+
+    const auto it = buckets.find(layer.bucket->name);
+    if (it == buckets.end()) {
+        return nullptr;
     }
+
+    assert(it->second);
+    return it->second.get();
 }
 
-Bucket* VectorTileData::getBucket(StyleLayer const& layer) {
-    if (state == State::parsed && layer.bucket) {
-        const auto it = buckets.find(layer.bucket->name);
-        if (it != buckets.end()) {
-            assert(it->second);
-            return it->second.get();
-        }
+void VectorTileData::setBucket(StyleLayer const& layer, std::unique_ptr<Bucket> bucket) {
+    assert(layer.bucket);
+
+    std::lock_guard<std::mutex> lock(bucketsMutex);
+
+    if (buckets.find(layer.bucket->name) != buckets.end()) {
+        return;
     }
-    return nullptr;
+
+    buckets[layer.bucket->name] = std::move(bucket);
 }
diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp
index 10eaf9c18..146bc9322 100644
--- a/src/mbgl/map/vector_tile_data.hpp
+++ b/src/mbgl/map/vector_tile_data.hpp
@@ -10,6 +10,7 @@
 
 #include <iosfwd>
 #include <memory>
+#include <mutex>
 #include <unordered_map>
 
 namespace mbgl {
@@ -42,6 +43,8 @@ public:
     void parse() override;
     virtual Bucket* getBucket(StyleLayer const &layer_desc) override;
 
+    void setBucket(StyleLayer const &layer_desc, std::unique_ptr<Bucket> bucket);
+
 protected:
     // Holds the actual geometries in this tile.
     FillVertexBuffer fillVertexBuffer;
@@ -50,16 +53,22 @@ protected:
     TriangleElementsBuffer triangleElementsBuffer;
     LineElementsBuffer lineElementsBuffer;
 
-    // Holds the buckets of this tile.
-    // They contain the location offsets in the buffers stored above
-    std::unordered_map<std::string, std::unique_ptr<Bucket>> buckets;
-
     GlyphAtlas& glyphAtlas;
     GlyphStore& glyphStore;
     SpriteAtlas& spriteAtlas;
     util::ptr<Sprite> sprite;
     Style& style;
 
+private:
+    // Contains all the Bucket objects for the tile. Buckets are render
+    // objects and they get added to this std::map<> by the workers doing
+    // the actual tile parsing as they get processed. Tiles partially
+    // parsed can get new buckets at any moment but are also fit for
+    // rendering. That said, access to this list needs locking unless
+    // the tile is completely parsed.
+    std::unordered_map<std::string, std::unique_ptr<Bucket>> buckets;
+    mutable std::mutex bucketsMutex;
+
 public:
     const float depth;
 };
-- 
cgit v1.2.3


From f10839bac21a067bd4eff50a160223e5796a587e Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 28 Apr 2015 10:21:49 +0300
Subject: Make GlyphStore observable

GlyphStore will notify observers when a glyph range is loaded. By now
the notification is generic, but could be fine grained, causing the
processing of buckets depending on this particular range.
---
 src/mbgl/map/resource_loader.cpp |  4 ++++
 src/mbgl/map/resource_loader.hpp |  9 ++++++++-
 src/mbgl/text/glyph_store.cpp    | 17 ++++++++++++++++-
 src/mbgl/text/glyph_store.hpp    | 14 ++++++++++++++
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 00688911a..5dc0ab9d6 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -73,6 +73,10 @@ void ResourceLoader::update(MapData& data,
     }
 }
 
+void ResourceLoader::onGlyphRangeLoaded() {
+    emitTileDataChanged();
+}
+
 void ResourceLoader::onSourceLoaded() {
     emitTileDataChanged();
 }
diff --git a/src/mbgl/map/resource_loader.hpp b/src/mbgl/map/resource_loader.hpp
index af518a48a..a4d10d038 100644
--- a/src/mbgl/map/resource_loader.hpp
+++ b/src/mbgl/map/resource_loader.hpp
@@ -3,6 +3,7 @@
 
 #include <mbgl/map/source.hpp>
 #include <mbgl/map/sprite.hpp>
+#include <mbgl/text/glyph_store.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <string>
@@ -21,7 +22,10 @@ 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, public Sprite::Observer, private util::noncopyable {
+class ResourceLoader : public GlyphStore::Observer,
+                       public Source::Observer,
+                       public Sprite::Observer,
+                       private util::noncopyable {
 public:
     class Observer {
     public:
@@ -52,6 +56,9 @@ public:
         return sprite_;
     }
 
+    // GlyphStore::Observer implementation.
+    void onGlyphRangeLoaded() override;
+
     // Source::Observer implementation.
     void onSourceLoaded() override;
     void onTileLoaded() override;
diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index 1871908b0..aed162ade 100644
--- a/src/mbgl/text/glyph_store.cpp
+++ b/src/mbgl/text/glyph_store.cpp
@@ -226,7 +226,13 @@ void GlyphPBF::parse(FontStack &stack) {
     data.clear();
 }
 
-GlyphStore::GlyphStore(Environment& env_) : env(env_), mtx(util::make_unique<uv::mutex>()) {}
+GlyphStore::GlyphStore(Environment& env_)
+    : env(env_), mtx(util::make_unique<uv::mutex>()), observer(nullptr) {
+}
+
+GlyphStore::~GlyphStore() {
+    observer = nullptr;
+}
 
 void GlyphStore::setURL(const std::string &url) {
     glyphURL = url;
@@ -289,5 +295,14 @@ uv::exclusive<FontStack> GlyphStore::getFontStack(const std::string &fontStack)
     return stack;
 }
 
+void GlyphStore::setObserver(Observer* observer_) {
+    observer = observer_;
+}
+
+void GlyphStore::emitGlyphRangeLoaded() {
+    if (observer) {
+        observer->onGlyphRangeLoaded();
+    }
+}
 
 }
diff --git a/src/mbgl/text/glyph_store.hpp b/src/mbgl/text/glyph_store.hpp
index ee2097536..f92161995 100644
--- a/src/mbgl/text/glyph_store.hpp
+++ b/src/mbgl/text/glyph_store.hpp
@@ -75,7 +75,15 @@ private:
 // Manages Glyphrange PBF loading.
 class GlyphStore {
 public:
+    class Observer {
+    public:
+        virtual ~Observer() = default;
+
+        virtual void onGlyphRangeLoaded() = 0;
+    };
+
     GlyphStore(Environment &);
+    ~GlyphStore();
 
     // Block until all specified GlyphRanges of the specified font stack are loaded.
     void waitForGlyphRanges(const std::string &fontStack, const std::set<GlyphRange> &glyphRanges);
@@ -84,7 +92,11 @@ public:
 
     void setURL(const std::string &url);
 
+    void setObserver(Observer* observer);
+
 private:
+    void emitGlyphRangeLoaded();
+
     // Loads an individual glyph range from the font stack and adds it to rangeSets
     std::shared_future<GlyphPBF &> loadGlyphRange(const std::string &fontStack, std::map<GlyphRange, std::unique_ptr<GlyphPBF>> &rangeSets, GlyphRange range);
 
@@ -95,6 +107,8 @@ private:
     std::unordered_map<std::string, std::map<GlyphRange, std::unique_ptr<GlyphPBF>>> ranges;
     std::unordered_map<std::string, std::unique_ptr<FontStack>> stacks;
     std::unique_ptr<uv::mutex> mtx;
+
+    Observer* observer;
 };
 
 
-- 
cgit v1.2.3


From 071e2e1731b447e195290936c9bf52d28f52b2d9 Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 28 Apr 2015 14:53:50 +0300
Subject: Do not process buckets depending on Glyphs that were not loaded

For now, the buckets depending on Glyphs that are not yet available
are simply not created (or created and discarded). In the future
we could refactor the code to not create the bucket at all or put
the bucket in a suspended state until the glyph range is available.
---
 src/mbgl/map/tile_parser.cpp        |  23 ++++++--
 src/mbgl/map/tile_parser.hpp        |   2 +-
 src/mbgl/renderer/symbol_bucket.cpp |  17 ++++--
 src/mbgl/renderer/symbol_bucket.hpp |   3 ++
 src/mbgl/text/glyph_store.cpp       | 102 ++++++++++++++++++------------------
 src/mbgl/text/glyph_store.hpp       |  41 ++++++++-------
 6 files changed, 108 insertions(+), 80 deletions(-)

diff --git a/src/mbgl/map/tile_parser.cpp b/src/mbgl/map/tile_parser.cpp
index 18bc3b2ae..65b445a6c 100644
--- a/src/mbgl/map/tile_parser.cpp
+++ b/src/mbgl/map/tile_parser.cpp
@@ -119,11 +119,13 @@ 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) {
-            if (sprite->isLoaded()) {
-                return createSymbolBucket(*layer, bucketDesc);
-            } else {
+            bool needsResources = false;
+            auto symbolBucket = createSymbolBucket(*layer, bucketDesc, needsResources);
+            if (needsResources) {
                 partialParse = true;
                 return nullptr;
+            } else {
+                return std::move(symbolBucket);
             }
         } else if (bucketDesc.type == StyleLayerType::Raster) {
             return nullptr;
@@ -185,7 +187,13 @@ std::unique_ptr<Bucket> TileParser::createLineBucket(const GeometryTileLayer& la
 }
 
 std::unique_ptr<Bucket> TileParser::createSymbolBucket(const GeometryTileLayer& layer,
-                                                       const StyleBucket& bucket_desc) {
+                                                       const StyleBucket& bucket_desc,
+                                                       bool& needsResources) {
+    if (!sprite->isLoaded()) {
+        needsResources = true;
+        return nullptr;
+    }
+
     auto bucket = util::make_unique<SymbolBucket>(*collision);
 
     const float z = tile.id.z;
@@ -231,6 +239,13 @@ std::unique_ptr<Bucket> TileParser::createSymbolBucket(const GeometryTileLayer&
 
     bucket->addFeatures(
         layer, bucket_desc.filter, reinterpret_cast<uintptr_t>(&tile), spriteAtlas, *sprite, glyphAtlas, glyphStore);
+
+    if (bucket->needsGlyphs()) {
+        needsResources = true;
+        return nullptr;
+    }
+
     return bucket->hasData() ? std::move(bucket) : nullptr;
 }
+
 }
diff --git a/src/mbgl/map/tile_parser.hpp b/src/mbgl/map/tile_parser.hpp
index dcada6fc1..18184e27a 100644
--- a/src/mbgl/map/tile_parser.hpp
+++ b/src/mbgl/map/tile_parser.hpp
@@ -54,7 +54,7 @@ private:
     std::unique_ptr<Bucket> createBucket(const StyleBucket&);
     std::unique_ptr<Bucket> createFillBucket(const GeometryTileLayer&, const StyleBucket&);
     std::unique_ptr<Bucket> createLineBucket(const GeometryTileLayer&, const StyleBucket&);
-    std::unique_ptr<Bucket> createSymbolBucket(const GeometryTileLayer&, const StyleBucket&);
+    std::unique_ptr<Bucket> createSymbolBucket(const GeometryTileLayer&, const StyleBucket&, bool& needsResources);
 
     template <class Bucket>
     void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&);
diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp
index d6ae6c397..dbaa177fc 100644
--- a/src/mbgl/renderer/symbol_bucket.cpp
+++ b/src/mbgl/renderer/symbol_bucket.cpp
@@ -29,7 +29,7 @@
 namespace mbgl {
 
 SymbolBucket::SymbolBucket(Collision &collision_)
-    : collision(collision_) {
+    : collision(collision_), needsGlyphs_(false) {
 }
 
 SymbolBucket::~SymbolBucket() {
@@ -134,7 +134,10 @@ std::vector<SymbolFeature> SymbolBucket::processFeatures(const GeometryTileLayer
         util::mergeLines(features);
     }
 
-    glyphStore.waitForGlyphRanges(layout.text.font, ranges);
+    if (glyphStore.requestGlyphRangesIfNeeded(layout.text.font, ranges)) {
+        needsGlyphs_ = true;
+        return {};
+    }
 
     return features;
 }
@@ -148,6 +151,12 @@ void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
                                GlyphStore& glyphStore) {
     const std::vector<SymbolFeature> features = processFeatures(layer, filter, glyphStore);
 
+    // Stop if we still need glyphs because the
+    // bucket will be discarded.
+    if (needsGlyphs()) {
+        return;
+    }
+
     float horizontalAlign = 0.5;
     float verticalAlign = 0.5;
 
@@ -189,7 +198,7 @@ void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
     if (layout.text.justify == TextJustifyType::Right) justify = 1;
     else if (layout.text.justify == TextJustifyType::Left) justify = 0;
 
-    const auto &fontStack = glyphStore.getFontStack(layout.text.font);
+    auto* fontStack = glyphStore.getFontStack(layout.text.font);
 
     for (const auto& feature : features) {
         if (!feature.geometry.size()) continue;
@@ -213,7 +222,7 @@ void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
 
             // Add the glyphs we need for this label to the glyph atlas.
             if (shaping.size()) {
-                glyphAtlas.addGlyphs(tileUID, feature.label, layout.text.font, fontStack, face);
+                glyphAtlas.addGlyphs(tileUID, feature.label, layout.text.font, *fontStack, face);
             }
         }
 
diff --git a/src/mbgl/renderer/symbol_bucket.hpp b/src/mbgl/renderer/symbol_bucket.hpp
index 54a77b509..9c67d66a5 100644
--- a/src/mbgl/renderer/symbol_bucket.hpp
+++ b/src/mbgl/renderer/symbol_bucket.hpp
@@ -69,6 +69,8 @@ public:
                      GlyphAtlas&,
                      GlyphStore&);
 
+    inline bool needsGlyphs() const { return needsGlyphs_; }
+
     void drawGlyphs(SDFShader& shader);
     void drawIcons(SDFShader& shader);
     void drawIcons(IconShader& shader);
@@ -103,6 +105,7 @@ private:
         std::vector<std::unique_ptr<IconElementGroup>> groups;
     } icon;
 
+    bool needsGlyphs_;
 };
 }
 
diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index aed162ade..80af0db98 100644
--- a/src/mbgl/text/glyph_store.cpp
+++ b/src/mbgl/text/glyph_store.cpp
@@ -10,6 +10,7 @@
 #include <mbgl/util/token.hpp>
 #include <mbgl/util/math.hpp>
 #include <mbgl/storage/file_source.hpp>
+#include <mbgl/platform/log.hpp>
 #include <mbgl/platform/platform.hpp>
 #include <mbgl/util/uv_detail.hpp>
 #include <algorithm>
@@ -138,11 +139,12 @@ void FontStack::lineWrap(Shaping &shaping, const float lineHeight, const float m
     align(shaping, justify, horizontalAlign, verticalAlign, maxLineLength, lineHeight, line);
 }
 
-GlyphPBF::GlyphPBF(const std::string &glyphURL,
-                   const std::string &fontStack,
+GlyphPBF::GlyphPBF(const std::string& glyphURL,
+                   const std::string& fontStack,
                    GlyphRange glyphRange,
-                   Environment &env)
-    : future(promise.get_future().share()) {
+                   Environment& env,
+                   const GlyphLoadedCallback& callback)
+    : parsed(false) {
     // Load the glyph set URL
     std::string url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string {
         if (name == "fontstack") return util::percentEncode(fontStack);
@@ -151,27 +153,23 @@ GlyphPBF::GlyphPBF(const std::string &glyphURL,
     });
 
     // The prepare call jumps back to the main thread.
-    env.requestAsync({ Resource::Kind::Glyphs, url }, [&, url](const Response &res) {
+    env.requestAsync({ Resource::Kind::Glyphs, url }, [&, url, callback](const Response &res) {
         if (res.status != Response::Successful) {
-            // Something went wrong with loading the glyph pbf. Pass on the error to the future listeners.
+            // Something went wrong with loading the glyph pbf.
             const std::string msg = std::string { "[ERROR] failed to load glyphs: " } + url + " message: " + res.message;
-            promise.set_exception(std::make_exception_ptr(std::runtime_error(msg)));
+            Log::Error(Event::HttpRequest, msg);
         } else {
             // Transfer the data to the GlyphSet and signal its availability.
             // Once it is available, the caller will need to call parse() to actually
             // parse the data we received. We are not doing this here since this callback is being
             // called from another (unknown) thread.
             data = res.data;
-            promise.set_value(*this);
+            parsed = true;
+            callback(this);
         }
     });
 }
 
-
-std::shared_future<GlyphPBF &> GlyphPBF::getFuture() {
-    return future;
-}
-
 void GlyphPBF::parse(FontStack &stack) {
     std::lock_guard<std::mutex> lock(mtx);
 
@@ -226,8 +224,12 @@ void GlyphPBF::parse(FontStack &stack) {
     data.clear();
 }
 
+bool GlyphPBF::isParsed() const {
+    return parsed;
+}
+
 GlyphStore::GlyphStore(Environment& env_)
-    : env(env_), mtx(util::make_unique<uv::mutex>()), observer(nullptr) {
+    : env(env_), observer(nullptr) {
 }
 
 GlyphStore::~GlyphStore() {
@@ -238,61 +240,59 @@ void GlyphStore::setURL(const std::string &url) {
     glyphURL = url;
 }
 
+bool GlyphStore::requestGlyphRangesIfNeeded(const std::string& fontStack,
+                                            const std::set<GlyphRange>& glyphRanges) {
+    bool requestIsNeeded = false;
 
-void GlyphStore::waitForGlyphRanges(const std::string &fontStack, const std::set<GlyphRange> &glyphRanges) {
-    // We are implementing a blocking wait with futures: Every GlyphSet has a future that we are
-    // waiting for until it is loaded.
     if (glyphRanges.empty()) {
-        return;
+        return requestIsNeeded;
     }
 
-    uv::exclusive<FontStack> stack(mtx);
-
-    std::vector<std::shared_future<GlyphPBF &>> futures;
-    futures.reserve(glyphRanges.size());
-    {
-        auto &rangeSets = ranges[fontStack];
-
-        stack << createFontStack(fontStack);
+    auto callback = [this, fontStack](GlyphPBF* glyph) {
+        glyph->parse(*createFontStack(fontStack));
+        emitGlyphRangeLoaded();
+    };
+
+    std::lock_guard<std::mutex> lock(rangesMutex);
+    auto& rangeSets = ranges[fontStack];
+
+    for (const auto& range : glyphRanges) {
+        const auto& rangeSets_it = rangeSets.find(range);
+        if (rangeSets_it == rangeSets.end()) {
+            auto glyph = util::make_unique<GlyphPBF>(glyphURL, fontStack, range, env, callback);
+            rangeSets.emplace(range, std::move(glyph));
+            requestIsNeeded = true;
+            continue;
+        }
 
-        // Attempt to load the glyph range. If the GlyphSet already exists, we are getting back
-        // the same shared_future.
-        for (const auto range : glyphRanges) {
-            futures.emplace_back(loadGlyphRange(fontStack, rangeSets, range));
+        if (!rangeSets_it->second->isParsed()) {
+            requestIsNeeded = true;
         }
     }
 
-    // Now that we potentially created all GlyphSets, we are waiting for the results, one by one.
-    // When we get a result (or the GlyphSet is aready loaded), we are attempting to parse the
-    // GlyphSet.
-    for (const auto& future : futures) {
-        future.get().parse(stack);
-    }
+    return requestIsNeeded;
 }
 
-std::shared_future<GlyphPBF &> GlyphStore::loadGlyphRange(const std::string &fontStack, std::map<GlyphRange, std::unique_ptr<GlyphPBF>> &rangeSets, const GlyphRange range) {
-    auto range_it = rangeSets.find(range);
-    if (range_it == rangeSets.end()) {
-        // We don't have this glyph set yet for this font stack.
-        range_it = rangeSets.emplace(range, util::make_unique<GlyphPBF>(glyphURL, fontStack, range, env)).first;
-    }
-
-    return range_it->second->getFuture();
-}
+FontStack* GlyphStore::createFontStack(const std::string &fontStack) {
+    std::lock_guard<std::mutex> lock(stacksMutex);
 
-FontStack &GlyphStore::createFontStack(const std::string &fontStack) {
     auto stack_it = stacks.find(fontStack);
     if (stack_it == stacks.end()) {
         stack_it = stacks.emplace(fontStack, util::make_unique<FontStack>()).first;
     }
 
-    return *stack_it->second.get();
+    return stack_it->second.get();
 }
 
-uv::exclusive<FontStack> GlyphStore::getFontStack(const std::string &fontStack) {
-    uv::exclusive<FontStack> stack(mtx);
-    stack << createFontStack(fontStack);
-    return stack;
+FontStack* GlyphStore::getFontStack(const std::string &fontStack) {
+    std::lock_guard<std::mutex> lock(stacksMutex);
+
+    const auto& stack_it = stacks.find(fontStack);
+    if (stack_it == stacks.end()) {
+        return nullptr;
+    }
+
+    return stack_it->second.get();
 }
 
 void GlyphStore::setObserver(Observer* observer_) {
diff --git a/src/mbgl/text/glyph_store.hpp b/src/mbgl/text/glyph_store.hpp
index f92161995..6b7e2b8bd 100644
--- a/src/mbgl/text/glyph_store.hpp
+++ b/src/mbgl/text/glyph_store.hpp
@@ -4,13 +4,13 @@
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/vec.hpp>
 #include <mbgl/util/ptr.hpp>
-#include <mbgl/util/uv.hpp>
 
 #include <cstdint>
 #include <vector>
 #include <future>
 #include <map>
 #include <set>
+#include <string>
 #include <unordered_map>
 
 namespace mbgl {
@@ -49,10 +49,16 @@ private:
 
 class GlyphPBF {
 public:
+    using GlyphLoadedCallback = std::function<void(GlyphPBF*)>;
+
     GlyphPBF(const std::string &glyphURL,
              const std::string &fontStack,
              GlyphRange glyphRange,
-             Environment &env);
+             Environment &env,
+             const GlyphLoadedCallback& callback);
+
+    void parse(FontStack &stack);
+    bool isParsed() const;
 
 private:
     GlyphPBF(const GlyphPBF &) = delete;
@@ -60,16 +66,9 @@ private:
     GlyphPBF &operator=(const GlyphPBF &) = delete;
     GlyphPBF &operator=(GlyphPBF &&) = delete;
 
-public:
-    void parse(FontStack &stack);
-
-    std::shared_future<GlyphPBF &> getFuture();
-
-private:
     std::string data;
-    std::promise<GlyphPBF &> promise;
-    std::shared_future<GlyphPBF &> future;
-    std::mutex mtx;
+    std::atomic_bool parsed;
+    mutable std::mutex mtx;
 };
 
 // Manages Glyphrange PBF loading.
@@ -85,10 +84,13 @@ public:
     GlyphStore(Environment &);
     ~GlyphStore();
 
-    // Block until all specified GlyphRanges of the specified font stack are loaded.
-    void waitForGlyphRanges(const std::string &fontStack, const std::set<GlyphRange> &glyphRanges);
+    // Asynchronously request for GlyphRanges and when it gets loaded, notifies the
+    // observer subscribed to this object. Successive requests for the same range are
+    // going to be discarded. Returns true if a request was made or false if all the
+    // GlyphRanges are already available, and thus, no request is performed.
+    bool requestGlyphRangesIfNeeded(const std::string &fontStack, const std::set<GlyphRange> &glyphRanges);
 
-    uv::exclusive<FontStack> getFontStack(const std::string &fontStack);
+    FontStack* getFontStack(const std::string &fontStack);
 
     void setURL(const std::string &url);
 
@@ -97,21 +99,20 @@ public:
 private:
     void emitGlyphRangeLoaded();
 
-    // Loads an individual glyph range from the font stack and adds it to rangeSets
-    std::shared_future<GlyphPBF &> loadGlyphRange(const std::string &fontStack, std::map<GlyphRange, std::unique_ptr<GlyphPBF>> &rangeSets, GlyphRange range);
-
-    FontStack &createFontStack(const std::string &fontStack);
+    FontStack* createFontStack(const std::string &fontStack);
 
     std::string glyphURL;
     Environment &env;
+
     std::unordered_map<std::string, std::map<GlyphRange, std::unique_ptr<GlyphPBF>>> ranges;
+    std::mutex rangesMutex;
+
     std::unordered_map<std::string, std::unique_ptr<FontStack>> stacks;
-    std::unique_ptr<uv::mutex> mtx;
+    std::mutex stacksMutex;
 
     Observer* observer;
 };
 
-
 }
 
 #endif
-- 
cgit v1.2.3


From 19881f8b24c94ef66cc3ee72eebc73eeb37af2af Mon Sep 17 00:00:00 2001
From: "Thiago Marcos P. Santos" <thiago@mapbox.com>
Date: Tue, 28 Apr 2015 21:56:42 +0300
Subject: Cancel all pending requests before leaving

This still has the unwanted side effect off blocking the Map thread
waiting for the workers to return (because they hold a reference
to the Map thread main loop).

Maybe the only way to solve this would be having a I/O thread for
dispatching messages.

The Request is not bound anymore to the invoker thread because the workers
will request for glyphs, but the reply will arrive at the Map thread.
---
 include/mbgl/storage/request.hpp |  2 --
 src/mbgl/map/environment.cpp     |  1 -
 src/mbgl/map/resource_loader.cpp |  3 +--
 src/mbgl/map/source.cpp          | 20 ++++++++++++++------
 src/mbgl/map/source.hpp          |  6 ++++--
 src/mbgl/storage/request.cpp     |  3 +--
 src/mbgl/text/glyph_store.cpp    | 14 +++++++++++---
 src/mbgl/text/glyph_store.hpp    |  6 ++++++
 8 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/include/mbgl/storage/request.hpp b/include/mbgl/storage/request.hpp
index a01808ce8..5d979a281 100644
--- a/include/mbgl/storage/request.hpp
+++ b/include/mbgl/storage/request.hpp
@@ -19,8 +19,6 @@ namespace mbgl {
 class Response;
 
 class Request : private util::noncopyable {
-    MBGL_STORE_THREAD(tid)
-
 public:
     using Callback = std::function<void(const Response &)>;
     Request(const Resource &resource, uv_loop_t *loop, Callback callback);
diff --git a/src/mbgl/map/environment.cpp b/src/mbgl/map/environment.cpp
index dd6c1f193..5857d4bdf 100644
--- a/src/mbgl/map/environment.cpp
+++ b/src/mbgl/map/environment.cpp
@@ -128,7 +128,6 @@ void Environment::requestAsync(const Resource& resource,
 
 Request* Environment::request(const Resource& resource,
                               std::function<void(const Response&)> callback) {
-    assert(currentlyOn(ThreadType::Map));
     return fileSource.request(resource, util::RunLoop::current.get()->get(), std::move(callback));
 }
 
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 5dc0ab9d6..b3aa35b74 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -37,10 +37,9 @@ void ResourceLoader::setStyle(Style* style) {
 
     style_ = style;
 
-    Environment& env = Environment::Get();
     for (const auto& source : style->sources) {
         source->setObserver(this);
-        source->load(accessToken_, env);
+        source->load(accessToken_);
     }
 }
 
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 9b6631da9..8b0bf905f 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -113,11 +113,13 @@ std::string SourceInfo::tileURL(const TileID& id, float pixelRatio) const {
     return result;
 }
 
-Source::Source()
-{
-}
+Source::Source() {}
 
-Source::~Source() {}
+Source::~Source() {
+    if (req) {
+        Environment::Get().cancelRequest(req);
+    }
+}
 
 bool Source::isLoaded() const {
     if (!loaded) {
@@ -136,7 +138,7 @@ bool Source::isLoaded() const {
 // 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 Source::load(const std::string& accessToken, Environment& env) {
+void Source::load(const std::string& accessToken) {
     if (info.url.empty()) {
         loaded = true;
         return;
@@ -145,7 +147,9 @@ void Source::load(const std::string& accessToken, Environment& env) {
     util::ptr<Source> source = shared_from_this();
 
     const std::string url = util::mapbox::normalizeSourceURL(info.url, accessToken);
-    env.request({ Resource::Kind::JSON, url }, [source](const Response &res) {
+    req = Environment::Get().request({ Resource::Kind::JSON, url }, [source](const Response &res) {
+        source->clearRequest();
+
         if (res.status != Response::Successful) {
             Log::Warning(Event::General, "Failed to load source TileJSON: %s", res.message.c_str());
             return;
@@ -505,6 +509,10 @@ void Source::onLowMemory() {
     cache.clear();
 }
 
+void Source::clearRequest() {
+    req = nullptr;
+}
+
 void Source::setObserver(Observer* observer) {
     observer_ = observer;
 }
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index dd02cdc3b..eb7950225 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -29,6 +29,7 @@ class Sprite;
 class TexturePool;
 class Style;
 class Painter;
+class Request;
 class TransformState;
 class Tile;
 struct ClipID;
@@ -63,7 +64,7 @@ public:
     Source();
     ~Source();
 
-    void load(const std::string& accessToken, Environment&);
+    void load(const std::string& accessToken);
     bool isLoaded() const;
 
     void load(MapData&, Environment&, std::function<void()> callback);
@@ -87,6 +88,7 @@ public:
 
     void setCacheSize(size_t);
     void onLowMemory();
+    void clearRequest();
 
     void setObserver(Observer* observer);
 
@@ -128,7 +130,7 @@ private:
     std::map<TileID, std::weak_ptr<TileData>> tile_data;
     TileCache cache;
 
-
+    Request* req = nullptr;
     Observer* observer_ = nullptr;
 };
 
diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp
index d413cabbe..53a882f2a 100644
--- a/src/mbgl/storage/request.cpp
+++ b/src/mbgl/storage/request.cpp
@@ -1,5 +1,6 @@
 #include <mbgl/storage/request.hpp>
 
+#include <mbgl/map/environment.hpp>
 #include <mbgl/storage/response.hpp>
 
 #include <mbgl/util/util.hpp>
@@ -33,7 +34,6 @@ Request::Request(const Resource &resource_, uv_loop_t *loop, Callback callback_)
 
 // Called in the originating thread.
 void Request::notifyCallback() {
-    MBGL_VERIFY_THREAD(tid)
     if (!canceled) {
         invoke();
     } else {
@@ -84,7 +84,6 @@ void Request::notify(const std::shared_ptr<const Response> &response_) {
 
 // Called in the originating thread.
 void Request::cancel() {
-    MBGL_VERIFY_THREAD(tid)
     assert(async);
     assert(!canceled);
     canceled = util::make_unique<Canceled>();
diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index 80af0db98..71c1be6d9 100644
--- a/src/mbgl/text/glyph_store.cpp
+++ b/src/mbgl/text/glyph_store.cpp
@@ -142,9 +142,9 @@ void FontStack::lineWrap(Shaping &shaping, const float lineHeight, const float m
 GlyphPBF::GlyphPBF(const std::string& glyphURL,
                    const std::string& fontStack,
                    GlyphRange glyphRange,
-                   Environment& env,
+                   Environment& env_,
                    const GlyphLoadedCallback& callback)
-    : parsed(false) {
+    : parsed(false), env(env_) {
     // Load the glyph set URL
     std::string url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string {
         if (name == "fontstack") return util::percentEncode(fontStack);
@@ -153,7 +153,9 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL,
     });
 
     // The prepare call jumps back to the main thread.
-    env.requestAsync({ Resource::Kind::Glyphs, url }, [&, url, callback](const Response &res) {
+    req = env.request({ Resource::Kind::Glyphs, url }, [&, url, callback](const Response &res) {
+        req = nullptr;
+
         if (res.status != Response::Successful) {
             // Something went wrong with loading the glyph pbf.
             const std::string msg = std::string { "[ERROR] failed to load glyphs: " } + url + " message: " + res.message;
@@ -170,6 +172,12 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL,
     });
 }
 
+GlyphPBF::~GlyphPBF() {
+    if (req) {
+        env.cancelRequest(req);
+    }
+}
+
 void GlyphPBF::parse(FontStack &stack) {
     std::lock_guard<std::mutex> lock(mtx);
 
diff --git a/src/mbgl/text/glyph_store.hpp b/src/mbgl/text/glyph_store.hpp
index 6b7e2b8bd..a41eec533 100644
--- a/src/mbgl/text/glyph_store.hpp
+++ b/src/mbgl/text/glyph_store.hpp
@@ -17,6 +17,7 @@ namespace mbgl {
 
 class FileSource;
 class Environment;
+class Request;
 
 class SDFGlyph {
 public:
@@ -56,6 +57,7 @@ public:
              GlyphRange glyphRange,
              Environment &env,
              const GlyphLoadedCallback& callback);
+    ~GlyphPBF();
 
     void parse(FontStack &stack);
     bool isParsed() const;
@@ -68,6 +70,10 @@ private:
 
     std::string data;
     std::atomic_bool parsed;
+
+    Environment& env;
+    Request* req = nullptr;
+
     mutable std::mutex mtx;
 };
 
-- 
cgit v1.2.3