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 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 TileParser::createLineBucket(const GeometryTileLayer& la } std::unique_ptr 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(*collision); const float z = tile.id.z; @@ -231,6 +239,13 @@ std::unique_ptr TileParser::createSymbolBucket(const GeometryTileLayer& bucket->addFeatures( layer, bucket_desc.filter, reinterpret_cast(&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 createBucket(const StyleBucket&); std::unique_ptr createFillBucket(const GeometryTileLayer&, const StyleBucket&); std::unique_ptr createLineBucket(const GeometryTileLayer&, const StyleBucket&); - std::unique_ptr createSymbolBucket(const GeometryTileLayer&, const StyleBucket&); + std::unique_ptr createSymbolBucket(const GeometryTileLayer&, const StyleBucket&, bool& needsResources); template 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 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 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> 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 #include #include +#include #include #include #include @@ -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::getFuture() { - return future; -} - void GlyphPBF::parse(FontStack &stack) { std::lock_guard 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()), 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& glyphRanges) { + bool requestIsNeeded = false; -void GlyphStore::waitForGlyphRanges(const std::string &fontStack, const std::set &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 stack(mtx); + auto callback = [this, fontStack](GlyphPBF* glyph) { + glyph->parse(*createFontStack(fontStack)); + emitGlyphRangeLoaded(); + }; - std::vector> futures; - futures.reserve(glyphRanges.size()); - { - auto &rangeSets = ranges[fontStack]; + std::lock_guard lock(rangesMutex); + auto& rangeSets = ranges[fontStack]; - stack << createFontStack(fontStack); + for (const auto& range : glyphRanges) { + const auto& rangeSets_it = rangeSets.find(range); + if (rangeSets_it == rangeSets.end()) { + auto glyph = util::make_unique(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 GlyphStore::loadGlyphRange(const std::string &fontStack, std::map> &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(glyphURL, fontStack, range, env)).first; - } +FontStack* GlyphStore::createFontStack(const std::string &fontStack) { + std::lock_guard lock(stacksMutex); - return range_it->second->getFuture(); -} - -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()).first; } - return *stack_it->second.get(); + return stack_it->second.get(); } -uv::exclusive GlyphStore::getFontStack(const std::string &fontStack) { - uv::exclusive stack(mtx); - stack << createFontStack(fontStack); - return stack; +FontStack* GlyphStore::getFontStack(const std::string &fontStack) { + std::lock_guard 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 #include #include -#include #include #include #include #include #include +#include #include namespace mbgl { @@ -49,10 +49,16 @@ private: class GlyphPBF { public: + using GlyphLoadedCallback = std::function; + 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 getFuture(); - -private: std::string data; - std::promise promise; - std::shared_future 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 &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 &glyphRanges); - uv::exclusive 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 loadGlyphRange(const std::string &fontStack, std::map> &rangeSets, GlyphRange range); - - FontStack &createFontStack(const std::string &fontStack); + FontStack* createFontStack(const std::string &fontStack); std::string glyphURL; Environment &env; + std::unordered_map>> ranges; + std::mutex rangesMutex; + std::unordered_map> stacks; - std::unique_ptr mtx; + std::mutex stacksMutex; Observer* observer; }; - } #endif