Skip to content

Quadtree performance #12907

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+94 −75
Open

Quadtree performance #12907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b7dd01
Improves performance of quadtree customdata
mzschwartz5 Sep 22, 2025
327a3e6
Changes quadtree tile customdata from array to set
mzschwartz5 Sep 22, 2025
7a98a4d
CHANGES.md for quadtree
mzschwartz5 Sep 22, 2025
7de48c2
Changes customData to set in GlobeSurfaceTile
mzschwartz5 Sep 22, 2025
6febd2b
Removes rectangle check from quadtree tile for performance
mzschwartz5 Sep 26, 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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Improved performance and reduced memory usage of `Event` class. [#12896](https://github.com/CesiumGS/cesium/pull/12896)
- Fixes vertical misalignment of glyphs in labels with small fonts [#8474](https://github.com/CesiumGS/cesium/issues/8474)
- Prevent runtime errors for certain forms of invalid PNTS files [#12872](https://github.com/CesiumGS/cesium/issues/12872)
- Improved performance of terrain Quadtree handling of custom data [#12907](https://github.com/CesiumGS/cesium/pull/12907)

#### Additions :tada:

Expand Down
4 changes: 1 addition & 3 deletions packages/engine/Source/Scene/GlobeSurfaceTile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,8 @@ GlobeSurfaceTile.prototype.updateExaggeration = function (
if (quadtree !== undefined) {
quadtree._tileToUpdateHeights.push(tile);
const customData = tile.customData;
const customDataLength = customData.length;
for (let i = 0; i < customDataLength; i++) {
for (const data of customData) {
// Restart the level so that a height update is triggered
const data = customData[i];
data.level = -1;
}
}
Expand Down
60 changes: 34 additions & 26 deletions packages/engine/Source/Scene/QuadtreePrimitive.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ function QuadtreePrimitive(options) {
this._removeHeightCallbacks = [];

this._tileToUpdateHeights = [];
this._lastTileIndex = 0;
this._updateHeightsTimeSlice = 2.0;

// If a culled tile contains _cameraPositionCartographic or _cameraReferenceFrameOriginCartographic, it will be marked
Expand Down Expand Up @@ -217,10 +216,8 @@ function invalidateAllTiles(primitive) {
for (let i = 0; i < levelZeroTiles.length; ++i) {
const tile = levelZeroTiles[i];
const customData = tile.customData;
const customDataLength = customData.length;

for (let j = 0; j < customDataLength; ++j) {
const data = customData[j];
for (const data of customData) {
data.level = 0;
primitive._addHeightCallbacks.push(data);
}
Expand Down Expand Up @@ -513,7 +510,6 @@ function selectTilesForRendering(primitive, frameState) {
tilesToRender.length = 0;

// We can't render anything before the level zero tiles exist.
let i;
const tileProvider = primitive._tileProvider;
if (!defined(primitive._levelZeroTiles)) {
const tilingScheme = tileProvider.tilingScheme;
Expand All @@ -524,7 +520,7 @@ function selectTilesForRendering(primitive, frameState) {
const numberOfRootTiles = primitive._levelZeroTiles.length;
if (rootTraversalDetails.length < numberOfRootTiles) {
rootTraversalDetails = new Array(numberOfRootTiles);
for (i = 0; i < numberOfRootTiles; ++i) {
for (let i = 0; i < numberOfRootTiles; ++i) {
if (rootTraversalDetails[i] === undefined) {
rootTraversalDetails[i] = new TraversalDetails();
}
Expand All @@ -537,7 +533,6 @@ function selectTilesForRendering(primitive, frameState) {

primitive._occluders.ellipsoid.cameraPosition = frameState.camera.positionWC;

let tile;
const levelZeroTiles = primitive._levelZeroTiles;
const occluders =
levelZeroTiles.length > 1 ? primitive._occluders : undefined;
Expand All @@ -550,18 +545,28 @@ function selectTilesForRendering(primitive, frameState) {

const customDataAdded = primitive._addHeightCallbacks;
const customDataRemoved = primitive._removeHeightCallbacks;
const frameNumber = frameState.frameNumber;

let len;
if (customDataAdded.length > 0 || customDataRemoved.length > 0) {
for (i = 0, len = levelZeroTiles.length; i < len; ++i) {
tile = levelZeroTiles[i];
tile._updateCustomData(frameNumber, customDataAdded, customDataRemoved);
customDataAdded.forEach((data) => {
const tile = levelZeroTiles.find((tile) =>
Rectangle.contains(tile.rectangle, data.positionCartographic),
);
if (tile) {
tile._addedCustomData.push(data);
}
});

customDataAdded.length = 0;
customDataRemoved.length = 0;
}
customDataRemoved.forEach((data) => {
const tile = levelZeroTiles.find((tile) =>
Rectangle.contains(tile.rectangle, data.positionCartographic),
);
if (tile) {
tile._removedCustomData.push(data);
}
});

levelZeroTiles.forEach((tile) => tile._updateCustomData());
customDataAdded.length = 0;
customDataRemoved.length = 0;

const camera = frameState.camera;

Expand All @@ -577,8 +582,8 @@ function selectTilesForRendering(primitive, frameState) {
);

// Traverse in depth-first, near-to-far order.
for (i = 0, len = levelZeroTiles.length; i < len; ++i) {
tile = levelZeroTiles[i];
for (let i = 0; i < levelZeroTiles.length; ++i) {
const tile = levelZeroTiles[i];
primitive._tileReplacementQueue.markTileRendered(tile);
if (!tile.renderable) {
queueTileLoad(primitive, primitive._tileLoadQueueHigh, tile, frameState);
Expand All @@ -596,7 +601,7 @@ function selectTilesForRendering(primitive, frameState) {
}
}

primitive._lastSelectionFrameNumber = frameNumber;
primitive._lastSelectionFrameNumber = frameState.frameNumber;
}

function queueTileLoad(primitive, queue, tile, frameState) {
Expand Down Expand Up @@ -716,7 +721,7 @@ function visitTile(
++debug.tilesVisited;

primitive._tileReplacementQueue.markTileRendered(tile);
tile._updateCustomData(frameState.frameNumber);
tile._updateCustomData();

if (tile.level > debug.maxDepthVisited) {
debug.maxDepthVisited = tile.level;
Expand Down Expand Up @@ -1417,15 +1422,18 @@ function updateHeights(primitive, frameState) {
// Ensure stale position cache is cleared
tile.clearPositionCache();
tilesToUpdateHeights.shift();
primitive._lastTileIndex = 0;
continue;
}
const customData = tile.customData;
const customDataLength = customData.length;
if (!defined(tile._customDataIterator)) {
tile._customDataIterator = customData.values();
}
const customDataIterator = tile._customDataIterator;

let timeSliceMax = false;
for (i = primitive._lastTileIndex; i < customDataLength; ++i) {
const data = customData[i];
let nextData;
while (!(nextData = customDataIterator.next()).done) {
const data = nextData.value;

// No need to run this code when the tile is upsampled, because the height will be the same as its parent.
const terrainData = tile.data.terrainData;
Expand Down Expand Up @@ -1543,10 +1551,10 @@ function updateHeights(primitive, frameState) {
}

if (timeSliceMax) {
primitive._lastTileIndex = i;
tile._customDataIterator = customDataIterator;
break;
} else {
primitive._lastTileIndex = 0;
tile._customDataIterator = undefined;
tilesToUpdateHeights.shift();
}
}
Expand Down
100 changes: 56 additions & 44 deletions packages/engine/Source/Scene/QuadtreeTile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import defined from "../Core/defined.js";
import DeveloperError from "../Core/DeveloperError.js";
import Rectangle from "../Core/Rectangle.js";
import Cartographic from "../Core/Cartographic.js";
import QuadtreeTileLoadState from "./QuadtreeTileLoadState.js";
import TileSelectionResult from "./TileSelectionResult.js";

Expand Down Expand Up @@ -107,8 +108,9 @@ function QuadtreeTile(options) {
this._distance = 0.0;
this._loadPriority = 0.0;

this._customData = [];
this._frameUpdated = undefined;
this._customData = new Set();
this._addedCustomData = [];
this._removedCustomData = [];
this._lastSelectionResult = TileSelectionResult.NONE;
this._lastSelectionResultFrame = undefined;
this._loadedCallbacks = {};
Expand Down Expand Up @@ -311,52 +313,62 @@ QuadtreeTile.prototype.clearPositionCache = function () {
}
};

QuadtreeTile.prototype._updateCustomData = function (
frameNumber,
added,
removed,
) {
let customData = this.customData;

let i;
let data;
let rectangle;

if (defined(added) && defined(removed)) {
customData = customData.filter(function (value) {
return removed.indexOf(value) === -1;
});
this._customData = customData;

rectangle = this._rectangle;
for (i = 0; i < added.length; ++i) {
data = added[i];
if (Rectangle.contains(rectangle, data.positionCartographic)) {
customData.push(data);
}
}
QuadtreeTile.prototype._updateCustomData = function () {
const added = this._addedCustomData;
const removed = this._removedCustomData;
if (added.length === 0 && removed.length === 0) {
return;
}

this._frameUpdated = frameNumber;
} else {
// interior or leaf tile, update from parent
const parent = this._parent;
if (defined(parent) && this._frameUpdated !== parent._frameUpdated) {
customData.length = 0;

rectangle = this._rectangle;
const parentCustomData = parent.customData;
for (i = 0; i < parentCustomData.length; ++i) {
data = parentCustomData[i];
if (Rectangle.contains(rectangle, data.positionCartographic)) {
customData.push(data);
}
}
const customData = this.customData;
for (let i = 0; i < added.length; ++i) {
const data = added[i];
customData.add(data);

const child = childTileAtPosition(this, data.positionCartographic);
child._addedCustomData.push(data);
}
this._addedCustomData.length = 0;

this._frameUpdated = parent._frameUpdated;
for (let i = 0; i < removed.length; ++i) {
const data = removed[i];
if (customData.has(data)) {
customData.delete(data);
}

const child = childTileAtPosition(this, data.positionCartographic);
child._removedCustomData.push(data);
}
this._removedCustomData.length = 0;
};

const centerScratch = new Cartographic();

/**
* Determines which child tile that contains the specified position. Assumes the position is within
* the bounds of the parent tile.
* @private
* @param {QuadtreeTile} tile - The parent tile.
* @param {Cartographic} positionCartographic - The cartographic position.
* @returns {QuadtreeTile} The child tile that contains the position.
*/
function childTileAtPosition(tile, positionCartographic) {
const center = Rectangle.center(tile.rectangle, centerScratch);
const x = positionCartographic.longitude >= center.longitude ? 1 : 0;
const y = positionCartographic.latitude < center.latitude ? 1 : 0;

switch (y * 2 + x) {
case 0:
return tile.northwestChild;
case 1:
return tile.northeastChild;
case 2:
return tile.southwestChild;
default:
return tile.southeastChild;
}
}

Comment on lines +355 to +371
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two comments / questions on this:

  1. Is this reliable? I don't think it works if the tile crosses the IDL, so is that a bad assumption to make here?
  2. Instead of a switch statement, we could define an array out of this function that contains the names of the children and index into that. But that means if the variable name changes, so must the array. I wasn't sure if that was a good tradeoff for a minimal speed boost (if there's any), but input is welcome.

Copy link
Member

Choose a reason for hiding this comment

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

@mzschwartz5 I'm responding here belatedly after you pinged me about this on Teams...

I don't think the IDL should be an issue, because I don't think our quadtree tiles can ever cross it?

But I think there's a bigger problem here.

First, consider a quadtree with a Web Mercator tiling scheme. The four child tiles aren't subdivided at the center latitude at all in that case (they're divided in the center in Web Mercator coordinates, which do not map linearly to latitude), so this check is very wrong.

Even in the Geographic projection, where this seems correct, it might still cause subtle problems. It's a floating point thing, and a bit hard to explain, so bear with me...

If we want to know the coordinates of a particular tile given its Level / X / Y quadtree coordinates, there are two ways we can do that. One is that we can repeatedly subdivide the appropriate tile until we reach the one we care about. Level zero spans -90 to 90 degrees latitude, let's say, so we can divide that level 0 tile at 0 degrees latitude, pick the appropriate level 1 tile from among the four, and then divide that to get our level 2 tiles, and so on. Mathematically, it's repeated division, right?

Or, the other way to do it is to divide the entire coordinate space by the number of tiles in the level, giving us a tile width/height, and then multiply that by the tile coordinate. One division, one multiplication.

Mathematically, they're identical. In floating point land, they're not. The repeated divisions in the first approach can lead to accumulation of rounding errors. After drilling down 20+ levels, those tiny errors get magnified to really actually matter. The "dividing line" of a tile computed with the two approaches can be a significant tile width away. We've seen this happen.

So, as a policy, in the name of both performance and precision, CesiumJS always uses the second approach to compute tile coordinates. The subtle problem is that the code above is effectively determining tile boundaries using the first approach. If there's any possibility of this operation happening repeatedly, and thus of errors accumulating, then it could lead to wrong results.

You should be able to solve both problems (the Web Mercator one, and this subtle one) by letting the tiling scheme compute the center point rather than averaging the coordinates.

Object.defineProperties(QuadtreeTile.prototype, {
/**
* Gets the tiling scheme used to tile the surface.
Expand Down Expand Up @@ -522,9 +534,9 @@ Object.defineProperties(QuadtreeTile.prototype, {
},

/**
* An array of objects associated with this tile.
* A set of objects associated with this tile.
* @memberof QuadtreeTile.prototype
* @type {Array}
* @type {Set}
*/
customData: {
get: function () {
Expand Down
4 changes: 2 additions & 2 deletions packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ describe("Scene/QuadtreePrimitive", function () {

let addedCallback = false;
quadtree.forEachLoadedTile(function (tile) {
addedCallback = addedCallback || tile.customData.length > 0;
addedCallback = addedCallback || tile.customData.size > 0;
});

expect(addedCallback).toEqual(true);
Expand All @@ -915,7 +915,7 @@ describe("Scene/QuadtreePrimitive", function () {

let removedCallback = true;
quadtree.forEachLoadedTile(function (tile) {
removedCallback = removedCallback && tile.customData.length === 0;
removedCallback = removedCallback && tile.customData.size === 0;
});

expect(removedCallback).toEqual(true);
Expand Down