Skip to content
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

- Adds Google2DImageryProvider to load imagery from [Google Maps](https://developers.google.com/maps/documentation/tile/2d-tiles-overview) [#12913](https://github.com/CesiumGS/cesium/pull/12913)
- Adds an async factory method for the Material class that allows callers to wait on resource loading. [#10566](https://github.com/CesiumGS/cesium/issues/10566)
- Adds new declusteredEvent: Fires with complete clustering information including both clustered and declustered entities [#5760](https://github.com/CesiumGS/cesium/issues/5760)

## 1.133.1 - 2025-09-08

Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -432,3 +432,4 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for details on how to contribute to Cesiu
- [Pamela Augustine](https://github.com/pamelaAugustine)
- [宋时旺](https://github.com/BlockCnFuture)
- [Marco Zhan](https://github.com/marcoYxz)
- [Alexander Remer](https://github.com/Oko-Tester)
54 changes: 50 additions & 4 deletions packages/engine/Source/DataSources/EntityCluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ function EntityCluster(options) {
this._removeEventListener = undefined;

this._clusterEvent = new Event();
this._declusterEvent = new Event();
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this is a bit misleading. clusterEvent fires whenever a new cluster is added, right? But declusterEvent only fires when all clusters are removed. (If I've understood this PR correctly)

It seems like declusterEvent should also fire whenever a cluster is removed. (I don't know if that's feasible to implement, though)

Copy link
Author

Choose a reason for hiding this comment

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

You are correct about the asymmetry. The behavior is as follows:
clusterEvent: Triggered every time addCluster is called, which happens in the following cases:

When new clusters are created
When existing clusters are reused (when the camera zooms in and they still meet the minimumClusterSize threshold)

declusterEvent: Only fires when transitioning from "has clusters" to "no clusters at all". It does NOT fire when individual clusters are removed.
However, users can still detect which clusters have been removed by tracking the entity IDs themselves. Since clusterEvent is triggered every frame with the current cluster composition, you can compare frame by frame to determine which entities were declustered.

Copy link
Contributor

Choose a reason for hiding this comment

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

A wild guess is that this question may have to do with the name.

When you have methods like
addSomething/removeSomething or
createSomething/destroySomething
then you'd also expect a certain "symmetry".
(And when you have addSomething, removeSomething, and deleteSomething, you'll wonder: Wait what...?)

In this case, one could expect this event to be raises when ... the opposite of that happened what causes a clusterEvent. Yes, there is the documentation that says that this event "... will be raised when all entities have been declustered.", but still.

@mzschwartz5 It's up to you, but I think that options could be to

  • rename this event (to something like declusteredAllEvent or allDeclusteredEvent or ... whatever is appropriate here)
  • and (!!!optional!!!) consider adding a declusterEvent that indeed is the opposite of what clusterEvent is doing

Copy link
Author

@Oko-Tester Oko-Tester Nov 6, 2025

Choose a reason for hiding this comment

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

Yes, you’re right about the naming confusion. I think we should rename the event to lastDeclusterEvent (or allDeclusteredEvent, whatever you prefer) so it’s clear that it only fires when the last cluster is removed. But there’s no real need for a new event that tracks every declustering — users can already detect removed clusters by comparing the cluster composition from the clusterEvent. So I don’t think an additional event for every declustering is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not deeply involved here - it was just a comment while looking at my GitHub notifications. So I don't know in how far it makes sense to define an event for individual removed clusters. But... (again: a bit shallow)...:

users can already detect removed clusters by comparing the cluster composition from the clusterEvent. So I don’t think an additional event for every declustering is necessary.

Users can do a lot of stuff. Users can track this and that and store this and that and compute some difference here and there. The question is whether there's a convincing use case for being informed about individual clusters. If there is, then uers should not be forced to write 200 lines of (possibly buggy) code to derive the information they need, when they could just listen for a single, predefined event.

Maybe there is no such use case. And in doubt, this new event could be added later, when the demand arises. And following the mantra: "When in doubt, leave it out!", it's probably OK to omit it for now (and certainly in the context of this PR)

Copy link
Contributor

Choose a reason for hiding this comment

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

I generally agree with @javagl, I and I think it would be sufficient for this PR to simply rename this event. I liked the proposed "allDeclusteredEvent"

this._previouslyClusteredEntities = [];

/**
* Determines if entities in this collection will be shown.
Expand Down Expand Up @@ -216,7 +218,7 @@ function getScreenSpacePositions(
}
}

const pointBoundinRectangleScratch = new BoundingRectangle();
const pointBoundingRectangleScratch = new BoundingRectangle();
const totalBoundingRectangleScratch = new BoundingRectangle();
const neighborBoundingRectangleScratch = new BoundingRectangle();

Expand Down Expand Up @@ -414,7 +416,7 @@ function createDeclutterCallback(entityCluster) {
point.coord,
pixelRange,
entityCluster,
pointBoundinRectangleScratch,
pointBoundingRectangleScratch,
);
const totalBBox = BoundingRectangle.clone(
bbox,
Expand Down Expand Up @@ -500,8 +502,28 @@ function createDeclutterCallback(entityCluster) {
entityCluster._clusterPointCollection = undefined;
}

entityCluster._previousClusters = newClusters;
entityCluster._previousHeight = currentHeight;
const currentlyClusteredIds = [];

if (newClusters.length > 0 && defined(clusteredLabelCollection)) {
for (let c = 0; c < clusteredLabelCollection.length; ++c) {
const clusterLabel = clusteredLabelCollection.get(c);
if (defined(clusterLabel) && defined(clusterLabel.id)) {
currentlyClusteredIds.push(...clusterLabel.id);
}
}
}
Comment on lines +507 to +514
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Why do we need to check newClusters.length > 0?
  2. Will it ever be the case that clusterLabel or clusterLabel.id is not defined? When?
  3. Why do we need to use the spread operator (...) when pushing an ID? To make a copy of it (if so, why isn't a reference sufficient?)

Copy link
Author

Choose a reason for hiding this comment

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

  1. Why check newClusters.length > 0?
    This check is redundant. If there are no clusters, clusteredLabelCollection.length would be 0 anyway, so the loop wouldn't execute. I'll remove this check.
  2. Will clusterLabel or clusterLabel.id ever be undefined?
    In normal operation, no. clusterLabel.id is always set in addCluster(). I think keeping these defensive checks for safety would be good.
  3. Why use the spread operator (...)?
    Because clusterLabel.id is an array of entity IDs (set in addCluster()), not a single ID. The spread operator flattens it. Without it, you'd have an array of arrays instead of a flat array of IDs.

Copy link
Contributor

@javagl javagl Nov 6, 2025

Choose a reason for hiding this comment

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

Random drive-by comment:

In normal operation, no. clusterLabel.id is always set in addCluster(). I think keeping these defensive checks for safety would be good.

When something should never be undefined, then this should be a contract that people can rely on. When one of these things is ever undefined, then this is a bug, and it will show up as a crash, with a clear stack trace. With these checks, it will not show up with a stack trace. But it will still be a bug. And this bug will only show up as an issue like "Sometimes some labels are not properly shown in some clusters at some zoom level" or so. And one can easily spend 5 or 10 hours of painful debugging to find the reason for an issue like that.

I know, there are different philosophies. But there's an important difference between being "defensive"/"resilient", and what I refer to as "fail silent".
"Defensive" would mean that there is a defined check, and when the thing is undefined, then it does not crash, but it still logs an error to the console, saying "This is undefined, but it never should be". Ideally, this should not be necessary within a library. It should only be necessary for things that are provided from outside, via the public API. But... that ship has sailed.
"Fail silent" means that something is wrong, and there's not way to figure out what is wrong. That's not good.

Copy link
Author

Choose a reason for hiding this comment

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

Good point, I didn’t consider that. I think it was mentioned earlier in this PR, but you’re right, better to fail visibly than silently.

Copy link
Contributor

@mzschwartz5 mzschwartz5 Nov 6, 2025

Choose a reason for hiding this comment

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

Okay so sounds like we can address points 1 and 2 easily.

For point 3 (the spread operator) - are we copying the all currently clustered IDs (i.e. every ID of every entity that is clustered) on every declutter callback? That seems like it could be bad for performance. It also doubles the memory we consume in tracking which entities are clustered (since we always have a copy of the previouslyClustered...)

Side note: does this only fire a declustered event for clusteredLabelCollection? But not for clusteredBillboardCollection or clusteredPointCollection?


const hasActiveClusters = currentlyClusteredIds.length > 0;
const hadPreviouslyClusters =
entityCluster._previouslyClusteredEntities.length > 0;

if (!hasActiveClusters && hadPreviouslyClusters) {
entityCluster._declusterEvent.raiseEvent(
entityCluster._previouslyClusteredEntities,
);
}

entityCluster._previouslyClusteredEntities = currentlyClusteredIds;
};
}

Expand Down Expand Up @@ -567,6 +589,16 @@ Object.defineProperties(EntityCluster.prototype, {
return this._clusterEvent;
},
},
/**
* Gets the event that will be raised when all entities have been declustered.
* @memberof EntityCluster.prototype
* @type {Event<EntityCluster.declusterCallback>}
*/
declusterEvent: {
get: function () {
return this._declusterEvent;
},
},
/**
* Gets or sets whether clustering billboard entities is enabled.
* @memberof EntityCluster.prototype
Expand Down Expand Up @@ -993,6 +1025,7 @@ EntityCluster.prototype.destroy = function () {

this._previousClusters = [];
this._previousHeight = undefined;
this._previouslyClusteredEntities = [];

this._enabledDirty = false;
this._pixelRangeDirty = false;
Expand All @@ -1019,4 +1052,17 @@ EntityCluster.prototype.destroy = function () {
* cluster.label.text = entities.length.toLocaleString();
* });
*/
/**
* A event listener function used when all entities have been declustered.
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo- "A event" --> "An event"

* @callback EntityCluster.declusterCallback
*
* @param {Entity[]} declusteredEntities An array of the entities that were in the last
* remaining clusters and are now displayed individually.
*
* @example
* // Listen for decluster events
* dataSource.clustering.declusterEvent.addEventListener(function(entities) {
* console.log('All clusters removed. Last entities declustered:', entities);
* });
*/
export default EntityCluster;
181 changes: 181 additions & 0 deletions packages/engine/Specs/DataSources/EntityClusterSpec.js
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not an expert on unit testing philosphy, so maybe this is a bad suggestion, but could we maybe have unit test(s) that check that clustering generally works for all three of: points, billboards, and labels (collections)?

I'm just a little wary given the discussion about using the item.show property as if these classes all have a shared abstract interface (which they don't). I don't expect that to be an issue, but if someone were to ever change that property name on one of the classes, at least having a failing unit test here would prevent the issue.

Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,187 @@ describe(
expect(cluster._billboardCollection).toBeDefined();
expect(cluster._billboardCollection.length).toEqual(2);
});

it("has declusteredEvent property", function () {
cluster = new EntityCluster();
expect(cluster.declusteredEvent).toBeDefined();
expect(typeof cluster.declusteredEvent.addEventListener).toEqual(
"function",
);
});

it("provides access to clustering data via new API methods", function () {
cluster = new EntityCluster();
cluster._initialize(scene);

expect(typeof cluster.getLastClusteredEntities).toEqual("function");
expect(typeof cluster.getLastDeclusteredEntities).toEqual("function");
expect(typeof cluster.getAllProcessedEntities).toEqual("function");

expect(cluster.getLastClusteredEntities()).toEqual([]);
expect(cluster.getLastDeclusteredEntities()).toEqual([]);
expect(cluster.getAllProcessedEntities()).toEqual([]);
});

it("fires declusteredEvent with both clustered and declustered entities", function () {
cluster = new EntityCluster();
cluster._initialize(scene);

let receivedData = null;
cluster.declusteredEvent.addEventListener(function (data) {
receivedData = data;
});

const entity1 = new Entity();
const point1 = cluster.getPoint(entity1);
point1.id = entity1;
point1.pixelSize = 1;
point1.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(0.0, 0.0),
depth,
);

const entity2 = new Entity();
const point2 = cluster.getPoint(entity2);
point2.id = entity2;
point2.pixelSize = 1;
point2.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(1.0, 1.0),
depth,
);

const entity3 = new Entity();
const point3 = cluster.getPoint(entity3);
point3.id = entity3;
point3.pixelSize = 1;
point3.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(scene.canvas.clientWidth, scene.canvas.clientHeight),
farDepth,
);

cluster.enabled = true;
return updateUntilDone(cluster).then(function () {
expect(receivedData).toBeDefined();
expect(receivedData.clustered).toBeDefined();
expect(receivedData.declustered).toBeDefined();
expect(Array.isArray(receivedData.clustered)).toEqual(true);
expect(Array.isArray(receivedData.declustered)).toEqual(true);
});
});

it("maintains backward compatibility - original clusterEvent still works", function () {
cluster = new EntityCluster();
cluster._initialize(scene);

let originalEventFired = false;
let newEventFired = false;

cluster.clusterEvent.addEventListener(function (entities, clusterObj) {
originalEventFired = true;
expect(Array.isArray(entities)).toEqual(true);
expect(clusterObj).toBeDefined();
});

cluster.declusteredEvent.addEventListener(function (data) {
newEventFired = true;
expect(data.clustered).toBeDefined();
expect(data.declustered).toBeDefined();
});

const entity1 = new Entity();
const point1 = cluster.getPoint(entity1);
point1.id = entity1;
point1.pixelSize = 1;
point1.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(0.0, 0.0),
depth,
);

const entity2 = new Entity();
const point2 = cluster.getPoint(entity2);
point2.id = entity2;
point2.pixelSize = 1;
point2.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(1.0, 1.0),
depth,
);

cluster.enabled = true;
return updateUntilDone(cluster).then(function () {
expect(originalEventFired).toEqual(true);
expect(newEventFired).toEqual(true);
});
});

it("tracks processed entities correctly", function () {
cluster = new EntityCluster();
cluster._initialize(scene);

const entity1 = new Entity();
const point1 = cluster.getPoint(entity1);
point1.id = entity1;
point1.pixelSize = 1;
point1.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(0.0, 0.0),
depth,
);

const entity2 = new Entity();
const point2 = cluster.getPoint(entity2);
point2.id = entity2;
point2.pixelSize = 1;
point2.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(scene.canvas.clientWidth, scene.canvas.clientHeight),
farDepth,
);

cluster.enabled = true;
return updateUntilDone(cluster).then(function () {
const clusteredEntities = cluster.getLastClusteredEntities();
const declusteredEntities = cluster.getLastDeclusteredEntities();
const allProcessed = cluster.getAllProcessedEntities();

expect(allProcessed.length).toBeGreaterThan(0);
expect(
clusteredEntities.length + declusteredEntities.length,
).toBeLessThanOrEqual(allProcessed.length);
});
});

it("cleans up tracking arrays on destroy", function () {
cluster = new EntityCluster();
cluster._initialize(scene);

const entity = new Entity();
const point = cluster.getPoint(entity);
point.id = entity;
point.pixelSize = 1;
point.position = SceneTransforms.drawingBufferToWorldCoordinates(
scene,
new Cartesian2(0.0, 0.0),
depth,
);

cluster.enabled = true;
return updateUntilDone(cluster).then(function () {
expect(cluster._allProcessedEntities).toBeDefined();
expect(cluster._lastClusteredEntities).toBeDefined();
expect(cluster._lastDeclusteredEntities).toBeDefined();

cluster.destroy();

expect(cluster._allProcessedEntities).toEqual([]);
expect(cluster._lastClusteredEntities).toEqual([]);
expect(cluster._lastDeclusteredEntities).toEqual([]);
});
});
},
"WebGL",
);