Skip to content
Open
Changes from 2 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
efaaea5
Optimize cross tile index searching
bradymadden97 Oct 31, 2025
c834bd2
newline
bradymadden97 Oct 31, 2025
1c0d4f2
clarify comments
bradymadden97 Nov 1, 2025
4600214
Fix iteration logic
bradymadden97 Nov 1, 2025
6dda944
Revert
bradymadden97 Nov 1, 2025
77a9904
Merge branch 'bmadden/cross-tile-symbol-index-optimization' of https:…
bradymadden97 Nov 1, 2025
91ad4b2
Add check to test
bradymadden97 Nov 1, 2025
88ae4e0
Merge branch 'bmadden/add-check-to-cross-tile-symbol-index-test' into…
bradymadden97 Nov 1, 2025
0c0f587
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 1, 2025
392f7d3
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
HarelM Nov 3, 2025
c1dc44f
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 5, 2025
99c7530
Merge branch 'bmadden/cross-tile-symbol-index-optimization' of https:…
bradymadden97 Nov 6, 2025
b0e7909
update for comments & add changelog
bradymadden97 Nov 6, 2025
1b92449
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 6, 2025
83c84a8
better perf
bradymadden97 Nov 7, 2025
a98ee11
update to test indexing code path for existing tests
bradymadden97 Nov 7, 2025
3404589
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 7, 2025
b6a37ef
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 10, 2025
1134558
refactor for clarity
bradymadden97 Nov 10, 2025
6574aee
indent
bradymadden97 Nov 10, 2025
4cf39a2
Remove double space
HarelM Nov 10, 2025
f4d0304
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 10, 2025
4beaad2
rename and types
bradymadden97 Nov 10, 2025
386288a
Merge branch 'bmadden/cross-tile-symbol-index-optimization' of https:…
bradymadden97 Nov 10, 2025
578fdd8
longer comment
bradymadden97 Nov 10, 2025
20eca82
pull out
bradymadden97 Nov 10, 2025
964916c
xy map
bradymadden97 Nov 10, 2025
4989773
add tests specifically for indexing
bradymadden97 Nov 10, 2025
a954ea8
Update CHANGELOG.md
bradymadden97 Nov 10, 2025
5c2c793
rename
bradymadden97 Nov 10, 2025
880833a
Merge branch 'main' into bmadden/cross-tile-symbol-index-optimization
bradymadden97 Nov 10, 2025
f729a85
pull out to method
bradymadden97 Nov 10, 2025
3d218f3
toBeDefined
bradymadden97 Nov 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 64 additions & 18 deletions src/symbol/cross_tile_symbol_index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number, Map<number, SymbolInstance[]>>();

for (let i = 0; i < symbolInstances.length; i++) {
const symbolInstance = symbolInstances.get(i);
if (symbolInstance.crossTileID) {
Expand All @@ -120,25 +122,21 @@ 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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to use numbers as keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't figure out another way to use the coordinates themselves as keys in a relatively efficient way

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's the tileID.key that is usually used, can it be used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This coordinate is within a given tile, so not equivalent to tileID.key I don't think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use strings maybe? I'm terrified of number overflows when I see this kind of code? How worse is the performance if this is string instead of number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to doubly nested map by x/y coordinates if that's better 964916c

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++) {
Expand All @@ -159,6 +157,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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to compare this to the previous code and I'm not sure I understand why there will be less calls to range, can you elaborate on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely! So the situation where this is faster is because we're now running a range query only once for each coordinate position that has any symbol instances within it. Then we are matching all symbol instances at that position with any results from the range query.

Previously, we would independently run a range query for every single symbol instance, and then match a result from that range query with the symbol instance.

A pathological case would be 10,000 symbol instances at the exact same coordinate. Previously that would be 10,000 range queries. Now that would be 1 range query.

Based on that algorithm, it appears to never be more range queries than before, but in bad cases it can be significantly less.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is for x and y that are exactly the same, right? If all the x and y are in slightly different places then it would behave as before, right?
I'm just trying to make sure I understand what you mean and where this will improve performance.

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++;
}
}
}
}

Expand Down