[core] Inline GlyphSet into GlyphAtlas

6 files changed, 73 insertions, 98 deletions

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<uint32_t, SDFGlyph>& 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<SDFGlyph> 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<GlyphDependencies>& 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<SDFGlyph> 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<GlyphDependencies>& 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 <mbgl/text/glyph.hpp>
 #include <mbgl/text/glyph_atlas_observer.hpp>
 #include <mbgl/text/glyph_range.hpp>
-#include <mbgl/text/glyph_set.hpp>
+#include <mbgl/text/glyph_pbf.hpp>
 #include <mbgl/geometry/binpack.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/optional.hpp>
@@ -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<uint32_t, SDFGlyph>& 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<GlyphRange, GlyphRequest> ranges;
-        GlyphSet glyphSet;
+        std::map<uint32_t, SDFGlyph> sdfs;
         std::map<uint32_t, GlyphValue> glyphValues;
     };
 
@@ -99,6 +100,7 @@ private:
     bool rangeIsParsed(const std::map<GlyphRange, GlyphRequest>&, 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 <mbgl/text/glyph_set.hpp>
-#include <mbgl/util/logging.hpp>
-
-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<uint32_t, SDFGlyph>& 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 <mbgl/text/glyph_pbf.hpp>
-
-#include <map>
-
-namespace mbgl {
-
-class GlyphSet {
-public:
-    void insert(SDFGlyph&&);
-    const std::map<uint32_t, SDFGlyph>& getSDFs() const;
-
-private:
-    std::map<uint32_t, SDFGlyph> 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 <mbgl/test/stub_file_source.hpp>
 #include <mbgl/test/stub_style_observer.hpp>
 
-#include <mbgl/text/glyph_set.hpp>
 #include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/util/run_loop.hpp>
 #include <mbgl/util/string.hpp>
@@ -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 */ } });