From ce1e46f00b786172390ed0136297d4422f3cbeb4 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Fri, 6 Oct 2017 23:04:56 +0300 Subject: [PATCH 1/2] [core] Update glyph requestors _before_ requesting from file source --- src/mbgl/text/glyph_manager.cpp | 11 ++++------- src/mbgl/text/glyph_manager.hpp | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/mbgl/text/glyph_manager.cpp b/src/mbgl/text/glyph_manager.cpp index 916d39ae620..c79a1938c1f 100644 --- a/src/mbgl/text/glyph_manager.cpp +++ b/src/mbgl/text/glyph_manager.cpp @@ -36,8 +36,9 @@ void GlyphManager::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphD for (const auto& range : ranges) { auto it = entry.ranges.find(range); if (it == entry.ranges.end() || !it->second.parsed) { - GlyphRequest& request = requestRange(entry, fontStack, range); + GlyphRequest& request = entry.ranges[range]; request.requestors[&requestor] = dependencies; + requestRange(request, fontStack, range); } } } @@ -49,18 +50,14 @@ void GlyphManager::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphD } } -GlyphManager::GlyphRequest& GlyphManager::requestRange(Entry& entry, const FontStack& fontStack, const GlyphRange& range) { - GlyphRequest& request = entry.ranges[range]; - +void GlyphManager::requestRange(GlyphRequest& request, const FontStack& fontStack, const GlyphRange& range) { if (request.req) { - return request; + return; } request.req = fileSource.request(Resource::glyphs(glyphURL, fontStack, range), [this, fontStack, range](Response res) { processResponse(res, fontStack, range); }); - - return request; } void GlyphManager::processResponse(const Response& res, const FontStack& fontStack, const GlyphRange& range) { diff --git a/src/mbgl/text/glyph_manager.hpp b/src/mbgl/text/glyph_manager.hpp index 00df079462a..de2b9cde7b8 100644 --- a/src/mbgl/text/glyph_manager.hpp +++ b/src/mbgl/text/glyph_manager.hpp @@ -58,7 +58,7 @@ class GlyphManager : public util::noncopyable { std::unordered_map entries; - GlyphRequest& requestRange(Entry&, const FontStack&, const GlyphRange&); + void requestRange(GlyphRequest&, const FontStack&, const GlyphRange&); void processResponse(const Response&, const FontStack&, const GlyphRange&); void notify(GlyphRequestor&, const GlyphDependencies&); From 18765578e040c8d268ec7098671997625c22b53c Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Fri, 6 Oct 2017 23:42:20 +0300 Subject: [PATCH 2/2] [test] Added GlyphManager.ImmediateFileSource --- test/src/mbgl/test/stub_file_source.cpp | 16 +++++++- test/src/mbgl/test/stub_file_source.hpp | 8 +++- test/text/glyph_loader.test.cpp | 49 +++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/test/src/mbgl/test/stub_file_source.cpp b/test/src/mbgl/test/stub_file_source.cpp index 7891d5d907d..0bbff84ff32 100644 --- a/test/src/mbgl/test/stub_file_source.cpp +++ b/test/src/mbgl/test/stub_file_source.cpp @@ -17,7 +17,12 @@ class StubFileRequest : public AsyncRequest { StubFileSource& fileSource; }; -StubFileSource::StubFileSource() { +StubFileSource::StubFileSource(ResponseType type_) + : type(type_) { + if (type == ResponseType::Synchronous) { + return; + } + timer.start(1ms, 1ms, [this] { // Explicit copy to avoid iterator invalidation if ~StubFileRequest gets called within the loop. auto pending_ = pending; @@ -46,7 +51,14 @@ StubFileSource::~StubFileSource() = default; std::unique_ptr StubFileSource::request(const Resource& resource, Callback callback) { auto req = std::make_unique(*this); - pending.emplace(req.get(), std::make_tuple(resource, response, callback)); + if (type == ResponseType::Synchronous) { + optional res = response(resource); + if (res) { + callback(*res); + } + } else { + pending.emplace(req.get(), std::make_tuple(resource, response, callback)); + } return std::move(req); } diff --git a/test/src/mbgl/test/stub_file_source.hpp b/test/src/mbgl/test/stub_file_source.hpp index 85118e1a777..6cee8377c69 100644 --- a/test/src/mbgl/test/stub_file_source.hpp +++ b/test/src/mbgl/test/stub_file_source.hpp @@ -9,7 +9,12 @@ namespace mbgl { class StubFileSource : public FileSource { public: - StubFileSource(); + enum class ResponseType { + Asynchronous = 0, + Synchronous + }; + + StubFileSource(ResponseType = ResponseType::Asynchronous); ~StubFileSource() override; std::unique_ptr request(const Resource&, Callback) override; @@ -36,6 +41,7 @@ class StubFileSource : public FileSource { optional defaultResponse(const Resource&); std::unordered_map> pending; + ResponseType type; util::Timer timer; }; diff --git a/test/text/glyph_loader.test.cpp b/test/text/glyph_loader.test.cpp index be197ebb462..20ac0459251 100644 --- a/test/text/glyph_loader.test.cpp +++ b/test/text/glyph_loader.test.cpp @@ -214,3 +214,52 @@ TEST(GlyphManager, LoadingInvalid) { {{{"Test Stack"}}, {u'A', u'E'}} }); } + +TEST(GlyphManager, ImmediateFileSource) { + class GlyphManagerTestSynchronous { + public: + util::RunLoop loop; + StubFileSource fileSource = { StubFileSource::ResponseType::Synchronous }; + StubGlyphManagerObserver observer; + StubGlyphRequestor requestor; + GlyphManager glyphManager { fileSource }; + + void run(const std::string& url, GlyphDependencies dependencies) { + // Squelch logging. + Log::setObserver(std::make_unique()); + + glyphManager.setURL(url); + glyphManager.setObserver(&observer); + glyphManager.getGlyphs(requestor, std::move(dependencies)); + + loop.run(); + } + + void end() { + loop.stop(); + } + }; + + GlyphManagerTestSynchronous test; + + test.fileSource.glyphsResponse = [&] (const Resource&) { + Response response; + response.data = std::make_shared(util::read_file("test/fixtures/resources/glyphs.pbf")); + return response; + }; + + test.observer.glyphsError = [&] (const FontStack&, const GlyphRange&, std::exception_ptr) { + FAIL(); + test.end(); + }; + + test.requestor.glyphsAvailable = [&] (GlyphMap) { + test.end(); + }; + + test.run( + "test/fixtures/resources/glyphs.pbf", + GlyphDependencies { + {{{"Test Stack"}}, {u'a', u'å', u' '}} + }); +}