From 7149c8044ce4aca244df11982ae28e8df4136197 Mon Sep 17 00:00:00 2001
From: Chris Loer <chris.loer@gmail.com>
Date: Wed, 17 Jan 2018 14:26:36 -0800
Subject: [core] Synchronize state of CollisionIndex and FeatureIndexes. Fixes
 issue #10778, in which mismatch between FeatureIndex and CollisionIndex could
 lead to inconsistent results or even invalid array access.

---
 src/mbgl/renderer/renderer_impl.cpp | 13 +++++++++++++
 src/mbgl/renderer/renderer_impl.hpp |  1 +
 src/mbgl/tile/geometry_tile.cpp     |  8 +++++++-
 src/mbgl/tile/geometry_tile.hpp     |  3 +++
 src/mbgl/tile/tile.hpp              |  5 +++++
 5 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp
index 9fe421fa14..8e5968d4c2 100644
--- a/src/mbgl/renderer/renderer_impl.cpp
+++ b/src/mbgl/renderer/renderer_impl.cpp
@@ -389,6 +389,10 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) {
         }
 
         placementChanged = newPlacement->commit(*placement, parameters.timePoint);
+        // commitFeatureIndexes depends on the assumption that no new FeatureIndex has been loaded since placement
+        // started. If we violate this assumption, then we need to either make CollisionIndex completely independendent of
+        // FeatureIndex, or find a way for its entries to point to multiple FeatureIndexes.
+        commitFeatureIndexes();
         if (placementChanged || symbolBucketsChanged) {
             placement = std::move(newPlacement);
         }
@@ -768,6 +772,15 @@ bool Renderer::Impl::hasTransitions(TimePoint timePoint) const {
     return false;
 }
 
+void Renderer::Impl::commitFeatureIndexes() {
+    for (auto& source : renderSources) {
+        for (auto& renderTile : source.second->getRenderTiles()) {
+            Tile& tile = renderTile.get().tile;
+            tile.commitFeatureIndex();
+        }
+    }
+}
+
 void Renderer::Impl::updateFadingTiles() {
     fadingTiles = false;
     for (auto& source : renderSources) {
diff --git a/src/mbgl/renderer/renderer_impl.hpp b/src/mbgl/renderer/renderer_impl.hpp
index 5d0200a5df..4f45d514a5 100644
--- a/src/mbgl/renderer/renderer_impl.hpp
+++ b/src/mbgl/renderer/renderer_impl.hpp
@@ -74,6 +74,7 @@ private:
     void onTileChanged(RenderSource&, const OverscaledTileID&) override;
     void onTileError(RenderSource&, const OverscaledTileID&, std::exception_ptr) override;
 
+    void commitFeatureIndexes();
     void updateFadingTiles();
 
     friend class Renderer;
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 7d93b61c5b..4965158a9f 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -130,7 +130,7 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio
     //  replacing a tile at a different zoom that _did_ have symbols.
     (void)resultCorrelationID;
     nonSymbolBuckets = std::move(result.nonSymbolBuckets);
-    featureIndex = std::move(result.featureIndex);
+    pendingFeatureIndex = std::move(result.featureIndex);
     data = std::move(result.tileData);
     observer->onTileChanged(*this);
 }
@@ -213,6 +213,12 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const {
     return it->second.get();
 }
 
+void GeometryTile::commitFeatureIndex() {
+    if (pendingFeatureIndex) {
+        featureIndex = std::move(pendingFeatureIndex);
+    }
+}
+
 void GeometryTile::queryRenderedFeatures(
     std::unordered_map<std::string, std::vector<Feature>>& result,
     const GeometryCoordinates& queryGeometry,
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 6667f095ed..1851ad7494 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -100,6 +100,8 @@ public:
     void markRenderedPreviously() override;
     void performedFadePlacement() override;
     
+    void commitFeatureIndex() override;
+    
 protected:
     const GeometryTileData* getData() {
         return data.get();
@@ -123,6 +125,7 @@ private:
 
     std::unordered_map<std::string, std::shared_ptr<Bucket>> nonSymbolBuckets;
     std::unique_ptr<FeatureIndex> featureIndex;
+    std::unique_ptr<FeatureIndex> pendingFeatureIndex;
     std::unique_ptr<const GeometryTileData> data;
 
     optional<AlphaImage> glyphAtlasImage;
diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp
index b7e200266f..1bed180f3d 100644
--- a/src/mbgl/tile/tile.hpp
+++ b/src/mbgl/tile/tile.hpp
@@ -109,6 +109,11 @@ public:
     // and will have time to finish by the second placement.
     virtual void performedFadePlacement() {}
     
+    // FeatureIndexes are loaded asynchronously, but must be used with a CollisionIndex
+    // generated from the same data. Calling commitFeatureIndex signals the current
+    // CollisionIndex is up-to-date and allows us to start using the last loaded FeatureIndex
+    virtual void commitFeatureIndex() {}
+    
     void dumpDebugLogs() const;
 
     const OverscaledTileID id;
-- 
cgit v1.2.1