[core] Evict unused font stacks from GlyphManagerupstream/fix-12388

7 files changed, 73 insertions, 32 deletions

diff --git a/include/mbgl/util/font_stack.hpp b/include/mbgl/util/font_stack.hpp
index d0b431e9ea..ace60a4ba6 100644
--- a/include/mbgl/util/font_stack.hpp
+++ b/include/mbgl/util/font_stack.hpp
@@ -1,7 +1,11 @@
 #pragma once
 
+#include <mbgl/util/immutable.hpp>
+#include <mbgl/style/layer.hpp>
+
 #include <string>
 #include <vector>
+#include <set>
 
 namespace mbgl {
 
@@ -14,4 +18,7 @@ struct FontStackHash {
     std::size_t operator()(const FontStack&) const;
 };
 
+// Statically evaluate layer properties to determine what font stacks are used.
+std::set<FontStack> fontStacks(const std::vector<Immutable<style::Layer::Impl>>&);
+
 } // namespace mbgl
diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp
index fea27403c9..d3f72b89b9 100644
--- a/src/mbgl/renderer/renderer_impl.cpp
+++ b/src/mbgl/renderer/renderer_impl.cpp
@@ -173,6 +173,10 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) {
         renderLayers.at(entry.first)->setImpl(entry.second.after);
     }
 
+    if (!layerDiff.removed.empty() || !layerDiff.added.empty() || !layerDiff.changed.empty()) {
+        glyphManager->evict(fontStacks(*updateParameters.layers));
+    }
+
     // Update layers for class and zoom changes.
     for (const auto& entry : renderLayers) {
         RenderLayer& layer = *entry.second;
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index 8d14d7972c..0a90919f0b 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -1,6 +1,5 @@
 #include <mbgl/style/parser.hpp>
 #include <mbgl/style/layer_impl.hpp>
-#include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/rapidjson_conversion.hpp>
 #include <mbgl/style/conversion.hpp>
 #include <mbgl/style/conversion/coordinate.hpp>
@@ -274,31 +273,12 @@ void Parser::parseLayer(const std::string& id, const JSValue& value, std::unique
 }
 
 std::vector<FontStack> Parser::fontStacks() const {
-    std::set<FontStack> result;
-
+    std::vector<Immutable<Layer::Impl>> impls;
+    impls.reserve(layers.size());
     for (const auto& layer : layers) {
-        if (layer->is<SymbolLayer>() && !layer->as<SymbolLayer>()->getTextField().isUndefined()) {
-            layer->as<SymbolLayer>()->getTextFont().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 work offline. Output values must be contained as literals within the expression.", layer->getID().c_str());
-                            break;
-                        }
-                    }
-                }
-            );
-        }
+        impls.emplace_back(layer->baseImpl);
     }
-
+    std::set<FontStack> result = mbgl::fontStacks(impls);
     return std::vector<FontStack>(result.begin(), result.end());
 }
 
diff --git a/src/mbgl/text/glyph_manager.cpp b/src/mbgl/text/glyph_manager.cpp
index 3130418908..8e7cfe5ba7 100644
--- a/src/mbgl/text/glyph_manager.cpp
+++ b/src/mbgl/text/glyph_manager.cpp
@@ -5,6 +5,7 @@
 #include <mbgl/storage/resource.hpp>
 #include <mbgl/storage/response.hpp>
 #include <mbgl/util/tiny_sdf.hpp>
+#include <mbgl/util/std.hpp>
 
 namespace mbgl {
 
@@ -153,4 +154,10 @@ void GlyphManager::removeRequestor(GlyphRequestor& requestor) {
     }
 }
 
+void GlyphManager::evict(const std::set<FontStack>& keep) {
+    util::erase_if(entries, [&] (const auto& entry) {
+        return keep.count(entry.first) == 0;
+    });
+}
+
 } // namespace mbgl
diff --git a/src/mbgl/text/glyph_manager.hpp b/src/mbgl/text/glyph_manager.hpp
index 84db2c4be5..642471bbf2 100644
--- a/src/mbgl/text/glyph_manager.hpp
+++ b/src/mbgl/text/glyph_manager.hpp
@@ -42,6 +42,9 @@ public:
 
     void setObserver(GlyphManagerObserver*);
 
+    // Remove glyphs for all but the supplied font stacks.
+    void evict(const std::set<FontStack>&);
+
 private:
     Glyph generateLocalSDF(const FontStack& fontStack, GlyphID glyphID);
 
diff --git a/src/mbgl/util/font_stack.cpp b/src/mbgl/util/font_stack.cpp
index fb3b1b60a2..177d5e6f31 100644
--- a/src/mbgl/util/font_stack.cpp
+++ b/src/mbgl/util/font_stack.cpp
@@ -1,10 +1,14 @@
 #include <mbgl/util/font_stack.hpp>
+#include <mbgl/util/logging.hpp>
+#include <mbgl/style/layers/symbol_layer_impl.hpp>
 
 #include <boost/functional/hash.hpp>
 #include <boost/algorithm/string/join.hpp>
 
 namespace mbgl {
 
+using namespace style;
+
 std::string fontStackToString(const FontStack& fontStack) {
     return boost::algorithm::join(fontStack, ",");
 }
@@ -13,4 +17,40 @@ std::size_t FontStackHash::operator()(const FontStack& fontStack) const {
     return boost::hash_range(fontStack.begin(), fontStack.end());
 }
 
+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;
+                    }
+                }
+            }
+        );
+    }
+
+    return result;
+}
+
 } // namespace mbgl
diff --git a/test/fixtures/style_parser/text-font.info.json b/test/fixtures/style_parser/text-font.info.json
index 0fb9658269..dba9c707df 100644
--- a/test/fixtures/style_parser/text-font.info.json
+++ b/test/fixtures/style_parser/text-font.info.json
@@ -1,14 +1,14 @@
 {
   "default": {
     "log": [
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - get' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - case' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - match' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - at' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - coalesce' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - step' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - let/var' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."],
-      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - identity function' has an invalid value for text-font and will not work offline. Output values must be contained as literals within the expression."]
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - get' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - case' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - match' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - at' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - coalesce' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - step' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - let/var' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."],
+      [1, "WARNING", "ParseStyle", "Layer 'invalid expression - identity function' has an invalid value for text-font and will not render text. Output values must be contained as literals within the expression."]
     ]
   }
 }