From 40051fb68ec710c5d83795740d0e3e8c75bb3cb3 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Fri, 6 Jan 2017 13:24:33 -0800
Subject: [core] Keep symbol and non-symbol buckets segregated

Discard prior symbol buckets only when new symbol buckets became available, in order to eliminate flickering when tiles are refreshed.
---
 src/mbgl/tile/geometry_tile.cpp |  8 ++++----
 src/mbgl/tile/geometry_tile.hpp |  7 ++++---
 test/tile/vector_tile.test.cpp  | 43 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 2a0047fecf..4c5a61672d 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -6,6 +6,7 @@
 #include <mbgl/style/layer_impl.hpp>
 #include <mbgl/style/layers/background_layer.hpp>
 #include <mbgl/style/layers/custom_layer.hpp>
+#include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/style.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/geometry/feature_index.hpp>
@@ -105,7 +106,7 @@ void GeometryTile::redoLayout() {
 
 void GeometryTile::onLayout(LayoutResult result) {
     availableData = DataAvailability::Some;
-    buckets = std::move(result.buckets);
+    nonSymbolBuckets = std::move(result.nonSymbolBuckets);
     featureIndex = std::move(result.featureIndex);
     data = std::move(result.tileData);
     observer->onTileChanged(*this);
@@ -115,9 +116,7 @@ void GeometryTile::onPlacement(PlacementResult result) {
     if (result.correlationID == correlationID) {
         availableData = DataAvailability::All;
     }
-    for (auto& bucket : result.buckets) {
-        buckets[bucket.first] = std::move(bucket.second);
-    }
+    symbolBuckets = std::move(result.symbolBuckets);
     featureIndex->setCollisionTile(std::move(result.collisionTile));
     observer->onTileChanged(*this);
 }
@@ -128,6 +127,7 @@ void GeometryTile::onError(std::exception_ptr err) {
 }
 
 Bucket* GeometryTile::getBucket(const Layer& layer) {
+    const auto& buckets = layer.is<SymbolLayer>() ? symbolBuckets : nonSymbolBuckets;
     const auto it = buckets.find(layer.baseImpl->id);
     if (it == buckets.end()) {
         return nullptr;
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 993f7b018e..5e7e501e89 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -50,7 +50,7 @@ public:
 
     class LayoutResult {
     public:
-        std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
+        std::unordered_map<std::string, std::shared_ptr<Bucket>> nonSymbolBuckets;
         std::unique_ptr<FeatureIndex> featureIndex;
         std::unique_ptr<GeometryTileData> tileData;
         uint64_t correlationID;
@@ -59,7 +59,7 @@ public:
 
     class PlacementResult {
     public:
-        std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
+        std::unordered_map<std::string, std::shared_ptr<Bucket>> symbolBuckets;
         std::unique_ptr<CollisionTile> collisionTile;
         uint64_t correlationID;
     };
@@ -80,7 +80,8 @@ private:
     uint64_t correlationID = 0;
     optional<PlacementConfig> requestedConfig;
 
-    std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
+    std::unordered_map<std::string, std::shared_ptr<Bucket>> nonSymbolBuckets;
+    std::unordered_map<std::string, std::shared_ptr<Bucket>> symbolBuckets;
     std::unique_ptr<FeatureIndex> featureIndex;
     std::unique_ptr<const GeometryTileData> data;
 };
diff --git a/test/tile/vector_tile.test.cpp b/test/tile/vector_tile.test.cpp
index 210422feec..9732f23b32 100644
--- a/test/tile/vector_tile.test.cpp
+++ b/test/tile/vector_tile.test.cpp
@@ -8,8 +8,14 @@
 #include <mbgl/map/transform.hpp>
 #include <mbgl/style/style.hpp>
 #include <mbgl/style/update_parameters.hpp>
+#include <mbgl/style/layers/symbol_layer.hpp>
+#include <mbgl/renderer/symbol_bucket.hpp>
+#include <mbgl/text/collision_tile.hpp>
+#include <mbgl/geometry/feature_index.hpp>
 #include <mbgl/annotation/annotation_manager.hpp>
 
+#include <memory>
+
 using namespace mbgl;
 
 class VectorTileTest {
@@ -47,3 +53,40 @@ TEST(VectorTile, onError) {
     tile.onError(std::make_exception_ptr(std::runtime_error("test")));
     EXPECT_TRUE(tile.isRenderable());
 }
+
+TEST(VectorTile, Issue7615) {
+    VectorTileTest test;
+    VectorTile tile(OverscaledTileID(0, 0, 0), "source", test.updateParameters, test.tileset);
+
+    style::SymbolLayer symbolLayer("symbol", "source");
+    auto symbolBucket = std::make_shared<SymbolBucket>(
+        MapMode::Continuous, style::SymbolLayoutProperties::Evaluated(), false, false);
+
+    // First onLayout is required so that a non-null FeatureIndex is available.
+    tile.onLayout(GeometryTile::LayoutResult {
+        {},
+        std::make_unique<FeatureIndex>(),
+        nullptr,
+        0
+    });
+
+    // Simulate placement of a symbol layer.
+    tile.onPlacement(GeometryTile::PlacementResult {
+        {{
+            symbolLayer.getID(),
+            symbolBucket
+        }},
+        nullptr,
+        0
+    });
+
+    // Second onLayout should not cause the existing symbol bucket to be discarded.
+    tile.onLayout(GeometryTile::LayoutResult {
+        {},
+        nullptr,
+        nullptr,
+        0
+    });
+
+    EXPECT_EQ(symbolBucket.get(), tile.getBucket(symbolLayer));
+}
-- 
cgit v1.2.1