Merge branch 'master' into new-labelling

Conflicts:
	src/mbgl/map/tile_parser.cpp
	src/mbgl/map/tile_parser.hpp
	src/mbgl/renderer/painter.hpp
	src/mbgl/renderer/painter_symbol.cpp
	src/mbgl/renderer/symbol_bucket.cpp
	src/mbgl/renderer/symbol_bucket.hpp
	src/mbgl/text/collision.cpp
	src/mbgl/text/collision.hpp
	src/mbgl/text/placement.cpp

5 files changed, 129 insertions, 71 deletions

diff --git a/src/mbgl/text/glyph.hpp b/src/mbgl/text/glyph.hpp
index bc46042d2a..03016f0fbf 100644
--- a/src/mbgl/text/glyph.hpp
+++ b/src/mbgl/text/glyph.hpp
@@ -35,7 +35,7 @@ struct Glyph {
         : rect(rect_), metrics(metrics_) {}
 
     operator bool() const {
-        return metrics || rect;
+        return metrics || rect.hasArea();
     }
 
     const Rect<uint16_t> rect;
diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index 55159f3131..37e26b193c 100644
--- a/src/mbgl/text/glyph_store.cpp
+++ b/src/mbgl/text/glyph_store.cpp
@@ -9,7 +9,9 @@
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/token.hpp>
 #include <mbgl/util/math.hpp>
+#include <mbgl/util/uv_detail.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>
@@ -155,11 +157,12 @@ void FontStack::lineWrap(Shaping &shaping, const float lineHeight, const float m
      shaping.right = shaping.left + maxLineLength;
 }
 
-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), 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);
@@ -168,25 +171,29 @@ 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) {
+    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. Pass on the error to the future listeners.
-            const std::string msg = std::string { "[ERROR] failed to load glyphs: " } + res.message;
-            promise.set_exception(std::make_exception_ptr(std::runtime_error(msg)));
+            // Something went wrong with loading the glyph pbf.
+            const std::string msg = std::string { "[ERROR] failed to load glyphs: " } + url + " message: " + res.message;
+            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;
+GlyphPBF::~GlyphPBF() {
+    if (req) {
+        env.cancelRequest(req);
+    }
 }
 
 void GlyphPBF::parse(FontStack &stack) {
@@ -243,68 +250,88 @@ void GlyphPBF::parse(FontStack &stack) {
     data.clear();
 }
 
-GlyphStore::GlyphStore(Environment& env_) : env(env_), mtx(util::make_unique<uv::mutex>()) {}
+bool GlyphPBF::isParsed() const {
+    return parsed;
+}
+
+GlyphStore::GlyphStore(uv_loop_t* loop, Environment& env_)
+    : env(env_),
+      asyncEmitGlyphRangeLoaded(util::make_unique<uv::async>(loop, [this] { emitGlyphRangeLoaded(); })),
+      observer(nullptr) {
+    asyncEmitGlyphRangeLoaded->unref();
+}
+
+GlyphStore::~GlyphStore() {
+    observer = nullptr;
+}
 
 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));
+        asyncEmitGlyphRangeLoaded->send();
+    };
+
+    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;
+FontStack* GlyphStore::createFontStack(const std::string &fontStack) {
+    std::lock_guard<std::mutex> lock(stacksMutex);
+
+    auto stack_it = stacks.find(fontStack);
+    if (stack_it == stacks.end()) {
+        stack_it = stacks.emplace(fontStack, util::make_unique<FontStack>()).first;
     }
 
-    return range_it->second->getFuture();
+    return stack_it->second.get();
 }
 
-FontStack &GlyphStore::createFontStack(const std::string &fontStack) {
-    auto stack_it = stacks.find(fontStack);
+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()) {
-        stack_it = stacks.emplace(fontStack, util::make_unique<FontStack>()).first;
+        return nullptr;
     }
 
-    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;
+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 ee2097536c..efa848dd08 100644
--- a/src/mbgl/text/glyph_store.hpp
+++ b/src/mbgl/text/glyph_store.hpp
@@ -4,19 +4,28 @@
 #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>
 
+typedef struct uv_loop_s uv_loop_t;
+
+namespace uv {
+
+class async;
+
+}
+
 namespace mbgl {
 
 class FileSource;
 class Environment;
+class Request;
 
 class SDFGlyph {
 public:
@@ -49,10 +58,17 @@ 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);
+    ~GlyphPBF();
+
+    void parse(FontStack &stack);
+    bool isParsed() const;
 
 private:
     GlyphPBF(const GlyphPBF &) = delete;
@@ -60,43 +76,58 @@ private:
     GlyphPBF &operator=(const GlyphPBF &) = delete;
     GlyphPBF &operator=(GlyphPBF &&) = delete;
 
-public:
-    void parse(FontStack &stack);
+    std::string data;
+    std::atomic_bool parsed;
 
-    std::shared_future<GlyphPBF &> getFuture();
+    Environment& env;
+    Request* req = nullptr;
 
-private:
-    std::string data;
-    std::promise<GlyphPBF &> promise;
-    std::shared_future<GlyphPBF &> future;
-    std::mutex mtx;
+    mutable std::mutex mtx;
 };
 
 // Manages Glyphrange PBF loading.
 class GlyphStore {
 public:
-    GlyphStore(Environment &);
+    class Observer {
+    public:
+        virtual ~Observer() = default;
 
-    // Block until all specified GlyphRanges of the specified font stack are loaded.
-    void waitForGlyphRanges(const std::string &fontStack, const std::set<GlyphRange> &glyphRanges);
+        virtual void onGlyphRangeLoaded() = 0;
+    };
 
-    uv::exclusive<FontStack> getFontStack(const std::string &fontStack);
+    GlyphStore(uv_loop_t* loop, Environment &);
+    ~GlyphStore();
+
+    // 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);
+
+    FontStack* getFontStack(const std::string &fontStack);
 
     void setURL(const std::string &url);
 
+    void setObserver(Observer* observer);
+
 private:
-    // 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);
+    void emitGlyphRangeLoaded();
 
-    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;
 
+    std::unique_ptr<uv::async> asyncEmitGlyphRangeLoaded;
+
+    Observer* observer;
+};
 
 }
 
diff --git a/src/mbgl/text/quads.cpp b/src/mbgl/text/quads.cpp
index a30269035c..6c517bab2e 100644
--- a/src/mbgl/text/quads.cpp
+++ b/src/mbgl/text/quads.cpp
@@ -136,7 +136,7 @@ SymbolQuads getGlyphQuads(Anchor &anchor, const Shaping &shapedText,
         if (!glyph)
             continue;
 
-        if (!rect)
+        if (!rect.hasArea())
             continue;
 
         const float centerX = (positionedGlyph.x + glyph.metrics.advance / 2.0f) * boxScale;
diff --git a/src/mbgl/text/shaping.hpp b/src/mbgl/text/shaping.hpp
index 41e889333e..c409e6ed0a 100644
--- a/src/mbgl/text/shaping.hpp
+++ b/src/mbgl/text/shaping.hpp
@@ -19,7 +19,7 @@ namespace mbgl {
             float left = 0;
             float right = 0;
 
-            operator bool() const { return image; }
+            operator bool() const { return image.hasArea(); }
     };
 
     class StyleLayoutSymbol;