[core] Use the proper type for font stacks

16 files changed, 89 insertions, 62 deletions

diff --git a/src/mbgl/geometry/glyph_atlas.cpp b/src/mbgl/geometry/glyph_atlas.cpp
index 92fc7903b2..fd43fb8c18 100644
--- a/src/mbgl/geometry/glyph_atlas.cpp
+++ b/src/mbgl/geometry/glyph_atlas.cpp
@@ -24,7 +24,7 @@ GlyphAtlas::~GlyphAtlas() {
 
 void GlyphAtlas::addGlyphs(uintptr_t tileUID,
                            const std::u32string& text,
-                           const std::string& stackName,
+                           const FontStack& fontStack,
                            const GlyphSet& glyphSet,
                            GlyphPositions& face)
 {
@@ -40,19 +40,19 @@ void GlyphAtlas::addGlyphs(uintptr_t tileUID,
         }
 
         const SDFGlyph& sdf = sdf_it->second;
-        Rect<uint16_t> rect = addGlyph(tileUID, stackName, sdf);
+        Rect<uint16_t> rect = addGlyph(tileUID, fontStack, sdf);
         face.emplace(chr, Glyph{rect, sdf.metrics});
     }
 }
 
 Rect<uint16_t> GlyphAtlas::addGlyph(uintptr_t tileUID,
-                                    const std::string& stackName,
+                                    const FontStack& fontStack,
                                     const SDFGlyph& glyph)
 {
     // Use constant value for now.
     const uint8_t buffer = 3;
 
-    std::map<uint32_t, GlyphValue>& face = index[stackName];
+    std::map<uint32_t, GlyphValue>& face = index[fontStack];
     std::map<uint32_t, GlyphValue>::iterator it = face.find(glyph.id);
 
     // The glyph is already in this texture.
diff --git a/src/mbgl/geometry/glyph_atlas.hpp b/src/mbgl/geometry/glyph_atlas.hpp
index 9b8af9f75d..df1446cd47 100644
--- a/src/mbgl/geometry/glyph_atlas.hpp
+++ b/src/mbgl/geometry/glyph_atlas.hpp
@@ -9,7 +9,7 @@
 
 #include <string>
 #include <set>
-#include <map>
+#include <unordered_map>
 #include <mutex>
 #include <atomic>
 
@@ -22,7 +22,7 @@ public:
 
     void addGlyphs(uintptr_t tileUID,
                    const std::u32string& text,
-                   const std::string& stackName,
+                   const FontStack&,
                    const GlyphSet&,
                    GlyphPositions&);
     void removeGlyphs(uintptr_t tileUID);
@@ -46,12 +46,12 @@ private:
     };
 
     Rect<uint16_t> addGlyph(uintptr_t tileID,
-                            const std::string& stackName,
+                            const FontStack&,
                             const SDFGlyph&);
 
     std::mutex mtx;
     BinPack<uint16_t> bin;
-    std::map<std::string, std::map<uint32_t, GlyphValue>> index;
+    std::unordered_map<FontStack, std::map<uint32_t, GlyphValue>, FontStackHash> index;
     const std::unique_ptr<uint8_t[]> data;
     std::atomic<bool> dirty;
     gl::TextureHolder texture;
diff --git a/src/mbgl/layer/symbol_layer.hpp b/src/mbgl/layer/symbol_layer.hpp
index 440631bcb4..f8e42ab821 100644
--- a/src/mbgl/layer/symbol_layer.hpp
+++ b/src/mbgl/layer/symbol_layer.hpp
@@ -28,7 +28,7 @@ public:
 
     LayoutProperty<RotationAlignmentType> textRotationAlignment { RotationAlignmentType::Viewport };
     LayoutProperty<std::string> textField { "" };
-    LayoutProperty<std::string> textFont { "Open Sans Regular, Arial Unicode MS Regular" };
+    LayoutProperty<std::vector<std::string>> textFont { { "Open Sans Regular", "Arial Unicode MS Regular" } };
     LayoutProperty<float> textSize { 16.0f };
     LayoutProperty<float> textMaxWidth { 10.0f /* em */ };
     LayoutProperty<float> textLineHeight { 1.2f /* em */ };
diff --git a/src/mbgl/storage/resource.cpp b/src/mbgl/storage/resource.cpp
index d5b88d9292..d633ae195e 100644
--- a/src/mbgl/storage/resource.cpp
+++ b/src/mbgl/storage/resource.cpp
@@ -33,12 +33,12 @@ Resource Resource::spriteJSON(const std::string& base, float pixelRatio) {
     };
 }
 
-Resource Resource::glyphs(const std::string& urlTemplate, const std::string& fontStack, const std::pair<uint16_t, uint16_t>& glyphRange) {
+Resource Resource::glyphs(const std::string& urlTemplate, const FontStack& fontStack, const std::pair<uint16_t, uint16_t>& glyphRange) {
     return Resource {
         Resource::Kind::Glyphs,
         util::replaceTokens(urlTemplate, [&](const std::string& token) {
             if (token == "fontstack") {
-                return util::percentEncode(fontStack);
+                return util::percentEncode(fontStackToString(fontStack));
             } else if (token == "range") {
                 return util::toString(glyphRange.first) + "-" + util::toString(glyphRange.second);
             } else {
diff --git a/src/mbgl/style/function.cpp b/src/mbgl/style/function.cpp
index 55643ed43f..bc3e6074fb 100644
--- a/src/mbgl/style/function.cpp
+++ b/src/mbgl/style/function.cpp
@@ -14,9 +14,10 @@ template <> inline bool defaultStopsValue() { return true; }
 template <> inline float defaultStopsValue() { return 1.0f; }
 template <> inline Color defaultStopsValue() { return {{ 0, 0, 0, 1 }}; }
 template <> inline std::vector<float> defaultStopsValue() { return {{ 1, 0 }}; }
+template <> inline std::vector<std::string> defaultStopsValue() { return {{}}; }
 template <> inline std::array<float, 2> defaultStopsValue() { return {{ 0, 0 }}; }
 
-template <> inline std:: string defaultStopsValue() { return {}; }
+template <> inline std::string defaultStopsValue() { return {}; }
 template <> inline TranslateAnchorType defaultStopsValue() { return {}; };
 template <> inline RotateAnchorType defaultStopsValue() { return {}; };
 template <> inline LineCapType defaultStopsValue() { return {}; };
@@ -79,6 +80,7 @@ template class Function<bool>;
 template class Function<float>;
 template class Function<Color>;
 template class Function<std::vector<float>>;
+template class Function<std::vector<std::string>>;
 template class Function<std::array<float, 2>>;
 
 template class Function<std::string>;
diff --git a/src/mbgl/style/property_parsing.cpp b/src/mbgl/style/property_parsing.cpp
index 879f61411a..6f052222e4 100644
--- a/src/mbgl/style/property_parsing.cpp
+++ b/src/mbgl/style/property_parsing.cpp
@@ -32,28 +32,6 @@ optional<float> parseProperty(const char* name, const JSValue& value) {
 
 template <>
 optional<std::string> parseProperty(const char* name, const JSValue& value) {
-    if (std::string { "text-font" } == name) {
-        if (!value.IsArray()) {
-            Log::Warning(Event::ParseStyle, "value of '%s' must be an array of strings", name);
-            return {};
-        }
-
-        std::string result = "";
-        for (rapidjson::SizeType i = 0; i < value.Size(); ++i) {
-            const JSValue& stop = value[i];
-            if (stop.IsString()) {
-                result += stop.GetString();
-                if (i < value.Size()-1) {
-                    result += ",";
-                }
-            } else {
-                Log::Warning(Event::ParseStyle, "text-font members must be strings");
-                return {};
-            }
-        }
-        return result;
-    }
-
     if (!value.IsString()) {
         Log::Warning(Event::ParseStyle, "value of '%s' must be a string", name);
         return {};
@@ -209,6 +187,29 @@ optional<std::vector<float>> parseProperty(const char* name, const JSValue& valu
 }
 
 template <>
+optional<std::vector<std::string>> parseProperty(const char* name, const JSValue& value) {
+    if (!value.IsArray()) {
+        Log::Warning(Event::ParseStyle, "value of '%s' must be an array of strings", name);
+        return {};
+    }
+
+    std::vector<std::string> result;
+
+    for (rapidjson::SizeType i = 0; i < value.Size(); ++i) {
+        const JSValue& part = value[i];
+
+        if (!part.IsString()) {
+            Log::Warning(Event::ParseStyle, "value of '%s' must be an array of strings", name);
+            return {};
+        }
+
+        result.push_back({ part.GetString(), part.GetStringLength() });
+    }
+
+    return result;
+}
+
+template <>
 optional<PropertyTransition> parseProperty(const char *, const JSValue& value) {
     PropertyTransition transition;
     if (value.IsObject()) {
@@ -362,6 +363,10 @@ template<> optional<Function<Color>> parseProperty(const char* name, const JSVal
     return parseFunction<Color>(name, value);
 }
 
+template<> optional<Function<std::vector<std::string>>> parseProperty(const char* name, const JSValue& value) {
+    return parseFunction<std::vector<std::string>>(name, value);
+}
+
 template <typename T>
 optional<Function<Faded<T>>> parseFadedFunction(const JSValue& value) {
     if (!value.HasMember("stops")) {
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index a480ef8d97..338f6113b0 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -330,16 +330,16 @@ void Style::setObserver(Observer* observer_) {
     observer = observer_;
 }
 
-void Style::onGlyphsLoaded(const std::string& fontStack, const GlyphRange& glyphRange) {
+void Style::onGlyphsLoaded(const FontStack& fontStack, const GlyphRange& glyphRange) {
     shouldReparsePartialTiles = true;
     observer->onGlyphsLoaded(fontStack, glyphRange);
     observer->onResourceLoaded();
 }
 
-void Style::onGlyphsError(const std::string& fontStack, const GlyphRange& glyphRange, std::exception_ptr error) {
+void Style::onGlyphsError(const FontStack& fontStack, const GlyphRange& glyphRange, std::exception_ptr error) {
     lastError = error;
     Log::Error(Event::Style, "Failed to load glyph range %d-%d for font stack %s: %s",
-               glyphRange.first, glyphRange.second, fontStack.c_str(), util::toString(error).c_str());
+               glyphRange.first, glyphRange.second, fontStackToString(fontStack).c_str(), util::toString(error).c_str());
     observer->onGlyphsError(fontStack, glyphRange, error);
     observer->onResourceError(error);
 }
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index b3c06d8214..790518d08e 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -128,8 +128,8 @@ private:
     std::vector<std::unique_ptr<StyleLayer>>::const_iterator findLayer(const std::string& layerID) const;
 
     // GlyphStore::Observer implementation.
-    void onGlyphsLoaded(const std::string& fontStack, const GlyphRange&) override;
-    void onGlyphsError(const std::string& fontStack, const GlyphRange&, std::exception_ptr) override;
+    void onGlyphsLoaded(const FontStack&, const GlyphRange&) override;
+    void onGlyphsError(const FontStack&, const GlyphRange&, std::exception_ptr) override;
 
     // SpriteStore::Observer implementation.
     void onSpriteLoaded() override;
diff --git a/src/mbgl/style/style_parser.cpp b/src/mbgl/style/style_parser.cpp
index e312dee3b3..ff0a3b13af 100644
--- a/src/mbgl/style/style_parser.cpp
+++ b/src/mbgl/style/style_parser.cpp
@@ -492,12 +492,12 @@ void StyleParser::parseVisibility(StyleLayer& layer, const JSValue& value) {
     layer.visibility = VisibilityTypeClass({ value["visibility"].GetString(), value["visibility"].GetStringLength() });
 }
 
-std::vector<std::string> StyleParser::fontStacks() const {
-    std::set<std::string> result;
+std::vector<FontStack> StyleParser::fontStacks() const {
+    std::set<FontStack> result;
 
     for (const auto& layer : layers) {
         if (layer->is<SymbolLayer>()) {
-            LayoutProperty<std::string> property = layer->as<SymbolLayer>()->layout.textFont;
+            LayoutProperty<FontStack> property = layer->as<SymbolLayer>()->layout.textFont;
             if (property.parsedValue) {
                 for (const auto& stop : property.parsedValue->getStops()) {
                     result.insert(stop.second);
@@ -508,7 +508,7 @@ std::vector<std::string> StyleParser::fontStacks() const {
         }
     }
 
-    return std::vector<std::string>(result.begin(), result.end());
+    return std::vector<FontStack>(result.begin(), result.end());
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/style/style_parser.hpp b/src/mbgl/style/style_parser.hpp
index 280cda530b..4cc4e2f2cb 100644
--- a/src/mbgl/style/style_parser.hpp
+++ b/src/mbgl/style/style_parser.hpp
@@ -1,6 +1,7 @@
 #ifndef MBGL_STYLE_STYLE_PARSER
 #define MBGL_STYLE_STYLE_PARSER
 
+#include <mbgl/style/types.hpp>
 #include <mbgl/style/style_layer.hpp>
 #include <mbgl/source/source.hpp>
 #include <mbgl/util/rapidjson.hpp>
@@ -29,7 +30,7 @@ public:
     std::vector<std::unique_ptr<StyleLayer>> layers;
 
     // Statically evaluate layer properties to determine what font stacks are used.
-    std::vector<std::string> fontStacks() const;
+    std::vector<FontStack> fontStacks() const;
 
     static std::unique_ptr<SourceInfo> parseTileJSON(const std::string& json, const std::string& sourceURL, SourceType, uint16_t tileSize);
     static std::unique_ptr<SourceInfo> parseTileJSON(const JSValue&);
diff --git a/src/mbgl/style/types.cpp b/src/mbgl/style/types.cpp
new file mode 100644
index 0000000000..27574afa93
--- /dev/null
+++ b/src/mbgl/style/types.cpp
@@ -0,0 +1,16 @@
+#include <mbgl/style/types.hpp>
+
+#include <boost/functional/hash.hpp>
+#include <boost/algorithm/string/join.hpp>
+
+namespace mbgl {
+
+std::string fontStackToString(const FontStack& fontStack) {
+    return boost::algorithm::join(fontStack, ",");
+}
+
+std::size_t FontStackHash::operator()(const FontStack& fontStack) const {
+    return boost::hash_range(fontStack.begin(), fontStack.end());
+}
+
+} // namespace mbgl
diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp
index 105ea45190..2588afaefa 100644
--- a/src/mbgl/text/glyph_pbf.cpp
+++ b/src/mbgl/text/glyph_pbf.cpp
@@ -61,7 +61,7 @@ void parseGlyphPBF(mbgl::GlyphSet& glyphSet, const std::string& data) {
 namespace mbgl {
 
 GlyphPBF::GlyphPBF(GlyphStore* store,
-                   const std::string& fontStack,
+                   const FontStack& fontStack,
                    const GlyphRange& glyphRange,
                    GlyphStore::Observer* observer_,
                    FileSource& fileSource)
diff --git a/src/mbgl/text/glyph_pbf.hpp b/src/mbgl/text/glyph_pbf.hpp
index 5b47563f7d..d281d2a04f 100644
--- a/src/mbgl/text/glyph_pbf.hpp
+++ b/src/mbgl/text/glyph_pbf.hpp
@@ -3,6 +3,7 @@
 
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/text/glyph_store.hpp>
+#include <mbgl/style/types.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <atomic>
@@ -12,14 +13,13 @@
 
 namespace mbgl {
 
-class GlyphSet;
 class AsyncRequest;
 class FileSource;
 
 class GlyphPBF : private util::noncopyable {
 public:
-    GlyphPBF(GlyphStore* store,
-             const std::string& fontStack,
+    GlyphPBF(GlyphStore*,
+             const FontStack&,
              const GlyphRange&,
              GlyphStore::Observer*,
              FileSource&);
diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index 9f6a6b7b72..540908ed63 100644
--- a/src/mbgl/text/glyph_store.cpp
+++ b/src/mbgl/text/glyph_store.cpp
@@ -11,9 +11,9 @@ GlyphStore::GlyphStore(FileSource& fileSource_)
 
 GlyphStore::~GlyphStore() = default;
 
-void GlyphStore::requestGlyphRange(const std::string& fontStackName, const GlyphRange& range) {
+void GlyphStore::requestGlyphRange(const FontStack& fontStack, const GlyphRange& range) {
     std::lock_guard<std::mutex> lock(rangesMutex);
-    auto& rangeSets = ranges[fontStackName];
+    auto& rangeSets = ranges[fontStack];
 
     const auto& rangeSetsIt = rangeSets.find(range);
     if (rangeSetsIt != rangeSets.end()) {
@@ -21,17 +21,17 @@ void GlyphStore::requestGlyphRange(const std::string& fontStackName, const Glyph
     }
 
     rangeSets.emplace(range,
-        std::make_unique<GlyphPBF>(this, fontStackName, range, observer, fileSource));
+        std::make_unique<GlyphPBF>(this, fontStack, range, observer, fileSource));
 }
 
 
-bool GlyphStore::hasGlyphRanges(const std::string& fontStackName, const std::set<GlyphRange>& glyphRanges) {
+bool GlyphStore::hasGlyphRanges(const FontStack& fontStack, const std::set<GlyphRange>& glyphRanges) {
     if (glyphRanges.empty()) {
         return true;
     }
 
     std::lock_guard<std::mutex> lock(rangesMutex);
-    const auto& rangeSets = ranges[fontStackName];
+    const auto& rangeSets = ranges[fontStack];
 
     bool hasRanges = true;
     for (const auto& range : glyphRanges) {
@@ -39,7 +39,7 @@ bool GlyphStore::hasGlyphRanges(const std::string& fontStackName, const std::set
         if (rangeSetsIt == rangeSets.end()) {
             // Push the request to the MapThread, so we can easly cancel
             // if it is still pending when we destroy this object.
-            workQueue.push(std::bind(&GlyphStore::requestGlyphRange, this, fontStackName, range));
+            workQueue.push(std::bind(&GlyphStore::requestGlyphRange, this, fontStack, range));
 
             hasRanges = false;
             continue;
@@ -53,7 +53,7 @@ bool GlyphStore::hasGlyphRanges(const std::string& fontStackName, const std::set
     return hasRanges;
 }
 
-util::exclusive<GlyphSet> GlyphStore::getGlyphSet(const std::string& fontStack) {
+util::exclusive<GlyphSet> GlyphStore::getGlyphSet(const FontStack& fontStack) {
     auto lock = std::make_unique<std::lock_guard<std::mutex>>(glyphSetsMutex);
 
     auto it = glyphSets.find(fontStack);
diff --git a/src/mbgl/text/glyph_store.hpp b/src/mbgl/text/glyph_store.hpp
index 5b80589f4b..f89ff80864 100644
--- a/src/mbgl/text/glyph_store.hpp
+++ b/src/mbgl/text/glyph_store.hpp
@@ -3,11 +3,13 @@
 
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/text/glyph_set.hpp>
+#include <mbgl/style/types.hpp>
 #include <mbgl/util/exclusive.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/work_queue.hpp>
 
 #include <exception>
+#include <vector>
 #include <set>
 #include <string>
 #include <unordered_map>
@@ -26,21 +28,21 @@ public:
     public:
         virtual ~Observer() = default;
 
-        virtual void onGlyphsLoaded(const std::string& /* fontStack */, const GlyphRange&) {};
-        virtual void onGlyphsError(const std::string& /* fontStack */, const GlyphRange&, std::exception_ptr) {};
+        virtual void onGlyphsLoaded(const FontStack&, const GlyphRange&) {};
+        virtual void onGlyphsError(const FontStack&, const GlyphRange&, std::exception_ptr) {};
     };
 
     GlyphStore(FileSource&);
     ~GlyphStore();
 
-    util::exclusive<GlyphSet> getGlyphSet(const std::string& fontStack);
+    util::exclusive<GlyphSet> getGlyphSet(const FontStack&);
 
     // Returns true if the set of GlyphRanges are available and parsed or false
     // if they are not. For the missing ranges, a request on the FileSource is
     // made and when the glyph if finally parsed, it gets added to the respective
     // GlyphSet and a signal is emitted to notify the observers. This method
     // can be called from any thread.
-    bool hasGlyphRanges(const std::string& fontStack, const std::set<GlyphRange>& glyphRanges);
+    bool hasGlyphRanges(const FontStack&, const std::set<GlyphRange>&);
 
     void setURL(const std::string &url) {
         glyphURL = url;
@@ -53,15 +55,15 @@ public:
     void setObserver(Observer* observer);
 
 private:
-    void requestGlyphRange(const std::string& fontStackName, const GlyphRange& range);
+    void requestGlyphRange(const FontStack&, const GlyphRange&);
 
     FileSource& fileSource;
     std::string glyphURL;
 
-    std::unordered_map<std::string, std::map<GlyphRange, std::unique_ptr<GlyphPBF>>> ranges;
+    std::unordered_map<FontStack, std::map<GlyphRange, std::unique_ptr<GlyphPBF>>, FontStackHash> ranges;
     std::mutex rangesMutex;
 
-    std::unordered_map<std::string, std::unique_ptr<GlyphSet>> glyphSets;
+    std::unordered_map<FontStack, std::unique_ptr<GlyphSet>, FontStackHash> glyphSets;
     std::mutex glyphSetsMutex;
 
     util::WorkQueue workQueue;
diff --git a/src/mbgl/util/interpolate.hpp b/src/mbgl/util/interpolate.hpp
index 7d1a857e8e..f456a0ecb1 100644
--- a/src/mbgl/util/interpolate.hpp
+++ b/src/mbgl/util/interpolate.hpp
@@ -35,6 +35,7 @@ inline std::array<T, 2> interpolate(const std::array<T, 2>& a, const std::array<
 // fake interpolations that just return the first value
 template<> inline bool interpolate(const bool a, const bool, const double) { return a; }
 template<> inline std::vector<float> interpolate(const std::vector<float> a, const std::vector<float>, const double) { return a; }
+template<> inline std::vector<std::string> interpolate(const std::vector<std::string> a, const std::vector<std::string>, const double) { return a; }
 template<> inline std::string interpolate(const std::string a, const std::string, const double) { return a; }
 template<> inline TranslateAnchorType interpolate(const TranslateAnchorType a, const TranslateAnchorType, const double) { return a; }
 template<> inline RotateAnchorType interpolate(const RotateAnchorType a, const RotateAnchorType, const double) { return a; }