Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve performance of symbol layers with identical or no text #12669

Merged
merged 4 commits into from
Apr 20, 2023
Merged

Improve performance of symbol layers with identical or no text #12669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
743db34
WIP speed up cross tile symbol index
mourner Apr 19, 2023
a5f96fd
fix flow
mourner Apr 19, 2023
d94b004
add a debug page for testing mass-symbol perf
mourner Apr 19, 2023
fa0deb7
smaller node size for faster index search
mourner Apr 19, 2023
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
84 changes: 84 additions & 0 deletions debug/symbols-generated.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<!DOCTYPE html>
<html>
<head>
<title>Mapbox GL JS debug page</title>
<meta charset='utf-8'>
<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
<link rel='stylesheet' href='../dist/mapbox-gl.css' />
<style>
body { margin: 0; padding: 0; }
html, body, #map { height: 100%; }
</style>
</head>

<body>
<div id='map'></div>

<script src='../dist/mapbox-gl-dev.js'></script>
<script src='../debug/access_token_generated.js'></script>
<script>

const map = new mapboxgl.Map({
container: 'map',
style: 'mapbox://styles/mapbox/satellite-v9',
center: [-79.197693, 43.832545],
zoom: 15
});

const canvas = document.createElement('canvas');
const ctx = canvas.getContext('2d', {willReadFrequently: true});

map.on('styleimagemissing', ({id}) => {
const [shape, fill, stroke] = id.split('-');
const size = 32;

ctx.canvas.width = size;
ctx.canvas.height = size;

ctx.fillStyle = fill;
ctx.strokeStyle = stroke;
ctx.lineWidth = 4;

if (shape === 'circle') {
ctx.arc(16, 16, 14, 0, Math.PI * 2);

} else if (shape === 'triangle') {
ctx.moveTo(2, 2);
ctx.lineTo(30, 2);
ctx.lineTo(16, 30);

} else if (shape === 'square') {
ctx.moveTo(4, 4);
ctx.lineTo(28, 4);
ctx.lineTo(28, 28);
ctx.lineTo(4, 28);
}
ctx.closePath();
ctx.fill();
ctx.stroke();

map.addImage(id, ctx.getImageData(0, 0, size, size), {pixelRatio: 2});
});

map.on('load', () => {
map.addSource('trees', {
'type': 'vector',
'url': 'mapbox://william-davis.b5m4mb3c'
});
map.addLayer({
'id': 'trees',
'source': 'trees',
'source-layer': 'trees',
'type': 'symbol',
'layout': {
// 'icon-image': ['concat', 'square-rgb(0,', ['*', ['round', ['get', 'd_height']], 5], ',0)-green'],
'icon-image': ['case', ['<', ['get', 'd_height'], 10], 'circle-red-white', 'triangle-blue-yellow'],
'icon-allow-overlap': true,
'icon-ignore-placement': true
}
});
});

</script>
</body>
</html>
8 changes: 8 additions & 0 deletions flow-typed/kdbush.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @flow strict
declare module 'kdbush' {
declare export default class KDBush<T> {
points: Array<T>;
constructor(points: Array<T>, getX: (T) => number, getY: (T) => number, nodeSize?: number, arrayType?: Class<$ArrayBufferView>): KDBush<T>;
range(minX: number, minY: number, maxX: number, maxY: number): Array<number>;
}
}
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"geojson-vt": "^3.2.1",
"gl-matrix": "^3.4.3",
"grid-index": "^1.1.0",
"kdbush": "^3.0.0",
"murmurhash-js": "^1.0.0",
"pbf": "^3.2.1",
"potpack": "^2.0.0",
Expand Down
4 changes: 0 additions & 4 deletions src/render/painter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {isMapAuthenticated} from '../util/mapbox.js';
import posAttributes from '../data/pos_attributes.js';
import boundsAttributes from '../data/bounds_attributes.js';
import ProgramConfiguration from '../data/program_configuration.js';
import CrossTileSymbolIndex from '../symbol/cross_tile_symbol_index.js';
import shaders from '../shaders/shaders.js';
import Program from './program.js';
import {programUniforms} from './program/program_uniforms.js';
Expand Down Expand Up @@ -147,7 +146,6 @@ class Painter {
id: string;
_showOverdrawInspector: boolean;
cache: {[_: string]: Program<*> };
crossTileSymbolIndex: CrossTileSymbolIndex;
symbolFadeChange: number;
gpuTimers: GPUTimers;
deferredRenderGpuTimeQueries: Array<any>;
Expand Down Expand Up @@ -177,8 +175,6 @@ class Painter {
this.numSublayers = SourceCache.maxUnderzooming + SourceCache.maxOverzooming + 1;
this.depthEpsilon = 1 / Math.pow(2, 16);

this.crossTileSymbolIndex = new CrossTileSymbolIndex();

this.deferredRenderGpuTimeQueries = [];
this.gpuTimers = {};
this.frameCounter = 0;
Expand Down
71 changes: 27 additions & 44 deletions src/symbol/cross_tile_symbol_index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import EXTENT from '../data/extent.js';

import {SymbolInstanceArray} from '../data/array_types.js';
import KDBush from 'kdbush';

import type Projection from '../geo/projection/projection.js';
import type {SymbolInstance} from '../data/array_types.js';
import type {OverscaledTileID} from '../source/tile_id.js';
Expand All @@ -29,33 +31,22 @@ const roundingFactor = 512 / EXTENT / 2;

class TileLayerIndex {
tileID: OverscaledTileID;
indexedSymbolInstances: {[_: number]: Array<{
crossTileID: number,
coord: {
x: number,
y: number
}
}>};
bucketInstanceId: number;
index: KDBush<{x: number, y: number, key: number, crossTileID: number}>;

constructor(tileID: OverscaledTileID, symbolInstances: SymbolInstanceArray, bucketInstanceId: number) {
this.tileID = tileID;
this.indexedSymbolInstances = {};
this.bucketInstanceId = bucketInstanceId;

const coords = [];
for (let i = 0; i < symbolInstances.length; i++) {
const symbolInstance = symbolInstances.get(i);
const key = symbolInstance.key;
if (!this.indexedSymbolInstances[key]) {
this.indexedSymbolInstances[key] = [];
}
// This tile may have multiple symbol instances with the same key
// Store each one along with its coordinates
this.indexedSymbolInstances[key].push({
crossTileID: symbolInstance.crossTileID,
coord: this.getScaledCoordinates(symbolInstance, tileID)
});
const {key, crossTileID} = symbolInstance;
const {x, y} = this.getScaledCoordinates(symbolInstance, tileID);
coords.push({x, y, key, crossTileID});
}
// create a spatial index for deduplicating symbol instances;
// use a low nodeSize because we're optimizing for search performance, not indexing
this.index = new KDBush(coords, p => p.x, p => p.y, 16, Int32Array);
}

// Converts the coordinates of the input symbol instance into coordinates that be can compared
Expand All @@ -65,16 +56,16 @@ class TileLayerIndex {
// (3) down-sampled by "roundingFactor" from tile coordinate precision in order to be
// more tolerant of small differences between tiles.
getScaledCoordinates(symbolInstance: SymbolInstance, childTileID: OverscaledTileID): {|x: number, y: number|} {
const zDifference = childTileID.canonical.z - this.tileID.canonical.z;
const scale = roundingFactor / Math.pow(2, zDifference);
const scale = roundingFactor / Math.pow(2, childTileID.canonical.z - this.tileID.canonical.z);
return {
x: Math.floor((childTileID.canonical.x * EXTENT + symbolInstance.tileAnchorX) * scale),
y: Math.floor((childTileID.canonical.y * EXTENT + symbolInstance.tileAnchorY) * scale)
};
}

findMatches(symbolInstances: SymbolInstanceArray, newTileID: OverscaledTileID, zoomCrossTileIDs: {[crossTileID: number]: boolean}) {
findMatches(symbolInstances: SymbolInstanceArray, newTileID: OverscaledTileID, zoomCrossTileIDs: Set<number>) {
const tolerance = this.tileID.canonical.z < newTileID.canonical.z ? 1 : Math.pow(2, this.tileID.canonical.z - newTileID.canonical.z);
const symbols = this.index.points;

for (let i = 0; i < symbolInstances.length; i++) {
const symbolInstance = symbolInstances.get(i);
Expand All @@ -83,25 +74,19 @@ class TileLayerIndex {
continue;
}

const indexedInstances = this.indexedSymbolInstances[symbolInstance.key];
if (!indexedInstances) {
// No symbol with this key in this bucket
continue;
}
const {x, y} = this.getScaledCoordinates(symbolInstance, newTileID);

const scaledSymbolCoord = this.getScaledCoordinates(symbolInstance, newTileID);

for (const thisTileSymbol of indexedInstances) {
// 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.coord.x - scaledSymbolCoord.x) <= tolerance &&
Math.abs(thisTileSymbol.coord.y - scaledSymbolCoord.y) <= tolerance &&
!zoomCrossTileIDs[thisTileSymbol.crossTileID]) {
// 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 matchedIds = this.index.range(x - tolerance, y - tolerance, x + tolerance, y + tolerance);
for (const id of matchedIds) {
const {key, crossTileID} = symbols[id];
if (key === symbolInstance.key && !zoomCrossTileIDs.has(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[thisTileSymbol.crossTileID] = true;
symbolInstance.crossTileID = thisTileSymbol.crossTileID;
zoomCrossTileIDs.add(crossTileID);
symbolInstance.crossTileID = crossTileID;
break;
}
}
Expand All @@ -121,7 +106,7 @@ class CrossTileIDs {

class CrossTileSymbolLayerIndex {
indexes: {[zoom: string | number]: {[tileId: string | number]: TileLayerIndex}};
usedCrossTileIDs: {[zoom: string | number]: {[crossTileID: number]: boolean}};
usedCrossTileIDs: {[zoom: string | number]: Set<number>};
lng: number;

constructor() {
Expand Down Expand Up @@ -176,7 +161,7 @@ class CrossTileSymbolLayerIndex {
}

if (!this.usedCrossTileIDs[tileID.overscaledZ]) {
this.usedCrossTileIDs[tileID.overscaledZ] = {};
this.usedCrossTileIDs[tileID.overscaledZ] = new Set();
mourner marked this conversation as resolved.
Show resolved Hide resolved
}
const zoomCrossTileIDs = this.usedCrossTileIDs[tileID.overscaledZ];

Expand All @@ -203,7 +188,7 @@ class CrossTileSymbolLayerIndex {
if (!symbolInstance.crossTileID) {
// symbol did not match any known symbol, assign a new id
symbolInstance.crossTileID = crossTileIDs.generate();
zoomCrossTileIDs[symbolInstance.crossTileID] = true;
zoomCrossTileIDs.add(symbolInstance.crossTileID);
}
}

Expand All @@ -216,10 +201,8 @@ class CrossTileSymbolLayerIndex {
}

removeBucketCrossTileIDs(zoom: string | number, removedBucket: TileLayerIndex) {
for (const key in removedBucket.indexedSymbolInstances) {
for (const symbolInstance of removedBucket.indexedSymbolInstances[(key: any)]) {
delete this.usedCrossTileIDs[zoom][symbolInstance.crossTileID];
}
for (const {crossTileID} of removedBucket.index.points) {
this.usedCrossTileIDs[zoom].delete(crossTileID);
}
}

Expand Down
10 changes: 5 additions & 5 deletions test/unit/symbol/cross_tile_symbol_index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ test('CrossTileSymbolIndex.addLayer', (t) => {
t.equal(mainInstances[1].crossTileID, 2);

const layerIndex = index.layerIndexes[styleLayer.id];
t.deepEqual(Object.keys(layerIndex.usedCrossTileIDs[6]), [1, 2]);
t.deepEqual([...layerIndex.usedCrossTileIDs[6]], [1, 2]);

// copies parent ids without duplicate ids in this tile
index.addLayer(styleLayer, [childTile], 0, {name: 'mercator'});
Expand All @@ -155,8 +155,8 @@ test('CrossTileSymbolIndex.addLayer', (t) => {
t.equal(childInstances[2].crossTileID, 3); // C' gets new ID

// Updates per-zoom usedCrossTileIDs
t.deepEqual(Object.keys(layerIndex.usedCrossTileIDs[6]), []);
t.deepEqual(Object.keys(layerIndex.usedCrossTileIDs[7]), [1, 2, 3]);
t.deepEqual([...layerIndex.usedCrossTileIDs[6]], []);
t.deepEqual([...layerIndex.usedCrossTileIDs[7]], [1, 2, 3]);

t.end();
});
Expand Down Expand Up @@ -184,15 +184,15 @@ test('CrossTileSymbolIndex.addLayer', (t) => {
t.equal(firstInstances[1].crossTileID, 2);

const layerIndex = index.layerIndexes[styleLayer.id];
t.deepEqual(Object.keys(layerIndex.usedCrossTileIDs[6]), [1, 2]);
t.deepEqual([...layerIndex.usedCrossTileIDs[6]], [1, 2]);

// uses same ids when tile gets updated
index.addLayer(styleLayer, [secondTile], 0, {name: 'mercator'});
t.equal(secondInstances[0].crossTileID, 1); // A' copies from A
t.equal(secondInstances[1].crossTileID, 2); // B' copies from B
t.equal(secondInstances[2].crossTileID, 3); // C' gets new ID

t.deepEqual(Object.keys(layerIndex.usedCrossTileIDs[6]), [1, 2, 3]);
t.deepEqual([...layerIndex.usedCrossTileIDs[6]], [1, 2, 3]);

t.end();
});
Expand Down