Headless label placement tests aren't deterministic

include/mbgl/util/exclusive.hpp

@@ -0,0 +1,30 @@
+#ifndef MBGL_UTIL_EXCLUSIVE
+#define MBGL_UTIL_EXCLUSIVE
+
+#include <memory>
+#include <mutex>
+
+
+namespace mbgl {
+namespace util {
+
+template <class T>
+class exclusive {
+public:
+    inline exclusive(T* val, std::unique_ptr<std::lock_guard<std::mutex>> mtx) : ptr(val), lock(std::move(mtx)) {}
+
+    inline T* operator->() { return ptr; }
+    inline const T* operator->() const { return ptr; }
+    inline T* operator*() { return ptr; }
+    inline const T* operator*() const { return ptr; }
+
+private:
+    T *ptr;
+    std::unique_ptr<std::lock_guard<std::mutex>> lock;
+};
+
+
+} // end namespace util
+} // end namespace mbgl
+
+#endif

src/mbgl/renderer/symbol_bucket.cpp

@@ -190,7 +190,7 @@ void SymbolBucket::addFeatures(uintptr_t tileUID,
     if (layout.text.justify == TextJustifyType::Right) justify = 1;
     else if (layout.text.justify == TextJustifyType::Left) justify = 0;
 
-    auto* fontStack = glyphStore.getFontStack(layout.text.font);
+    auto fontStack = glyphStore.getFontStack(layout.text.font);
 
     for (const auto& feature : features) {
         if (!feature.geometry.size()) continue;
@@ -214,7 +214,7 @@ void SymbolBucket::addFeatures(uintptr_t tileUID,
 
             // Add the glyphs we need for this label to the glyph atlas.
             if (shaping.size()) {
-                glyphAtlas.addGlyphs(tileUID, feature.label, layout.text.font, *fontStack, face);
+                glyphAtlas.addGlyphs(tileUID, feature.label, layout.text.font, **fontStack, face);
             }
         }
 

src/mbgl/text/glyph_store.cpp

@@ -20,28 +20,23 @@ namespace mbgl {
 
 
 void FontStack::insert(uint32_t id, const SDFGlyph &glyph) {
-    std::lock_guard<std::mutex> lock(mtx);
     metrics.emplace(id, glyph.metrics);
     bitmaps.emplace(id, glyph.bitmap);
     sdfs.emplace(id, glyph);
 }
 
 const std::map<uint32_t, GlyphMetrics> &FontStack::getMetrics() const {
-    std::lock_guard<std::mutex> lock(mtx);
     return metrics;
 }
 
 const std::map<uint32_t, SDFGlyph> &FontStack::getSDFs() const {
-    std::lock_guard<std::mutex> lock(mtx);
     return sdfs;
 }
 
 const Shaping FontStack::getShaping(const std::u32string &string, const float maxWidth,
                                     const float lineHeight, const float horizontalAlign,
                                     const float verticalAlign, const float justify,
                                     const float spacing, const vec2<float> &translate) const {
-    std::lock_guard<std::mutex> lock(mtx);
-
     Shaping shaping;
 
     int32_t x = std::round(translate.x * 24); // one em
@@ -167,7 +162,6 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL,
             // parse the data we received. We are not doing this here since this callback is being
             // called from another (unknown) thread.
             data = res.data;
-            parsed = true;
             callback(this);
         }
     });
@@ -180,8 +174,6 @@ GlyphPBF::~GlyphPBF() {
 }
 
 void GlyphPBF::parse(FontStack &stack) {
-    std::lock_guard<std::mutex> lock(mtx);
-
     if (!data.size()) {
         // If there is no data, this means we either haven't received any data, or
         // we have already parsed the data.
@@ -231,6 +223,8 @@ void GlyphPBF::parse(FontStack &stack) {
     }
 
     data.clear();
+
+    parsed = true;
 }
 
 bool GlyphPBF::isParsed() const {
@@ -252,26 +246,27 @@ void GlyphStore::setURL(const std::string &url) {
     glyphURL = url;
 }
 
-bool GlyphStore::requestGlyphRangesIfNeeded(const std::string& fontStack,
+bool GlyphStore::requestGlyphRangesIfNeeded(const std::string& fontStackName,
                                             const std::set<GlyphRange>& glyphRanges) {
     bool requestIsNeeded = false;
 
     if (glyphRanges.empty()) {
         return requestIsNeeded;
     }
 
-    auto callback = [this, fontStack](GlyphPBF* glyph) {
-        glyph->parse(*createFontStack(fontStack));
+    auto callback = [this, fontStackName](GlyphPBF* glyph) {
+        auto fontStack = createFontStack(fontStackName);
+        glyph->parse(**fontStack);
         asyncEmitGlyphRangeLoaded->send();
     };
 
     std::lock_guard<std::mutex> lock(rangesMutex);
-    auto& rangeSets = ranges[fontStack];
+    auto& rangeSets = ranges[fontStackName];
 
     for (const auto& range : glyphRanges) {
         const auto& rangeSets_it = rangeSets.find(range);
         if (rangeSets_it == rangeSets.end()) {
-            auto glyph = util::make_unique<GlyphPBF>(glyphURL, fontStack, range, env, callback);
+            auto glyph = util::make_unique<GlyphPBF>(glyphURL, fontStackName, range, env, callback);
             rangeSets.emplace(range, std::move(glyph));
             requestIsNeeded = true;
             continue;
@@ -285,26 +280,26 @@ bool GlyphStore::requestGlyphRangesIfNeeded(const std::string& fontStack,
     return requestIsNeeded;
 }
 
-FontStack* GlyphStore::createFontStack(const std::string &fontStack) {
-    std::lock_guard<std::mutex> lock(stacksMutex);
+util::exclusive<FontStack> GlyphStore::createFontStack(const std::string &fontStack) {
+    auto lock = util::make_unique<std::lock_guard<std::mutex>>(stacksMutex);
 
     auto stack_it = stacks.find(fontStack);
     if (stack_it == stacks.end()) {
         stack_it = stacks.emplace(fontStack, util::make_unique<FontStack>()).first;
     }
 
-    return stack_it->second.get();
+    return { stack_it->second.get(), std::move(lock) };
 }
 
-FontStack* GlyphStore::getFontStack(const std::string &fontStack) {
-    std::lock_guard<std::mutex> lock(stacksMutex);
+util::exclusive<FontStack> GlyphStore::getFontStack(const std::string &fontStack) {
+    auto lock = util::make_unique<std::lock_guard<std::mutex>>(stacksMutex);
 
     const auto& stack_it = stacks.find(fontStack);
     if (stack_it == stacks.end()) {
-        return nullptr;
+        return { nullptr, nullptr };
     }
 
-    return stack_it->second.get();
+    return { stack_it->second.get(), std::move(lock) };
 }
 
 void GlyphStore::setObserver(Observer* observer_) {

src/mbgl/text/glyph_store.hpp

@@ -4,6 +4,7 @@
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/vec.hpp>
 #include <mbgl/util/ptr.hpp>
+#include <mbgl/util/exclusive.hpp>
 
 #include <cstdint>
 #include <vector>
@@ -53,7 +54,6 @@ class FontStack {
     std::map<uint32_t, std::string> bitmaps;
     std::map<uint32_t, GlyphMetrics> metrics;
     std::map<uint32_t, SDFGlyph> sdfs;
-    mutable std::mutex mtx;
 };
 
 class GlyphPBF {
@@ -81,8 +81,6 @@ class GlyphPBF {
 
     Environment& env;
     Request* req = nullptr;
-
-    mutable std::mutex mtx;
 };
 
 // Manages Glyphrange PBF loading.
@@ -104,7 +102,7 @@ class GlyphStore {
     // GlyphRanges are already available, and thus, no request is performed.
     bool requestGlyphRangesIfNeeded(const std::string &fontStack, const std::set<GlyphRange> &glyphRanges);
 
-    FontStack* getFontStack(const std::string &fontStack);
+    util::exclusive<FontStack> getFontStack(const std::string &fontStack);
 
     void setURL(const std::string &url);
 
@@ -113,7 +111,7 @@ class GlyphStore {
 private:
     void emitGlyphRangeLoaded();
 
-    FontStack* createFontStack(const std::string &fontStack);
+    util::exclusive<FontStack> createFontStack(const std::string &fontStack);
 
     std::string glyphURL;
     Environment &env;