[core] Make FeatureIndex own GeometryTileData.upstream/one-phase-tiles

Prevents querying a FeatureIndex built against a separate set of data, which can lead to invalid index exceptions.
The GeometryTileWorker 'data' member can still change independently of the data in the feature index, at the time 'setData' is called. The GeometryTileWorker maintains ownership of its local data (which may be used to re-parse) and clones the data for use by the FeatureIndex in the foreground.

5 files changed, 27 insertions, 30 deletions

diff --git a/src/mbgl/geometry/feature_index.cpp b/src/mbgl/geometry/feature_index.cpp
index 6fb0d5e446..c67786274a 100644
--- a/src/mbgl/geometry/feature_index.cpp
+++ b/src/mbgl/geometry/feature_index.cpp
@@ -17,8 +17,9 @@
 
 namespace mbgl {
 
-FeatureIndex::FeatureIndex()
-    : grid(util::EXTENT, util::EXTENT, util::EXTENT / 16) { // 16x16 grid -> 32px cell
+FeatureIndex::FeatureIndex(std::unique_ptr<const GeometryTileData> tileData_)
+    : grid(util::EXTENT, util::EXTENT, util::EXTENT / 16) // 16x16 grid -> 32px cell
+    , tileData(std::move(tileData_)) {
 }
 
 void FeatureIndex::insert(const GeometryCollection& geometries,
@@ -47,12 +48,15 @@ void FeatureIndex::query(
         const double tileSize,
         const double scale,
         const RenderedQueryOptions& queryOptions,
-        const GeometryTileData& geometryTileData,
         const UnwrappedTileID& tileID,
         const std::string& sourceID,
         const std::vector<const RenderLayer*>& layers,
         const CollisionIndex& collisionIndex,
         const float additionalQueryRadius) const {
+    
+    if (!tileData) {
+        return;
+    }
 
     // Determine query radius
     const float pixelsToTileUnits = util::EXTENT / tileSize / scale;
@@ -72,13 +76,13 @@ void FeatureIndex::query(
         if (indexedFeature.sortIndex == previousSortIndex) continue;
         previousSortIndex = indexedFeature.sortIndex;
 
-        addFeature(result, indexedFeature, queryGeometry, queryOptions, geometryTileData, tileID.canonical, layers, bearing, pixelsToTileUnits);
+        addFeature(result, indexedFeature, queryGeometry, queryOptions, tileID.canonical, layers, bearing, pixelsToTileUnits);
     }
 
     std::vector<IndexedSubfeature> symbolFeatures = collisionIndex.queryRenderedSymbols(queryGeometry, tileID, sourceID);
     std::sort(symbolFeatures.begin(), symbolFeatures.end(), topDownSymbols);
     for (const auto& symbolFeature : symbolFeatures) {
-        addFeature(result, symbolFeature, queryGeometry, queryOptions, geometryTileData, tileID.canonical, layers, bearing, pixelsToTileUnits);
+        addFeature(result, symbolFeature, queryGeometry, queryOptions, tileID.canonical, layers, bearing, pixelsToTileUnits);
     }
 }
 
@@ -87,7 +91,6 @@ void FeatureIndex::addFeature(
     const IndexedSubfeature& indexedFeature,
     const GeometryCoordinates& queryGeometry,
     const RenderedQueryOptions& options,
-    const GeometryTileData& geometryTileData,
     const CanonicalTileID& tileID,
     const std::vector<const RenderLayer*>& layers,
     const float bearing,
@@ -113,7 +116,7 @@ void FeatureIndex::addFeature(
         }
 
         if (!geometryTileFeature) {
-            sourceLayer = geometryTileData.getLayer(indexedFeature.sourceLayerName);
+            sourceLayer = tileData->getLayer(indexedFeature.sourceLayerName);
             assert(sourceLayer);
 
             geometryTileFeature = sourceLayer->getFeature(indexedFeature.index);
diff --git a/src/mbgl/geometry/feature_index.hpp b/src/mbgl/geometry/feature_index.hpp
index e95bb94da6..9e0c172342 100644
--- a/src/mbgl/geometry/feature_index.hpp
+++ b/src/mbgl/geometry/feature_index.hpp
@@ -50,8 +50,10 @@ public:
 
 class FeatureIndex {
 public:
-    FeatureIndex();
+    FeatureIndex(std::unique_ptr<const GeometryTileData> tileData_);
 
+    const GeometryTileData* getData() { return tileData.get(); }
+    
     void insert(const GeometryCollection&, std::size_t index, const std::string& sourceLayerName, const std::string& bucketName);
 
     void query(
@@ -61,7 +63,6 @@ public:
             const double tileSize,
             const double scale,
             const RenderedQueryOptions& options,
-            const GeometryTileData&,
             const UnwrappedTileID&,
             const std::string&,
             const std::vector<const RenderLayer*>&,
@@ -83,7 +84,6 @@ private:
             const IndexedSubfeature&,
             const GeometryCoordinates& queryGeometry,
             const RenderedQueryOptions& options,
-            const GeometryTileData&,
             const CanonicalTileID&,
             const std::vector<const RenderLayer*>&,
             const float bearing,
@@ -93,5 +93,6 @@ private:
     unsigned int sortIndex = 0;
 
     std::unordered_map<std::string, std::vector<std::string>> bucketLayerIDs;
+    std::unique_ptr<const GeometryTileData> tileData;
 };
 } // namespace mbgl
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 5089f95022..a99cb91d26 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -133,7 +133,7 @@ void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelatio
     
     buckets = std::move(result.buckets);
     
-    dataPendingCommit = {{ std::move(result.tileData), std::move(result.featureIndex) }};
+    featureIndexPendingCommit = { std::move(result.featureIndex) };
 
     if (result.glyphAtlasImage) {
         glyphAtlasImage = std::move(*result.glyphAtlasImage);
@@ -202,12 +202,11 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const {
 }
 
 void GeometryTile::commitFeatureIndex() {
-    // We commit our pending FeatureIndex and GeometryTileData when a global placement has run,
-    // synchronizing the global CollisionIndex with the latest buckets/FeatureIndex/GeometryTileData
-    if (dataPendingCommit) {
-        data = std::move(dataPendingCommit->first);
-        featureIndex = std::move(dataPendingCommit->second);
-        dataPendingCommit = nullopt;
+    // We commit our pending FeatureIndex when a global placement has run,
+    // synchronizing the global CollisionIndex with the latest buckets/FeatureIndex
+    if (featureIndexPendingCommit) {
+        featureIndex = std::move(*featureIndexPendingCommit);
+        featureIndexPendingCommit = nullopt;
     }
 }
 
@@ -219,7 +218,7 @@ void GeometryTile::queryRenderedFeatures(
     const RenderedQueryOptions& options,
     const CollisionIndex& collisionIndex) {
 
-    if (!featureIndex || !data) return;
+    if (!getData()) return;
 
     // Determine the additional radius needed factoring in property functions
     float additionalRadius = 0;
@@ -236,7 +235,6 @@ void GeometryTile::queryRenderedFeatures(
                         util::tileSize * id.overscaleFactor(),
                         std::pow(2, transformState.getZoom() - id.overscaledZ),
                         options,
-                        *data,
                         id.toUnwrapped(),
                         sourceID,
                         layers,
@@ -248,8 +246,8 @@ void GeometryTile::querySourceFeatures(
     std::vector<Feature>& result,
     const SourceQueryOptions& options) {
 
-    // Data not yet available
-    if (!data) {
+    // Data not yet available, or tile is empty
+    if (!getData()) {
         return;
     }
     
@@ -262,7 +260,7 @@ void GeometryTile::querySourceFeatures(
     for (auto sourceLayer : *options.sourceLayers) {
         // Go throught all sourceLayers, if any
         // to gather all the features
-        auto layer = data->getLayer(sourceLayer);
+        auto layer = getData()->getLayer(sourceLayer);
         
         if (layer) {
             auto featureCount = layer->featureCount();
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 0161e00efd..418db4a0b2 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -67,18 +67,15 @@ public:
     public:
         std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
         std::unique_ptr<FeatureIndex> featureIndex;
-        std::unique_ptr<GeometryTileData> tileData;
         optional<AlphaImage> glyphAtlasImage;
         optional<PremultipliedImage> iconAtlasImage;
 
         LayoutResult(std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets_,
                      std::unique_ptr<FeatureIndex> featureIndex_,
-                     std::unique_ptr<GeometryTileData> tileData_,
                      optional<AlphaImage> glyphAtlasImage_,
                      optional<PremultipliedImage> iconAtlasImage_)
             : buckets(std::move(buckets_)),
               featureIndex(std::move(featureIndex_)),
-              tileData(std::move(tileData_)),
               glyphAtlasImage(std::move(glyphAtlasImage_)),
               iconAtlasImage(std::move(iconAtlasImage_)) {}
     };
@@ -95,7 +92,7 @@ public:
     
 protected:
     const GeometryTileData* getData() {
-        return data.get();
+        return featureIndex ? featureIndex->getData() : nullptr;
     }
 
 private:
@@ -116,9 +113,8 @@ private:
 
     std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
     
-    optional<std::pair<std::unique_ptr<const GeometryTileData>, std::unique_ptr<FeatureIndex>>> dataPendingCommit;
+    optional<std::unique_ptr<FeatureIndex>> featureIndexPendingCommit;
     std::unique_ptr<FeatureIndex> featureIndex;
-    std::unique_ptr<const GeometryTileData> data;
 
     optional<AlphaImage> glyphAtlasImage;
     optional<PremultipliedImage> iconAtlasImage;
diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp
index f57732117b..1378ad5d3a 100644
--- a/src/mbgl/tile/geometry_tile_worker.cpp
+++ b/src/mbgl/tile/geometry_tile_worker.cpp
@@ -329,7 +329,7 @@ void GeometryTileWorker::parse() {
 
     std::unordered_map<std::string, std::unique_ptr<SymbolLayout>> symbolLayoutMap;
     buckets.clear();
-    featureIndex = std::make_unique<FeatureIndex>();
+    featureIndex = std::make_unique<FeatureIndex>(*data ? (*data)->clone() : nullptr);
     BucketParameters parameters { id, mode, pixelRatio };
 
     GlyphDependencies glyphDependencies;
@@ -471,7 +471,6 @@ void GeometryTileWorker::performSymbolLayout() {
     parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult {
         std::move(buckets),
         std::move(featureIndex),
-        *data ? (*data)->clone() : nullptr,
         std::move(glyphAtlasImage),
         std::move(iconAtlasImage)
     }, correlationID);