add stopgap to for querying updated symbol buckets

The index relied on a source layer and index to identify a feature. If a data layer gets updated a new bucket is formed where these identifiers probably don't match the previous bucket. Since there is a gap between a bucket being loaded and the index being updated (during the next placement) it was possible for a query to return wrong results. This prevents it from returning incorrect features but it does not return the correct features. This might cause it to return empty results.
mapbox · Jan 8, 2018 · b0aa49b · b0aa49b
1 parent c86dd66
commit b0aa49b
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 11 deletions.
diff --git a/src/data/feature_index.js b/src/data/feature_index.js
@@ -32,6 +32,7 @@ type QueryParameters = {
     },
     collisionBoxArray: CollisionBoxArray,
     sourceID: string,
+    bucketInstanceIds: { [number]: boolean },
     collisionIndex: CollisionIndex
 }
 
@@ -121,7 +122,7 @@ class FeatureIndex {
         this.filterMatching(result, matching, this.featureIndexArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits);
 
         const matchingSymbols = args.collisionIndex ?
-            args.collisionIndex.queryRenderedSymbols(queryGeometry, this.tileID, EXTENT / args.tileSize, args.collisionBoxArray, args.sourceID) :
+            args.collisionIndex.queryRenderedSymbols(queryGeometry, this.tileID, EXTENT / args.tileSize, args.collisionBoxArray, args.sourceID, args.bucketInstanceIds) :
             [];
         matchingSymbols.sort();
         this.filterMatching(result, matchingSymbols, args.collisionBoxArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits);

diff --git a/src/source/tile.js b/src/source/tile.js
@@ -8,6 +8,7 @@ const Protobuf = require('pbf');
 const GeoJSONFeature = require('../util/vectortile_to_geojson');
 const featureFilter = require('../style-spec/feature_filter');
 const CollisionIndex = require('../symbol/collision_index');
+const SymbolBucket = require('../data/bucket/symbol_bucket');
 const {
     RasterBoundsArray,
     CollisionBoxArray
@@ -230,10 +231,14 @@ class Tile {
 
         // Determine the additional radius needed factoring in property functions
         let additionalRadius = 0;
+        const bucketInstanceIds = {};
         for (const id in layers) {
             const bucket = this.getBucket(layers[id]);
             if (bucket) {
                 additionalRadius = Math.max(additionalRadius, layers[id].queryRadius(bucket));
+                if (bucket instanceof SymbolBucket) {
+                    bucketInstanceIds[bucket.bucketInstanceId] = true;
+                }
             }
         }
 
@@ -246,7 +251,8 @@ class Tile {
             additionalRadius: additionalRadius,
             collisionBoxArray: this.collisionBoxArray,
             sourceID: sourceID,
-            collisionIndex: collisionIndex
+            collisionIndex: collisionIndex,
+            bucketInstanceIds: bucketInstanceIds
         }, layers);
     }
 

diff --git a/src/symbol/collision_index.js b/src/symbol/collision_index.js
@@ -245,7 +245,7 @@ class CollisionIndex {
      *
      * @private
      */
-    queryRenderedSymbols(queryGeometry: any, tileCoord: OverscaledTileID, textPixelRatio: number, collisionBoxArray: CollisionBoxArray, sourceID: string) {
+    queryRenderedSymbols(queryGeometry: any, tileCoord: OverscaledTileID, textPixelRatio: number, collisionBoxArray: CollisionBoxArray, sourceID: string, bucketInstanceIds: {[number]: boolean}) {
         const sourceLayerFeatures = {};
         const result = [];
 
@@ -277,7 +277,7 @@ class CollisionIndex {
         const thisTileFeatures = [];
         const features = this.grid.query(minX, minY, maxX, maxY);
         for (let i = 0; i < features.length; i++) {
-            if (features[i].sourceID === sourceID && features[i].tileID === tileID) {
+            if (features[i].sourceID === sourceID && features[i].tileID === tileID && bucketInstanceIds[features[i].bucketInstanceId]) {
                 thisTileFeatures.push(features[i].boxIndex);
             }
         }
@@ -333,18 +333,18 @@ class CollisionIndex {
         return result;
     }
 
-    insertCollisionBox(collisionBox: Array<number>, ignorePlacement: boolean, tileID: number, sourceID: string, boxStartIndex: number) {
+    insertCollisionBox(collisionBox: Array<number>, ignorePlacement: boolean, tileID: number, sourceID: string, bucketInstanceId: number, boxStartIndex: number) {
         const grid = ignorePlacement ? this.ignoredGrid : this.grid;
 
-        const key = { tileID: tileID, sourceID: sourceID, boxIndex: boxStartIndex };
+        const key = { tileID: tileID, sourceID: sourceID, bucketInstanceId: bucketInstanceId, boxIndex: boxStartIndex };
         grid.insert(key, collisionBox[0], collisionBox[1], collisionBox[2], collisionBox[3]);
     }
 
-    insertCollisionCircles(collisionCircles: Array<number>, ignorePlacement: boolean, tileID: number, sourceID: string, boxStartIndex: number) {
+    insertCollisionCircles(collisionCircles: Array<number>, ignorePlacement: boolean, tileID: number, sourceID: string, bucketInstanceId: number, boxStartIndex: number) {
         const grid = ignorePlacement ? this.ignoredGrid : this.grid;
 
         for (let k = 0; k < collisionCircles.length; k += 4) {
-            const key = { tileID: tileID, sourceID: sourceID, boxIndex: boxStartIndex + collisionCircles[k + 3] };
+            const key = { tileID: tileID, sourceID: sourceID, bucketInstanceId: bucketInstanceId, boxIndex: boxStartIndex + collisionCircles[k + 3] };
             grid.insertCircle(key, collisionCircles[k], collisionCircles[k + 1], collisionCircles[k + 2]);
         }
     }

diff --git a/src/symbol/placement.js b/src/symbol/placement.js
@@ -181,16 +181,20 @@ class Placement {
                 }
 
                 if (placeText && placedGlyphBoxes) {
-                    this.collisionIndex.insertCollisionBox(placedGlyphBoxes.box, layout.get('text-ignore-placement'), tileKey, sourceID, symbolInstance.textBoxStartIndex);
+                    this.collisionIndex.insertCollisionBox(placedGlyphBoxes.box, layout.get('text-ignore-placement'),
+                            tileKey, sourceID, bucket.bucketInstanceId, symbolInstance.textBoxStartIndex);
                 }
                 if (placeIcon && placedIconBoxes) {
-                    this.collisionIndex.insertCollisionBox(placedIconBoxes.box, layout.get('icon-ignore-placement'), tileKey, sourceID, symbolInstance.iconBoxStartIndex);
+                    this.collisionIndex.insertCollisionBox(placedIconBoxes.box, layout.get('icon-ignore-placement'),
+                            tileKey, sourceID, bucket.bucketInstanceId, symbolInstance.iconBoxStartIndex);
                 }
                 if (placeText && placedGlyphCircles) {
-                    this.collisionIndex.insertCollisionCircles(placedGlyphCircles.circles, layout.get('text-ignore-placement'), tileKey, sourceID, symbolInstance.textBoxStartIndex);
+                    this.collisionIndex.insertCollisionCircles(placedGlyphCircles.circles, layout.get('text-ignore-placement'),
+                            tileKey, sourceID, bucket.bucketInstanceId, symbolInstance.textBoxStartIndex);
                 }
 
                 assert(symbolInstance.crossTileID !== 0);
+                assert(bucket.bucketInstanceId !== 0);
 
                 this.placements[symbolInstance.crossTileID] = new JointPlacement(placeText, placeIcon, offscreen || justReloaded);
                 seenCrossTileIDs[symbolInstance.crossTileID] = true;