From f63337b33636a94a0918a0bb8c9a2dcf4ab253d5 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 19 Nov 2018 15:30:19 +0200 Subject: Refactor util::fontStacks() So that it does not downcast `Layer::Impl` instances. Also, an unneeded `std::vector` construction was removed from `Parser::fontStacks()`. --- src/mbgl/style/layer_impl.cpp | 2 ++ src/mbgl/style/layer_impl.hpp | 4 +++ src/mbgl/style/layers/symbol_layer_impl.cpp | 27 ++++++++++++++ src/mbgl/style/layers/symbol_layer_impl.hpp | 1 + src/mbgl/style/parser.cpp | 5 ++- src/mbgl/style/parser.hpp | 2 +- src/mbgl/util/font_stack.cpp | 30 +--------------- test/fixtures/offline_database/offline_sideload.db | Bin 4096 -> 73728 bytes test/style/style_parser.test.cpp | 39 ++++++++++++--------- 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&) 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&) 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) 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 +#include + 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) const { + if (layout.get().isUndefined()) { + return; + } + + layout.get().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&) const override; const LayerTypeInfo* getTypeInfo() const noexcept final; + void populateFontStack(std::set& 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 Parser::fontStacks() const { +std::set Parser::fontStacks() const { std::vector> impls; impls.reserve(layers.size()); for (const auto& layer : layers) { impls.emplace_back(layer->baseImpl); } - std::set result = mbgl::fontStacks(impls); - return std::vector(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 fontStacks() const; + std::set 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 -#include #include #include @@ -23,35 +22,8 @@ FontStackHash FontStackHasher::operator()(const FontStack& fontStack) const { std::set fontStacks(const std::vector>& layers) { std::set result; - for (const auto& layer : layers) { - if (layer->type != LayerType::Symbol) { - continue; - } - - const SymbolLayer::Impl& impl = dynamic_cast(*layer); - if (impl.layout.get().isUndefined()) { - continue; - } - - impl.layout.get().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 Binary files a/test/fixtures/offline_database/offline_sideload.db and b/test/fixtures/offline_database/offline_sideload.db differ 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 #include +#include #include @@ -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 expected; + expected.insert(FontStack({"a"})); + expected.insert(FontStack({"a", "b"})); + expected.insert(FontStack({"a", "b", "c"})); + std::set 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 expected; + expected.insert(FontStack({"Arial"})); + expected.insert(FontStack({"Helvetica"})); + std::set 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 expected; + expected.insert(FontStack({"Arial"})); + expected.insert(FontStack({"Helvetica"})); + std::set 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 expected; + expected.insert(FontStack({"Arial"})); + expected.insert(FontStack({"Helvetica"})); + std::set result = parser.fontStacks(); + ASSERT_EQ(expected, result); } TEST(StyleParser, FontStacksGetExpression) { -- cgit v1.2.1