[core] Avoid bucket downcasting in Placement

5 files changed, 65 insertions, 44 deletions

diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp
index a074743c9a..c02e2ec341 100644
--- a/src/mbgl/renderer/bucket.hpp
+++ b/src/mbgl/renderer/bucket.hpp
@@ -17,6 +17,8 @@ class CrossTileSymbolLayerIndex;
 class OverscaledTileID;
 class PatternDependency;
 using PatternLayerMap = std::map<std::string, PatternDependency>;
+class Placement;
+class BucketPlacementParameters;
 
 class Bucket {
 public:
@@ -55,6 +57,9 @@ public:
     virtual std::pair<uint32_t, bool> registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&, uint32_t&) {
         return std::make_pair(0u, false);
     }
+    // Places this bucket to the given placement. Returns bucket cross-tile id on success call; `0` otherwise.
+    virtual uint32_t place(Placement&, const BucketPlacementParameters&, std::set<uint32_t>&) { return 0u; }
+    virtual void updateOpacities(Placement&, std::set<uint32_t>&) {}
 
 protected:
     Bucket() = default;
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index 8af40f6e3e..381ef5b24b 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -4,6 +4,7 @@
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
 #include <mbgl/text/cross_tile_symbol_index.hpp>
 #include <mbgl/text/glyph_atlas.hpp>
+#include <mbgl/text/placement.hpp>
 
 namespace mbgl {
 
@@ -245,4 +246,13 @@ std::pair<uint32_t, bool> SymbolBucket::registerAtCrossTileIndex(CrossTileSymbol
     return std::make_pair(bucketInstanceId, added);
 }
 
+uint32_t SymbolBucket::place(Placement& placement, const BucketPlacementParameters& params, std::set<uint32_t>& seenIds) {
+    placement.placeLayerBucket(*this, params, seenIds);
+    return bucketInstanceId;
+}
+
+void SymbolBucket::updateOpacities(Placement& placement, std::set<uint32_t>& seenIds) {
+    placement.updateBucketOpacities(*this, seenIds);
+}
+
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
index dc3aa64567..3b61002890 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -59,6 +59,8 @@ public:
     void upload(gfx::UploadPass&) override;
     bool hasData() const override;
     std::pair<uint32_t, bool> registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&, uint32_t& maxCrossTileID) override;
+    uint32_t place(Placement&, const BucketPlacementParameters&, std::set<uint32_t>&) override;
+    void updateOpacities(Placement&, std::set<uint32_t>&) override;
     bool hasTextData() const;
     bool hasIconData() const;
     bool hasCollisionBoxData() const;
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index bcb193d02a..49a57ae7c0 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -73,15 +73,13 @@ Placement::Placement(const TransformState& state_, MapMode mapMode_, style::Tran
 
 void Placement::placeLayer(const RenderLayer& layer, const mat4& projMatrix, bool showCollisionBoxes) {
 
-    std::unordered_set<uint32_t> seenCrossTileIDs;
+    std::set<uint32_t> seenCrossTileIDs;
 
     for (const auto& item : layer.getPlacementData()) {
         RenderTile& renderTile = item.tile;
         assert(renderTile.tile.kind == Tile::Kind::Geometry);
         auto& geometryTile = static_cast<GeometryTile&>(renderTile.tile);
-
         Bucket& bucket = item.bucket;
-        auto& symbolBucket = static_cast<SymbolBucket&>(bucket);
 
         const float pixelsToTileUnits = renderTile.id.pixelsToTileUnits(1, state.getZoom());
 
@@ -103,17 +101,27 @@ void Placement::placeLayer(const RenderLayer& layer, const mat4& projMatrix, boo
                 item.rotateWithMap,
                 state,
                 pixelsToTileUnits);
-        
+
+        const auto& collisionGroup = collisionGroups.get(geometryTile.sourceID);
+        BucketPlacementParameters params{
+                posMatrix,
+                textLabelPlaneMatrix,
+                iconLabelPlaneMatrix,
+                scale,
+                textPixelRatio,
+                showCollisionBoxes,
+                renderTile.tile.holdForFade(),
+                collisionGroup};
+        auto bucketInstanceId = bucket.place(*this, params, seenCrossTileIDs);
+        assert(bucketInstanceId != 0u);
         
         // As long as this placement lives, we have to hold onto this bucket's
         // matching FeatureIndex/data for querying purposes
         retainedQueryData.emplace(std::piecewise_construct,
-                                  std::forward_as_tuple(symbolBucket.bucketInstanceId),
-                                  std::forward_as_tuple(symbolBucket.bucketInstanceId, geometryTile.getFeatureIndex(), geometryTile.id));
+                                  std::forward_as_tuple(bucketInstanceId),
+                                  std::forward_as_tuple(bucketInstanceId, geometryTile.getFeatureIndex(), geometryTile.id));
         
-        const auto collisionGroup = collisionGroups.get(geometryTile.sourceID);
-        
-        placeLayerBucket(symbolBucket, posMatrix, textLabelPlaneMatrix, iconLabelPlaneMatrix, scale, textPixelRatio, showCollisionBoxes, seenCrossTileIDs, renderTile.tile.holdForFade(), collisionGroup);
+
     }
 }
 
@@ -132,15 +140,8 @@ Point<float> calculateVariableLayoutOffset(style::SymbolAnchorType anchor, float
 
 void Placement::placeLayerBucket(
         SymbolBucket& bucket,
-        const mat4& posMatrix,
-        const mat4& textLabelPlaneMatrix,
-        const mat4& iconLabelPlaneMatrix,
-        const float scale,
-        const float textPixelRatio,
-        const bool showCollisionBoxes,
-        std::unordered_set<uint32_t>& seenCrossTileIDs,
-        const bool holdingForFade,
-        const CollisionGroups::CollisionGroup& collisionGroup) {
+        const BucketPlacementParameters& params,
+        std::set<uint32_t>& seenCrossTileIDs) {
 
     auto partiallyEvaluatedTextSize = bucket.textSizeBinder->evaluateForZoom(state.getZoom());
     auto partiallyEvaluatedIconSize = bucket.iconSizeBinder->evaluateForZoom(state.getZoom());
@@ -149,7 +150,7 @@ void Placement::placeLayerBucket(
     if (mapMode == MapMode::Tile &&
         (bucket.layout.get<style::SymbolAvoidEdges>() ||
          bucket.layout.get<style::SymbolPlacement>() == style::SymbolPlacementType::Line)) {
-        avoidEdges = collisionIndex.projectTileBoundaries(posMatrix);
+        avoidEdges = collisionIndex.projectTileBoundaries(params.posMatrix);
     }
     
     const bool textAllowOverlap = bucket.layout.get<style::TextAllowOverlap>();
@@ -179,7 +180,7 @@ void Placement::placeLayerBucket(
     auto placeSymbol = [&] (SymbolInstance& symbolInstance) {
         if (seenCrossTileIDs.count(symbolInstance.crossTileID) != 0u) return;
 
-        if (holdingForFade) {
+        if (params.holdingForFade) {
             // Mark all symbols from this tile as "not placed", but don't add to seenCrossTileIDs, because we don't
             // know yet if we have a duplicate in a parent tile that _should_ be placed.
             placements.emplace(symbolInstance.crossTileID, JointPlacement(false, false, false));
@@ -196,11 +197,11 @@ void Placement::placeLayerBucket(
             const float fontSize = evaluateSizeForFeature(partiallyEvaluatedTextSize, placedSymbol);
             if (variableTextAnchors.empty()) {
                 auto placed = collisionIndex.placeFeature(textCollisionFeature, {},
-                        posMatrix, textLabelPlaneMatrix, textPixelRatio,
-                        placedSymbol, scale, fontSize,
+                        params.posMatrix, params.textLabelPlaneMatrix, params.pixelRatio,
+                        placedSymbol, params.scale, fontSize,
                         bucket.layout.get<style::TextAllowOverlap>(),
                         pitchWithMap,
-                        showCollisionBoxes, avoidEdges, collisionGroup.second);
+                        params.showCollisionBoxes, avoidEdges, params.collisionGroup.second);
                 placeText = placed.first;
                 offscreen &= placed.second;
             } else if (!textCollisionFeature.alongLine && !textCollisionFeature.boxes.empty()) {
@@ -235,11 +236,11 @@ void Placement::placeLayerBucket(
                     }
 
                     auto placed = collisionIndex.placeFeature(textCollisionFeature, shift,
-                                                                posMatrix, mat4(), textPixelRatio,
-                                                                placedSymbol, scale, fontSize,
+                                                                params.posMatrix, mat4(), params.pixelRatio,
+                                                                placedSymbol, params.scale, fontSize,
                                                                 bucket.layout.get<style::TextAllowOverlap>(),
                                                                 pitchWithMap,
-                                                                showCollisionBoxes, avoidEdges, collisionGroup.second);
+                                                                params.showCollisionBoxes, avoidEdges, params.collisionGroup.second);
 
                     if (placed.first) {
                         assert(symbolInstance.crossTileID != 0u);
@@ -290,11 +291,11 @@ void Placement::placeLayerBucket(
             const float fontSize = evaluateSizeForFeature(partiallyEvaluatedIconSize, placedSymbol);
 
             auto placed = collisionIndex.placeFeature(symbolInstance.iconCollisionFeature, {},
-                    posMatrix, iconLabelPlaneMatrix, textPixelRatio,
-                    placedSymbol, scale, fontSize,
+                    params.posMatrix, params.iconLabelPlaneMatrix, params.pixelRatio,
+                    placedSymbol, params.scale, fontSize,
                     bucket.layout.get<style::IconAllowOverlap>(),
                     pitchWithMap,
-                    showCollisionBoxes, avoidEdges, collisionGroup.second);
+                    params.showCollisionBoxes, avoidEdges, params.collisionGroup.second);
             placeIcon = placed.first;
             offscreen &= placed.second;
         }
@@ -312,11 +313,11 @@ void Placement::placeLayerBucket(
         }
 
         if (placeText) {
-            collisionIndex.insertFeature(symbolInstance.textCollisionFeature, bucket.layout.get<style::TextIgnorePlacement>(), bucket.bucketInstanceId, collisionGroup.first);
+            collisionIndex.insertFeature(symbolInstance.textCollisionFeature, bucket.layout.get<style::TextIgnorePlacement>(), bucket.bucketInstanceId, params.collisionGroup.first);
         }
 
         if (placeIcon) {
-            collisionIndex.insertFeature(symbolInstance.iconCollisionFeature, bucket.layout.get<style::IconIgnorePlacement>(), bucket.bucketInstanceId, collisionGroup.first);
+            collisionIndex.insertFeature(symbolInstance.iconCollisionFeature, bucket.layout.get<style::IconIgnorePlacement>(), bucket.bucketInstanceId, params.collisionGroup.first);
         }
 
         assert(symbolInstance.crossTileID != 0);
@@ -398,10 +399,7 @@ void Placement::commit(TimePoint now) {
 void Placement::updateLayerOpacities(const RenderLayer& layer) {
     std::set<uint32_t> seenCrossTileIDs;
     for (const auto& item : layer.getPlacementData()) {
-        Bucket& bucket = item.bucket;
-        auto& symbolBucket = static_cast<SymbolBucket&>(bucket);
-
-        updateBucketOpacities(symbolBucket, seenCrossTileIDs);
+        item.bucket.get().updateOpacities(*this, seenCrossTileIDs);
     }
 }
 
diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp
index 8dcb7e3ff9..e0fcac3350 100644
--- a/src/mbgl/text/placement.hpp
+++ b/src/mbgl/text/placement.hpp
@@ -87,6 +87,18 @@ private:
     uint16_t maxGroupID;
     bool crossSourceCollisions;
 };
+
+class BucketPlacementParameters {
+public:
+    const mat4& posMatrix;
+    const mat4& textLabelPlaneMatrix;
+    const mat4& iconLabelPlaneMatrix;
+    float scale;
+    float pixelRatio;
+    bool showCollisionBoxes;
+    bool holdingForFade;
+    const CollisionGroups::CollisionGroup& collisionGroup;
+};
     
 class Placement {
 public:
@@ -108,17 +120,11 @@ public:
     const VariableOffsets& getVariableOffsets() const { return variableOffsets; }
 
 private:
+    friend SymbolBucket;
     void placeLayerBucket(
             SymbolBucket&,
-            const mat4& posMatrix,
-            const mat4& textLabelPlaneMatrix,
-            const mat4& iconLabelPlaneMatrix,
-            const float scale,
-            const float pixelRatio,
-            const bool showCollisionBoxes,
-            std::unordered_set<uint32_t>& seenCrossTileIDs,
-            const bool holdingForFade,
-            const CollisionGroups::CollisionGroup& collisionGroup);
+            const BucketPlacementParameters&,
+            std::set<uint32_t>& seenCrossTileIDs);
 
     void updateBucketOpacities(SymbolBucket&, std::set<uint32_t>&);
     void markUsedJustification(SymbolBucket&, style::TextVariableAnchorType, SymbolInstance&);