[core] SymbolBucket updates complete at placement stage

`RenderSymbolLayer` does not have to update dynamic vertices of its buckets, this logic is moved to placement (which is already updates opacity vertices).

* fixes clustering of labels when text variable placement enabled - as assignes `usesVariablePlacement` per bucket
* simplifies the code in `RenderSymbolLayer` (the `RenderSymbolLayer::upload()`	is now omitted).
* symbol buckets are not modified after orchestration finishes

2 files changed, 24 insertions, 14 deletions

diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index 381ef5b24b..06fccb8ffd 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -10,7 +10,7 @@ namespace mbgl {
 
 using namespace style;
 namespace {
-    std::atomic<uint32_t> maxBucketInstanceId;
+std::atomic<uint32_t> maxBucketInstanceId;
 } // namespace
 
 SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layout_,
@@ -62,7 +62,11 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {
         }
 
         if (!dynamicUploaded) {
-            text.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(text.dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            if (!text.dynamicVertexBuffer) {
+                text.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(text.dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            } else {
+                uploadPass.updateVertexBuffer(*text.dynamicVertexBuffer, std::move(text.dynamicVertices));
+            }
         }
         if (!placementChangesUploaded) {
             if (!text.opacityVertexBuffer) {
@@ -84,7 +88,11 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {
             uploadPass.updateIndexBuffer(*icon.indexBuffer, std::move(icon.triangles));
         }
         if (!dynamicUploaded) {
-            icon.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(icon.dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            if (!icon.dynamicVertexBuffer) {
+                icon.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(icon.dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            } else {
+                uploadPass.updateVertexBuffer(*icon.dynamicVertexBuffer, std::move(icon.dynamicVertices));
+            }
         }
         if (!placementChangesUploaded) {
             if (!icon.opacityVertexBuffer) {
@@ -150,11 +158,6 @@ bool SymbolBucket::hasCollisionCircleData() const {
     return !collisionCircle.segments.empty();
 }
 
-void SymbolBucket::updateOpacity() {
-    placementChangesUploaded = false;
-    uploaded = false;
-}
-
 void addPlacedSymbol(gfx::IndexVector<gfx::Triangles>& triangles, const PlacedSymbol& placedSymbol) {
     auto endIndex = placedSymbol.vertexStartIndex + placedSymbol.glyphOffsets.size() * 4;
     for (auto vertexIndex = placedSymbol.vertexStartIndex; vertexIndex < endIndex; vertexIndex += 4) {
@@ -242,8 +245,8 @@ bool SymbolBucket::hasFormatSectionOverrides() const {
 }
 
 std::pair<uint32_t, bool> SymbolBucket::registerAtCrossTileIndex(CrossTileSymbolLayerIndex& index, const OverscaledTileID& tileID, uint32_t& maxCrossTileID) {
-    bool added = index.addBucket(tileID, *this, maxCrossTileID);
-    return std::make_pair(bucketInstanceId, added);
+    bool firstTimeAdded = index.addBucket(tileID, *this, maxCrossTileID);
+    return std::make_pair(bucketInstanceId, firstTimeAdded);
 }
 
 uint32_t SymbolBucket::place(Placement& placement, const BucketPlacementParameters& params, std::set<uint32_t>& seenIds) {
@@ -251,8 +254,14 @@ uint32_t SymbolBucket::place(Placement& placement, const BucketPlacementParamete
     return bucketInstanceId;
 }
 
-void SymbolBucket::updateOpacities(Placement& placement, std::set<uint32_t>& seenIds) {
-    placement.updateBucketOpacities(*this, seenIds);
+void SymbolBucket::updateVertices(Placement& placement, bool updateOpacities, const RenderTile& tile, std::set<uint32_t>& seenIds) {
+    if (updateOpacities) {
+        placement.updateBucketOpacities(*this, seenIds);
+        placementChangesUploaded = false;
+    }
+    placement.updateBucketDynamicVertices(*this, tile);
+    dynamicUploaded = false;
+    uploaded = false;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
index 3b61002890..e47672f1cd 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -60,14 +60,14 @@ public:
     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;
+    void updateVertices(Placement&, bool updateOpacities, const RenderTile&, std::set<uint32_t>&) override;
     bool hasTextData() const;
     bool hasIconData() const;
     bool hasCollisionBoxData() const;
     bool hasCollisionCircleData() const;
     bool hasFormatSectionOverrides() const;
 
-    void updateOpacity();
+
     void sortFeatures(const float angle);
     // The result contains references to the `symbolInstances` items, sorted by viewport Y.
     std::vector<std::reference_wrapper<SymbolInstance>> getSortedSymbols(const float angle);
@@ -138,6 +138,7 @@ public:
     const float tilePixelRatio;
     uint32_t bucketInstanceId;
     bool justReloaded = false;
+    bool hasVariablePlacement = false;
     mutable optional<bool> hasFormatSectionOverrides_;
 
     std::shared_ptr<std::vector<size_t>> featureSortOrder;