[core] De-mutex GlyphAtlas.upstream/cloer_glyph_sanity_check

 - Expose glyph information to workers via message interface.
 - Symbol preparation for a tile now depends on all glyphs becoming
    available before it can start.

4 files changed, 37 insertions, 13 deletions

diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 5f1fc5de66..602c43148a 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -32,12 +32,13 @@ GeometryTile::GeometryTile(const OverscaledTileID& id_,
       worker(parameters.workerScheduler,
              ActorRef<GeometryTile>(*this, mailbox),
              id_,
-             *parameters.style.glyphAtlas,
              obsolete,
-             parameters.mode) {
+             parameters.mode),
+             glyphAtlas(*parameters.style.glyphAtlas) {
 }
 
 GeometryTile::~GeometryTile() {
+    glyphAtlas.removeGlyphs(*this);
     cancel();
 }
 
@@ -130,6 +131,14 @@ void GeometryTile::onError(std::exception_ptr err) {
     availableData = DataAvailability::All;
     observer->onTileError(*this, err);
 }
+    
+void GeometryTile::onGlyphsAvailable(GlyphPositionMap glyphPositions) {
+    worker.invoke(&GeometryTileWorker::onGlyphsAvailable, std::move(glyphPositions));
+}
+
+void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) {
+    glyphAtlas.getGlyphs(*this, std::move(glyphDependencies));
+}
 
 Bucket* GeometryTile::getBucket(const Layer& layer) {
     const auto& buckets = layer.is<SymbolLayer>() ? symbolBuckets : nonSymbolBuckets;
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index cabe193467..c7b0264c5e 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -2,6 +2,7 @@
 
 #include <mbgl/tile/tile.hpp>
 #include <mbgl/tile/geometry_tile_worker.hpp>
+#include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/text/placement_config.hpp>
 #include <mbgl/util/feature.hpp>
 #include <mbgl/actor/actor.hpp>
@@ -24,7 +25,7 @@ class UpdateParameters;
 class SourceQueryOptions;
 } // namespace style
 
-class GeometryTile : public Tile {
+class GeometryTile : public Tile, public GlyphRequestor {
 public:
     GeometryTile(const OverscaledTileID&,
                  std::string sourceID,
@@ -38,6 +39,8 @@ public:
     void setPlacementConfig(const PlacementConfig&) override;
     void symbolDependenciesChanged() override;
     void redoLayout() override;
+    
+    void getGlyphs(GlyphDependencies);
 
     Bucket* getBucket(const style::Layer&) override;
 
@@ -71,6 +74,8 @@ public:
     void onPlacement(PlacementResult);
 
     void onError(std::exception_ptr);
+
+    virtual void onGlyphsAvailable(GlyphPositionMap) override;
     
 protected:
     const GeometryTileData* getData() {
@@ -87,6 +92,8 @@ private:
     std::shared_ptr<Mailbox> mailbox;
     Actor<GeometryTileWorker> worker;
 
+    GlyphAtlas& glyphAtlas;
+    
     uint64_t correlationID = 0;
     optional<PlacementConfig> requestedConfig;
 
diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp
index b1fd7a852e..0d17526cb1 100644
--- a/src/mbgl/tile/geometry_tile_worker.cpp
+++ b/src/mbgl/tile/geometry_tile_worker.cpp
@@ -2,7 +2,6 @@
 #include <mbgl/tile/geometry_tile_data.hpp>
 #include <mbgl/tile/geometry_tile.hpp>
 #include <mbgl/text/collision_tile.hpp>
-#include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/layout/symbol_layout.hpp>
 #include <mbgl/style/bucket_parameters.hpp>
 #include <mbgl/style/group_by_layout.hpp>
@@ -25,19 +24,16 @@ using namespace style;
 GeometryTileWorker::GeometryTileWorker(ActorRef<GeometryTileWorker> self_,
                                        ActorRef<GeometryTile> parent_,
                                        OverscaledTileID id_,
-                                       GlyphAtlas& glyphAtlas_,
                                        const std::atomic<bool>& obsolete_,
                                        const MapMode mode_)
     : self(std::move(self_)),
       parent(std::move(parent_)),
       id(std::move(id_)),
-      glyphAtlas(glyphAtlas_),
       obsolete(obsolete_),
       mode(mode_) {
 }
 
 GeometryTileWorker::~GeometryTileWorker() {
-    glyphAtlas.removeGlyphs(reinterpret_cast<uintptr_t>(this));
 }
 
 /*
@@ -168,6 +164,11 @@ void GeometryTileWorker::symbolDependenciesChanged() {
     }
 }
 
+void GeometryTileWorker::onGlyphsAvailable(GlyphPositionMap glyphs) {
+    glyphPositions = std::move(glyphs);
+    symbolDependenciesChanged(); // TODO: This is a clumsy way to join the "glyphs loaded" and "symbol dependencies changed" signals
+}
+
 void GeometryTileWorker::coalesced() {
     try {
         switch (state) {
@@ -215,6 +216,8 @@ void GeometryTileWorker::redoLayout() {
     std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
     auto featureIndex = std::make_unique<FeatureIndex>();
     BucketParameters parameters { id, mode };
+    GlyphDependencies glyphDependencies;
+    glyphPositions.clear();
 
     std::vector<std::vector<const Layer*>> groups = groupByLayout(*layers);
     for (auto& group : groups) {
@@ -242,7 +245,7 @@ void GeometryTileWorker::redoLayout() {
 
         if (leader.is<SymbolLayer>()) {
             symbolLayoutMap.emplace(leader.getID(),
-                leader.as<SymbolLayer>()->impl->createLayout(parameters, group, *geometryLayer));
+                leader.as<SymbolLayer>()->impl->createLayout(parameters, group, *geometryLayer, glyphDependencies));
         } else {
             const Filter& filter = leader.baseImpl->filter;
             const std::string& sourceLayerID = leader.baseImpl->sourceLayer;
@@ -276,6 +279,10 @@ void GeometryTileWorker::redoLayout() {
             symbolLayouts.push_back(std::move(it->second));
         }
     }
+    
+    if (!glyphDependencies.empty()) {
+        parent.invoke(&GeometryTile::getGlyphs, std::move(glyphDependencies));
+    }
 
     parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult {
         std::move(buckets),
@@ -313,10 +320,9 @@ void GeometryTileWorker::attemptPlacement() {
         }
 
         if (symbolLayout->state == SymbolLayout::Pending) {
-            if (symbolLayout->canPrepare(glyphAtlas)) {
+            if (symbolLayout->canPrepare(glyphPositions)) {
                 symbolLayout->state = SymbolLayout::Prepared;
-                symbolLayout->prepare(reinterpret_cast<uintptr_t>(this),
-                                      glyphAtlas);
+                symbolLayout->prepare(glyphPositions);
             } else {
                 canPlace = false;
             }
diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp
index 91bf81a697..3a62203b10 100644
--- a/src/mbgl/tile/geometry_tile_worker.hpp
+++ b/src/mbgl/tile/geometry_tile_worker.hpp
@@ -2,6 +2,7 @@
 
 #include <mbgl/map/mode.hpp>
 #include <mbgl/tile/tile_id.hpp>
+#include <mbgl/text/glyph.hpp>
 #include <mbgl/text/placement_config.hpp>
 #include <mbgl/actor/actor_ref.hpp>
 #include <mbgl/util/optional.hpp>
@@ -25,7 +26,6 @@ public:
     GeometryTileWorker(ActorRef<GeometryTileWorker> self,
                        ActorRef<GeometryTile> parent,
                        OverscaledTileID,
-                       GlyphAtlas&,
                        const std::atomic<bool>&,
                        const MapMode);
     ~GeometryTileWorker();
@@ -34,6 +34,8 @@ public:
     void setData(std::unique_ptr<const GeometryTileData>, uint64_t correlationID);
     void setPlacementConfig(PlacementConfig, uint64_t correlationID);
     void symbolDependenciesChanged();
+    
+    void onGlyphsAvailable(GlyphPositionMap glyphs);
 
 private:
     void coalesce();
@@ -46,7 +48,6 @@ private:
     ActorRef<GeometryTile> parent;
 
     const OverscaledTileID id;
-    GlyphAtlas& glyphAtlas;
     const std::atomic<bool>& obsolete;
     const MapMode mode;
 
@@ -66,6 +67,7 @@ private:
     optional<PlacementConfig> placementConfig;
 
     std::vector<std::unique_ptr<SymbolLayout>> symbolLayouts;
+    GlyphPositionMap glyphPositions;
 };
 
 } // namespace mbgl