From 08101921633ded8978c003179c5f324c91aac266 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 6 Apr 2017 07:06:51 -0700 Subject: [core] Inline GlyphSet into GlyphAtlas --- cmake/core-files.cmake | 2 - src/mbgl/text/glyph_atlas.cpp | 102 +++++++++++++++++++++++++---------------- src/mbgl/text/glyph_atlas.hpp | 8 ++-- src/mbgl/text/glyph_set.cpp | 30 ------------ src/mbgl/text/glyph_set.hpp | 18 -------- test/text/glyph_atlas.test.cpp | 11 ++--- 6 files changed, 73 insertions(+), 98 deletions(-) delete mode 100644 src/mbgl/text/glyph_set.cpp delete mode 100644 src/mbgl/text/glyph_set.hpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index d40fb76211..98c51b2d5d 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -399,8 +399,6 @@ set(MBGL_CORE_FILES src/mbgl/text/glyph_pbf.cpp src/mbgl/text/glyph_pbf.hpp src/mbgl/text/glyph_range.hpp - src/mbgl/text/glyph_set.cpp - src/mbgl/text/glyph_set.hpp src/mbgl/text/placement_config.hpp src/mbgl/text/quads.cpp src/mbgl/text/quads.hpp diff --git a/src/mbgl/text/glyph_atlas.cpp b/src/mbgl/text/glyph_atlas.cpp index 3e8bb0fe9c..2256805606 100644 --- a/src/mbgl/text/glyph_atlas.cpp +++ b/src/mbgl/text/glyph_atlas.cpp @@ -25,10 +25,10 @@ GlyphAtlas::GlyphAtlas(const Size size, FileSource& fileSource_) GlyphAtlas::~GlyphAtlas() = default; -GlyphSet& GlyphAtlas::getGlyphSet(const FontStack& fontStack) { - return entries[fontStack].glyphSet; +std::map& GlyphAtlas::getGlyphSet(const FontStack& fontStack) { + return entries[fontStack].sdfs; } - + bool GlyphAtlas::hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet& ranges) const { for (const auto& range : ranges) { if (!hasGlyphRange(fontStack,range)) { @@ -87,45 +87,70 @@ GlyphAtlas::GlyphRequest& GlyphAtlas::requestRange(Entry& entry, const FontStack return request; } - request.req = fileSource.request(Resource::glyphs(glyphURL, fontStack, range), - [this, &entry, &request, fontStack, range](Response res) { - if (res.error) { - observer->onGlyphsError(fontStack, range, std::make_exception_ptr(std::runtime_error(res.error->message))); - } else if (res.notModified) { - return; - } else { - if (!res.noContent) { - std::vector glyphs; - - try { - glyphs = parseGlyphPBF(range, *res.data); - } catch (...) { - observer->onGlyphsError(fontStack, range, std::current_exception()); - return; - } - - for (auto& glyph : glyphs) { - entry.glyphSet.insert(std::move(glyph)); - } - } + request.req = fileSource.request(Resource::glyphs(glyphURL, fontStack, range), [this, fontStack, range](Response res) { + processResponse(res, fontStack, range); + }); + + return request; +} - request.parsed = true; +void GlyphAtlas::processResponse(const Response& res, const FontStack& fontStack, const GlyphRange& range) { + if (res.error) { + observer->onGlyphsError(fontStack, range, std::make_exception_ptr(std::runtime_error(res.error->message))); + return; + } - for (auto& pair : request.requestors) { - GlyphRequestor& requestor = *pair.first; - const std::shared_ptr& dependencies = pair.second; - if (dependencies.unique()) { - addGlyphs(requestor, *dependencies); - } - } + if (res.notModified) { + return; + } + + Entry& entry = entries[fontStack]; + GlyphRequest& request = entry.ranges[range]; + + if (!res.noContent) { + std::vector glyphs; - request.requestors.clear(); + try { + glyphs = parseGlyphPBF(range, *res.data); + } catch (...) { + observer->onGlyphsError(fontStack, range, std::current_exception()); + return; + } - observer->onGlyphsLoaded(fontStack, range); + for (auto& glyph : glyphs) { + auto it = entry.sdfs.find(glyph.id); + if (it == entry.sdfs.end()) { + // Glyph doesn't exist yet. + entry.sdfs.emplace(glyph.id, std::move(glyph)); + } else if (it->second.metrics == glyph.metrics) { + if (it->second.bitmap != glyph.bitmap) { + // The actual bitmap was updated; this is unsupported. + Log::Warning(Event::Glyph, "Modified glyph changed bitmap represenation"); + } + // At least try to update it in case it's currently unused. + // If it is already used, we won't attempt to update the glyph atlas texture. + it->second.bitmap = std::move(glyph.bitmap); + } else { + // The metrics were updated; this is unsupported. + Log::Warning(Event::Glyph, "Modified glyph has different metrics"); + return; } - }); + } + } - return request; + request.parsed = true; + + for (auto& pair : request.requestors) { + GlyphRequestor& requestor = *pair.first; + const std::shared_ptr& dependencies = pair.second; + if (dependencies.unique()) { + addGlyphs(requestor, *dependencies); + } + } + + request.requestors.clear(); + + observer->onGlyphsLoaded(fontStack, range); } void GlyphAtlas::setObserver(GlyphAtlasObserver* observer_) { @@ -142,12 +167,11 @@ void GlyphAtlas::addGlyphs(GlyphRequestor& requestor, const GlyphDependencies& g GlyphPositions& positions = glyphPositions[fontStack]; Entry& entry = entries[fontStack]; - const auto& sdfs = entry.glyphSet.getSDFs(); for (const auto& glyphID : glyphIDs) { loadedRanges.insert(getGlyphRange(glyphID)); - auto it = sdfs.find(glyphID); - if (it == sdfs.end()) + auto it = entry.sdfs.find(glyphID); + if (it == entry.sdfs.end()) continue; addGlyph(requestor, fontStack, it->second); diff --git a/src/mbgl/text/glyph_atlas.hpp b/src/mbgl/text/glyph_atlas.hpp index f6491a6d81..2003c8e593 100644 --- a/src/mbgl/text/glyph_atlas.hpp +++ b/src/mbgl/text/glyph_atlas.hpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include #include @@ -23,6 +23,7 @@ namespace mbgl { class FileSource; class AsyncRequest; +class Response; namespace gl { class Context; @@ -38,7 +39,7 @@ public: GlyphAtlas(Size, FileSource&); ~GlyphAtlas(); - GlyphSet& getGlyphSet(const FontStack&); + std::map& getGlyphSet(const FontStack&); // Workers send a `getGlyphs` message to the main thread once they have determined // which glyphs they will need. Invoking this method will increment reference @@ -86,7 +87,7 @@ private: struct Entry { std::map ranges; - GlyphSet glyphSet; + std::map sdfs; std::map glyphValues; }; @@ -99,6 +100,7 @@ private: bool rangeIsParsed(const std::map&, const GlyphRange&) const; GlyphRequest& requestRange(Entry&, const FontStack&, const GlyphRange&); + void processResponse(const Response&, const FontStack&, const GlyphRange&); void addGlyphs(GlyphRequestor&, const GlyphDependencies&); void addGlyph(GlyphRequestor&, const FontStack&, const SDFGlyph&); diff --git a/src/mbgl/text/glyph_set.cpp b/src/mbgl/text/glyph_set.cpp deleted file mode 100644 index 3305d4136e..0000000000 --- a/src/mbgl/text/glyph_set.cpp +++ /dev/null @@ -1,30 +0,0 @@ -#include -#include - -namespace mbgl { - -void GlyphSet::insert(SDFGlyph&& glyph) { - auto it = sdfs.find(glyph.id); - if (it == sdfs.end()) { - // Glyph doesn't exist yet. - sdfs.emplace(glyph.id, std::move(glyph)); - } else if (it->second.metrics == glyph.metrics) { - if (it->second.bitmap != glyph.bitmap) { - // The actual bitmap was updated; this is unsupported. - Log::Warning(Event::Glyph, "Modified glyph changed bitmap represenation"); - } - // At least try to update it in case it's currently unsused. - // If it is already used; we won't attempt to update the glyph atlas texture. - it->second.bitmap = std::move(glyph.bitmap); - } else { - // The metrics were updated; this is unsupported. - Log::Warning(Event::Glyph, "Modified glyph has different metrics"); - return; - } -} - -const std::map& GlyphSet::getSDFs() const { - return sdfs; -} - -} // end namespace mbgl diff --git a/src/mbgl/text/glyph_set.hpp b/src/mbgl/text/glyph_set.hpp deleted file mode 100644 index 8b56ccd7a3..0000000000 --- a/src/mbgl/text/glyph_set.hpp +++ /dev/null @@ -1,18 +0,0 @@ -#pragma once - -#include - -#include - -namespace mbgl { - -class GlyphSet { -public: - void insert(SDFGlyph&&); - const std::map& getSDFs() const; - -private: - std::map sdfs; -}; - -} // end namespace mbgl diff --git a/test/text/glyph_atlas.test.cpp b/test/text/glyph_atlas.test.cpp index f38c18c28c..3ea77a16f3 100644 --- a/test/text/glyph_atlas.test.cpp +++ b/test/text/glyph_atlas.test.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -73,7 +72,7 @@ TEST(GlyphAtlas, LoadingSuccess) { return; auto& glyphSet = test.glyphAtlas.getGlyphSet({{"Test Stack"}}); - ASSERT_FALSE(glyphSet.getSDFs().empty()); + ASSERT_FALSE(glyphSet.empty()); test.end(); }; @@ -102,7 +101,7 @@ TEST(GlyphAtlas, LoadingFail) { EXPECT_TRUE(error != nullptr); EXPECT_EQ(util::toString(error), "Failed by the test case"); - ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}}).getSDFs().empty()); + ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}}).empty()); ASSERT_FALSE(test.hasGlyphRanges({{"Test Stack"}}, {{0, 255}})); test.end(); @@ -130,7 +129,7 @@ TEST(GlyphAtlas, LoadingCorrupted) { EXPECT_TRUE(error != nullptr); EXPECT_EQ(util::toString(error), "unknown pbf field type exception"); - ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}}).getSDFs().empty()); + ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}}).empty()); ASSERT_FALSE(test.hasGlyphRanges({{"Test Stack"}}, {{0, 255}})); test.end(); @@ -166,11 +165,11 @@ TEST(GlyphAtlas, InvalidSDFGlyph) { GlyphAtlasTest test; auto& glyphSet = test.glyphAtlas.getGlyphSet(fontStack); - glyphSet.insert(SDFGlyph{ 66 /* ASCII 'B' */, + glyphSet.emplace(66, SDFGlyph{ 66 /* ASCII 'B' */, AlphaImage({7, 7}), /* correct */ { 1 /* width */, 1 /* height */, 0 /* left */, 0 /* top */, 0 /* advance */ } }); - glyphSet.insert(SDFGlyph{ 67 /* ASCII 'C' */, + glyphSet.emplace(67, SDFGlyph{ 67 /* ASCII 'C' */, AlphaImage({518, 8}), /* correct */ { 512 /* width */, 2 /* height */, 0 /* left */, 0 /* top */, 0 /* advance */ } }); -- cgit v1.2.1