diff --git a/CHANGELOG.md b/CHANGELOG.md index 966b4a93810..f9c64ec0005 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### ✨ Features and improvements - Add `padding` option to `Popup` class to prevent popups from being positioned too close to map container edges ([#5978](https://github.com/maplibre/maplibre-gl-js/issues/5978)) (by [@yuiseki](https://github.com/yuiseki) and [@lucaswoj](https://github.com/lucaswoj)) +- 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.test.ts b/src/symbol/cross_tile_symbol_index.test.ts index 497ae31b515..113aa9bdbbe 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)).toBeDefined(); + } + + 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)).toBeDefined(); + } + + // 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)).toBeDefined(); + } + + 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)).toBeDefined(); + } + + // 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)]).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)).toBeDefined(); + } + // 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)]).toBeDefined(); + } + }); + + 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)]).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)).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)]).toBeDefined(); + } + }); + + 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)).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)).toBeDefined(); + } + }); + test('indexes data for findMatches perf', () => { const index = new CrossTileSymbolIndex(); @@ -223,7 +430,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 +440,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); }); }); diff --git a/src/symbol/cross_tile_symbol_index.ts b/src/symbol/cross_tile_symbol_index.ts index c9d56647e63..e0c25e1dfc4 100644 --- a/src/symbol/cross_tile_symbol_index.ts +++ b/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 = {}; + _symbolsByKey: Record = {}; 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,11 +106,42 @@ 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 instancesByKey = this.groupSymbolInstancesByKey(symbolInstances); + // 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.matchForIndexedEntry(entry, instances, newTileID, zoomCrossTileIDs, tolerance); + } else { + this.matchForUnindexedEntry(entry, instances, newTileID, zoomCrossTileIDs, tolerance); + } + } + } + + /** + * 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) { @@ -111,60 +149,120 @@ class TileLayerIndex { continue; } - const entry = this._symbolsByKey[symbolInstance.key]; - if (!entry) { - // No symbol with this key in this bucket - 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 + * {@link zoomCrossTileIDs} when it finds a symbol within {@link entry} + * that can be matched with a {@link symbolInstances} instance. + */ + private matchForUnindexedEntry( + entry: UnindexedSymbolKind, + symbolInstances: SymbolInstance[], + newTileID: OverscaledTileID, + zoomCrossTileIDs: {[crossTileID: number]: boolean}, + tolerance: number + ): void { + 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(); + 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; + } + } + } + } - for (const i of indexes) { - const crossTileID = entry.crossTileIDs[i]; + /** + * 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, + symbolInstances: SymbolInstance[], + newTileID: OverscaledTileID, + zoomCrossTileIDs: {[crossTileID: number]: boolean}, + tolerance: number + ): void { + // 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); - 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; - } + const xInstances = instancesByScaledCoordinate.get(scaledSymbolCoord.x); + if (xInstances) { + const xyInstances = xInstances.get(scaledSymbolCoord.y); + if (xyInstances) { + xyInstances.push(symbolInstance); + } else { + xInstances.set(scaledSymbolCoord.y, [symbolInstance]); } - } 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 && - Math.abs(thisTileSymbol.y - scaledSymbolCoord.y) <= tolerance && - !zoomCrossTileIDs[crossTileID]) { + } else { + instancesByScaledCoordinate.set( + scaledSymbolCoord.x, + new Map([[scaledSymbolCoord.y, [symbolInstance]]]) + ); + } + } + + // For each scaled coordinate, match instances with indexed results + // at the same location. + 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; - symbolInstance.crossTileID = crossTileID; - break; + instances[j].crossTileID = crossTileID; + j++; } + i++; } } } } - - getCrossTileIDsLists() { - return Object.values(this._symbolsByKey).map(({crossTileIDs}) => crossTileIDs); - } } class CrossTileIDs { diff --git a/test/build/bundle_size.json b/test/build/bundle_size.json index 9478a5e4a8b..300c3a80316 100644 --- a/test/build/bundle_size.json +++ b/test/build/bundle_size.json @@ -1 +1 @@ -1014715 +1016271 \ No newline at end of file