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.

1 files changed, 51 insertions, 51 deletions

diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index aed162ade2..80af0db98f 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_) {