From efaaea5a2a7fa201626277cdb9f4b1f879bf11e4 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Fri, 31 Oct 2025 13:32:38 -0400 Subject: [PATCH 01/21] Optimize cross tile index searching --- src/symbol/cross_tile_symbol_index.ts | 83 +++++++++++++++++++++------ 1 file changed, 64 insertions(+), 19 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index c9d56647e6..fa989ddd81 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -104,6 +104,8 @@ class TileLayerIndex { }) { const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z); + const symbolKeyToScaledCoordinatesToSymbolInstanceMap = new Map>(); + for (let i = 0; i < symbolInstances.length; i++) { const symbolInstance = symbolInstances.get(i); if (symbolInstance.crossTileID) { @@ -118,27 +120,22 @@ class TileLayerIndex { } const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); - if (entry.index) { - // 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; + // Build a map keyed by common symbol instance keys, which are also + // used to key indexes. For each symbol index key, build a reverse map + // of the scaled coordinates back to a set of SymbolInstance that + // resolve to that coordinate. + const scaledCoordinateId = scaledSymbolCoord.x << 16 | scaledSymbolCoord.y; + const existingSymbolKey = symbolKeyToScaledCoordinatesToSymbolInstanceMap.get(symbolInstance.key); + if (existingSymbolKey) { + const existingSymbolsAtCoordinate = existingSymbolKey.get(scaledCoordinateId); + if (existingSymbolsAtCoordinate) { + existingSymbolsAtCoordinate.push(symbolInstance); + } else { + existingSymbolKey.set(scaledCoordinateId, [symbolInstance]); } + } else { + symbolKeyToScaledCoordinatesToSymbolInstanceMap.set(symbolInstance.key, new Map([[scaledCoordinateId, [symbolInstance]]])); } } else if (entry.positions) { for (let i = 0; i < entry.positions.length; i++) { @@ -159,6 +156,54 @@ class TileLayerIndex { } } } + + } + + for (const [symbolKey, coordinateMap] of symbolKeyToScaledCoordinatesToSymbolInstanceMap.entries()) { + const entry = this._symbolsByKey[symbolKey]; + if (!entry) { + // Shouldn't happen + continue; + } + + if (!entry.index){ + // Shouldn't happen + continue; + } + + for (const [coordinateId, symbolInstancesAtCoordinate] of coordinateMap.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 < symbolInstancesAtCoordinate.length) { + const crossTileID = entry.crossTileIDs[i]; + const symbolInstanceAtCoordinate = symbolInstancesAtCoordinate[j]; + + 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; + symbolInstanceAtCoordinate.crossTileID = crossTileID; + j++; + + } + i++; + } + } } } From c834bd2c6102b24d91c32062b8e879599b5bbc54 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Fri, 31 Oct 2025 13:41:48 -0400 Subject: [PATCH 02/21] newline --- src/symbol/cross_tile_symbol_index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index fa989ddd81..1200176b2c 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -120,6 +120,7 @@ class TileLayerIndex { } const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); + if (entry.index) { // Build a map keyed by common symbol instance keys, which are also // used to key indexes. For each symbol index key, build a reverse map From 1c0d4f2d429bc80a5b1852aeb50e5be2fdfc6835 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Sat, 1 Nov 2025 13:42:12 -0400 Subject: [PATCH 03/21] clarify comments --- src/symbol/cross_tile_symbol_index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 1200176b2c..682f6eee8a 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -157,18 +157,19 @@ class TileLayerIndex { } } } - } for (const [symbolKey, coordinateMap] of symbolKeyToScaledCoordinatesToSymbolInstanceMap.entries()) { const entry = this._symbolsByKey[symbolKey]; if (!entry) { - // Shouldn't happen + // Shouldn't happen, as we only populate keys in this + // map above when an entry exists. continue; } if (!entry.index){ - // Shouldn't happen + // Shouldn't happen, as we only populate keys in this + // map above when the entry has an index built. continue; } @@ -200,7 +201,6 @@ class TileLayerIndex { zoomCrossTileIDs[crossTileID] = true; symbolInstanceAtCoordinate.crossTileID = crossTileID; j++; - } i++; } From 4600214b62ec239e976b4a6bb98fd4a8f1e042ae Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Sat, 1 Nov 2025 15:37:31 -0400 Subject: [PATCH 04/21] Fix iteration logic --- src/symbol/cross_tile_symbol_index.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 682f6eee8a..5a09c427c1 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -190,16 +190,14 @@ class TileLayerIndex { // paired with any entry in the index. let i = 0; let j = 0; - while (i < indexes.length && j < symbolInstancesAtCoordinate.length) { + while (i < entry.crossTileIDs.length && j < Math.min(indexes.length, symbolInstancesAtCoordinate.length)) { const crossTileID = entry.crossTileIDs[i]; - const symbolInstanceAtCoordinate = symbolInstancesAtCoordinate[j]; - 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; - symbolInstanceAtCoordinate.crossTileID = crossTileID; + symbolInstancesAtCoordinate[j].crossTileID = crossTileID; j++; } i++; From 6dda944347406caa6520d0a1fca4dedd37951800 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Sat, 1 Nov 2025 15:51:31 -0400 Subject: [PATCH 05/21] Revert --- src/symbol/cross_tile_symbol_index.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 682f6eee8a..5a09c427c1 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -190,16 +190,14 @@ class TileLayerIndex { // paired with any entry in the index. let i = 0; let j = 0; - while (i < indexes.length && j < symbolInstancesAtCoordinate.length) { + while (i < entry.crossTileIDs.length && j < Math.min(indexes.length, symbolInstancesAtCoordinate.length)) { const crossTileID = entry.crossTileIDs[i]; - const symbolInstanceAtCoordinate = symbolInstancesAtCoordinate[j]; - 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; - symbolInstanceAtCoordinate.crossTileID = crossTileID; + symbolInstancesAtCoordinate[j].crossTileID = crossTileID; j++; } i++; From 91ad4b2b76353c49f99d05625c7f74ba31d3a4cc Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Sat, 1 Nov 2025 16:02:24 -0400 Subject: [PATCH 06/21] Add check to test --- src/symbol/cross_tile_symbol_index.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index 497ae31b51..d09baf25e8 100644 --- a/src/symbol/cross_tile_symbol_index.test.ts +++ b/src/symbol/cross_tile_symbol_index.test.ts @@ -214,7 +214,7 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test('indexes data for findMatches perf', () => { + test('matches ids when indexing', () => { const index = new CrossTileSymbolIndex(); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); @@ -223,9 +223,10 @@ describe('CrossTileSymbolIndex.addLayer', () => { const mainInstances: any[] = []; const childInstances: any[] = []; - for (let i = 0; i < KDBUSH_THRESHHOLD + 1; i++) { - mainInstances.push(makeSymbolInstance(0, 0, '')); - childInstances.push(makeSymbolInstance(0, 0, '')); + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + for (let i = 0; i < INSTANCE_COUNT; i++) { + mainInstances.push(makeSymbolInstance(i, i, '')); + childInstances.push(makeSymbolInstance(i, i, '')); } const mainTile = makeTile(mainID, mainInstances); const childTile = makeTile(childID, childInstances); @@ -234,7 +235,10 @@ describe('CrossTileSymbolIndex.addLayer', () => { // 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(Math.max(...childInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT); }); }); From b0e7909120b33ecbc252e993247a4104dad63b80 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Thu, 6 Nov 2025 18:16:49 -0500 Subject: [PATCH 07/21] update for comments & add changelog --- CHANGELOG.md | 1 + src/symbol/cross_tile_symbol_index.ts | 143 +++++++++++++++----------- 2 files changed, 85 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c899c04aec..7017576cc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### ✨ Features and improvements - 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 Safar crashes in some scenarios by setting it to 4 or less. (by [@HarelM](https://github.com/HarelM)) +- 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 diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 5a09c427c1..9fdfb8d38c 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -28,14 +28,20 @@ const roundingFactor = 512 / EXTENT / 2; export const KDBUSH_THRESHHOLD = 128; -interface SymbolsByKeyEntry { - index?: KDBush; - positions?: {x: number; y: number}[]; - crossTileIDs: number[]; +interface IndexedSymbolKind { + readonly type: 'indexed'; + readonly index: KDBush; + readonly crossTileIDs: number[]; +} + +interface UnindexedSymbolKind { + readonly type: 'unindexed'; + readonly positions: {x: number; y: number}[]; + readonly crossTileIDs: number[]; } class TileLayerIndex { - _symbolsByKey: Record = {}; + _symbolsByKey: Record = {}; constructor(public tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, public bucketInstanceId: number) { // group the symbolInstances by key @@ -57,21 +63,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: 'indexed', index, crossTileIDs}; + } else { + this._symbolsByKey[key] = {type: 'unindexed', positions, crossTileIDs}; } - this._symbolsByKey[key] = entry; } } @@ -99,12 +101,16 @@ 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); - const symbolKeyToScaledCoordinatesToSymbolInstanceMap = new Map>(); + const symbolKeyToScaledCoordinatesToSymbolInstanceMap = new Map>(); for (let i = 0; i < symbolInstances.length; i++) { const symbolInstance = symbolInstances.get(i); @@ -121,7 +127,7 @@ class TileLayerIndex { const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); - if (entry.index) { + if (entry.type === 'indexed') { // Build a map keyed by common symbol instance keys, which are also // used to key indexes. For each symbol index key, build a reverse map // of the scaled coordinates back to a set of SymbolInstance that @@ -138,7 +144,7 @@ class TileLayerIndex { } else { symbolKeyToScaledCoordinatesToSymbolInstanceMap.set(symbolInstance.key, new Map([[scaledCoordinateId, [symbolInstance]]])); } - } else if (entry.positions) { + } else { for (let i = 0; i < entry.positions.length; i++) { const thisTileSymbol = entry.positions[i]; const crossTileID = entry.crossTileIDs[i]; @@ -161,53 +167,72 @@ class TileLayerIndex { for (const [symbolKey, coordinateMap] of symbolKeyToScaledCoordinatesToSymbolInstanceMap.entries()) { const entry = this._symbolsByKey[symbolKey]; - if (!entry) { - // Shouldn't happen, as we only populate keys in this - // map above when an entry exists. - continue; + if (entry && entry.type === 'indexed') { + this.findMatchesForInstances( + entry, + coordinateMap, + zoomCrossTileIDs, + tolerance + ); } + } - if (!entry.index){ - // Shouldn't happen, as we only populate keys in this - // map above when the entry has an index built. - continue; - } + } - for (const [coordinateId, symbolInstancesAtCoordinate] of coordinateMap.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 < entry.crossTileIDs.length && j < Math.min(indexes.length, symbolInstancesAtCoordinate.length)) { - 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; - symbolInstancesAtCoordinate[j].crossTileID = crossTileID; - j++; - } - i++; - } - } + private findMatchesForInstances( + kind: IndexedSymbolKind, + coordinateMap: Map, + zoomCrossTileIDs: {[crossTileID: number]: boolean}, + tolerance: number + ) { + for (const [coordinateId, instances] of coordinateMap.entries()) { + const x = coordinateId >>> 16; + const y = coordinateId % (1 << 16); + this.findMatchesForInstancesAtCoordinate( + kind, + instances, + x, + y, + zoomCrossTileIDs, + tolerance, + ); } } - getCrossTileIDsLists() { - return Object.values(this._symbolsByKey).map(({crossTileIDs}) => crossTileIDs); + private findMatchesForInstancesAtCoordinate( + kind: IndexedSymbolKind, + instances: SymbolInstance[], + x: number, + y: number, + zoomCrossTileIDs: {[crossTileID: number]: boolean}, + tolerance: number + ) { + const indexes = kind.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 < kind.crossTileIDs.length && j < Math.min(indexes.length, instances.length)) { + const crossTileID = kind.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; + instances[j].crossTileID = crossTileID; + j++; + } + i++; + } } } From 83c84a88b54e3bd44398791d42e642fc985fca95 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Thu, 6 Nov 2025 19:36:43 -0500 Subject: [PATCH 08/21] better perf --- src/symbol/cross_tile_symbol_index.test.ts | 7 ++- src/symbol/cross_tile_symbol_index.ts | 58 +++++++++------------- 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index d09baf25e8..d85342a65f 100644 --- a/src/symbol/cross_tile_symbol_index.test.ts +++ b/src/symbol/cross_tile_symbol_index.test.ts @@ -225,19 +225,18 @@ describe('CrossTileSymbolIndex.addLayer', () => { const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; for (let i = 0; i < INSTANCE_COUNT; i++) { - mainInstances.push(makeSymbolInstance(i, i, '')); - childInstances.push(makeSymbolInstance(i, i, '')); + mainInstances.push(makeSymbolInstance(0, 0, '')); + childInstances.push(makeSymbolInstance(0, 0, '')); } const mainTile = makeTile(mainID, mainInstances); const childTile = makeTile(childID, childInstances); 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); }); }); diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 9fdfb8d38c..58818049f4 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -28,14 +28,18 @@ const roundingFactor = 512 / EXTENT / 2; export const KDBUSH_THRESHHOLD = 128; +const SymbolKindType = { + INDEXED: 0, + UNINDEXED: 1, +} as const; interface IndexedSymbolKind { - readonly type: 'indexed'; + readonly type: typeof SymbolKindType.INDEXED; readonly index: KDBush; readonly crossTileIDs: number[]; } interface UnindexedSymbolKind { - readonly type: 'unindexed'; + readonly type: typeof SymbolKindType.UNINDEXED; readonly positions: {x: number; y: number}[]; readonly crossTileIDs: number[]; } @@ -69,9 +73,9 @@ class TileLayerIndex { const index = new KDBush(positions.length, 16, Uint16Array); for (const {x, y} of positions) index.add(x, y); index.finish(); - this._symbolsByKey[key] = {type: 'indexed', index, crossTileIDs}; + this._symbolsByKey[key] = {type: SymbolKindType.INDEXED, index, crossTileIDs}; } else { - this._symbolsByKey[key] = {type: 'unindexed', positions, crossTileIDs}; + this._symbolsByKey[key] = {type: SymbolKindType.UNINDEXED, positions, crossTileIDs}; } } @@ -127,7 +131,7 @@ class TileLayerIndex { const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); - if (entry.type === 'indexed') { + if (entry.type === SymbolKindType.INDEXED) { // Build a map keyed by common symbol instance keys, which are also // used to key indexes. For each symbol index key, build a reverse map // of the scaled coordinates back to a set of SymbolInstance that @@ -167,38 +171,24 @@ class TileLayerIndex { for (const [symbolKey, coordinateMap] of symbolKeyToScaledCoordinatesToSymbolInstanceMap.entries()) { const entry = this._symbolsByKey[symbolKey]; - if (entry && entry.type === 'indexed') { - this.findMatchesForInstances( - entry, - coordinateMap, - zoomCrossTileIDs, - tolerance - ); + if (entry && entry.type === SymbolKindType.INDEXED) { + for (const [coordinateId, instances] of coordinateMap.entries()) { + const x = coordinateId >>> 16; + const y = coordinateId % (1 << 16); + this.findMatchesForInstancesAtCoordinate( + entry, + instances, + x, + y, + zoomCrossTileIDs, + tolerance, + ); + } } } } - private findMatchesForInstances( - kind: IndexedSymbolKind, - coordinateMap: Map, - zoomCrossTileIDs: {[crossTileID: number]: boolean}, - tolerance: number - ) { - for (const [coordinateId, instances] of coordinateMap.entries()) { - const x = coordinateId >>> 16; - const y = coordinateId % (1 << 16); - this.findMatchesForInstancesAtCoordinate( - kind, - instances, - x, - y, - zoomCrossTileIDs, - tolerance, - ); - } - } - private findMatchesForInstancesAtCoordinate( kind: IndexedSymbolKind, instances: SymbolInstance[], @@ -221,8 +211,8 @@ class TileLayerIndex { // paired with any entry in the index. let i = 0; let j = 0; - while (i < kind.crossTileIDs.length && j < Math.min(indexes.length, instances.length)) { - const crossTileID = kind.crossTileIDs[i]; + while (i < indexes.length && j < instances.length) { + const crossTileID = kind.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 From a98ee1118d54baee70ca11d14ca02f35680cfd7f Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Fri, 7 Nov 2025 16:26:57 -0500 Subject: [PATCH 09/21] update to test indexing code path for existing tests --- src/symbol/cross_tile_symbol_index.test.ts | 28 ++++++++++++---------- src/symbol/cross_tile_symbol_index.ts | 14 +++++------ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index d85342a65f..b906476848 100644 --- a/src/symbol/cross_tile_symbol_index.test.ts +++ b/src/symbol/cross_tile_symbol_index.test.ts @@ -1,5 +1,5 @@ import {describe, test, expect} from 'vitest'; -import {CrossTileSymbolIndex, KDBUSH_THRESHHOLD} from './cross_tile_symbol_index'; +import {CrossTileSymbolIndex} from './cross_tile_symbol_index'; import {OverscaledTileID} from '../tile/tile_id'; import {type StyleLayer} from '../style/style_layer'; @@ -30,10 +30,12 @@ const makeTile = (tileID, symbolInstances): any => { }; }; +const INDEX_THRESHOLDS = [1, 128]; + describe('CrossTileSymbolIndex.addLayer', () => { - test('matches ids', () => { - const index = new CrossTileSymbolIndex(); + test.each(INDEX_THRESHOLDS)('matches ids: indexThreshold %s', (indexThreshold) => { + const index = new CrossTileSymbolIndex(indexThreshold); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); const mainInstances = [ @@ -92,8 +94,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test('overwrites ids when re-adding', () => { - const index = new CrossTileSymbolIndex(); + test.each(INDEX_THRESHOLDS)('overwrites ids when re-adding: indexThreshold %s', (indexThreshold) => { + const index = new CrossTileSymbolIndex(indexThreshold); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); const mainInstances = [makeSymbolInstance(1000, 1000, 'Detroit')]; @@ -121,7 +123,7 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test('does not duplicate ids within one zoom level', () => { + test.each(INDEX_THRESHOLDS)('does not duplicate ids within one zoom level: indexThreshold %s', () => { const index = new CrossTileSymbolIndex(); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); @@ -159,8 +161,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test('does not regenerate ids for same zoom', () => { - const index = new CrossTileSymbolIndex(); + test.each(INDEX_THRESHOLDS)('does not regenerate ids for same zoom: indexThreshold %s', (indexThreshold) => { + const index = new CrossTileSymbolIndex(indexThreshold); const tileID = new OverscaledTileID(6, 0, 6, 8, 8); const firstInstances = [ @@ -194,8 +196,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test('reuses indexes when longitude is wrapped', () => { - const index = new CrossTileSymbolIndex(); + test.each(INDEX_THRESHOLDS)('reuses indexes when longitude is wrapped: indexThreshold %s', (indexThreshold) => { + const index = new CrossTileSymbolIndex(indexThreshold); const longitude = 370; const tileID = new OverscaledTileID(6, 1, 6, 8, 8); @@ -214,8 +216,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test('matches ids when indexing', () => { - const index = new CrossTileSymbolIndex(); + test.each(INDEX_THRESHOLDS)('matches ids when indexing: indexThreshold %s', (indexThreshold) => { + const index = new CrossTileSymbolIndex(indexThreshold); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); const childID = new OverscaledTileID(7, 0, 7, 16, 16); @@ -223,7 +225,7 @@ describe('CrossTileSymbolIndex.addLayer', () => { const mainInstances: any[] = []; const childInstances: any[] = []; - const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + const INSTANCE_COUNT = indexThreshold + 1; for (let i = 0; i < INSTANCE_COUNT; i++) { mainInstances.push(makeSymbolInstance(0, 0, '')); childInstances.push(makeSymbolInstance(0, 0, '')); diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 58818049f4..9769a23dc9 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -26,8 +26,6 @@ import type {Tile} from '../tile/tile'; // Round anchor positions to roughly 4 pixel grid const roundingFactor = 512 / EXTENT / 2; -export const KDBUSH_THRESHHOLD = 128; - const SymbolKindType = { INDEXED: 0, UNINDEXED: 1, @@ -47,7 +45,7 @@ interface UnindexedSymbolKind { class TileLayerIndex { _symbolsByKey: Record = {}; - constructor(public tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, public bucketInstanceId: number) { + constructor(public tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, public bucketInstanceId: number, readonly indexThreshold: number) { // group the symbolInstances by key const symbolInstancesByKey = new Map(); for (let i = 0; i < symbolInstances.length; i++) { @@ -69,7 +67,7 @@ class TileLayerIndex { const crossTileIDs = symbols.map(v => v.crossTileID); // once we get too many symbols for a given key, it becomes much faster to index it before queries - if (positions.length > KDBUSH_THRESHHOLD) { + if (positions.length > indexThreshold) { const index = new KDBush(positions.length, 16, Uint16Array); for (const {x, y} of positions) index.add(x, y); index.finish(); @@ -249,7 +247,7 @@ class CrossTileSymbolLayerIndex { }; lng: number; - constructor() { + constructor(private readonly indexThreshold: number) { this.indexes = {}; this.usedCrossTileIDs = {}; this.lng = 0; @@ -335,7 +333,7 @@ class CrossTileSymbolLayerIndex { if (this.indexes[tileID.overscaledZ] === undefined) { this.indexes[tileID.overscaledZ] = {}; } - this.indexes[tileID.overscaledZ][tileID.key] = new TileLayerIndex(tileID, bucket.symbolInstances, bucket.bucketInstanceId); + this.indexes[tileID.overscaledZ][tileID.key] = new TileLayerIndex(tileID, bucket.symbolInstances, bucket.bucketInstanceId, this.indexThreshold); return true; } @@ -372,7 +370,7 @@ export class CrossTileSymbolIndex { maxBucketInstanceId: number; bucketsInCurrentPlacement: {[_: number]: boolean}; - constructor() { + constructor(private readonly indexThreshold = 128) { this.layerIndexes = {}; this.crossTileIDs = new CrossTileIDs(); this.maxBucketInstanceId = 0; @@ -382,7 +380,7 @@ export class CrossTileSymbolIndex { addLayer(styleLayer: StyleLayer, tiles: Array, lng: number) { let layerIndex = this.layerIndexes[styleLayer.id]; if (layerIndex === undefined) { - layerIndex = this.layerIndexes[styleLayer.id] = new CrossTileSymbolLayerIndex(); + layerIndex = this.layerIndexes[styleLayer.id] = new CrossTileSymbolLayerIndex(this.indexThreshold); } let symbolBucketsChanged = false; From 1134558ec2697c2e8ceac0b2beee0cbb47101591 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Sun, 9 Nov 2025 23:08:04 -0500 Subject: [PATCH 10/21] refactor for clarity --- src/symbol/cross_tile_symbol_index.test.ts | 28 ++- src/symbol/cross_tile_symbol_index.ts | 201 +++++++++++---------- 2 files changed, 121 insertions(+), 108 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index b906476848..054fd57ef5 100644 --- a/src/symbol/cross_tile_symbol_index.test.ts +++ b/src/symbol/cross_tile_symbol_index.test.ts @@ -1,5 +1,5 @@ import {describe, test, expect} from 'vitest'; -import {CrossTileSymbolIndex} from './cross_tile_symbol_index'; +import {CrossTileSymbolIndex, KDBUSH_THRESHHOLD} from './cross_tile_symbol_index'; import {OverscaledTileID} from '../tile/tile_id'; import {type StyleLayer} from '../style/style_layer'; @@ -30,12 +30,10 @@ const makeTile = (tileID, symbolInstances): any => { }; }; -const INDEX_THRESHOLDS = [1, 128]; - describe('CrossTileSymbolIndex.addLayer', () => { - test.each(INDEX_THRESHOLDS)('matches ids: indexThreshold %s', (indexThreshold) => { - const index = new CrossTileSymbolIndex(indexThreshold); + test('matches ids', () => { + const index = new CrossTileSymbolIndex(); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); const mainInstances = [ @@ -94,8 +92,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test.each(INDEX_THRESHOLDS)('overwrites ids when re-adding: indexThreshold %s', (indexThreshold) => { - const index = new CrossTileSymbolIndex(indexThreshold); + test('overwrites ids when re-adding', () => { + const index = new CrossTileSymbolIndex(); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); const mainInstances = [makeSymbolInstance(1000, 1000, 'Detroit')]; @@ -123,7 +121,7 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test.each(INDEX_THRESHOLDS)('does not duplicate ids within one zoom level: indexThreshold %s', () => { + test('does not duplicate ids within one zoom level', () => { const index = new CrossTileSymbolIndex(); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); @@ -161,8 +159,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test.each(INDEX_THRESHOLDS)('does not regenerate ids for same zoom: indexThreshold %s', (indexThreshold) => { - const index = new CrossTileSymbolIndex(indexThreshold); + test('does not regenerate ids for same zoom', () => { + const index = new CrossTileSymbolIndex(); const tileID = new OverscaledTileID(6, 0, 6, 8, 8); const firstInstances = [ @@ -196,8 +194,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test.each(INDEX_THRESHOLDS)('reuses indexes when longitude is wrapped: indexThreshold %s', (indexThreshold) => { - const index = new CrossTileSymbolIndex(indexThreshold); + test('reuses indexes when longitude is wrapped', () => { + const index = new CrossTileSymbolIndex(); const longitude = 370; const tileID = new OverscaledTileID(6, 1, 6, 8, 8); @@ -216,8 +214,8 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); - test.each(INDEX_THRESHOLDS)('matches ids when indexing: indexThreshold %s', (indexThreshold) => { - const index = new CrossTileSymbolIndex(indexThreshold); + test('indexes data for findMatches perf', () => { + const index = new CrossTileSymbolIndex(); const mainID = new OverscaledTileID(6, 0, 6, 8, 8); const childID = new OverscaledTileID(7, 0, 7, 16, 16); @@ -225,7 +223,7 @@ describe('CrossTileSymbolIndex.addLayer', () => { const mainInstances: any[] = []; const childInstances: any[] = []; - const INSTANCE_COUNT = indexThreshold + 1; + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; for (let i = 0; i < INSTANCE_COUNT; i++) { mainInstances.push(makeSymbolInstance(0, 0, '')); childInstances.push(makeSymbolInstance(0, 0, '')); diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 9769a23dc9..ad5682dfed 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -26,26 +26,29 @@ import type {Tile} from '../tile/tile'; // Round anchor positions to roughly 4 pixel grid const roundingFactor = 512 / EXTENT / 2; +export const KDBUSH_THRESHHOLD = 128; + const SymbolKindType = { INDEXED: 0, UNINDEXED: 1, } as const; -interface IndexedSymbolKind { + +type IndexedSymbolKind = { readonly type: typeof SymbolKindType.INDEXED; readonly index: KDBush; readonly crossTileIDs: number[]; -} +}; -interface UnindexedSymbolKind { +type UnindexedSymbolKind = { readonly type: typeof SymbolKindType.UNINDEXED; readonly positions: {x: number; y: number}[]; readonly crossTileIDs: number[]; -} +}; class TileLayerIndex { _symbolsByKey: Record = {}; - constructor(public tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, public bucketInstanceId: number, readonly indexThreshold: number) { + constructor(public tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, public bucketInstanceId: number) { // group the symbolInstances by key const symbolInstancesByKey = new Map(); for (let i = 0; i < symbolInstances.length; i++) { @@ -67,7 +70,7 @@ class TileLayerIndex { const crossTileIDs = symbols.map(v => v.crossTileID); // once we get too many symbols for a given key, it becomes much faster to index it before queries - if (positions.length > indexThreshold) { + 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(); @@ -110,10 +113,8 @@ class TileLayerIndex { 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); - - const symbolKeyToScaledCoordinatesToSymbolInstanceMap = new Map>(); - + // Group symbol instances by key + const instancesByKey = new Map(); for (let i = 0; i < symbolInstances.length; i++) { const symbolInstance = symbolInstances.get(i); if (symbolInstance.crossTileID) { @@ -121,105 +122,119 @@ class TileLayerIndex { 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 + // No symbol with this key in this bucket continue; } - const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); - + const instances = instancesByKey.get(key); if (entry.type === SymbolKindType.INDEXED) { - // Build a map keyed by common symbol instance keys, which are also - // used to key indexes. For each symbol index key, build a reverse map - // of the scaled coordinates back to a set of SymbolInstance that - // resolve to that coordinate. - const scaledCoordinateId = scaledSymbolCoord.x << 16 | scaledSymbolCoord.y; - const existingSymbolKey = symbolKeyToScaledCoordinatesToSymbolInstanceMap.get(symbolInstance.key); - if (existingSymbolKey) { - const existingSymbolsAtCoordinate = existingSymbolKey.get(scaledCoordinateId); - if (existingSymbolsAtCoordinate) { - existingSymbolsAtCoordinate.push(symbolInstance); - } else { - existingSymbolKey.set(scaledCoordinateId, [symbolInstance]); - } - } else { - symbolKeyToScaledCoordinatesToSymbolInstanceMap.set(symbolInstance.key, new Map([[scaledCoordinateId, [symbolInstance]]])); - } + this.findMatchesForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs); } else { - for (let i = 0; i < entry.positions.length; i++) { - const thisTileSymbol = entry.positions[i]; - const crossTileID = entry.crossTileIDs[i]; + this.findMatchesForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs); + } + } + } - // 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 && + /** + * 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); + + 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 && 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; } } } + } - for (const [symbolKey, coordinateMap] of symbolKeyToScaledCoordinatesToSymbolInstanceMap.entries()) { - const entry = this._symbolsByKey[symbolKey]; - if (entry && entry.type === SymbolKindType.INDEXED) { - for (const [coordinateId, instances] of coordinateMap.entries()) { - const x = coordinateId >>> 16; - const y = coordinateId % (1 << 16); - this.findMatchesForInstancesAtCoordinate( - entry, - instances, - x, - y, - zoomCrossTileIDs, - tolerance, - ); - } + /** + * 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(); + 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]); } } - } - - private findMatchesForInstancesAtCoordinate( - kind: IndexedSymbolKind, - instances: SymbolInstance[], - x: number, - y: number, - zoomCrossTileIDs: {[crossTileID: number]: boolean}, - tolerance: number - ) { - const indexes = kind.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 = kind.crossTileIDs[indexes[i]]; - if (!zoomCrossTileIDs[crossTileID]) { + // 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++; + zoomCrossTileIDs[crossTileID] = true; + instances[j].crossTileID = crossTileID; + j++; + } + i++; } - i++; } } } @@ -247,7 +262,7 @@ class CrossTileSymbolLayerIndex { }; lng: number; - constructor(private readonly indexThreshold: number) { + constructor() { this.indexes = {}; this.usedCrossTileIDs = {}; this.lng = 0; @@ -333,7 +348,7 @@ class CrossTileSymbolLayerIndex { if (this.indexes[tileID.overscaledZ] === undefined) { this.indexes[tileID.overscaledZ] = {}; } - this.indexes[tileID.overscaledZ][tileID.key] = new TileLayerIndex(tileID, bucket.symbolInstances, bucket.bucketInstanceId, this.indexThreshold); + this.indexes[tileID.overscaledZ][tileID.key] = new TileLayerIndex(tileID, bucket.symbolInstances, bucket.bucketInstanceId); return true; } @@ -370,7 +385,7 @@ export class CrossTileSymbolIndex { maxBucketInstanceId: number; bucketsInCurrentPlacement: {[_: number]: boolean}; - constructor(private readonly indexThreshold = 128) { + constructor() { this.layerIndexes = {}; this.crossTileIDs = new CrossTileIDs(); this.maxBucketInstanceId = 0; @@ -380,7 +395,7 @@ export class CrossTileSymbolIndex { addLayer(styleLayer: StyleLayer, tiles: Array, lng: number) { let layerIndex = this.layerIndexes[styleLayer.id]; if (layerIndex === undefined) { - layerIndex = this.layerIndexes[styleLayer.id] = new CrossTileSymbolLayerIndex(this.indexThreshold); + layerIndex = this.layerIndexes[styleLayer.id] = new CrossTileSymbolLayerIndex(); } let symbolBucketsChanged = false; From 6574aee2d9ef0abbf116c69a28682768296b4ba8 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Sun, 9 Nov 2025 23:10:27 -0500 Subject: [PATCH 11/21] indent --- src/symbol/cross_tile_symbol_index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index ad5682dfed..de190891e1 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -134,7 +134,7 @@ class TileLayerIndex { for (const key of instancesByKey.keys()) { const entry = this._symbolsByKey[key]; if (!entry) { - // No symbol with this key in this bucket + // No symbol with this key in this bucket continue; } From 4cf39a2bf69f0e3a6ba4dd5626da6a3d75b284b9 Mon Sep 17 00:00:00 2001 From: Harel M Date: Mon, 10 Nov 2025 11:31:52 +0200 Subject: [PATCH 12/21] Remove double space --- src/symbol/cross_tile_symbol_index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index de190891e1..0edcc29c3b 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -192,7 +192,7 @@ class TileLayerIndex { 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(); + const instancesByScaledCoordinate = new Map(); for (const symbolInstance of symbolInstances) { const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); const scaledCoordinateId = scaledSymbolCoord.x << 16 | scaledSymbolCoord.y; From 4beaad2248ab88e91d671d68fccfa9b15dd6521e Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 10:53:53 -0500 Subject: [PATCH 13/21] rename and types --- src/symbol/cross_tile_symbol_index.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index de190891e1..52b152895e 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -140,9 +140,9 @@ class TileLayerIndex { const instances = instancesByKey.get(key); if (entry.type === SymbolKindType.INDEXED) { - this.findMatchesForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs); + this.matchForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs); } else { - this.findMatchesForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs); + this.matchForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs); } } } @@ -150,12 +150,12 @@ class TileLayerIndex { /** * Finds matches for entries without an index built */ - private findMatchesForUnindexedEntry( + private matchForUnindexedEntry( entry: UnindexedSymbolKind, symbolInstances: SymbolInstance[], newTileID: OverscaledTileID, zoomCrossTileIDs: {[crossTileID: number]: boolean} - ) { + ): void { 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); @@ -183,12 +183,12 @@ class TileLayerIndex { /** * Find matches for entries with an index built */ - private findMatchesForIndexedEntry( + private matchForIndexedEntry( entry: IndexedSymbolKind, symbolInstances: SymbolInstance[], newTileID: OverscaledTileID, zoomCrossTileIDs: {[crossTileID: number]: boolean} - ) { + ): void { 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 From 578fdd8a89626434c6225cee0e6a562378aa8e97 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 10:57:28 -0500 Subject: [PATCH 14/21] longer comment --- src/symbol/cross_tile_symbol_index.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 1cde596eb0..38b3b4c531 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -148,7 +148,10 @@ class TileLayerIndex { } /** - * Finds matches for entries without an index built + * Finds matches for entries without an index built. This method modifies + * instances within {@link symbolInstances} as well as adds entries to + * {@link zoomCrossTileIDs} when it finds a symbol within {@link entry} + * that can be matched with a {@link symbolInstances} instance. */ private matchForUnindexedEntry( entry: UnindexedSymbolKind, @@ -181,7 +184,11 @@ class TileLayerIndex { } /** - * Find matches for entries with an index built + * Finds matches for entries with an index built. This method modifies + * instances within {@link symbolInstances} as well as adds entries to + * {@link zoomCrossTileIDs} when it finds a symbol from the index within + * {@link entry} that can be matched with a {@link symbolInstances} + * instance. */ private matchForIndexedEntry( entry: IndexedSymbolKind, From 20eca8219772f19c9d1f1a8859231492a03fe6f5 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 10:58:59 -0500 Subject: [PATCH 15/21] pull out --- src/symbol/cross_tile_symbol_index.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 38b3b4c531..2cfdbde47f 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -129,6 +129,8 @@ class TileLayerIndex { } } + const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z); + // For each key, find the entry, then match the symbol instances // to the entry contents for (const key of instancesByKey.keys()) { @@ -140,9 +142,9 @@ class TileLayerIndex { const instances = instancesByKey.get(key); if (entry.type === SymbolKindType.INDEXED) { - this.matchForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs); + this.matchForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs, tolerance); } else { - this.matchForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs); + this.matchForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs, tolerance); } } } @@ -157,9 +159,9 @@ class TileLayerIndex { entry: UnindexedSymbolKind, symbolInstances: SymbolInstance[], newTileID: OverscaledTileID, - zoomCrossTileIDs: {[crossTileID: number]: boolean} + zoomCrossTileIDs: {[crossTileID: number]: boolean}, + tolerance: number ): void { - 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); @@ -194,10 +196,9 @@ class TileLayerIndex { entry: IndexedSymbolKind, symbolInstances: SymbolInstance[], newTileID: OverscaledTileID, - zoomCrossTileIDs: {[crossTileID: number]: boolean} + zoomCrossTileIDs: {[crossTileID: number]: boolean}, + tolerance: number ): void { - 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(); for (const symbolInstance of symbolInstances) { From 964916cbdc502fe687bf3a0ffa75a4188ac8c810 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 11:09:34 -0500 Subject: [PATCH 16/21] xy map --- src/symbol/cross_tile_symbol_index.ts | 75 +++++++++++++++------------ 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 2cfdbde47f..0d278334c8 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -199,49 +199,58 @@ class TileLayerIndex { zoomCrossTileIDs: {[crossTileID: number]: boolean}, tolerance: number ): void { - // Map instances by their scaled coordinate - const instancesByScaledCoordinate = new Map(); + // Map instances by their scaled coordinate. The map is keyed by X, + // then keyed by Y coordinate. + const instancesByScaledCoordinate = new Map>(); 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); + const xMap = instancesByScaledCoordinate.get(scaledSymbolCoord.x); + if (xMap) { + const yMap = xMap.get(scaledSymbolCoord.y); + if (yMap) { + yMap.push(symbolInstance); + } else { + xMap.set(scaledSymbolCoord.y, [symbolInstance]); + } } else { - instancesByScaledCoordinate.set(scaledCoordinateId, [symbolInstance]); + instancesByScaledCoordinate.set( + scaledSymbolCoord.x, + new Map([[scaledSymbolCoord.y, [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++; + for (const [x, yMap] of instancesByScaledCoordinate.entries()) { + for (const [y, instances] of yMap.entries()) { + 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++; } - i++; } } } From 4989773bb7b3424b1caa80b2ecc0967e9907d368 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 12:18:16 -0500 Subject: [PATCH 17/21] add tests specifically for indexing --- src/symbol/cross_tile_symbol_index.test.ts | 207 +++++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/src/symbol/cross_tile_symbol_index.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index 054fd57ef5..91375d195f 100644 --- a/src/symbol/cross_tile_symbol_index.test.ts +++ b/src/symbol/cross_tile_symbol_index.test.ts @@ -214,6 +214,213 @@ describe('CrossTileSymbolIndex.addLayer', () => { }); +}); + +describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () => { + test('matches ids', () => { + const index = new CrossTileSymbolIndex(); + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + + const mainID = new OverscaledTileID(6, 0, 6, 8, 8); + const mainInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(1000, 1000, 'Detroit')); + mainInstances.push(makeSymbolInstance(2000, 2000, 'Toronto')); + const mainTile = makeTile(mainID, mainInstances); + + index.addLayer(styleLayer, [mainTile], 0); + // Assigned new IDs + for (let i = 1; i <= INSTANCE_COUNT + 1; i++) { + expect(mainInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + + const childID = new OverscaledTileID(7, 0, 7, 16, 16); + const childInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(2000, 2000, 'Detroit')); + childInstances.push(makeSymbolInstance(2000, 2000, 'Windsor')); + childInstances.push(makeSymbolInstance(3000, 3000, 'Toronto')); + childInstances.push(makeSymbolInstance(4001, 4001, 'Toronto')); + const childTile = makeTile(childID, childInstances); + + index.addLayer(styleLayer, [mainTile, childTile], 0); + // matched parent tile for all Detroit + const detroitChildren = childInstances.filter(i => i.key === 'Detroit'); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + expect(detroitChildren.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + + // does not match Windsor because of different key + const windsorInstance = childInstances.find(i => i.key === 'Windsor'); + expect(windsorInstance.crossTileID).toEqual(131); + + // does not match Toronto @ 3000 because of different location + const toronto3000Instance = childInstances.find(i => i.key === 'Toronto' && i.anchorX === 3000); + expect(toronto3000Instance.crossTileID).toEqual(132); + + // matches Toronto @ 4001 even though it has a slightly updated location + const toronto4001Instance = childInstances.find(i => i.key === 'Toronto' && i.anchorX === 4001); + expect(toronto4001Instance.crossTileID).toBeLessThanOrEqual(INSTANCE_COUNT + 1); + + const parentID = new OverscaledTileID(5, 0, 5, 4, 4); + const parentInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(500, 500, 'Detroit')); + const parentTile = makeTile(parentID, parentInstances); + + index.addLayer(styleLayer, [mainTile, childTile, parentTile], 0); + // matched Detroit children tiles from parent + for (let i = 1; i < INSTANCE_COUNT; i++) { + expect(parentInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + + const grandchildID = new OverscaledTileID(8, 0, 8, 32, 32); + const grandchildInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(4000, 4000, 'Detroit')); + grandchildInstances.push(makeSymbolInstance(4000, 4000, 'Windsor')); + const grandchildTile = makeTile(grandchildID, grandchildInstances); + + index.addLayer(styleLayer, [mainTile], 0); + index.addLayer(styleLayer, [mainTile, grandchildTile], 0); + // matches Detroit grandchildren with mainBucket + const detroitGrandchildren = grandchildInstances.filter(i => i.key === 'Detroit'); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + expect(detroitGrandchildren.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + + // Does not match the Windsor value because that was removed + const windsorGrandchild = grandchildInstances.find(i => i.key === 'Windsor'); + expect(windsorGrandchild.crossTileID).toEqual(133); + }); + + test('overwrites ids when re-adding', () => { + const index = new CrossTileSymbolIndex(); + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + + const mainID = new OverscaledTileID(6, 0, 6, 8, 8); + const mainInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(1000, 1000, 'Detroit')); + const mainTile = makeTile(mainID, mainInstances); + + const childID = new OverscaledTileID(7, 0, 7, 16, 16); + const childInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(2000, 2000, 'Detroit')); + const childTile = makeTile(childID, childInstances); + + // Assigns new ids 1 -> INSTANCE_COUNT + index.addLayer(styleLayer, [mainTile], 0); + expect(Math.max(...mainInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT); + + // Removes the layer + index.addLayer(styleLayer, [], 0); + + // Assigns new ids INSTANCE_COUNT + 1 -> 2 * INSTANCE_COUNT + index.addLayer(styleLayer, [childTile], 0); + expect(Math.min(...childInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); + expect(Math.max(...childInstances.map(i => i.crossTileID))).toBe(2 * INSTANCE_COUNT); + + // Expect all to have a crossTileID + expect(mainInstances.some(i => i.crossTileID === 0)).toBeFalsy(); + expect(childInstances.some(i => i.crossTileID === 0)).toBeFalsy(); + + // Overwrites the old id to match the already-added tile + index.addLayer(styleLayer, [mainTile, childTile], 0); + expect(Math.min(...mainInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); + expect(Math.max(...mainInstances.map(i => i.crossTileID))).toBe(2 * INSTANCE_COUNT); + expect(Math.min(...childInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); + expect(Math.max(...childInstances.map(i => i.crossTileID))).toBe(2 * INSTANCE_COUNT); + }); + + test('does not duplicate ids within one zoom level', () => { + const index = new CrossTileSymbolIndex(); + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + + const mainID = new OverscaledTileID(6, 0, 6, 8, 8); + const mainInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(1000, 1000, '')); + const mainTile = makeTile(mainID, mainInstances); + + const childID = new OverscaledTileID(7, 0, 7, 16, 16); + const childInstances = Array.from({length: INSTANCE_COUNT + 1}, () => makeSymbolInstance(2000, 2000, '')); + const childTile = makeTile(childID, childInstances); + + // Assigns new ids 1 -> INSTANCE_COUNT + index.addLayer(styleLayer, [mainTile], 0); + expect(mainInstances.some(i => i.crossTileID === 0)).toBeFalsy(); + expect(Math.min(...mainInstances.map(i => i.crossTileID))).toBe(1); + expect(Math.max(...mainInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT); + + const layerIndex = index.layerIndexes[styleLayer.id]; + expect(Object.keys(layerIndex.usedCrossTileIDs[6]).length).toEqual(INSTANCE_COUNT); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + expect(layerIndex.usedCrossTileIDs[6][String(i)]).not.toBeUndefined(); + } + + // copies parent ids without duplicate ids in this tile + index.addLayer(styleLayer, [childTile], 0); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + // 1 -> INSTANCE_COUNT are copied + expect(childInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + // We have one new key generated for INSTANCE_COUNT + 1 + expect(Math.max(...childInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); + + // Updates per-zoom usedCrossTileIDs + expect(Object.keys(layerIndex.usedCrossTileIDs[6])).toEqual([]); + for (let i = 1; i <= INSTANCE_COUNT + 1; i++) { + expect(layerIndex.usedCrossTileIDs[7][String(i)]).not.toBeUndefined(); + } + }); + + test('does not regenerate ids for same zoom', () => { + const index = new CrossTileSymbolIndex(); + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + + const tileID = new OverscaledTileID(6, 0, 6, 8, 8); + const firstInstances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(1000, 1000, '')); + const firstTile = makeTile(tileID, firstInstances); + + const secondInstances = Array.from({length: INSTANCE_COUNT + 1}, () => makeSymbolInstance(1000, 1000, '')); + const secondTile = makeTile(tileID, secondInstances); + + // Assigns new ids 1 -> INSTANCE_COUNT + index.addLayer(styleLayer, [firstTile], 0); + expect(firstInstances.some(i => i.crossTileID === 0)).toBeFalsy(); + expect(Math.min(...firstInstances.map(i => i.crossTileID))).toBe(1); + expect(Math.max(...firstInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT); + + const layerIndex = index.layerIndexes[styleLayer.id]; + for (let i = 1; i <= INSTANCE_COUNT; i++) { + expect(layerIndex.usedCrossTileIDs[6][String(i)]).not.toBeUndefined(); + } + + // Uses same ids when tile gets updated + index.addLayer(styleLayer, [secondTile], 0); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + // 1 -> INSTANCE_COUNT are copied + expect(secondInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + // We have one new key generated for INSTANCE_COUNT + 1 + expect(Math.max(...secondInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); + + // Updates usedCrossTileIDs + for (let i = 1; i <= INSTANCE_COUNT + 1; i++) { + expect(layerIndex.usedCrossTileIDs[6][String(i)]).not.toBeUndefined(); + } + }); + + test('reuses indexes when longitude is wrapped', () => { + const index = new CrossTileSymbolIndex(); + const INSTANCE_COUNT = KDBUSH_THRESHHOLD + 1; + const longitude = 370; + + const tileID = new OverscaledTileID(6, 1, 6, 8, 8); + const instances = Array.from({length: INSTANCE_COUNT}, () => makeSymbolInstance(1000, 1000, '')); + const tile = makeTile(tileID, instances); + + index.addLayer(styleLayer, [tile], longitude); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + expect(instances.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + + tile.tileID = tileID.wrapped(); + + index.addLayer(styleLayer, [tile], longitude % 360); + for (let i = 1; i <= INSTANCE_COUNT; i++) { + expect(instances.find(j => j.crossTileID === i)).not.toBeUndefined(); + } + }); + test('indexes data for findMatches perf', () => { const index = new CrossTileSymbolIndex(); From a954ea8d0fbae6c0b175db15f664c627febf6a33 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 12:21:33 -0500 Subject: [PATCH 18/21] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd0bc5bed6..e6265d82b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,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)) From 5c2c7937d5925bba3edc22b0dd3058905fcb693f Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 16:22:12 -0500 Subject: [PATCH 19/21] rename --- package-lock.json | 37 +++++---------------------- src/symbol/cross_tile_symbol_index.ts | 12 ++++----- 2 files changed, 12 insertions(+), 37 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7d02b07d0a..af251c408b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -469,8 +469,7 @@ "resolved": "https://registry.npmjs.org/@cspell/dict-css/-/dict-css-4.0.18.tgz", "integrity": "sha512-EF77RqROHL+4LhMGW5NTeKqfUd/e4OOv6EDFQ/UQQiFyWuqkEKyEz0NDILxOFxWUEVdjT2GQ2cC7t12B6pESwg==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@cspell/dict-dart": { "version": "2.3.1", @@ -610,16 +609,14 @@ "resolved": "https://registry.npmjs.org/@cspell/dict-html/-/dict-html-4.0.12.tgz", "integrity": "sha512-JFffQ1dDVEyJq6tCDWv0r/RqkdSnV43P2F/3jJ9rwLgdsOIXwQbXrz6QDlvQLVvNSnORH9KjDtenFTGDyzfCaA==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@cspell/dict-html-symbol-entities": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/@cspell/dict-html-symbol-entities/-/dict-html-symbol-entities-4.0.4.tgz", "integrity": "sha512-afea+0rGPDeOV9gdO06UW183Qg6wRhWVkgCFwiO3bDupAoyXRuvupbb5nUyqSTsLXIKL8u8uXQlJ9pkz07oVXw==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@cspell/dict-java": { "version": "5.0.12", @@ -817,8 +814,7 @@ "resolved": "https://registry.npmjs.org/@cspell/dict-typescript/-/dict-typescript-3.2.3.tgz", "integrity": "sha512-zXh1wYsNljQZfWWdSPYwQhpwiuW0KPW1dSd8idjMRvSD0aSvWWHoWlrMsmZeRl4qM4QCEAjua8+cjflm41cQBg==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@cspell/dict-vue": { "version": "3.0.5", @@ -983,7 +979,6 @@ } ], "license": "MIT", - "peer": true, "engines": { "node": ">=18" }, @@ -1027,7 +1022,6 @@ } ], "license": "MIT", - "peer": true, "engines": { "node": ">=18" } @@ -3992,7 +3986,6 @@ "integrity": "sha512-qzQZRBqkFsYyaSWXuEHc2WR9c0a0CXwiE5FWUvn7ZM+vdy1uZLfCunD38UzhuB7YN/J11ndbDBcTmOdxJo9Q7A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -4030,7 +4023,6 @@ "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4196,7 +4188,6 @@ "integrity": "sha512-6m1I5RmHBGTnUGS113G04DMu3CpSdxCAU/UvtjNWL4Nuf3MW9tQhiJqRlHzChIkhy6kZSAQmc+I1bcGjE3yNKg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.46.3", "@typescript-eslint/types": "8.46.3", @@ -4562,7 +4553,6 @@ "integrity": "sha512-aIFPci9xoTmVkxpqsSKcRG/Hn0lTy421jsCehHydYeIMd+getn0Pue0JqY5cW8yZglZjMeX0YfIy5wDtQDHEcA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "4.0.7", "fflate": "^0.8.2", @@ -4612,7 +4602,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -5344,7 +5333,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.19", "caniuse-lite": "^1.0.30001751", @@ -5536,7 +5524,6 @@ "dev": true, "hasInstallScript": true, "license": "MIT", - "peer": true, "dependencies": { "node-addon-api": "^7.0.0", "prebuild-install": "^7.1.3" @@ -6703,7 +6690,6 @@ "integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==", "dev": true, "license": "ISC", - "peer": true, "engines": { "node": ">=12" } @@ -7046,8 +7032,7 @@ "resolved": "https://registry.npmjs.org/devtools-protocol/-/devtools-protocol-0.0.1541592.tgz", "integrity": "sha512-FUUSjWbtgaCWSQ9bDC7mXEM4pL/V6GZ9bv05E33+/7TD1DFg3rJVTw3kxx+NPyzrgFfwneOGLEJ7MYoLpcLm/g==", "dev": true, - "license": "BSD-3-Clause", - "peer": true + "license": "BSD-3-Clause" }, "node_modules/diff": { "version": "8.0.2", @@ -7543,7 +7528,6 @@ "integrity": "sha512-BhHmn2yNOFA9H9JmmIVKJmd288g9hrVRDkdoIgRCRuSySRUHH7r/DI6aAXW9T1WwUuY3DFgrcaqB+deURBLR5g==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -11765,7 +11749,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -12365,7 +12348,6 @@ "integrity": "sha512-8sLjZwK0R+JlxlYcTuVnyT2v+htpdrjDOKuMcOVdYjt52Lh8hWRYpxBPoKx/Zg+bcjc3wx6fmQevMmUztS/ccA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "cssesc": "^3.0.0", "util-deprecate": "^1.0.2" @@ -12742,7 +12724,6 @@ "integrity": "sha512-tmbWg6W31tQLeB5cdIBOicJDJRR2KzXsV7uSK9iNfLWQ5bIZfxuPEHp7M8wiHyHnn0DD1i7w3Zmin0FtkrwoCQ==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -12978,7 +12959,6 @@ "integrity": "sha512-3GuObel8h7Kqdjt0gxkEzaifHTqLVW56Y/bjN7PSQtkKr0w3V/QYSdt6QWYtd7A1xUtYQigtdUfgj1RvWVtorw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/estree": "1.0.8" }, @@ -14774,8 +14754,7 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", "dev": true, - "license": "0BSD", - "peer": true + "license": "0BSD" }, "node_modules/tunnel-agent": { "version": "0.6.0", @@ -14904,7 +14883,6 @@ "integrity": "sha512-ftJYPvpVfQvFzpkoSfHLkJybdA/geDJ8BGQt/ZnkkhnBYoYW6lBgPQXu6vqLxO4X75dA55hX8Af847H5KXlEFA==", "dev": true, "license": "Apache-2.0", - "peer": true, "dependencies": { "@gerrit0/mini-shiki": "^3.12.0", "lunr": "^2.3.9", @@ -14942,7 +14920,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -15066,7 +15043,6 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -15142,7 +15118,6 @@ "integrity": "sha512-xQroKAadK503CrmbzCISvQUjeuvEZzv6U0wlnlVFOi5i3gnzfH4onyQ29f3lzpe0FresAiTAd3aqK0Bi/jLI8w==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/expect": "4.0.7", "@vitest/mocker": "4.0.7", diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 0d278334c8..58e07504a6 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -205,13 +205,13 @@ class TileLayerIndex { for (const symbolInstance of symbolInstances) { const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID); - const xMap = instancesByScaledCoordinate.get(scaledSymbolCoord.x); - if (xMap) { - const yMap = xMap.get(scaledSymbolCoord.y); - if (yMap) { - yMap.push(symbolInstance); + const xInstances = instancesByScaledCoordinate.get(scaledSymbolCoord.x); + if (xInstances) { + const xyInstances = xInstances.get(scaledSymbolCoord.y); + if (xyInstances) { + xyInstances.push(symbolInstance); } else { - xMap.set(scaledSymbolCoord.y, [symbolInstance]); + xInstances.set(scaledSymbolCoord.y, [symbolInstance]); } } else { instancesByScaledCoordinate.set( From f729a857f9062fdbf3483bbf83af70ef8f205726 Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 16:27:51 -0500 Subject: [PATCH 20/21] pull out to method --- src/symbol/cross_tile_symbol_index.ts | 41 ++++++++++++++++----------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index 58e07504a6..e0c25e1dfc 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/src/symbol/cross_tile_symbol_index.ts @@ -113,23 +113,8 @@ class TileLayerIndex { findMatches(symbolInstances: SymbolInstanceArray, newTileID: OverscaledTileID, zoomCrossTileIDs: { [crossTileID: number]: boolean; }) { - // Group symbol instances by key - const instancesByKey = new Map(); - for (let i = 0; i < symbolInstances.length; i++) { - const symbolInstance = symbolInstances.get(i); - if (symbolInstance.crossTileID) { - // already has a match, skip - continue; - } - - if (instancesByKey.has(symbolInstance.key)) { - instancesByKey.get(symbolInstance.key).push(symbolInstance); - } else { - instancesByKey.set(symbolInstance.key, [symbolInstance]); - } - } - const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z); + const instancesByKey = this.groupSymbolInstancesByKey(symbolInstances); // For each key, find the entry, then match the symbol instances // to the entry contents @@ -149,6 +134,30 @@ class TileLayerIndex { } } + /** + * Groups all symbol instances by common key. + * + * @returns A map keyed by {@link SymbolInstance.key} to an array of + * {@link SymbolInstance}. + */ + private groupSymbolInstancesByKey(symbolInstances: SymbolInstanceArray): Map { + const instancesByKey = new Map(); + for (let i = 0; i < symbolInstances.length; i++) { + const symbolInstance = symbolInstances.get(i); + if (symbolInstance.crossTileID) { + // already has a match, skip + continue; + } + + if (instancesByKey.has(symbolInstance.key)) { + instancesByKey.get(symbolInstance.key).push(symbolInstance); + } else { + instancesByKey.set(symbolInstance.key, [symbolInstance]); + } + } + return instancesByKey; + } + /** * Finds matches for entries without an index built. This method modifies * instances within {@link symbolInstances} as well as adds entries to From 3d218f37d258077ad3ac9a6491271f980c61fe4c Mon Sep 17 00:00:00 2001 From: Brady Madden Date: Mon, 10 Nov 2025 16:29:54 -0500 Subject: [PATCH 21/21] toBeDefined --- src/symbol/cross_tile_symbol_index.test.ts | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/symbol/cross_tile_symbol_index.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index 91375d195f..113aa9bdbb 100644 --- a/src/symbol/cross_tile_symbol_index.test.ts +++ b/src/symbol/cross_tile_symbol_index.test.ts @@ -229,7 +229,7 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = index.addLayer(styleLayer, [mainTile], 0); // Assigned new IDs for (let i = 1; i <= INSTANCE_COUNT + 1; i++) { - expect(mainInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(mainInstances.find(j => j.crossTileID === i)).toBeDefined(); } const childID = new OverscaledTileID(7, 0, 7, 16, 16); @@ -243,7 +243,7 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = // matched parent tile for all Detroit const detroitChildren = childInstances.filter(i => i.key === 'Detroit'); for (let i = 1; i <= INSTANCE_COUNT; i++) { - expect(detroitChildren.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(detroitChildren.find(j => j.crossTileID === i)).toBeDefined(); } // does not match Windsor because of different key @@ -265,7 +265,7 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = index.addLayer(styleLayer, [mainTile, childTile, parentTile], 0); // matched Detroit children tiles from parent for (let i = 1; i < INSTANCE_COUNT; i++) { - expect(parentInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(parentInstances.find(j => j.crossTileID === i)).toBeDefined(); } const grandchildID = new OverscaledTileID(8, 0, 8, 32, 32); @@ -278,7 +278,7 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = // matches Detroit grandchildren with mainBucket const detroitGrandchildren = grandchildInstances.filter(i => i.key === 'Detroit'); for (let i = 1; i <= INSTANCE_COUNT; i++) { - expect(detroitGrandchildren.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(detroitGrandchildren.find(j => j.crossTileID === i)).toBeDefined(); } // Does not match the Windsor value because that was removed @@ -343,14 +343,14 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = const layerIndex = index.layerIndexes[styleLayer.id]; expect(Object.keys(layerIndex.usedCrossTileIDs[6]).length).toEqual(INSTANCE_COUNT); for (let i = 1; i <= INSTANCE_COUNT; i++) { - expect(layerIndex.usedCrossTileIDs[6][String(i)]).not.toBeUndefined(); + expect(layerIndex.usedCrossTileIDs[6][String(i)]).toBeDefined(); } // copies parent ids without duplicate ids in this tile index.addLayer(styleLayer, [childTile], 0); for (let i = 1; i <= INSTANCE_COUNT; i++) { // 1 -> INSTANCE_COUNT are copied - expect(childInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(childInstances.find(j => j.crossTileID === i)).toBeDefined(); } // We have one new key generated for INSTANCE_COUNT + 1 expect(Math.max(...childInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); @@ -358,7 +358,7 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = // Updates per-zoom usedCrossTileIDs expect(Object.keys(layerIndex.usedCrossTileIDs[6])).toEqual([]); for (let i = 1; i <= INSTANCE_COUNT + 1; i++) { - expect(layerIndex.usedCrossTileIDs[7][String(i)]).not.toBeUndefined(); + expect(layerIndex.usedCrossTileIDs[7][String(i)]).toBeDefined(); } }); @@ -381,21 +381,21 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = const layerIndex = index.layerIndexes[styleLayer.id]; for (let i = 1; i <= INSTANCE_COUNT; i++) { - expect(layerIndex.usedCrossTileIDs[6][String(i)]).not.toBeUndefined(); + expect(layerIndex.usedCrossTileIDs[6][String(i)]).toBeDefined(); } // Uses same ids when tile gets updated index.addLayer(styleLayer, [secondTile], 0); for (let i = 1; i <= INSTANCE_COUNT; i++) { // 1 -> INSTANCE_COUNT are copied - expect(secondInstances.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(secondInstances.find(j => j.crossTileID === i)).toBeDefined(); } // We have one new key generated for INSTANCE_COUNT + 1 expect(Math.max(...secondInstances.map(i => i.crossTileID))).toBe(INSTANCE_COUNT + 1); // Updates usedCrossTileIDs for (let i = 1; i <= INSTANCE_COUNT + 1; i++) { - expect(layerIndex.usedCrossTileIDs[6][String(i)]).not.toBeUndefined(); + expect(layerIndex.usedCrossTileIDs[6][String(i)]).toBeDefined(); } }); @@ -410,14 +410,14 @@ describe('CrossTileSymbolIndex.addLayer with a scale that causes indexing', () = index.addLayer(styleLayer, [tile], longitude); for (let i = 1; i <= INSTANCE_COUNT; i++) { - expect(instances.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(instances.find(j => j.crossTileID === i)).toBeDefined(); } tile.tileID = tileID.wrapped(); index.addLayer(styleLayer, [tile], longitude % 360); for (let i = 1; i <= INSTANCE_COUNT; i++) { - expect(instances.find(j => j.crossTileID === i)).not.toBeUndefined(); + expect(instances.find(j => j.crossTileID === i)).toBeDefined(); } });