[core] port minor collision changes from -js (#10764)

8 files changed, 80 insertions, 66 deletions

diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp
index e474737f8d..f6eb62f655 100644
--- a/src/mbgl/renderer/tile_pyramid.cpp
+++ b/src/mbgl/renderer/tile_pyramid.cpp
@@ -210,13 +210,6 @@ void TilePyramid::update(const std::vector<Immutable<style::Layer::Impl>>& layer
         auto tilesIt = tiles.begin();
         auto retainIt = retain.begin();
         while (tilesIt != tiles.end()) {
-            auto renderedIt = rendered.find(tilesIt->first.toUnwrapped());
-            if (renderedIt == rendered.end()) {
-                // Since this tile isn't in the render set, crossTileIDs won't be kept
-                // updated by CrossTileSymbolIndex. We need to reset the stored crossTileIDs
-                // so they're not reused if/when this tile is re-added to the render set
-                tilesIt->second->resetCrossTileIDs();
-            }
             if (retainIt == retain.end() || tilesIt->first < *retainIt) {
                 if (!needsRelayout) {
                     tilesIt->second->setNecessity(TileNecessity::Optional);
diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp
index 177615857f..6efded8e4a 100644
--- a/src/mbgl/text/cross_tile_symbol_index.cpp
+++ b/src/mbgl/text/cross_tile_symbol_index.cpp
@@ -57,50 +57,35 @@ void TileLayerIndex::findMatches(std::vector<SymbolInstance>& symbolInstances, c
 CrossTileSymbolLayerIndex::CrossTileSymbolLayerIndex() {
 }
 
-void CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& coord, SymbolBucket& bucket, uint32_t& maxCrossTileID) {
-    if (bucket.bucketInstanceId) return;
-    bucket.bucketInstanceId = ++maxBucketInstanceId;
-
-    uint8_t minZoom = 25;
-    uint8_t maxZoom = 0;
-    for (auto& it : indexes) {
-        auto z = it.first;
-        minZoom = std::min(minZoom, z);
-        maxZoom = std::max(maxZoom, z);
+bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket, uint32_t& maxCrossTileID) {
+    auto thisZoomIndexes = indexes[tileID.overscaledZ];
+    auto previousIndex = thisZoomIndexes.find(tileID);
+    if (previousIndex != thisZoomIndexes.end() && previousIndex->second.bucketInstanceId == bucket.bucketInstanceId) {
+        return false;
     }
 
+    for (auto& symbolInstance: bucket.symbolInstances) {
+        symbolInstance.crossTileID = 0;
+    }
 
-    // make all higher-res child tiles block duplicate labels in this tile
-    for (auto z = maxZoom; z > coord.overscaledZ; z--) {
-        auto zoomIndexes = indexes.find(z);
-        if (zoomIndexes != indexes.end()) {
-            for (auto& childIndex : zoomIndexes->second) {
-                if (!childIndex.second.coord.isChildOf(coord)) {
-                    continue;
+    for (auto& it : indexes) {
+        auto zoom = it.first;
+        auto zoomIndexes = it.second;
+        if (zoom > tileID.overscaledZ) {
+            for (auto& childIndex : zoomIndexes) {
+                if (childIndex.second.coord.isChildOf(tileID)) {
+                    childIndex.second.findMatches(bucket.symbolInstances, tileID);
                 }
-                childIndex.second.findMatches(bucket.symbolInstances, coord);
             }
-        }
-        if (z == 0) {
-            break;
-        }
-    }
-
-    // make this tile block duplicate labels in lower-res parent tiles
-    for (auto z = coord.overscaledZ; z >= minZoom; z--) {
-        auto parentCoord = coord.scaledTo(z);
-        auto zoomIndexes = indexes.find(z);
-        if (zoomIndexes != indexes.end()) {
-            auto parentIndex = zoomIndexes->second.find(parentCoord);
-            if (parentIndex != zoomIndexes->second.end()) {
-                parentIndex->second.findMatches(bucket.symbolInstances, coord);
+        } else {
+            auto parentTileID = tileID.scaledTo(zoom);
+            auto parentIndex = zoomIndexes.find(parentTileID);
+            if (parentIndex != zoomIndexes.end()) {
+                parentIndex->second.findMatches(bucket.symbolInstances, tileID);
             }
         }
-        if (z == 0) {
-            break;
-        }
     }
-    
+
     for (auto& symbolInstance : bucket.symbolInstances) {
         if (!symbolInstance.crossTileID) {
             // symbol did not match any known symbol, assign a new id
@@ -108,7 +93,8 @@ void CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& coord, SymbolB
         }
     }
 
-    indexes[coord.overscaledZ].emplace(coord, TileLayerIndex(coord, bucket.symbolInstances, bucket.bucketInstanceId));
+    indexes[tileID.overscaledZ].emplace(tileID, TileLayerIndex(tileID, bucket.symbolInstances, bucket.bucketInstanceId));
+    return true;
 }
 
 bool CrossTileSymbolLayerIndex::removeStaleBuckets(const std::unordered_set<uint32_t>& currentIDs) {
@@ -145,9 +131,11 @@ bool CrossTileSymbolIndex::addLayer(RenderSymbolLayer& symbolLayer) {
         SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
 
         if (!symbolBucket.bucketInstanceId) {
-            symbolBucketsChanged = true;
+            symbolBucket.bucketInstanceId = ++maxBucketInstanceId;
         }
-        layerIndex.addBucket(renderTile.tile.id, symbolBucket, maxCrossTileID);
+
+        const bool bucketAdded = layerIndex.addBucket(renderTile.tile.id, symbolBucket, maxCrossTileID);
+        symbolBucketsChanged = symbolBucketsChanged || bucketAdded;
         currentBucketIDs.insert(symbolBucket.bucketInstanceId);
     }
 
diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp
index c67cd37e00..995e0061b9 100644
--- a/src/mbgl/text/cross_tile_symbol_index.hpp
+++ b/src/mbgl/text/cross_tile_symbol_index.hpp
@@ -42,11 +42,10 @@ public:
 class CrossTileSymbolLayerIndex {
 public:
     CrossTileSymbolLayerIndex();
-    void addBucket(const OverscaledTileID&, SymbolBucket&, uint32_t& maxCrossTileID);
+    bool addBucket(const OverscaledTileID&, SymbolBucket&, uint32_t& maxCrossTileID);
     bool removeStaleBuckets(const std::unordered_set<uint32_t>& currentIDs);
 private:
     std::map<uint8_t,std::map<OverscaledTileID,TileLayerIndex>> indexes;
-    uint32_t maxBucketInstanceId = 0;
 };
 
 class CrossTileSymbolIndex {
@@ -59,6 +58,7 @@ public:
 private:
     std::map<std::string, CrossTileSymbolLayerIndex> layerIndexes;
     uint32_t maxCrossTileID = 0;
+    uint32_t maxBucketInstanceId = 0;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index b27d8f5baf..84e53f74eb 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -228,10 +228,15 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, std::set<uint32_t>&
     if (bucket.hasCollisionBoxData()) bucket.collisionBox.dynamicVertices.clear();
     if (bucket.hasCollisionCircleData()) bucket.collisionCircle.dynamicVertices.clear();
 
+    JointOpacityState defaultOpacityState(
+            bucket.layout.get<style::TextAllowOverlap>(),
+            bucket.layout.get<style::IconAllowOverlap>(),
+            true);
+
     for (SymbolInstance& symbolInstance : bucket.symbolInstances) {
         auto opacityState = seenCrossTileIDs.count(symbolInstance.crossTileID) == 0 ?
             getOpacity(symbolInstance.crossTileID) :
-            JointOpacityState(false, false, false);
+            defaultOpacityState;
 
         seenCrossTileIDs.insert(symbolInstance.crossTileID);
 
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 701e7cf2d6..7d93b61c5b 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -282,18 +282,6 @@ void GeometryTile::querySourceFeatures(
     }
 }
 
-void GeometryTile::resetCrossTileIDs() {
-    for (auto& bucket : symbolBuckets) {
-        auto symbolBucket = dynamic_cast<SymbolBucket*>(bucket.second.get());
-        if (symbolBucket && symbolBucket->bucketInstanceId) {
-            symbolBucket->bucketInstanceId = 0;
-            for (auto& symbolInstance : symbolBucket->symbolInstances) {
-                symbolInstance.crossTileID = 0;
-            }
-        }
-    }
-}
-
 bool GeometryTile::holdForFade() const {
     return mode == MapMode::Continuous &&
            (fadeState == FadeState::NeedsFirstPlacement || fadeState == FadeState::NeedsSecondPlacement);
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 6f871819a4..6667f095ed 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -95,8 +95,6 @@ public:
 
     void onError(std::exception_ptr, uint64_t correlationID);
     
-    void resetCrossTileIDs() override;
-    
     bool holdForFade() const override;
     void markRenderedIdeal() override;
     void markRenderedPreviously() override;
diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp
index 1186b74111..b7e200266f 100644
--- a/src/mbgl/tile/tile.hpp
+++ b/src/mbgl/tile/tile.hpp
@@ -109,8 +109,6 @@ public:
     // and will have time to finish by the second placement.
     virtual void performedFadePlacement() {}
     
-    virtual void resetCrossTileIDs() {};
-
     void dumpDebugLogs() const;
 
     const OverscaledTileID id;
diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp
index b40827d63e..ef5bd4a101 100644
--- a/test/text/cross_tile_symbol_index.test.cpp
+++ b/test/text/cross_tile_symbol_index.test.cpp
@@ -18,6 +18,7 @@ SymbolInstance makeSymbolInstance(float x, float y, std::u16string key) {
 TEST(CrossTileSymbolLayerIndex, addBucket) {
 
     uint32_t maxCrossTileID = 0;
+    uint32_t maxBucketInstanceId = 0;
     CrossTileSymbolLayerIndex index;
 
     style::SymbolLayoutProperties::PossiblyEvaluated layout;
@@ -25,12 +26,12 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
     bool iconsNeedLinear = false;
     bool sortFeaturesByY = false;
 
-
     OverscaledTileID mainID(6, 0, 6, 8, 8);
     std::vector<SymbolInstance> mainInstances;
     mainInstances.push_back(makeSymbolInstance(1000, 1000, u"Detroit"));
     mainInstances.push_back(makeSymbolInstance(2000, 2000, u"Toronto"));
     SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(mainInstances) };
+    mainBucket.bucketInstanceId = ++maxBucketInstanceId;
     index.addBucket(mainID, mainBucket, maxCrossTileID);
 
     // Assigned new IDs
@@ -45,6 +46,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
     childInstances.push_back(makeSymbolInstance(3000, 3000, u"Toronto"));
     childInstances.push_back(makeSymbolInstance(4001, 4001, u"Toronto"));
     SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(childInstances) };
+    childBucket.bucketInstanceId = ++maxBucketInstanceId;
     index.addBucket(childID, childBucket, maxCrossTileID);
 
     // matched parent tile
@@ -60,6 +62,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
     std::vector<SymbolInstance> parentInstances;
     parentInstances.push_back(makeSymbolInstance(500, 500, u"Detroit"));
     SymbolBucket parentBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(parentInstances) };
+    parentBucket.bucketInstanceId = ++maxBucketInstanceId;
     index.addBucket(parentID, parentBucket, maxCrossTileID);
 
     // matched child tile
@@ -75,6 +78,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
     grandchildInstances.push_back(makeSymbolInstance(4000, 4000, u"Detroit"));
     grandchildInstances.push_back(makeSymbolInstance(4000, 4000, u"Windsor"));
     SymbolBucket grandchildBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(grandchildInstances) };
+    grandchildBucket.bucketInstanceId = ++maxBucketInstanceId;
     index.addBucket(grandchildID, grandchildBucket, maxCrossTileID);
 
     // Matches the symbol in `mainBucket`
@@ -83,3 +87,43 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
     ASSERT_EQ(grandchildBucket.symbolInstances.at(1).crossTileID, 5u);
 
 }
+
+TEST(CrossTileSymbolLayerIndex, resetIDs) {
+
+    uint32_t maxCrossTileID = 0;
+    uint32_t maxBucketInstanceId = 0;
+    CrossTileSymbolLayerIndex index;
+
+    style::SymbolLayoutProperties::PossiblyEvaluated layout;
+    bool sdfIcons = false;
+    bool iconsNeedLinear = false;
+    bool sortFeaturesByY = false;
+
+    OverscaledTileID mainID(6, 0, 6, 8, 8);
+    std::vector<SymbolInstance> mainInstances;
+    mainInstances.push_back(makeSymbolInstance(1000, 1000, u"Detroit"));
+    SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(mainInstances) };
+    mainBucket.bucketInstanceId = ++maxBucketInstanceId;
+
+    OverscaledTileID childID(7, 0, 7, 16, 16);
+    std::vector<SymbolInstance> childInstances;
+    childInstances.push_back(makeSymbolInstance(2000, 2000, u"Detroit"));
+    SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(childInstances) };
+    childBucket.bucketInstanceId = ++maxBucketInstanceId;
+
+    // assigns a new id
+    index.addBucket(mainID, mainBucket, maxCrossTileID);
+    ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u);
+
+    // removes the tile
+    std::unordered_set<uint32_t> currentIDs;
+    index.removeStaleBuckets(currentIDs);
+
+    // assigns a new id
+    index.addBucket(childID, childBucket, maxCrossTileID);
+    ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 2u);
+
+    // overwrites the old id to match the already-added tile
+    index.addBucket(mainID, mainBucket, maxCrossTileID);
+    ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 2u);
+}