Refactor util::fontStacks()

So that it does not downcast `Layer::Impl` instances.
Also, an unneeded `std::vector` construction was removed from
`Parser::fontStacks()`.

9 files changed, 60 insertions, 50 deletions

diff --git a/src/mbgl/style/layer_impl.cpp b/src/mbgl/style/layer_impl.cpp
index a9a3941f3e..c6a248de90 100644
--- a/src/mbgl/style/layer_impl.cpp
+++ b/src/mbgl/style/layer_impl.cpp
@@ -9,5 +9,7 @@ Layer::Impl::Impl(LayerType type_, std::string layerID, std::string sourceID)
       source(std::move(sourceID)) {
 }
 
+void Layer::Impl::populateFontStack(std::set<FontStack>&) const {}
+
 } // namespace style
 } // namespace mbgl
diff --git a/src/mbgl/style/layer_impl.hpp b/src/mbgl/style/layer_impl.hpp
index 014fb60348..948bbab619 100644
--- a/src/mbgl/style/layer_impl.hpp
+++ b/src/mbgl/style/layer_impl.hpp
@@ -41,8 +41,12 @@ public:
     // Utility function for automatic layer grouping.
     virtual void stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>&) const = 0;
 
+    // Returns pointer to the statically allocated layer type info structure.
     virtual const LayerTypeInfo* getTypeInfo() const noexcept = 0;
 
+    // Populates the given \a fontStack with fonts being used by the layer.
+    virtual void populateFontStack(std::set<FontStack>& fontStack) const;
+
     // Note: LayerType is deprecated, do not use it.
     const LayerType type;
     std::string id;
diff --git a/src/mbgl/style/layers/symbol_layer_impl.cpp b/src/mbgl/style/layers/symbol_layer_impl.cpp
index 753b2fa184..e177391686 100644
--- a/src/mbgl/style/layers/symbol_layer_impl.cpp
+++ b/src/mbgl/style/layers/symbol_layer_impl.cpp
@@ -1,5 +1,7 @@
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
 
+#include <mbgl/util/logging.hpp>
+
 namespace mbgl {
 namespace style {
 
@@ -12,5 +14,30 @@ bool SymbolLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const {
            paint.hasDataDrivenPropertyDifference(impl.paint);
 }
 
+void SymbolLayer::Impl::populateFontStack(std::set<FontStack>& fontStack) const {
+    if (layout.get<TextField>().isUndefined()) {
+        return;
+    }
+
+    layout.get<TextFont>().match(
+        [&] (Undefined) {
+            fontStack.insert({"Open Sans Regular", "Arial Unicode MS Regular"});
+        },
+        [&] (const FontStack& constant) {
+            fontStack.insert(constant);
+        },
+        [&] (const auto& function) {
+            for (const auto& value : function.possibleOutputs()) {
+                if (value) {
+                    fontStack.insert(*value);
+                } else {
+                    Log::Warning(Event::ParseStyle, "Layer '%s' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression.", id.c_str());
+                    break;
+                }
+            }
+        }
+    );
+}
+
 } // namespace style
 } // namespace mbgl
diff --git a/src/mbgl/style/layers/symbol_layer_impl.hpp b/src/mbgl/style/layers/symbol_layer_impl.hpp
index 21e47ad89a..01ff9772b8 100644
--- a/src/mbgl/style/layers/symbol_layer_impl.hpp
+++ b/src/mbgl/style/layers/symbol_layer_impl.hpp
@@ -14,6 +14,7 @@ public:
     bool hasLayoutDifference(const Layer::Impl&) const override;
     void stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>&) const override;
     const LayerTypeInfo* getTypeInfo() const noexcept final;
+    void populateFontStack(std::set<FontStack>& fontStack) const final;
 
     SymbolLayoutProperties::Unevaluated layout;
     SymbolPaintProperties::Transitionable paint;
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index 77cf23bad0..ae298bd915 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -267,14 +267,13 @@ void Parser::parseLayer(const std::string& id, const JSValue& value, std::unique
     }
 }
 
-std::vector<FontStack> Parser::fontStacks() const {
+std::set<FontStack> Parser::fontStacks() const {
     std::vector<Immutable<Layer::Impl>> impls;
     impls.reserve(layers.size());
     for (const auto& layer : layers) {
         impls.emplace_back(layer->baseImpl);
     }
-    std::set<FontStack> result = mbgl::fontStacks(impls);
-    return std::vector<FontStack>(result.begin(), result.end());
+    return mbgl::fontStacks(impls);
 }
 
 } // namespace style
diff --git a/src/mbgl/style/parser.hpp b/src/mbgl/style/parser.hpp
index ec7be038fb..f992519083 100644
--- a/src/mbgl/style/parser.hpp
+++ b/src/mbgl/style/parser.hpp
@@ -43,7 +43,7 @@ public:
     double pitch = 0;
 
     // Statically evaluate layer properties to determine what font stacks are used.
-    std::vector<FontStack> fontStacks() const;
+    std::set<FontStack> fontStacks() const;
 
 private:
     void parseTransition(const JSValue&);
diff --git a/src/mbgl/util/font_stack.cpp b/src/mbgl/util/font_stack.cpp
index 4093a21793..362fdf1845 100644
--- a/src/mbgl/util/font_stack.cpp
+++ b/src/mbgl/util/font_stack.cpp
@@ -1,5 +1,4 @@
 #include <mbgl/util/font_stack.hpp>
-#include <mbgl/util/logging.hpp>
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
 #include <mbgl/util/hash.hpp>
 
@@ -23,35 +22,8 @@ FontStackHash FontStackHasher::operator()(const FontStack& fontStack) const {
 
 std::set<FontStack> fontStacks(const std::vector<Immutable<style::Layer::Impl>>& layers) {
     std::set<FontStack> result;
-
     for (const auto& layer : layers) {
-        if (layer->type != LayerType::Symbol) {
-            continue;
-        }
-
-        const SymbolLayer::Impl& impl = dynamic_cast<const SymbolLayer::Impl&>(*layer);
-        if (impl.layout.get<TextField>().isUndefined()) {
-            continue;
-        }
-
-        impl.layout.get<TextFont>().match(
-            [&] (Undefined) {
-                result.insert({"Open Sans Regular", "Arial Unicode MS Regular"});
-            },
-            [&] (const FontStack& constant) {
-                result.insert(constant);
-            },
-            [&] (const auto& function) {
-                for (const auto& value : function.possibleOutputs()) {
-                    if (value) {
-                        result.insert(*value);
-                    } else {
-                        Log::Warning(Event::ParseStyle, "Layer '%s' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression.", impl.id.c_str());
-                        break;
-                    }
-                }
-            }
-        );
+        layer->populateFontStack(result);
     }
 
     return result;
diff --git a/test/fixtures/offline_database/offline_sideload.db b/test/fixtures/offline_database/offline_sideload.db
index 8909c402b2..6146e30872 100644
--- a/test/fixtures/offline_database/offline_sideload.db
+++ b/test/fixtures/offline_database/offline_sideload.db
diff --git a/test/style/style_parser.test.cpp b/test/style/style_parser.test.cpp
index 43b982c3b9..a18a9bbf57 100644
--- a/test/style/style_parser.test.cpp
+++ b/test/style/style_parser.test.cpp
@@ -11,6 +11,7 @@
 
 #include <iostream>
 #include <fstream>
+#include <set>
 
 #include <dirent.h>
 
@@ -96,11 +97,12 @@ INSTANTIATE_TEST_CASE_P(StyleParser, StyleParserTest, ::testing::ValuesIn([] {
 TEST(StyleParser, FontStacks) {
     style::Parser parser;
     parser.parse(util::read_file("test/fixtures/style_parser/font_stacks.json"));
-    auto result = parser.fontStacks();
-    ASSERT_EQ(3u, result.size());
-    ASSERT_EQ(FontStack({"a"}), result[0]);
-    ASSERT_EQ(FontStack({"a", "b"}), result[1]);
-    ASSERT_EQ(FontStack({"a", "b", "c"}), result[2]);
+    std::set<mbgl::FontStack> expected;
+    expected.insert(FontStack({"a"}));
+    expected.insert(FontStack({"a", "b"}));
+    expected.insert(FontStack({"a", "b", "c"}));
+    std::set<mbgl::FontStack> result = parser.fontStacks();
+    ASSERT_EQ(expected, result);
 }
 
 TEST(StyleParser, FontStacksNoTextField) {
@@ -134,10 +136,11 @@ TEST(StyleParser, FontStacksCaseExpression) {
             }
         }]
     })");
-    auto result = parser.fontStacks();
-    ASSERT_EQ(2u, result.size());
-    ASSERT_EQ(FontStack({"Arial"}), result[0]);
-    ASSERT_EQ(FontStack({"Helvetica"}), result[1]);
+    std::set<mbgl::FontStack> expected;
+    expected.insert(FontStack({"Arial"}));
+    expected.insert(FontStack({"Helvetica"}));
+    std::set<mbgl::FontStack> result = parser.fontStacks();
+    ASSERT_EQ(expected, result);
 }
 
 TEST(StyleParser, FontStacksMatchExpression) {
@@ -154,10 +157,11 @@ TEST(StyleParser, FontStacksMatchExpression) {
             }
         }]
     })");
-    auto result = parser.fontStacks();
-    ASSERT_EQ(2u, result.size());
-    ASSERT_EQ(FontStack({"Arial"}), result[0]);
-    ASSERT_EQ(FontStack({"Helvetica"}), result[1]);
+    std::set<mbgl::FontStack> expected;
+    expected.insert(FontStack({"Arial"}));
+    expected.insert(FontStack({"Helvetica"}));
+    std::set<mbgl::FontStack> result = parser.fontStacks();
+    ASSERT_EQ(expected, result);
 }
 
 TEST(StyleParser, FontStacksStepExpression) {
@@ -174,10 +178,11 @@ TEST(StyleParser, FontStacksStepExpression) {
             }
         }]
     })");
-    auto result = parser.fontStacks();
-    ASSERT_EQ(2u, result.size());
-    ASSERT_EQ(FontStack({"Arial"}), result[0]);
-    ASSERT_EQ(FontStack({"Helvetica"}), result[1]);
+    std::set<mbgl::FontStack> expected;
+    expected.insert(FontStack({"Arial"}));
+    expected.insert(FontStack({"Helvetica"}));
+    std::set<mbgl::FontStack> result = parser.fontStacks();
+    ASSERT_EQ(expected, result);
 }
 
 TEST(StyleParser, FontStacksGetExpression) {