[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.
This commit is contained in:
Chris Loer
2018-03-31 19:47:40 -07:00
committed by Chris Loer
parent 11961d5c31
commit a2399a57a2
5 changed files with 27 additions and 30 deletions

View File

@@ -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);

View File

@@ -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

View File

@@ -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();

View File

@@ -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;

View File

@@ -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);