From 071e2e1731b447e195290936c9bf52d28f52b2d9 Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Tue, 28 Apr 2015 14:53:50 +0300 Subject: [PATCH] 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 | 98 ++++++++++++++--------------- src/mbgl/text/glyph_store.hpp | 41 ++++++------ 6 files changed, 106 insertions(+), 78 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 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