diff options
author | Thiago Marcos P. Santos <thiago@mapbox.com> | 2015-07-15 12:02:58 +0300 |
---|---|---|
committer | Thiago Marcos P. Santos <thiago@mapbox.com> | 2015-07-17 21:00:34 +0300 |
commit | 234a6539311bca387a982ad1c6f51dde77d3c450 (patch) | |
tree | 6fb2fee309fc98edb4c6a8f3d0549c91548a21c1 | |
parent | dbf3edad8ae7a7cbaf7c109daa3f7f8c78c04c87 (diff) | |
download | qtlocation-mapboxgl-234a6539311bca387a982ad1c6f51dde77d3c450.tar.gz |
Use the observer pattern for GlyphPBF loading
This will make the code a lot more clear and it will
also move how parsing is initiated to the GlyphPBF
class, to be initiated after the request, like we do
for other resources.
-rw-r--r-- | src/mbgl/text/glyph_pbf.cpp | 141 | ||||
-rw-r--r-- | src/mbgl/text/glyph_pbf.hpp | 47 | ||||
-rw-r--r-- | src/mbgl/text/glyph_store.cpp | 62 | ||||
-rw-r--r-- | src/mbgl/text/glyph_store.hpp | 24 | ||||
-rw-r--r-- | test/style/glyph_store.cpp | 10 | ||||
-rw-r--r-- | test/style/resource_loading.cpp | 2 |
6 files changed, 144 insertions, 142 deletions
diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp index 24993ed72d..ba92ce6b4e 100644 --- a/src/mbgl/text/glyph_pbf.cpp +++ b/src/mbgl/text/glyph_pbf.cpp @@ -1,77 +1,32 @@ #include <mbgl/text/glyph_pbf.hpp> -#include <mbgl/text/font_stack.hpp> #include <mbgl/storage/file_source.hpp> #include <mbgl/storage/resource.hpp> #include <mbgl/storage/response.hpp> - +#include <mbgl/text/font_stack.hpp> +#include <mbgl/text/glyph_store.hpp> +#include <mbgl/util/exception.hpp> #include <mbgl/util/pbf.hpp> #include <mbgl/util/string.hpp> -#include <mbgl/util/thread.hpp> +#include <mbgl/util/thread_context.hpp> #include <mbgl/util/token.hpp> #include <mbgl/util/url.hpp> #include <sstream> -namespace mbgl { - -GlyphPBF::GlyphPBF(const std::string& glyphURL, - const std::string& fontStack, - GlyphRange glyphRange, - const GlyphLoadedCallback& successCallback, - const GlyphLoadingFailedCallback& failureCallback) - : parsed(false) { - // Load the glyph set URL - url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string { - if (name == "fontstack") return util::percentEncode(fontStack); - if (name == "range") return util::toString(glyphRange.first) + "-" + util::toString(glyphRange.second); - return ""; - }); +namespace { - // The prepare call jumps back to the main thread. - FileSource* fs = util::ThreadContext::getFileSource(); - req = fs->request({ Resource::Kind::Glyphs, url }, util::RunLoop::getLoop(), [&, successCallback, failureCallback](const Response &res) { - req = nullptr; - - if (res.status != Response::Successful) { - std::stringstream message; - message << "Failed to load [" << url << "]: " << res.message; - failureCallback(message.str()); - } 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; - successCallback(this); - } - }); -} - -GlyphPBF::~GlyphPBF() { - if (req) { - util::ThreadContext::getFileSource()->cancel(req); - } -} - -void GlyphPBF::parse(FontStack &stack) { - if (!data.size()) { - // If there is no data, this means we either haven't received any data, or - // we have already parsed the data. - return; - } - - // Parse the glyph PBF - pbf glyphs_pbf(reinterpret_cast<const uint8_t *>(data.data()), data.size()); +void parseGlyphPBF(mbgl::FontStack& stack, const std::string& data) { + mbgl::pbf glyphs_pbf(reinterpret_cast<const uint8_t *>(data.data()), data.size()); while (glyphs_pbf.next()) { if (glyphs_pbf.tag == 1) { // stacks - pbf fontstack_pbf = glyphs_pbf.message(); + mbgl::pbf fontstack_pbf = glyphs_pbf.message(); while (fontstack_pbf.next()) { if (fontstack_pbf.tag == 3) { // glyphs - pbf glyph_pbf = fontstack_pbf.message(); + mbgl::pbf glyph_pbf = fontstack_pbf.message(); - SDFGlyph glyph; + mbgl::SDFGlyph glyph; while (glyph_pbf.next()) { if (glyph_pbf.tag == 1) { // id @@ -102,14 +57,84 @@ void GlyphPBF::parse(FontStack &stack) { glyphs_pbf.skip(); } } +} + +} + +namespace mbgl { + +GlyphPBF::GlyphPBF(GlyphStore* store, + const std::string& fontStack, + const GlyphRange& glyphRange) + : parsed(false) { + // Load the glyph set URL + std::string url = util::replaceTokens(store->getURL(), [&](const std::string &name) -> std::string { + if (name == "fontstack") return util::percentEncode(fontStack); + if (name == "range") return util::toString(glyphRange.first) + "-" + util::toString(glyphRange.second); + return ""; + }); + + auto requestCallback = [this, store, fontStack, url](const Response &res) { + req = nullptr; + + if (res.status != Response::Successful) { + std::stringstream message; + message << "Failed to load [" << url << "]: " << res.message; + emitGlyphPBFLoadingFailed(message.str()); + } else { + data = res.data; + parse(store, fontStack, url); + } + }; + + FileSource* fs = util::ThreadContext::getFileSource(); + req = fs->request({ Resource::Kind::Glyphs, url }, util::RunLoop::getLoop(), requestCallback); +} + +GlyphPBF::~GlyphPBF() { + if (req) { + util::ThreadContext::getFileSource()->cancel(req); + } +} + +void GlyphPBF::parse(GlyphStore* store, const std::string& fontStack, const std::string& url) { + if (data.empty()) { + // If there is no data, this means we either haven't + // received any data. + return; + } - data.clear(); + try { + parseGlyphPBF(**store->getFontStack(fontStack), std::move(data)); + } catch (const std::exception& ex) { + std::stringstream message; + message << "Failed to parse [" << url << "]: " << ex.what(); + emitGlyphPBFLoadingFailed(message.str()); + return; + } parsed = true; + + emitGlyphPBFLoaded(); +} + +void GlyphPBF::setObserver(Observer* observer_) { + observer = observer_; } -bool GlyphPBF::isParsed() const { - return parsed; +void GlyphPBF::emitGlyphPBFLoaded() { + if (observer) { + observer->onGlyphPBFLoaded(); + } +} + +void GlyphPBF::emitGlyphPBFLoadingFailed(const std::string& message) { + if (!observer) { + return; + } + + auto error = std::make_exception_ptr(util::GlyphRangeLoadingException(message)); + observer->onGlyphPBFLoadingFailed(error); } } diff --git a/src/mbgl/text/glyph_pbf.hpp b/src/mbgl/text/glyph_pbf.hpp index cc083e7f10..2aa2134d16 100644 --- a/src/mbgl/text/glyph_pbf.hpp +++ b/src/mbgl/text/glyph_pbf.hpp @@ -2,48 +2,53 @@ #define MBGL_TEXT_GLYPH_PBF #include <mbgl/text/glyph.hpp> +#include <mbgl/util/noncopyable.hpp> -#include <functional> #include <atomic> +#include <functional> #include <string> namespace mbgl { +class GlyphStore; class FontStack; class Request; -class GlyphPBF { +class GlyphPBF : private util::noncopyable { public: - using GlyphLoadedCallback = std::function<void(GlyphPBF*)>; - using GlyphLoadingFailedCallback = std::function<void(const std::string&)>; + class Observer { + public: + virtual ~Observer() = default; + + virtual void onGlyphPBFLoaded() = 0; + virtual void onGlyphPBFLoadingFailed(std::exception_ptr error) = 0; + }; - GlyphPBF(const std::string &glyphURL, - const std::string &fontStack, - GlyphRange glyphRange, - const GlyphLoadedCallback& successCallback, - const GlyphLoadingFailedCallback& failureCallback); - ~GlyphPBF(); + GlyphPBF(GlyphStore* store, + const std::string& fontStack, + const GlyphRange& glyphRange); + virtual ~GlyphPBF(); - void parse(FontStack &stack); - bool isParsed() const; + bool isParsed() const { + return parsed; + }; - std::string getURL() const { - return url; - } + void setObserver(Observer* observer); private: - GlyphPBF(const GlyphPBF &) = delete; - GlyphPBF(GlyphPBF &&) = delete; - GlyphPBF &operator=(const GlyphPBF &) = delete; - GlyphPBF &operator=(GlyphPBF &&) = delete; + void emitGlyphPBFLoaded(); + void emitGlyphPBFLoadingFailed(const std::string& message); + + void parse(GlyphStore* store, const std::string& fontStack, const std::string& url); std::string data; - std::string url; std::atomic<bool> parsed; Request* req = nullptr; + + Observer* observer = nullptr; }; -} // end namespace mbgl +} #endif diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp index f34f8ecd04..eb770fbf6e 100644 --- a/src/mbgl/text/glyph_store.cpp +++ b/src/mbgl/text/glyph_store.cpp @@ -1,38 +1,13 @@ #include <mbgl/text/glyph_store.hpp> -#include <mbgl/text/font_stack.hpp> #include <mbgl/text/glyph_pbf.hpp> -#include <mbgl/util/exception.hpp> #include <mbgl/util/thread_context.hpp> namespace mbgl { -GlyphStore::GlyphStore() { -} - -GlyphStore::~GlyphStore() { -} - void GlyphStore::requestGlyphRange(const std::string& fontStackName, const GlyphRange& range) { assert(util::ThreadContext::currentlyOn(util::ThreadType::Map)); - auto successCallback = [this, fontStackName](GlyphPBF* glyph) { - try { - { - auto fontStack = createFontStack(fontStackName); - glyph->parse(**fontStack); - } - emitGlyphRangeLoaded(); - } catch (const std::exception&) { - std::string message = "Failed to parse [" + glyph->getURL() + "]"; - emitGlyphRangeLoadingFailed(message); - } - }; - - auto failureCallback = [this](const std::string& message) { - emitGlyphRangeLoadingFailed(message); - }; - std::lock_guard<std::mutex> lock(rangesMutex); auto& rangeSets = ranges[fontStackName]; @@ -41,8 +16,10 @@ void GlyphStore::requestGlyphRange(const std::string& fontStackName, const Glyph return; } - rangeSets.emplace(range, std::make_unique<GlyphPBF>(glyphURL, fontStackName, range, - successCallback, failureCallback)); + auto glyphPBF = std::make_unique<GlyphPBF>(this, fontStackName, range); + glyphPBF->setObserver(this); + + rangeSets.emplace(range, std::move(glyphPBF)); } @@ -55,7 +32,6 @@ bool GlyphStore::hasGlyphRanges(const std::string& fontStackName, const std::set const auto& rangeSets = ranges[fontStackName]; bool hasRanges = true; - for (const auto& range : glyphRanges) { const auto& rangeSetsIt = rangeSets.find(range); if (rangeSetsIt == rangeSets.end()) { @@ -75,7 +51,7 @@ bool GlyphStore::hasGlyphRanges(const std::string& fontStackName, const std::set return hasRanges; } -util::exclusive<FontStack> GlyphStore::createFontStack(const std::string& fontStack) { +util::exclusive<FontStack> GlyphStore::getFontStack(const std::string& fontStack) { auto lock = std::make_unique<std::lock_guard<std::mutex>>(stacksMutex); auto it = stacks.find(fontStack); @@ -88,34 +64,20 @@ util::exclusive<FontStack> GlyphStore::createFontStack(const std::string& fontSt return { it->second.get(), std::move(lock) }; } -util::exclusive<FontStack> GlyphStore::getFontStack(const std::string &fontStack) { - auto lock = std::make_unique<std::lock_guard<std::mutex>>(stacksMutex); - - const auto& it = stacks.find(fontStack); - if (it == stacks.end()) { - return { nullptr, nullptr }; - } - - return { it->second.get(), std::move(lock) }; -} - -void GlyphStore::setObserver(Observer* observer_) { - observer = observer_; -} - -void GlyphStore::emitGlyphRangeLoaded() { +void GlyphStore::onGlyphPBFLoaded() { if (observer) { observer->onGlyphRangeLoaded(); } } -void GlyphStore::emitGlyphRangeLoadingFailed(const std::string& message) { - if (!observer) { - return; +void GlyphStore::onGlyphPBFLoadingFailed(std::exception_ptr error) { + if (observer) { + observer->onGlyphRangeLoadingFailed(error); } +} - auto error = std::make_exception_ptr(util::GlyphRangeLoadingException(message)); - observer->onGlyphRangeLoadingFailed(error); +void GlyphStore::setObserver(Observer* observer_) { + observer = observer_; } } diff --git a/src/mbgl/text/glyph_store.hpp b/src/mbgl/text/glyph_store.hpp index 1e31e77bf5..f8de6cbaf5 100644 --- a/src/mbgl/text/glyph_store.hpp +++ b/src/mbgl/text/glyph_store.hpp @@ -1,8 +1,11 @@ #ifndef MBGL_TEXT_GLYPH_STORE #define MBGL_TEXT_GLYPH_STORE +#include <mbgl/text/font_stack.hpp> #include <mbgl/text/glyph.hpp> +#include <mbgl/text/glyph_pbf.hpp> #include <mbgl/util/exclusive.hpp> +#include <mbgl/util/noncopyable.hpp> #include <mbgl/util/run_loop.hpp> #include <mbgl/util/work_queue.hpp> @@ -13,13 +16,10 @@ namespace mbgl { -class FontStack; -class GlyphPBF; - // The GlyphStore manages the loading and storage of Glyphs // and creation of FontStack objects. The GlyphStore lives // on the MapThread but can be queried from any thread. -class GlyphStore { +class GlyphStore : public GlyphPBF::Observer, private util::noncopyable { public: class Observer { public: @@ -29,8 +29,8 @@ public: virtual void onGlyphRangeLoadingFailed(std::exception_ptr error) = 0; }; - GlyphStore(); - ~GlyphStore(); + GlyphStore() = default; + virtual ~GlyphStore() = default; util::exclusive<FontStack> getFontStack(const std::string& fontStack); @@ -45,13 +45,17 @@ public: glyphURL = url; } + std::string getURL() const { + return glyphURL; + } + + // GlyphPBF::Observer implementation. + void onGlyphPBFLoaded() override; + void onGlyphPBFLoadingFailed(std::exception_ptr error) override; + void setObserver(Observer* observer); private: - void emitGlyphRangeLoaded(); - void emitGlyphRangeLoadingFailed(const std::string& message); - - util::exclusive<FontStack> createFontStack(const std::string &fontStack); void requestGlyphRange(const std::string& fontStackName, const GlyphRange& range); std::string glyphURL; diff --git a/test/style/glyph_store.cpp b/test/style/glyph_store.cpp index fb4355b98b..fe614e8c60 100644 --- a/test/style/glyph_store.cpp +++ b/test/style/glyph_store.cpp @@ -138,7 +138,8 @@ TEST_F(GlyphStoreTest, LoadingFail) { ASSERT_TRUE(error != nullptr); auto fontStack = store->getFontStack(params.stack); - ASSERT_EQ(*fontStack, nullptr); + ASSERT_TRUE(fontStack->getMetrics().empty()); + ASSERT_TRUE(fontStack->getSDFs().empty()); for (const auto& range : params.ranges) { ASSERT_FALSE(store->hasGlyphRanges(params.stack, {range})); @@ -171,6 +172,10 @@ TEST_F(GlyphStoreTest, LoadingCorrupted) { ASSERT_TRUE(error != nullptr); + auto fontStack = store->getFontStack(params.stack); + ASSERT_TRUE(fontStack->getMetrics().empty()); + ASSERT_TRUE(fontStack->getSDFs().empty()); + for (const auto& range : params.ranges) { ASSERT_FALSE(store->hasGlyphRanges(params.stack, {range})); } @@ -215,7 +220,8 @@ TEST_F(GlyphStoreTest, InvalidURL) { ASSERT_TRUE(error != nullptr); auto fontStack = store->getFontStack(params.stack); - ASSERT_EQ(*fontStack, nullptr); + ASSERT_TRUE(fontStack->getMetrics().empty()); + ASSERT_TRUE(fontStack->getSDFs().empty()); stopTest(); }; diff --git a/test/style/resource_loading.cpp b/test/style/resource_loading.cpp index 025b9b0271..b4fdbc7c5a 100644 --- a/test/style/resource_loading.cpp +++ b/test/style/resource_loading.cpp @@ -164,4 +164,4 @@ INSTANTIATE_TEST_CASE_P(Style, ResourceLoading, std::make_pair("sprite.png", "Could not parse sprite image"), std::make_pair("raster.png", "Failed to parse \\[17\\/6553(4|5|6|7)\\/6553(4|5|6|7)\\]\\: error parsing raster image"), std::make_pair("vector.pbf", "Failed to parse \\[1(5|6)\\/1638(3|4)\\/1638(3|4)\\]\\: pbf unknown field type exception"), - std::make_pair("glyphs.pbf", "Failed to parse \\[test\\/fixtures\\/resources\\/glyphs.pbf\\]"))); + std::make_pair("glyphs.pbf", "Failed to parse \\[test\\/fixtures\\/resources\\/glyphs.pbf\\]: pbf unknown field type exception"))); |