Further optimize cross tile symbol index findMatches

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## main
 
 ### ✨ Features and improvements
+- Speed up the cross tile symbol index in certain circumstances ([#6641](https://github.com/maplibre/maplibre-gl-js/pull/6641)) (by [@bradymadden97](https://github.com/bradymadden97))
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
@@ -10,7 +11,7 @@
 
 ### ✨ Features and improvements
 
-- Add support for MapLibre Tiles (MLT) by using `encoding: 'mlt'` in vector source definition ([#6570](https://github.com/maplibre/maplibre-gl-js/pull/6570)) (by [@Salkin975](https://github.com/Salkin975) and [@HarelM](https://github.com/HArelM)) 
+- Add support for MapLibre Tiles (MLT) by using `encoding: 'mlt'` in vector source definition ([#6570](https://github.com/maplibre/maplibre-gl-js/pull/6570)) (by [@Salkin975](https://github.com/Salkin975) and [@HarelM](https://github.com/HArelM))
 - Slice vector tiles to improve over scale vector handling ([#6521](https://github.com/maplibre/maplibre-gl-js/pull/6521)). It adds the `experimentalZoomLevelsToOverscale` flag to `MapOptions` to allow controlling how many zoom levels to slice and how many to scale. It seems to have better performance at high zoom levels. It can prevent Safari crashes in some scenarios by setting it to 4 or less. (by [@HarelM](https://github.com/HarelM))
 - Add reduceMotion option to Map Options ([#6661](https://github.com/maplibre/maplibre-gl-js/pull/6661)) (by [@wayofthefuture](https://github.com/wayofthefuture))
 

src/symbol/cross_tile_symbol_index.test.ts

@@ -223,7 +223,8 @@ describe('CrossTileSymbolIndex.addLayer', () => {
         const mainInstances: any[] = [];
         const childInstances: any[] = [];
 
-        for (let i = 0; i < KDBUSH_THRESHHOLD + 1; i++) {
+        const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1;
+        for (let i = 0; i < INSTANCE_COUNT; i++) {
             mainInstances.push(makeSymbolInstance(0, 0, ''));
             childInstances.push(makeSymbolInstance(0, 0, ''));
         }
@@ -232,9 +233,11 @@ describe('CrossTileSymbolIndex.addLayer', () => {
         index.addLayer(styleLayer, [mainTile], 0);
         index.addLayer(styleLayer, [childTile], 0);
 
-        // check that we matched the parent tile
-        expect(childInstances[0].crossTileID).toBe(1);
-
+        // all child instances matched a crossTileID from the parent, otherwise
+        // we would have generated a new crossTileID, and the number would
+        // exceed INSTANCE_COUNT
+        expect(childInstances.every(i => i.crossTileID <= INSTANCE_COUNT)).toBeTruthy();
+        expect(Math.max(...childInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT);
     });
 });
 

src/symbol/cross_tile_symbol_index.ts

@@ -28,14 +28,25 @@ const roundingFactor = 512 / EXTENT / 2;
 
 export const KDBUSH_THRESHHOLD = 128;
 
-interface SymbolsByKeyEntry {
-    index?: KDBush;
-    positions?: {x: number; y: number}[];
-    crossTileIDs: number[];
-}
+const SymbolKindType = {
+    INDEXED: 0,
+    UNINDEXED: 1,
+} as const;
+
+type IndexedSymbolKind  = {
+    readonly type: typeof SymbolKindType.INDEXED;
+    readonly index: KDBush;
+    readonly crossTileIDs: number[];
+};
+
+type UnindexedSymbolKind = {
+    readonly type: typeof SymbolKindType.UNINDEXED;
+    readonly positions: {x: number; y: number}[];
+    readonly crossTileIDs: number[];
+};
 
 class TileLayerIndex {
-    _symbolsByKey: Record<number, SymbolsByKeyEntry> = {};
+    _symbolsByKey: Record<number, IndexedSymbolKind | UnindexedSymbolKind> = {};
 
     constructor(public tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, public bucketInstanceId: number) {
         // group the symbolInstances by key
@@ -57,21 +68,17 @@ class TileLayerIndex {
         for (const [key, symbols] of symbolInstancesByKey) {
             const positions = symbols.map(symbolInstance => ({x: Math.floor(symbolInstance.anchorX * roundingFactor), y: Math.floor(symbolInstance.anchorY * roundingFactor)}));
             const crossTileIDs = symbols.map(v => v.crossTileID);
-            const entry: SymbolsByKeyEntry = {positions, crossTileIDs};
 
             // once we get too many symbols for a given key, it becomes much faster to index it before queries
-            if (entry.positions.length > KDBUSH_THRESHHOLD) {
-
-                const index = new KDBush(entry.positions.length, 16, Uint16Array);
-                for (const {x, y} of entry.positions) index.add(x, y);
+            if (positions.length > KDBUSH_THRESHHOLD) {
+                const index = new KDBush(positions.length, 16, Uint16Array);
+                for (const {x, y} of positions) index.add(x, y);
                 index.finish();
-
-                // clear all references to the original positions data
-                delete entry.positions;
-                entry.index = index;
+                this._symbolsByKey[key] = {type: SymbolKindType.INDEXED, index, crossTileIDs};
+            } else {
+                this._symbolsByKey[key] = {type: SymbolKindType.UNINDEXED, positions, crossTileIDs};
             }
 
-            this._symbolsByKey[key] = entry;
         }
     }
 
@@ -99,71 +106,136 @@ class TileLayerIndex {
         return result;
     }
 
+    getCrossTileIDsLists() {
+        return Object.values(this._symbolsByKey).map(({crossTileIDs}) => crossTileIDs);
+    }
+
     findMatches(symbolInstances: SymbolInstanceArray, newTileID: OverscaledTileID, zoomCrossTileIDs: {
         [crossTileID: number]: boolean;
     }) {
-        const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z);
-
+        // Group symbol instances by key
+        const instancesByKey = new Map<number, SymbolInstance[]>();
         for (let i = 0; i < symbolInstances.length; i++) {
             const symbolInstance = symbolInstances.get(i);
             if (symbolInstance.crossTileID) {
                 // already has a match, skip
                 continue;
             }
 
-            const entry = this._symbolsByKey[symbolInstance.key];
+            if (instancesByKey.has(symbolInstance.key)) {
+                instancesByKey.get(symbolInstance.key).push(symbolInstance);
+            } else {
+                instancesByKey.set(symbolInstance.key, [symbolInstance]);
+            }
+        }
+
+        // For each key, find the entry, then match the symbol instances
+        // to the entry contents
+        for (const key of instancesByKey.keys()) {
+            const entry = this._symbolsByKey[key];
             if (!entry) {
                 // No symbol with this key in this bucket
                 continue;
             }
 
+            const instances = instancesByKey.get(key);
+            if (entry.type === SymbolKindType.INDEXED) {
+                this.findMatchesForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs);
+            } else {
+                this.findMatchesForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs);
+            }
+        }
+    }
+
+    /**
+     * Finds matches for entries without an index built
+     */
+    private findMatchesForUnindexedEntry(
+        entry: UnindexedSymbolKind,
+        symbolInstances: SymbolInstance[],
+        newTileID: OverscaledTileID,
+        zoomCrossTileIDs: {[crossTileID: number]: boolean}
+    ) {
+        const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z);
+        for (const symbolInstance of symbolInstances) {
             const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID);
 
-            if (entry.index) {
+            for (let i = 0; i < entry.positions.length; i++) {
+                const thisTileSymbol = entry.positions[i];
+                const crossTileID = entry.crossTileIDs[i];
+
                 // Return any symbol with the same keys whose coordinates are within 1
                 // grid unit. (with a 4px grid, this covers a 12px by 12px area)
-                const indexes = entry.index.range(
-                    scaledSymbolCoord.x - tolerance,
-                    scaledSymbolCoord.y - tolerance,
-                    scaledSymbolCoord.x + tolerance,
-                    scaledSymbolCoord.y + tolerance).sort();
-
-                for (const i of indexes) {
-                    const crossTileID = entry.crossTileIDs[i];
-
-                    if (!zoomCrossTileIDs[crossTileID]) {
-                        // Once we've marked ourselves duplicate against this parent symbol,
-                        // don't let any other symbols at the same zoom level duplicate against
-                        // the same parent (see issue #5993)
-                        zoomCrossTileIDs[crossTileID] = true;
-                        symbolInstance.crossTileID = crossTileID;
-                        break;
-                    }
-                }
-            } else if (entry.positions) {
-                for (let i = 0; i < entry.positions.length; i++) {
-                    const thisTileSymbol = entry.positions[i];
-                    const crossTileID = entry.crossTileIDs[i];
-
-                    // Return any symbol with the same keys whose coordinates are within 1
-                    // grid unit. (with a 4px grid, this covers a 12px by 12px area)
-                    if (Math.abs(thisTileSymbol.x - scaledSymbolCoord.x) <= tolerance &&
+                if (Math.abs(thisTileSymbol.x - scaledSymbolCoord.x) <= tolerance &&
                         Math.abs(thisTileSymbol.y - scaledSymbolCoord.y) <= tolerance &&
                         !zoomCrossTileIDs[crossTileID]) {
-                        // Once we've marked ourselves duplicate against this parent symbol,
-                        // don't let any other symbols at the same zoom level duplicate against
-                        // the same parent (see issue #5993)
-                        zoomCrossTileIDs[crossTileID] = true;
-                        symbolInstance.crossTileID = crossTileID;
-                        break;
-                    }
+                // Once we've marked ourselves duplicate against this parent symbol,
+                // don't let any other symbols at the same zoom level duplicate against
+                // the same parent (see issue #5993)
+                    zoomCrossTileIDs[crossTileID] = true;
+                    symbolInstance.crossTileID = crossTileID;
+                    break;
                 }
             }
         }
     }
 
-    getCrossTileIDsLists() {
-        return Object.values(this._symbolsByKey).map(({crossTileIDs}) => crossTileIDs);
+    /**
+     * Find matches for entries with an index built
+     */
+    private findMatchesForIndexedEntry(
+        entry: IndexedSymbolKind,
+        symbolInstances: SymbolInstance[],
+        newTileID: OverscaledTileID,
+        zoomCrossTileIDs: {[crossTileID: number]: boolean}
+    ) {
+        const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z);
+
+        // Map instances by their scaled coordinate
+        const instancesByScaledCoordinate = new Map<number, SymbolInstance[]>();
+        for (const symbolInstance of symbolInstances) {
+            const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID);
+            const scaledCoordinateId = scaledSymbolCoord.x << 16 | scaledSymbolCoord.y;
+
+            if (instancesByScaledCoordinate.has(scaledCoordinateId)) {
+                instancesByScaledCoordinate.get(scaledCoordinateId).push(symbolInstance);
+            } else {
+                instancesByScaledCoordinate.set(scaledCoordinateId, [symbolInstance]);
+            }
+        }
+
+        // For each scaled coordinate, match instances with indexed results
+        // at the same location.
+        for (const [coordinateId, instances] of instancesByScaledCoordinate.entries()) {
+            const x = coordinateId >>> 16;
+            const y = coordinateId % (1 << 16);
+            const indexes = entry.index.range(
+                x - tolerance,
+                y - tolerance,
+                x + tolerance,
+                y + tolerance);
+
+            // Iterate through cross tile entries at this quadrant _and_
+            // symbol instances and pair them up until one of the lists
+            // runs out. This is faster than re-running a range query
+            // for every symbol instance, as each of these symbol
+            // instances already have the same key, and thus can be
+            // paired with any entry in the index.
+            let i = 0;
+            let j = 0;
+            while (i < indexes.length && j < instances.length) {
+                const crossTileID = entry.crossTileIDs[indexes[i]];
+                if (!zoomCrossTileIDs[crossTileID]) {
+                // Once we've marked ourselves duplicate against this parent symbol,
+                // don't let any other symbols at the same zoom level duplicate against
+                // the same parent (see issue #5993)
+                    zoomCrossTileIDs[crossTileID] = true;
+                    instances[j].crossTileID = crossTileID;
+                    j++;
+                }
+                i++;
+            }
+        }
     }
 }