[tests] Rewrite GlyphAtlas tests to use public API

3 files changed, 79 insertions, 98 deletions

diff --git a/src/mbgl/text/glyph_atlas.cpp b/src/mbgl/text/glyph_atlas.cpp
index 2256805606..2b09bc5763 100644
--- a/src/mbgl/text/glyph_atlas.cpp
+++ b/src/mbgl/text/glyph_atlas.cpp
@@ -25,29 +25,6 @@ GlyphAtlas::GlyphAtlas(const Size size, FileSource& fileSource_)
 
 GlyphAtlas::~GlyphAtlas() = default;
 
-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)) {
-            return false;
-        }
-    }
-    return true;
-}
-
-bool GlyphAtlas::rangeIsParsed(const std::map<GlyphRange, GlyphRequest>& ranges, const GlyphRange& range) const {
-    auto it = ranges.find(range);
-    return it != ranges.end() && it->second.parsed;
-}
-
-bool GlyphAtlas::hasGlyphRange(const FontStack& fontStack, const GlyphRange& range) const {
-    auto it = entries.find(fontStack);
-    return it != entries.end() && rangeIsParsed(it->second.ranges, range);
-}
-    
 void GlyphAtlas::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphDependencies) {
     auto dependencies = std::make_shared<GlyphDependencies>(std::move(glyphDependencies));
 
@@ -66,7 +43,8 @@ void GlyphAtlas::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphDep
         }
 
         for (const auto& range : ranges) {
-            if (!rangeIsParsed(entry.ranges, range)) {
+            auto it = entry.ranges.find(range);
+            if (it == entry.ranges.end() || !it->second.parsed) {
                 GlyphRequest& request = requestRange(entry, fontStack, range);
                 request.requestors[&requestor] = dependencies;
             }
diff --git a/src/mbgl/text/glyph_atlas.hpp b/src/mbgl/text/glyph_atlas.hpp
index 2003c8e593..ef1ddc85ed 100644
--- a/src/mbgl/text/glyph_atlas.hpp
+++ b/src/mbgl/text/glyph_atlas.hpp
@@ -39,8 +39,6 @@ public:
     GlyphAtlas(Size, FileSource&);
     ~GlyphAtlas();
 
-    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
     // counts for all the glyphs in `GlyphDependencies`. If all glyphs are already
@@ -93,12 +91,6 @@ private:
 
     std::unordered_map<FontStack, Entry, FontStackHash> entries;
 
-    // Only used by GlyphAtlasTest
-    friend class ::GlyphAtlasTest;
-    bool hasGlyphRanges(const FontStack&, const GlyphRangeSet&) const;
-    bool hasGlyphRange(const FontStack&, const GlyphRange&) const;
-    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&);
 
diff --git a/test/text/glyph_atlas.test.cpp b/test/text/glyph_atlas.test.cpp
index 3ea77a16f3..e57a7944a4 100644
--- a/test/text/glyph_atlas.test.cpp
+++ b/test/text/glyph_atlas.test.cpp
@@ -10,46 +10,37 @@
 
 using namespace mbgl;
 
-class GlyphAtlasTest : public GlyphRequestor {
+class StubGlyphRequestor : public GlyphRequestor {
+public:
+    void onGlyphsAvailable(GlyphPositionMap positions, GlyphRangeSet ranges) override {
+        if (glyphsAvailable) glyphsAvailable(std::move(positions), std::move(ranges));
+    }
+
+    std::function<void (GlyphPositionMap, GlyphRangeSet)> glyphsAvailable;
+};
+
+class GlyphAtlasTest {
 public:
     util::RunLoop loop;
     StubFileSource fileSource;
     StubStyleObserver observer;
+    StubGlyphRequestor requestor;
     GlyphAtlas glyphAtlas{ { 32, 32 }, fileSource };
-    GlyphPositionMap glyphPositions;
-    GlyphRangeSet loadedRanges;
 
-    void run(const std::string& url, const FontStack& fontStack, const GlyphRangeSet& glyphRanges) {
+    void run(const std::string& url, GlyphDependencies dependencies) {
         // Squelch logging.
         Log::setObserver(std::make_unique<Log::NullObserver>());
 
-        glyphAtlas.setObserver(&observer);
         glyphAtlas.setURL(url);
-        GlyphDependencies glyphDependencies;
-        for (const auto& range : glyphRanges) {
-            glyphDependencies[fontStack].insert(range.first);
-        }
-        glyphAtlas.getGlyphs(*this, glyphDependencies);
-        glyphAtlas.hasGlyphRanges(fontStack, glyphRanges);
+        glyphAtlas.setObserver(&observer);
+        glyphAtlas.getGlyphs(requestor, std::move(dependencies));
 
         loop.run();
     }
-    void addGlyphs(GlyphRequestor& requestor, const GlyphDependencies& glyphDependencies) {
-        glyphAtlas.addGlyphs(requestor, glyphDependencies);
-    }
-    
-    bool hasGlyphRanges(const FontStack& fontStack, const GlyphRangeSet& ranges) const {
-        return glyphAtlas.hasGlyphRanges(fontStack, ranges);
-    }
 
     void end() {
         loop.stop();
     }
-    
-    virtual void onGlyphsAvailable(GlyphPositionMap positions, GlyphRangeSet _loadedRanges) {
-        glyphPositions = std::move(positions);
-        loadedRanges = std::move(_loadedRanges);
-    }
 };
 
 TEST(GlyphAtlas, LoadingSuccess) {
@@ -67,20 +58,29 @@ TEST(GlyphAtlas, LoadingSuccess) {
         test.end();
     };
 
-    test.observer.glyphsLoaded = [&] (const FontStack&, const GlyphRange&) {
-        if (!test.hasGlyphRanges({{"Test Stack"}}, {{0, 255}, {256, 511}}))
-            return;
+    test.observer.glyphsLoaded = [&] (const FontStack& fontStack, const GlyphRange& range) {
+        ASSERT_EQ(fontStack, FontStack {{"Test Stack"}});
+        ASSERT_EQ(range, GlyphRange(0, 255));
+    };
+
+    test.requestor.glyphsAvailable = [&] (GlyphPositionMap positions, GlyphRangeSet ranges) {
+        const auto& testPositions = positions.at({{"Test Stack"}});
+
+        ASSERT_EQ(testPositions.size(), 2u);
+        ASSERT_EQ(testPositions.count(u'a'), 1u);
+        ASSERT_EQ(testPositions.count(u'å'), 1u);
 
-        auto& glyphSet = test.glyphAtlas.getGlyphSet({{"Test Stack"}});
-        ASSERT_FALSE(glyphSet.empty());
+        ASSERT_EQ(ranges.size(), 1u);
+        ASSERT_EQ((ranges.count({0, 255})), 1u);
 
         test.end();
     };
 
     test.run(
         "test/fixtures/resources/glyphs.pbf",
-        {{"Test Stack"}},
-        {{0, 255}, {256, 511}});
+        GlyphDependencies {
+            {{{"Test Stack"}}, {u'a', u'å'}}
+        });
 }
 
 TEST(GlyphAtlas, LoadingFail) {
@@ -101,16 +101,19 @@ TEST(GlyphAtlas, LoadingFail) {
         EXPECT_TRUE(error != nullptr);
         EXPECT_EQ(util::toString(error), "Failed by the test case");
 
-        ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}}).empty());
-        ASSERT_FALSE(test.hasGlyphRanges({{"Test Stack"}}, {{0, 255}}));
+        test.end();
+    };
 
+    test.requestor.glyphsAvailable = [&] (GlyphPositionMap, GlyphRangeSet) {
+        FAIL();
         test.end();
     };
 
     test.run(
         "test/fixtures/resources/glyphs.pbf",
-        {{"Test Stack"}},
-        {{0, 255}});
+        GlyphDependencies {
+            {{{"Test Stack"}}, {u'a', u'å'}}
+        });
 }
 
 TEST(GlyphAtlas, LoadingCorrupted) {
@@ -129,16 +132,19 @@ TEST(GlyphAtlas, LoadingCorrupted) {
         EXPECT_TRUE(error != nullptr);
         EXPECT_EQ(util::toString(error), "unknown pbf field type exception");
 
-        ASSERT_TRUE(test.glyphAtlas.getGlyphSet({{"Test Stack"}}).empty());
-        ASSERT_FALSE(test.hasGlyphRanges({{"Test Stack"}}, {{0, 255}}));
+        test.end();
+    };
 
+    test.requestor.glyphsAvailable = [&] (GlyphPositionMap, GlyphRangeSet) {
+        FAIL();
         test.end();
     };
 
     test.run(
         "test/fixtures/resources/glyphs.pbf",
-        {{"Test Stack"}},
-        {{0, 255}});
+        GlyphDependencies {
+            {{{"Test Stack"}}, {u'a', u'å'}}
+        });
 }
 
 TEST(GlyphAtlas, LoadingCancel) {
@@ -155,42 +161,47 @@ TEST(GlyphAtlas, LoadingCancel) {
 
     test.run(
         "test/fixtures/resources/glyphs.pbf",
-        {{"Test Stack"}},
-        {{0, 255}});
+        GlyphDependencies {
+            {{{"Test Stack"}}, {u'a', u'å'}}
+        });
 }
 
-TEST(GlyphAtlas, InvalidSDFGlyph) {
-    const FontStack fontStack{ "Mock Font" };
-
+TEST(GlyphAtlas, LoadingInvalid) {
     GlyphAtlasTest test;
 
-    auto& glyphSet = test.glyphAtlas.getGlyphSet(fontStack);
-    glyphSet.emplace(66, SDFGlyph{ 66 /* ASCII 'B' */,
-                                  AlphaImage({7, 7}), /* correct */
-                                  { 1 /* width */, 1 /* height */, 0 /* left */, 0 /* top */,
-                                    0 /* advance */ } });
-    glyphSet.emplace(67, SDFGlyph{ 67 /* ASCII 'C' */,
-                                  AlphaImage({518, 8}), /* correct */
-                                  { 512 /* width */, 2 /* height */, 0 /* left */, 0 /* top */,
-                                    0 /* advance */ } });
+    test.fileSource.glyphsResponse = [&] (const Resource& resource) {
+        EXPECT_EQ(Resource::Kind::Glyphs, resource.kind);
+        Response response;
+        response.data = std::make_shared<std::string>(util::read_file("test/fixtures/resources/fake_glyphs-0-255.pbf"));
+        return response;
+    };
+
+    test.observer.glyphsError = [&] (const FontStack&, const GlyphRange&, std::exception_ptr) {
+        FAIL();
+        test.end();
+    };
 
-    GlyphDependencies glyphDependencies = {{fontStack, {'A','B','C'}}};
-    test.addGlyphs(test, glyphDependencies);
-    GlyphPositions positions = test.glyphPositions[fontStack];
+    test.observer.glyphsLoaded = [&] (const FontStack& fontStack, const GlyphRange& range) {
+        ASSERT_EQ(fontStack, FontStack {{"Test Stack"}});
+        ASSERT_EQ(range, GlyphRange(0, 255));
+    };
 
-    ASSERT_EQ(2u, positions.size());
+    test.requestor.glyphsAvailable = [&] (GlyphPositionMap positions, GlyphRangeSet ranges) {
+        const auto& testPositions = positions.at({{"Test Stack"}});
 
-    // 'A' was not placed because not in the glyph set.
-    ASSERT_EQ(positions.end(), positions.find(65));
+        ASSERT_EQ(testPositions.size(), 1u);
+        ASSERT_EQ(testPositions.count(u'A'), 0u);
+        ASSERT_EQ(testPositions.count(u'E'), 1u);
 
-    // 'B' was placed at the top left.
-    ASSERT_NE(positions.end(), positions.find(66));
-    // Width is 12 because actual dimensions are 1+6 pixels, with 1px border added, rounded up to
-    // the next multiple of 4.
-    ASSERT_EQ((Rect<uint16_t>{ 0, 0, 12, 12 }), positions[66].rect);
+        ASSERT_EQ(ranges.size(), 1u);
+        ASSERT_EQ((ranges.count({0, 255})), 1u);
 
-    // 'C' was not placed because the width is too big.
-    ASSERT_NE(positions.end(), positions.find(67));
-    ASSERT_EQ((Rect<uint16_t>{ 0, 0, 0, 0 }), positions[67].rect);
+        test.end();
+    };
 
+    test.run(
+        "test/fixtures/resources/glyphs.pbf",
+        GlyphDependencies {
+            {{{"Test Stack"}}, {u'A', u'E'}}
+        });
 }