From e6ddcf9beb8e0aad52d4185dac0c0b1d15205a16 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 16 Jan 2020 20:12:13 +0200 Subject: [core] Simplify CrossTileSymbolIndex::addLayer() --- src/mbgl/renderer/bucket.hpp | 2 +- src/mbgl/renderer/buckets/symbol_bucket.cpp | 5 +++-- src/mbgl/renderer/buckets/symbol_bucket.hpp | 2 +- src/mbgl/text/cross_tile_symbol_index.cpp | 16 +++++++++++---- src/mbgl/text/cross_tile_symbol_index.hpp | 5 +++-- test/text/cross_tile_symbol_index.test.cpp | 30 ++++++++++++++--------------- 6 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp index 6da0f280e6..3e756a06a4 100644 --- a/src/mbgl/renderer/bucket.hpp +++ b/src/mbgl/renderer/bucket.hpp @@ -56,7 +56,7 @@ public: // Returns a pair, the first element of which is a bucket cross-tile id // on success call; `0` otherwise. The second element is `true` if // the bucket was originally registered; `false` otherwise. - virtual std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&, uint32_t&) { + virtual std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&) { return std::make_pair(0u, false); } // Places this bucket to the given placement. diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 7efd81053e..382aac6563 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -295,8 +295,9 @@ bool SymbolBucket::hasFormatSectionOverrides() const { return *hasFormatSectionOverrides_; } -std::pair SymbolBucket::registerAtCrossTileIndex(CrossTileSymbolLayerIndex& index, const OverscaledTileID& tileID, uint32_t& maxCrossTileID) { - bool firstTimeAdded = index.addBucket(tileID, *this, maxCrossTileID); +std::pair SymbolBucket::registerAtCrossTileIndex(CrossTileSymbolLayerIndex& index, + const OverscaledTileID& tileID) { + bool firstTimeAdded = index.addBucket(tileID, *this); return std::make_pair(bucketInstanceId, firstTimeAdded); } diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index 1d1aa13dd0..432589a52b 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -81,7 +81,7 @@ public: void upload(gfx::UploadPass&) override; bool hasData() const override; - std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&, uint32_t& maxCrossTileID) override; + std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&) override; void place(Placement&, const BucketPlacementParameters&, std::set&) override; void updateVertices( const Placement&, bool updateOpacities, const TransformState&, const RenderTile&, std::set&) override; diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index d8bd0292ec..5b9d6ae69c 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -67,7 +67,7 @@ void TileLayerIndex::findMatches(SymbolBucket& bucket, } } -CrossTileSymbolLayerIndex::CrossTileSymbolLayerIndex() = default; +CrossTileSymbolLayerIndex::CrossTileSymbolLayerIndex(uint32_t& maxCrossTileID_) : maxCrossTileID(maxCrossTileID_) {} /* * Sometimes when a user pans across the antimeridian the longitude value gets wrapped. @@ -95,7 +95,7 @@ void CrossTileSymbolLayerIndex::handleWrapJump(float newLng) { lng = newLng; } -bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket, uint32_t& maxCrossTileID) { +bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket) { auto& thisZoomIndexes = indexes[tileID.overscaledZ]; auto previousIndex = thisZoomIndexes.find(tileID); if (previousIndex != thisZoomIndexes.end()) { @@ -177,7 +177,15 @@ bool CrossTileSymbolLayerIndex::removeStaleBuckets(const std::unordered_set AddLayerResult { - auto& layerIndex = layerIndexes[layer.getID()]; + auto found = layerIndexes.find(layer.getID()); + if (found == layerIndexes.end()) { + found = layerIndexes + .emplace(std::piecewise_construct, + std::forward_as_tuple(layer.getID()), + std::forward_as_tuple(maxCrossTileID)) + .first; + } + auto& layerIndex = found->second; AddLayerResult result = AddLayerResult::NoChanges; std::unordered_set currentBucketIDs; @@ -187,7 +195,7 @@ auto CrossTileSymbolIndex::addLayer(const RenderLayer& layer, float lng) -> AddL for (const auto& item : layer.getPlacementData()) { const RenderTile& renderTile = item.tile; Bucket& bucket = item.bucket; - auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID(), maxCrossTileID); + auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID()); assert(pair.first != 0u); if (pair.second) result |= AddLayerResult::BucketsAdded; currentBucketIDs.insert(pair.first); diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp index ddcdb814c6..3b7c367726 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -47,8 +47,8 @@ public: class CrossTileSymbolLayerIndex { public: - CrossTileSymbolLayerIndex(); - bool addBucket(const OverscaledTileID&, SymbolBucket&, uint32_t& maxCrossTileID); + CrossTileSymbolLayerIndex(uint32_t& maxCrossTileID); + bool addBucket(const OverscaledTileID&, SymbolBucket&); bool removeStaleBuckets(const std::unordered_set& currentIDs); void handleWrapJump(float newLng); private: @@ -57,6 +57,7 @@ private: std::map> indexes; std::map> usedCrossTileIDs; float lng = 0; + uint32_t& maxCrossTileID; }; class CrossTileSymbolIndex { diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp index fe1375c66d..f0c78ce632 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -36,7 +36,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -62,7 +62,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; mainBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); // Assigned new IDs ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); @@ -89,7 +89,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; childBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(childID, childBucket, maxCrossTileID); + index.addBucket(childID, childBucket); // matched parent tile ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); @@ -117,7 +117,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; parentBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(parentID, parentBucket, maxCrossTileID); + index.addBucket(parentID, parentBucket); // matched child tile ASSERT_EQ(parentBucket.symbolInstances.at(0).crossTileID, 1u); @@ -145,7 +145,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; grandchildBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(grandchildID, grandchildBucket, maxCrossTileID); + index.addBucket(grandchildID, grandchildBucket); // Matches the symbol in `mainBucket` ASSERT_EQ(grandchildBucket.symbolInstances.at(0).crossTileID, 1u); @@ -158,7 +158,7 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -203,7 +203,7 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { childBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns a new id - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); // removes the tile @@ -211,18 +211,18 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { index.removeStaleBuckets(currentIDs); // assigns a new id - index.addBucket(childID, childBucket, maxCrossTileID); + index.addBucket(childID, childBucket); ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 2u); // overwrites the old id to match the already-added tile - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 2u); } TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -270,12 +270,12 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { childBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns new ids - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); ASSERT_EQ(mainBucket.symbolInstances.at(1).crossTileID, 2u); // copies parent ids without duplicate ids in this tile - index.addBucket(childID, childBucket, maxCrossTileID); + index.addBucket(childID, childBucket); ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); // A' copies from A ASSERT_EQ(childBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B ASSERT_EQ(childBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID @@ -284,7 +284,7 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { TEST(CrossTileSymbolLayerIndex, bucketReplacement) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -331,12 +331,12 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) { secondBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns new ids - index.addBucket(tileID, firstBucket, maxCrossTileID); + index.addBucket(tileID, firstBucket); ASSERT_EQ(firstBucket.symbolInstances.at(0).crossTileID, 1u); ASSERT_EQ(firstBucket.symbolInstances.at(1).crossTileID, 2u); // copies parent ids without duplicate ids in this tile - index.addBucket(tileID, secondBucket, maxCrossTileID); + index.addBucket(tileID, secondBucket); ASSERT_EQ(secondBucket.symbolInstances.at(0).crossTileID, 1u); // A' copies from A ASSERT_EQ(secondBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B ASSERT_EQ(secondBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID -- cgit v1.2.1