Fix queryRenderedFeatures race conditions.

Addresses hover flicker from issues #5887 and #5506.
Moves all data necessary for symbol querying into FeatureIndex.
Retains old FeatureIndexes if they're associated with a bucket that was used in the current placement.

src/data/feature_index.js

@@ -17,10 +17,9 @@ import { register } from '../util/web_worker_transfer';
 import type CollisionIndex from '../symbol/collision_index';
 import type StyleLayer from '../style/style_layer';
 import type {FeatureFilter} from '../style-spec/feature_filter';
-import type {CollisionBoxArray} from './array_types';
 import type Transform from '../geo/transform';
 
-import { FeatureIndexArray } from './array_types';
+import { FeatureIndexArray, CollisionBoxArray } from './array_types';
 
 type QueryParameters = {
     scale: number,
@@ -33,20 +32,19 @@ type QueryParameters = {
         filter: FilterSpecification,
         layers: Array<string>,
     },
-    collisionBoxArray: CollisionBoxArray,
     sourceID: string,
     bucketInstanceIds: { [number]: boolean },
     collisionIndex: ?CollisionIndex
 }
 
 class FeatureIndex {
     tileID: OverscaledTileID;
-    overscaling: number;
     x: number;
     y: number;
     z: number;
     grid: Grid;
     featureIndexArray: FeatureIndexArray;
+    collisionBoxArray: CollisionBoxArray;
 
     rawTileData: ArrayBuffer;
     bucketLayerIDs: Array<Array<string>>;
@@ -55,16 +53,16 @@ class FeatureIndex {
     sourceLayerCoder: DictionaryCoder;
 
     constructor(tileID: OverscaledTileID,
-                overscaling: number,
                 grid?: Grid,
-                featureIndexArray?: FeatureIndexArray) {
+                featureIndexArray?: FeatureIndexArray,
+                collisionBoxArray?: CollisionBoxArray) {
         this.tileID = tileID;
-        this.overscaling = overscaling;
         this.x = tileID.canonical.x;
         this.y = tileID.canonical.y;
         this.z = tileID.canonical.z;
         this.grid = grid || new Grid(EXTENT, 16, 0);
         this.featureIndexArray = featureIndexArray || new FeatureIndexArray();
+        this.collisionBoxArray = collisionBoxArray || new CollisionBoxArray();
     }
 
     insert(feature: VectorTileFeature, geometry: Array<Array<Point>>, featureIndex: number, sourceLayerIndex: number, bucketIndex: number) {
@@ -93,47 +91,47 @@ class FeatureIndex {
     }
 
     // Finds features in this tile at a particular position.
-    query(args: QueryParameters, styleLayers: {[string]: StyleLayer}) {
+    query(result: {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>}, args: QueryParameters, styleLayers: {[string]: StyleLayer}) {
         if (!this.vtLayers) {
             this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
             this.sourceLayerCoder = new DictionaryCoder(this.vtLayers ? Object.keys(this.vtLayers).sort() : ['_geojsonTileLayer']);
         }
 
-        const result = {};
-
         const params = args.params || {},
             pixelsToTileUnits = EXTENT / args.tileSize / args.scale,
             filter = featureFilter(params.filter);
 
         const queryGeometry = args.queryGeometry;
-        const queryPadding = args.queryPadding * pixelsToTileUnits;
-
-        let minX = Infinity;
-        let minY = Infinity;
-        let maxX = -Infinity;
-        let maxY = -Infinity;
-        for (let i = 0; i < queryGeometry.length; i++) {
-            const ring = queryGeometry[i];
-            for (let k = 0; k < ring.length; k++) {
-                const p = ring[k];
-                minX = Math.min(minX, p.x);
-                minY = Math.min(minY, p.y);
-                maxX = Math.max(maxX, p.x);
-                maxY = Math.max(maxY, p.y);
-            }
-        }
-
-        const matching = this.grid.query(minX - queryPadding, minY - queryPadding, maxX + queryPadding, maxY + queryPadding);
-        matching.sort(topDownFeatureComparator);
-        this.filterMatching(result, matching, this.featureIndexArray, queryGeometry, filter, params.layers, styleLayers, pixelsToTileUnits, args.posMatrix, args.transform);
 
-        const matchingSymbols = args.collisionIndex ?
-            args.collisionIndex.queryRenderedSymbols(queryGeometry, this.tileID, args.tileSize / EXTENT, args.collisionBoxArray, args.sourceID, args.bucketInstanceIds) :
-            [];
-        matchingSymbols.sort();
-        this.filterMatching(result, matchingSymbols, args.collisionBoxArray, queryGeometry, filter, params.layers, styleLayers, pixelsToTileUnits, args.posMatrix, args.transform);
+        if (args.collisionIndex) {
+            // Querying symbol features
+            const matchingSymbols =
+                args.collisionIndex.queryRenderedSymbols(queryGeometry, this.tileID, args.tileSize / EXTENT, this.collisionBoxArray, args.sourceID, args.bucketInstanceIds);
+            matchingSymbols.sort();
+            this.filterMatching(result, matchingSymbols, this.collisionBoxArray, queryGeometry, filter, params.layers, styleLayers, pixelsToTileUnits, args.posMatrix, args.transform);
+        } else {
+            // Querying non-symbol features
+            const queryPadding = args.queryPadding * pixelsToTileUnits;
+
+            let minX = Infinity;
+            let minY = Infinity;
+            let maxX = -Infinity;
+            let maxY = -Infinity;
+            for (let i = 0; i < queryGeometry.length; i++) {
+                const ring = queryGeometry[i];
+                for (let k = 0; k < ring.length; k++) {
+                    const p = ring[k];
+                    minX = Math.min(minX, p.x);
+                    minY = Math.min(minY, p.y);
+                    maxX = Math.max(maxX, p.x);
+                    maxY = Math.max(maxY, p.y);
+                }
+            }
 
-        return result;
+            const matching = this.grid.query(minX - queryPadding, minY - queryPadding, maxX + queryPadding, maxY + queryPadding);
+            matching.sort(topDownFeatureComparator);
+            this.filterMatching(result, matching, this.featureIndexArray, queryGeometry, filter, params.layers, styleLayers, pixelsToTileUnits, args.posMatrix, args.transform);
+        }
     }
 
     filterMatching(

src/source/geojson_source.js

@@ -238,7 +238,6 @@ class GeoJSONSource extends Evented implements Source {
             tileSize: this.tileSize,
             source: this.id,
             pixelRatio: browser.devicePixelRatio,
-            overscaling: tile.tileID.overscaleFactor(),
             showCollisionBoxes: this.map.showCollisionBoxes
         };
 

src/source/source_cache.js

@@ -757,6 +757,14 @@ class SourceCache extends Evented {
 
         return false;
     }
+
+    commitPlacement(usedBucketInstanceIds: {[number]: boolean}) {
+        const ids = this.getIds();
+        for (let i = 0; i < ids.length; i++) {
+            const tile = this.getTileByID(ids[i]);
+            tile.pruneFeatureIndexes(usedBucketInstanceIds);
+        }
+    }
 }
 
 SourceCache.maxOverzooming = 10;
@@ -779,4 +787,3 @@ function isRasterType(type) {
 }
 
 export default SourceCache;
-

src/source/tile.js

@@ -9,7 +9,7 @@ import GeoJSONFeature from '../util/vectortile_to_geojson';
 import featureFilter from '../style-spec/feature_filter';
 import CollisionIndex from '../symbol/collision_index';
 import SymbolBucket from '../data/bucket/symbol_bucket';
-import { RasterBoundsArray, CollisionBoxArray } from '../data/array_types';
+import { RasterBoundsArray } from '../data/array_types';
 import rasterBoundsAttributes from '../data/raster_bounds_attributes';
 import EXTENT from '../data/extent';
 import Point from '@mapbox/point-geometry';
@@ -55,6 +55,9 @@ class Tile {
     uses: number;
     tileSize: number;
     buckets: {[string]: Bucket};
+    latestFeatureIndex: ?FeatureIndex;
+    latestRawTileData: ?ArrayBuffer;
+    retainedFeatureIndexes: {[number]: FeatureIndex};
     iconAtlasImage: ?RGBAImage;
     iconAtlasTexture: Texture;
     glyphAtlasImage: ?AlphaImage;
@@ -64,10 +67,6 @@ class Tile {
     state: TileState;
     timeAdded: any;
     fadeEndTime: any;
-    rawTileData: ArrayBuffer;
-    collisionBoxArray: ?CollisionBoxArray;
-    collisionIndex: ?CollisionIndex;
-    featureIndex: ?FeatureIndex;
     redoWhenDone: boolean;
     showCollisionBoxes: boolean;
     placementSource: any;
@@ -111,6 +110,8 @@ class Tile {
         this.expiredRequestCount = 0;
 
         this.state = 'loading';
+
+        this.retainedFeatureIndexes = {};
     }
 
     registerFadeDuration(duration: number) {
@@ -144,17 +145,23 @@ class Tile {
 
         // empty GeoJSON tile
         if (!data) {
-            this.collisionBoxArray = new CollisionBoxArray();
+            this.latestFeatureIndex = new FeatureIndex(this.tileID);
             return;
         }
 
-        if (data.rawTileData) {
-            // Only vector tiles have rawTileData
-            this.rawTileData = data.rawTileData;
+        if (data.featureIndex) {
+            this.latestFeatureIndex = data.featureIndex;
+            if (data.rawTileData) {
+                // Only vector tiles have rawTileData, and they won't update it for
+                // 'reloadTile'
+                this.latestRawTileData = data.rawTileData;
+                this.latestFeatureIndex.rawTileData = data.rawTileData;
+            } else if (this.latestRawTileData) {
+                // If rawTileData hasn't updated, hold onto a pointer to the last
+                // one we received
+                this.latestFeatureIndex.rawTileData = this.latestRawTileData;
+            }
         }
-        this.collisionBoxArray = data.collisionBoxArray;
-        this.featureIndex = data.featureIndex;
-        this.featureIndex.rawTileData = this.rawTileData;
         this.buckets = deserializeBucket(data.buckets, painter.style);
 
         if (justReloaded) {
@@ -198,8 +205,7 @@ class Tile {
             this.glyphAtlasTexture.destroy();
         }
 
-        this.collisionBoxArray = null;
-        this.featureIndex = null;
+        this.latestFeatureIndex = null;
         this.state = 'unloaded';
     }
 
@@ -244,46 +250,72 @@ class Tile {
                           posMatrix: Float32Array,
                           sourceID: string,
                           collisionIndex: ?CollisionIndex): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} {
-        if (!this.featureIndex || !this.collisionBoxArray)
+        if (!this.latestFeatureIndex || !this.latestFeatureIndex.rawTileData)
             return {};
 
-        // Create the set of the current bucket instance ids
-        const bucketInstanceIds = {};
-        for (const id in layers) {
-            const bucket = this.getBucket(layers[id]);
-            if (bucket) {
-                // Add the bucket instance's id to the set of current ids.
-                // The query will only include results from current buckets.
-                if (bucket instanceof SymbolBucket && bucket.bucketInstanceId !== undefined) {
-                    bucketInstanceIds[bucket.bucketInstanceId] = true;
-                }
-            }
-        }
-
-        return this.featureIndex.query({
+        // First, query non-symbol features in the latest feature index
+        const results = {};
+        this.latestFeatureIndex.query(results, {
             queryGeometry: queryGeometry,
             scale: scale,
             tileSize: this.tileSize,
             posMatrix: posMatrix,
             transform: transform,
             params: params,
             queryPadding: this.queryPadding * maxPitchScaleFactor,
-            collisionBoxArray: this.collisionBoxArray,
             sourceID: sourceID,
-            collisionIndex: collisionIndex,
-            bucketInstanceIds: bucketInstanceIds
+            collisionIndex: null,
+            bucketInstanceIds: {}
         }, layers);
+
+        // Build up set of unique featureIndexes used by different buckets
+        const symbolFeatureIndexes = [];
+        for (const bucketInstanceString in this.retainedFeatureIndexes) {
+            const bucketInstanceId = parseInt(bucketInstanceString); // For flow -- what's a good way to use object keys _as if_ they're really numbers?
+            const featureIndex = this.retainedFeatureIndexes[bucketInstanceId];
+            const existingIndex = symbolFeatureIndexes.indexOf(featureIndex);
+            if (existingIndex === -1) {
+                const bucketIds = {};
+                bucketIds[bucketInstanceId] = true;
+                symbolFeatureIndexes.push({ index: featureIndex, bucketIds: bucketIds});
+            } else {
+                symbolFeatureIndexes[existingIndex].bucketIds[bucketInstanceId] = true;
+            }
+        }
+
+        for (const symbolFeatureIndex of symbolFeatureIndexes) {
+            if (!symbolFeatureIndex.index.rawTileData) {
+                continue;
+            }
+
+            symbolFeatureIndex.index.query(results, {
+                queryGeometry: queryGeometry,
+                scale: scale,
+                tileSize: this.tileSize,
+                posMatrix: posMatrix,
+                transform: transform,
+                params: params,
+                queryPadding: this.queryPadding * maxPitchScaleFactor,
+                sourceID: sourceID,
+                collisionIndex: collisionIndex,
+                bucketInstanceIds: symbolFeatureIndex.bucketIds
+            }, layers);
+        }
+
+        return results;
     }
 
     querySourceFeatures(result: Array<GeoJSONFeature>, params: any) {
-        if (!this.rawTileData) return;
+        if (!this.latestFeatureIndex || !this.latestFeatureIndex.rawTileData) return;
 
-        if (!this.vtLayers) {
-            this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
+        if (!this.latestFeatureIndex.vtLayers) {
+            this.latestFeatureIndex.vtLayers =
+                new vt.VectorTile(new Protobuf(this.latestFeatureIndex.rawTileData)).layers;
         }
+        const vtLayers = this.latestFeatureIndex.vtLayers;
 
         const sourceLayer = params ? params.sourceLayer : '';
-        const layer = this.vtLayers._geojsonTileLayer || this.vtLayers[sourceLayer];
+        const layer = vtLayers._geojsonTileLayer || vtLayers[sourceLayer];
 
         if (!layer) return;
 
@@ -427,6 +459,17 @@ class Tile {
             }
         }
     }
+
+    pruneFeatureIndexes(usedBucketInstanceIds: {[number]: boolean}) {
+        for (const bucketInstanceId of Object.keys(this.retainedFeatureIndexes).map(Number)) {
+            if (!usedBucketInstanceIds[bucketInstanceId]) {
+                // This bucket was no longer used in the latest placement,
+                // so discard the associated querying data if this was the
+                // last bucket to use it.
+                delete this.retainedFeatureIndexes[bucketInstanceId];
+            }
+        }
+    }
 }
 
 export default Tile;

src/source/vector_tile_source.js

@@ -93,18 +93,16 @@ class VectorTileSource extends Evented implements Source {
     }
 
     loadTile(tile: Tile, callback: Callback<void>) {
-        const overscaling = tile.tileID.overscaleFactor();
         const url = normalizeURL(tile.tileID.canonical.url(this.tiles, this.scheme), this.url);
         const params = {
             request: this.map._transformRequest(url, ResourceType.Tile),
             uid: tile.uid,
             tileID: tile.tileID,
             zoom: tile.tileID.overscaledZ,
-            tileSize: this.tileSize * overscaling,
+            tileSize: this.tileSize * tile.tileID.overscaleFactor(),
             type: this.type,
             source: this.id,
             pixelRatio: browser.devicePixelRatio,
-            overscaling: overscaling,
             showCollisionBoxes: this.map.showCollisionBoxes,
         };
         params.request.collectResourceTiming = this._collectResourceTiming;

src/source/worker_source.js

@@ -5,7 +5,6 @@ import type {RGBAImage, AlphaImage} from '../util/image';
 import type {OverscaledTileID} from './tile_id';
 import type {Bucket} from '../data/bucket';
 import type FeatureIndex from '../data/feature_index';
-import type {CollisionBoxArray} from '../data/array_types';
 import type DEMData from '../data/dem_data';
 import type {PerformanceResourceTiming} from '../types/performance_resource_timing';
 
@@ -21,7 +20,6 @@ export type WorkerTileParameters = TileParameters & {
     maxZoom: number,
     tileSize: number,
     pixelRatio: number,
-    overscaling: number,
     showCollisionBoxes: boolean,
     collectResourceTiming?: boolean
 };
@@ -37,7 +35,6 @@ export type WorkerTileResult = {
     iconAtlasImage: RGBAImage,
     glyphAtlasImage: AlphaImage,
     featureIndex: FeatureIndex,
-    collisionBoxArray: CollisionBoxArray,
     rawTileData?: ArrayBuffer,
     resourceTiming?: Array<PerformanceResourceTiming>
 };