[core] Extract SymbolLayout from SymbolBucket

SymbolLayout lives on the worker thread and contains the persistent data needed for repeated placement. SymbolBucket contains the data generated during placement, and is transferred to the main thread for rendering.

This eliminates the risky sharing of GeometryTile::buckets between the main thread and worker thread during TileWorker::redoPlacement.

While here, rationalize the names of states a SymbolLayout may be in:

* Pending: Waiting for the necessary glyphs or icons to be available.
* Prepared: The potential positions of text and icons have been determined.
* Placed: The final positions have been determined, taking into account prior layers.

In TileWorker, all SymbolLayouts are stored in a single vector. Each SymbolLayout knows what state it is in, and TileWorker can easily determine how much progress it can make toward a final result.

3 files changed, 112 insertions, 128 deletions

diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index f74f813e48..c432869fa5 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -193,19 +193,19 @@ void GeometryTile::redoPlacement() {
         return;
     }
 
-    workRequest = worker.redoPlacement(tileWorker, buckets, targetConfig, [this, config = targetConfig](std::unique_ptr<CollisionTile> collisionTile) {
+    workRequest = worker.redoPlacement(tileWorker, targetConfig, [this, config = targetConfig](TilePlacementResult result) {
         workRequest.reset();
 
         // Persist the configuration we just placed so that we can later check whether we need to
         // place again in case the configuration has changed.
         placedConfig = config;
 
-        for (auto& bucket : buckets) {
-            bucket.second->swapRenderData();
+        for (auto& bucket : result.buckets) {
+            buckets[bucket.first] = std::move(bucket.second);
         }
 
         if (featureIndex) {
-            featureIndex->setCollisionTile(std::move(collisionTile));
+            featureIndex->setCollisionTile(std::move(result.collisionTile));
         }
 
         // The target configuration could have changed since we started placement. In this case,
diff --git a/src/mbgl/tile/tile_worker.cpp b/src/mbgl/tile/tile_worker.cpp
index d5e551d118..572f9054cd 100644
--- a/src/mbgl/tile/tile_worker.cpp
+++ b/src/mbgl/tile/tile_worker.cpp
@@ -1,6 +1,7 @@
 #include <mbgl/text/collision_tile.hpp>
 #include <mbgl/tile/tile_worker.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
+#include <mbgl/layout/symbol_layout.hpp>
 #include <mbgl/style/bucket_parameters.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
@@ -12,7 +13,7 @@
 #include <mbgl/util/string.hpp>
 #include <mbgl/util/exception.hpp>
 
-#include <utility>
+#include <unordered_set>
 
 namespace mbgl {
 
@@ -38,149 +39,141 @@ TileWorker::~TileWorker() {
 
 TileParseResult TileWorker::parseAllLayers(std::vector<std::unique_ptr<Layer>> layers_,
                                            std::unique_ptr<const GeometryTileData> tileData_,
-                                           PlacementConfig config) {
+                                           const PlacementConfig& config) {
     tileData = std::move(tileData_);
     return redoLayout(std::move(layers_), config);
 }
 
 TileParseResult TileWorker::redoLayout(std::vector<std::unique_ptr<Layer>> layers_,
-                                       const PlacementConfig config) {
+                                       const PlacementConfig& config) {
     layers = std::move(layers_);
 
     // We're doing a fresh parse of the tile, because the underlying data or style has changed.
-    pending.clear();
-    placementPending.clear();
-    partialParse = false;
     featureIndex = std::make_unique<FeatureIndex>();
+    symbolLayouts.clear();
 
     // We're storing a set of bucket names we've parsed to avoid parsing a bucket twice that is
     // referenced from more than one layer
-    std::set<std::string> parsed;
+    std::unordered_map<std::string, std::unique_ptr<Bucket>> buckets;
+    std::unordered_set<std::string> parsed;
 
     for (auto i = layers.rbegin(); i != layers.rend(); i++) {
-        const Layer* layer = i->get();
-        if (parsed.find(layer->baseImpl->bucketName()) == parsed.end()) {
-            parsed.emplace(layer->baseImpl->bucketName());
-            parseLayer(layer);
+        if (obsolete) {
+            break;
         }
-        featureIndex->addBucketLayerName(layer->baseImpl->bucketName(), layer->baseImpl->id);
-    }
 
-    return prepareResult(config);
-}
+        // Temporary prevention for crashing due to https://github.com/mapbox/mapbox-gl-native/issues/6263.
+        // Instead, the race condition will produce a blank tile.
+        if (!tileData) {
+            break;
+        }
 
-TileParseResult TileWorker::parsePendingLayers(const PlacementConfig config) {
-    // Try parsing the remaining layers that we couldn't parse in the first step due to missing
-    // dependencies.
-    for (auto it = pending.begin(); it != pending.end();) {
-        const SymbolLayer& symbolLayer = *it->first;
-        SymbolBucket* symbolBucket = dynamic_cast<SymbolBucket*>(it->second.get());
+        const Layer* layer = i->get();
+        const std::string& bucketName = layer->baseImpl->bucketName();
 
-        if (!symbolBucket->needsDependencies(glyphStore, spriteStore)) {
-            symbolBucket->addFeatures(reinterpret_cast<uintptr_t>(this),
-                                      *symbolLayer.impl->spriteAtlas,
-                                      glyphAtlas,
-                                      glyphStore);
-            placementPending.emplace(symbolLayer.impl->bucketName(), std::move(it->second));
-            pending.erase(it++);
+        featureIndex->addBucketLayerName(bucketName, layer->baseImpl->id);
+
+        if (parsed.find(bucketName) != parsed.end()) {
+            continue;
+        }
+
+        parsed.emplace(bucketName);
+
+        auto geometryLayer = tileData->getLayer(layer->baseImpl->sourceLayer);
+        if (!geometryLayer) {
             continue;
         }
 
-        // Advance the iterator here; we're skipping this when erasing an element from this list.
-        ++it;
+        BucketParameters parameters(id,
+                                    *geometryLayer,
+                                    obsolete,
+                                    reinterpret_cast<uintptr_t>(this),
+                                    spriteStore,
+                                    glyphAtlas,
+                                    glyphStore,
+                                    *featureIndex,
+                                    mode);
+
+        if (layer->is<SymbolLayer>()) {
+            symbolLayouts.push_back(layer->as<SymbolLayer>()->impl->createLayout(parameters));
+        } else {
+            std::unique_ptr<Bucket> bucket = layer->baseImpl->createBucket(parameters);
+            if (bucket->hasData()) {
+                buckets.emplace(layer->baseImpl->bucketName(), std::move(bucket));
+            }
+        }
     }
 
-    return prepareResult(config);
+    return parsePendingLayers(config, std::move(buckets));
 }
 
-TileParseResult TileWorker::prepareResult(const PlacementConfig& config) {
-    result.complete = pending.empty();
+TileParseResult TileWorker::parsePendingLayers(const PlacementConfig& config) {
+    return parsePendingLayers(config, std::unordered_map<std::string, std::unique_ptr<Bucket>>());
+}
 
-    if (result.complete) {
-        featureIndex->setCollisionTile(placeLayers(config));
-        result.featureIndex = std::move(featureIndex);
-        result.tileData = tileData ? tileData->clone() : nullptr;
-    }
+TileParseResult TileWorker::parsePendingLayers(const PlacementConfig& config,
+                                               std::unordered_map<std::string, std::unique_ptr<Bucket>> buckets) {
+    TileParseResultData result;
 
-    return std::move(result);
-}
+    result.complete = true;
+    result.buckets = std::move(buckets);
 
-std::unique_ptr<CollisionTile> TileWorker::placeLayers(const PlacementConfig config) {
-    auto collisionTile = redoPlacement(&placementPending, config);
-    for (auto &p : placementPending) {
-        p.second->swapRenderData();
-        insertBucket(p.first, std::move(p.second));
+    // Prepare as many SymbolLayouts as possible.
+    for (auto& symbolLayout : symbolLayouts) {
+        if (symbolLayout->state == SymbolLayout::Pending) {
+            if (symbolLayout->canPrepare(glyphStore, spriteStore)) {
+                symbolLayout->state = SymbolLayout::Prepared;
+                symbolLayout->prepare(reinterpret_cast<uintptr_t>(this),
+                                      glyphAtlas,
+                                      glyphStore);
+            } else {
+                result.complete = false;
+            }
+        }
     }
-    placementPending.clear();
-    return collisionTile;
-}
 
-std::unique_ptr<CollisionTile> TileWorker::redoPlacement(
-    const std::unordered_map<std::string, std::unique_ptr<Bucket>>* buckets,
-    PlacementConfig config) {
+    // If all SymbolLayouts are prepared, then perform placement. Otherwise, parsePendingLayers
+    // will eventually be re-run.
+    if (result.complete) {
+        TilePlacementResult placementResult = redoPlacement(config);
 
-    auto collisionTile = std::make_unique<CollisionTile>(config);
+        featureIndex->setCollisionTile(std::move(placementResult.collisionTile));
 
-    for (auto i = layers.rbegin(); i != layers.rend(); i++) {
-        const auto it = buckets->find((*i)->baseImpl->id);
-        if (it != buckets->end()) {
-            it->second->placeFeatures(*collisionTile);
+        for (auto& bucket : placementResult.buckets) {
+            result.buckets.emplace(std::move(bucket));
+        }
+
+        result.featureIndex = std::move(featureIndex);
+
+        if (tileData) {
+            result.tileData = tileData->clone();
         }
     }
 
-    return collisionTile;
+    return std::move(result);
 }
 
-void TileWorker::parseLayer(const Layer* layer) {
-    // Cancel early when parsing.
-    if (obsolete)
-        return;
+TilePlacementResult TileWorker::redoPlacement(const PlacementConfig& config) {
+    TilePlacementResult result;
 
-    // Temporary prevention for crashing due to https://github.com/mapbox/mapbox-gl-native/issues/6263.
-    // Instead, the race condition will produce a blank tile.
-    if (!tileData) {
-        return;
-    }
+    result.collisionTile = std::make_unique<CollisionTile>(config);
 
-    auto geometryLayer = tileData->getLayer(layer->baseImpl->sourceLayer);
-    if (!geometryLayer) {
-        // The layer specified in the bucket does not exist. Do nothing.
-        if (debug::tileParseWarnings) {
-            Log::Warning(Event::ParseTile, "layer '%s' does not exist in tile %s",
-                    layer->baseImpl->sourceLayer.c_str(), util::toString(id).c_str());
+    for (auto& symbolLayout : symbolLayouts) {
+        if (symbolLayout->state == SymbolLayout::Pending) {
+            // Can't do placement until all layouts are prepared.
+            return result;
         }
-        return;
     }
 
-    BucketParameters parameters(id,
-                                     *geometryLayer,
-                                     obsolete,
-                                     reinterpret_cast<uintptr_t>(this),
-                                     partialParse,
-                                     spriteStore,
-                                     glyphAtlas,
-                                     glyphStore,
-                                     *featureIndex,
-                                     mode);
-
-    std::unique_ptr<Bucket> bucket = layer->baseImpl->createBucket(parameters);
-
-    if (layer->is<SymbolLayer>()) {
-        if (partialParse) {
-            // We cannot parse this bucket yet. Instead, we're saving it for later.
-            pending.emplace_back(layer->as<SymbolLayer>(), std::move(bucket));
-        } else {
-            placementPending.emplace(layer->baseImpl->bucketName(), std::move(bucket));
+    for (auto& symbolLayout : symbolLayouts) {
+        symbolLayout->state = SymbolLayout::Placed;
+        std::unique_ptr<Bucket> bucket = symbolLayout->place(*result.collisionTile);
+        if (bucket->hasData() || symbolLayout->hasSymbolInstances()) {
+            result.buckets.emplace(symbolLayout->bucketName, std::move(bucket));
         }
-    } else {
-        insertBucket(layer->baseImpl->bucketName(), std::move(bucket));
     }
-}
 
-void TileWorker::insertBucket(const std::string& name, std::unique_ptr<Bucket> bucket) {
-    if (bucket->hasData()) {
-        result.buckets.emplace(name, std::move(bucket));
-    }
+    return result;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/tile/tile_worker.hpp b/src/mbgl/tile/tile_worker.hpp
index 5f38c898ff..e64e7dee19 100644
--- a/src/mbgl/tile/tile_worker.hpp
+++ b/src/mbgl/tile/tile_worker.hpp
@@ -4,7 +4,6 @@
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/variant.hpp>
-#include <mbgl/text/placement_config.hpp>
 #include <mbgl/geometry/feature_index.hpp>
 
 #include <atomic>
@@ -22,10 +21,11 @@ class SpriteStore;
 class GlyphAtlas;
 class GlyphStore;
 class Bucket;
+class SymbolLayout;
+class PlacementConfig;
 
 namespace style {
 class Layer;
-class SymbolLayer;
 } // namespace style
 
 // We're using this class to shuttle the resulting buckets from the worker thread to the MapContext
@@ -42,6 +42,12 @@ using TileParseResult = variant<
     TileParseResultData, // success
     std::exception_ptr>;    // error
 
+class TilePlacementResult {
+public:
+    std::unordered_map<std::string, std::unique_ptr<Bucket>> buckets;
+    std::unique_ptr<CollisionTile> collisionTile;
+};
+
 class TileWorker : public util::noncopyable {
 public:
     TileWorker(OverscaledTileID,
@@ -54,21 +60,18 @@ public:
 
     TileParseResult parseAllLayers(std::vector<std::unique_ptr<style::Layer>>,
                                    std::unique_ptr<const GeometryTileData>,
-                                   PlacementConfig);
+                                   const PlacementConfig&);
 
-    TileParseResult parsePendingLayers(PlacementConfig);
+    TileParseResult parsePendingLayers(const PlacementConfig&);
 
     TileParseResult redoLayout(std::vector<std::unique_ptr<style::Layer>>,
-                               PlacementConfig);
+                               const PlacementConfig&);
 
-    std::unique_ptr<CollisionTile> redoPlacement(const std::unordered_map<std::string, std::unique_ptr<Bucket>>*,
-                       PlacementConfig);
+    TilePlacementResult redoPlacement(const PlacementConfig&);
 
 private:
-    TileParseResult prepareResult(const PlacementConfig& config);
-    void parseLayer(const style::Layer*);
-    void insertBucket(const std::string& name, std::unique_ptr<Bucket>);
-    std::unique_ptr<CollisionTile> placeLayers(PlacementConfig);
+    TileParseResult parsePendingLayers(const PlacementConfig&,
+                                       std::unordered_map<std::string, std::unique_ptr<Bucket>>);
 
     const OverscaledTileID id;
 
@@ -78,23 +81,11 @@ private:
     const std::atomic<bool>& obsolete;
     const MapMode mode;
 
-    bool partialParse = false;
-
     std::vector<std::unique_ptr<style::Layer>> layers;
-
-    std::unique_ptr<FeatureIndex> featureIndex;
     std::unique_ptr<const GeometryTileData> tileData;
 
-    // Contains buckets that we couldn't parse so far due to missing resources.
-    // They will be attempted on subsequent parses.
-    std::list<std::pair<const style::SymbolLayer*, std::unique_ptr<Bucket>>> pending;
-
-    // Contains buckets that have been parsed, but still need placement.
-    // They will be placed when all buckets have been parsed.
-    std::unordered_map<std::string, std::unique_ptr<Bucket>> placementPending;
-
-    // Temporary holder
-    TileParseResultData result;
+    std::unique_ptr<FeatureIndex> featureIndex;
+    std::vector<std::unique_ptr<SymbolLayout>> symbolLayouts;
 };
 
 } // namespace mbgl