From a2399a57a22aa453dbc5a9b16244bbc953e2d632 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Sat, 31 Mar 2018 19:47:40 -0700 Subject: [core] Make FeatureIndex own GeometryTileData. 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. --- src/mbgl/geometry/feature_index.cpp | 17 ++++++++++------- src/mbgl/geometry/feature_index.hpp | 7 ++++--- src/mbgl/tile/geometry_tile.cpp | 22 ++++++++++------------ src/mbgl/tile/geometry_tile.hpp | 8 ++------ src/mbgl/tile/geometry_tile_worker.cpp | 3 +-- 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 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& 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 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& 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 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&, @@ -83,7 +84,6 @@ private: const IndexedSubfeature&, const GeometryCoordinates& queryGeometry, const RenderedQueryOptions& options, - const GeometryTileData&, const CanonicalTileID&, const std::vector&, const float bearing, @@ -93,5 +93,6 @@ private: unsigned int sortIndex = 0; std::unordered_map> bucketLayerIDs; + std::unique_ptr 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& 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> buckets; std::unique_ptr featureIndex; - std::unique_ptr tileData; optional glyphAtlasImage; optional iconAtlasImage; LayoutResult(std::unordered_map> buckets_, std::unique_ptr featureIndex_, - std::unique_ptr tileData_, optional glyphAtlasImage_, optional 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> buckets; - optional, std::unique_ptr>> dataPendingCommit; + optional> featureIndexPendingCommit; std::unique_ptr featureIndex; - std::unique_ptr data; optional glyphAtlasImage; optional 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> symbolLayoutMap; buckets.clear(); - featureIndex = std::make_unique(); + featureIndex = std::make_unique(*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); -- cgit v1.2.1