make addGlyphsToAtlas static method of SymbolBucket + add addGlyphs method to minimise number of mutex locking

4 files changed, 34 insertions, 16 deletions

diff --git a/include/mbgl/geometry/glyph_atlas.hpp b/include/mbgl/geometry/glyph_atlas.hpp
index 5b09cbcd6d..639c6cc511 100644
--- a/include/mbgl/geometry/glyph_atlas.hpp
+++ b/include/mbgl/geometry/glyph_atlas.hpp
@@ -24,6 +24,8 @@ private:
         std::set<uint64_t> ids;
     };
 
+    Rect<uint16_t> addGlyph_impl(uint64_t tile_id, const std::string& face_name,
+                                 const SDFGlyph& glyph);
 public:
     GlyphAtlas(uint16_t width, uint16_t height);
     ~GlyphAtlas();
@@ -31,6 +33,8 @@ public:
 
     Rect<uint16_t> addGlyph(uint64_t tile_id, const std::string& face_name,
                             const SDFGlyph& glyph);
+    void addGlyphs(uint64_t tileid, std::u32string const& text, std::string const& stackname,
+                   FontStack const& fontStack, GlyphPositions & face);
     void removeGlyphs(uint64_t tile_id);
     void bind();
 
diff --git a/include/mbgl/renderer/symbol_bucket.hpp b/include/mbgl/renderer/symbol_bucket.hpp
index 42682401ef..9a5da1d012 100644
--- a/include/mbgl/renderer/symbol_bucket.hpp
+++ b/include/mbgl/renderer/symbol_bucket.hpp
@@ -86,8 +86,8 @@ private:
     void addSymbols(Buffer &buffer, const PlacedGlyphs &symbols, float scale, PlacementRange placementRange);
 
     // Adds glyphs to the glyph atlas so that they have a left/top/width/height coordinates associated to them that we can use for writing to a buffer.
-    void addGlyphsToAtlas(uint64_t tileid, const std::string stackname, const std::u32string &string,
-                  const FontStack &fontStack, GlyphAtlas &glyphAtlas, GlyphPositions &face);
+    static void addGlyphsToAtlas(uint64_t tileid, const std::string stackname, const std::u32string &string,
+                          const FontStack &fontStack, GlyphAtlas &glyphAtlas, GlyphPositions &face);
 
 public:
     const StyleBucketSymbol &properties;
diff --git a/src/geometry/glyph_atlas.cpp b/src/geometry/glyph_atlas.cpp
index 6bee475da3..72c961fda5 100644
--- a/src/geometry/glyph_atlas.cpp
+++ b/src/geometry/glyph_atlas.cpp
@@ -22,9 +22,15 @@ GlyphAtlas::~GlyphAtlas() {
 }
 
 Rect<uint16_t> GlyphAtlas::addGlyph(uint64_t tile_id, const std::string& face_name,
-                                    const SDFGlyph& glyph) {
+                                    const SDFGlyph& glyph)
+{
     std::lock_guard<std::mutex> lock(mtx);
+    return addGlyph_impl(tile_id, face_name, glyph);
+}
 
+Rect<uint16_t> GlyphAtlas::addGlyph_impl(uint64_t tile_id, const std::string& face_name,
+                                    const SDFGlyph& glyph)
+{
     // Use constant value for now.
     const uint8_t buffer = 3;
 
@@ -83,6 +89,23 @@ Rect<uint16_t> GlyphAtlas::addGlyph(uint64_t tile_id, const std::string& face_na
     return rect;
 }
 
+void GlyphAtlas::addGlyphs(uint64_t tileid, std::u32string const& text, std::string const& stackname, FontStack const& fontStack, GlyphPositions & face)
+{
+    std::lock_guard<std::mutex> lock(mtx);
+
+    std::map<uint32_t, SDFGlyph> const& sdfs = fontStack.getSDFs();
+    for (uint32_t chr : text)
+    {
+        auto sdf_it = sdfs.find(chr);
+        if (sdf_it != sdfs.end())
+        {
+            SDFGlyph const& sdf = sdf_it->second;
+            Rect<uint16_t> rect = addGlyph_impl(tileid, stackname, sdf);
+            face.emplace(chr, Glyph{rect, sdf.metrics});
+        }
+    }
+}
+
 void GlyphAtlas::removeGlyphs(uint64_t tile_id) {
     std::lock_guard<std::mutex> lock(mtx);
 
diff --git a/src/renderer/symbol_bucket.cpp b/src/renderer/symbol_bucket.cpp
index 47cc4db44c..7d5036eea3 100644
--- a/src/renderer/symbol_bucket.cpp
+++ b/src/renderer/symbol_bucket.cpp
@@ -34,18 +34,9 @@ bool SymbolBucket::hasTextData() const { return !text.groups.empty(); }
 bool SymbolBucket::hasIconData() const { return !icon.groups.empty(); }
 
 void SymbolBucket::addGlyphsToAtlas(uint64_t tileid, const std::string stackname,
-                                    const std::u32string &string, const FontStack &fontStack,
+                                    const std::u32string &text, const FontStack &fontStack,
                                     GlyphAtlas &glyphAtlas, GlyphPositions &face) {
-    const std::map<uint32_t, SDFGlyph> &sdfs = fontStack.getSDFs();
-    // Loop through all characters and add glyph to atlas, positions.
-    for (uint32_t chr : string) {
-        auto sdf_it = sdfs.find(chr);
-        if (sdf_it != sdfs.end()) {
-            const SDFGlyph &sdf = sdf_it->second;
-            const Rect<uint16_t> rect = glyphAtlas.addGlyph(tileid, stackname, sdf);
-            face.emplace(chr, Glyph{rect, sdf.metrics});
-        }
-    }
+    glyphAtlas.addGlyphs(tileid, text, stackname, fontStack,face);
 }
 
 std::vector<SymbolFeature> SymbolBucket::processFeatures(const VectorTileLayer &layer,
@@ -150,8 +141,8 @@ void SymbolBucket::addFeatures(const VectorTileLayer &layer, const FilterExpress
 
             // Add the glyphs we need for this label to the glyph atlas.
             if (shaping.size()) {
-                addGlyphsToAtlas(id.to_uint64(), properties.text.font, feature.label, fontStack,
-                                 glyphAtlas, face);
+                SymbolBucket::addGlyphsToAtlas(id.to_uint64(), properties.text.font, feature.label, fontStack,
+                                               glyphAtlas, face);
             }
         }