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.

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

Change DataSourceDisplay ready state behavior for bounding sphere calculations #12429

Merged
merged 7 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@

#### @cesium/engine

##### Breaking Changes :mega:

- Changed behavior of `DataSourceDisplay.ready` to always stay `true` once it is initially set to `true`. [#12429](https://github.com/CesiumGS/cesium/pull/12429)

##### Fixes :wrench:

- Fixed error when resetting `Cesium3DTileset.modelMatrix` to its initial value. [#12409](https://github.com/CesiumGS/cesium/pull/12409)
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to add a link to the fixed issue as well. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was waiting to see the PR number, updated 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, too soon 🏃‍♀️

Copy link
Contributor

Choose a reason for hiding this comment

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

@jjspace I don't see a link to the fixed issue (#12303). Can you please add it?

- Fixed the parameter types of the `ClippingPolygon.equals` function, and fixed cases where parameters to `equals` functions had erroneously not been marked as 'optional'. [#12394](https://github.com/CesiumGS/cesium/pull/12394)
- Fixed type of `ImageryLayer.fromProviderAsync`, to correctly show that the param `options` is optional. [#12400](https://github.com/CesiumGS/cesium/pull/12400)
- Fixed Draco decoding for vertex colors that are normalized `UNSIGNED_BYTE` or `UNSIGNED_SHORT`. [#12417](https://github.com/CesiumGS/cesium/pull/12417)
- Fixed type error when setting `Viewer.selectedEntity` [#12303](https://github.com/CesiumGS/cesium/issues/12303)

### 1.125 - 2025-01-02

Expand Down
6 changes: 3 additions & 3 deletions packages/engine/Source/DataSources/BoundingSphereState.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* @enum {number}
* @private
*/
const BoundingSphereState = {
const BoundingSphereState = Object.freeze({
/**
* The BoundingSphere has been computed.
* @type BoundingSphereState
Expand All @@ -22,5 +22,5 @@ const BoundingSphereState = {
* @constant
*/
FAILED: 2,
};
export default Object.freeze(BoundingSphereState);
});
export default BoundingSphereState;
7 changes: 5 additions & 2 deletions packages/engine/Source/DataSources/DataSourceDisplay.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,10 @@ DataSourceDisplay.prototype.update = function (time) {
if (!this._ready && result) {
this._scene.requestRender();
}
this._ready = result;

// once the DataSourceDisplay is ready it should stay ready to prevent
// entities from breaking updates when they become "un-ready"
this._ready = this._ready || result;

return result;
};
Expand Down Expand Up @@ -386,7 +389,7 @@ DataSourceDisplay.prototype.getBoundingSphere = function (
Check.defined("result", result);
//>>includeEnd('debug');

if (!this._ready && !allowPartial) {
if (!this._ready) {
return BoundingSphereState.PENDING;
}

Expand Down
26 changes: 16 additions & 10 deletions packages/engine/Source/Widget/CesiumWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -1134,8 +1134,6 @@ CesiumWidget.prototype._updateCanAnimate = function (isUpdated) {
this._clock.canAnimate = isUpdated;
};

const boundingSphereScratch = new BoundingSphere();

/**
* @private
*/
Expand All @@ -1148,15 +1146,15 @@ CesiumWidget.prototype._onTick = function (clock) {
}

const entityView = this._entityView;
if (defined(entityView)) {
if (defined(entityView) && defined(entityView.boundingSphere)) {
const trackedEntity = this._trackedEntity;
const trackedState = this._dataSourceDisplay.getBoundingSphere(
trackedEntity,
true,
boundingSphereScratch,
false,
entityView.boundingSphere,
);
if (trackedState === BoundingSphereState.DONE) {
entityView.update(time, boundingSphereScratch);
entityView.update(time, entityView.boundingSphere);
}
}
};
Expand Down Expand Up @@ -1414,6 +1412,8 @@ CesiumWidget.prototype._postRender = function () {
updateTrackedEntity(this);
};

const zoomTargetBoundingSphereScratch = new BoundingSphere();

function updateZoomTarget(widget) {
const target = widget._zoomTarget;
if (!defined(target) || widget.scene.mode === SceneMode.MORPHING) {
Expand Down Expand Up @@ -1511,13 +1511,15 @@ function updateZoomTarget(widget) {
const state = widget._dataSourceDisplay.getBoundingSphere(
entities[i],
false,
boundingSphereScratch,
zoomTargetBoundingSphereScratch,
);

if (state === BoundingSphereState.PENDING) {
return;
} else if (state !== BoundingSphereState.FAILED) {
boundingSpheres.push(BoundingSphere.clone(boundingSphereScratch));
boundingSpheres.push(
BoundingSphere.clone(zoomTargetBoundingSphereScratch),
);
}
}

Expand Down Expand Up @@ -1552,6 +1554,8 @@ function updateZoomTarget(widget) {
}
}

const trackedEntityBoundingSphereScratch = new BoundingSphere();

function updateTrackedEntity(widget) {
if (!widget._needTrackedEntityUpdate) {
return;
Expand All @@ -1577,7 +1581,7 @@ function updateTrackedEntity(widget) {
const state = widget._dataSourceDisplay.getBoundingSphere(
trackedEntity,
false,
boundingSphereScratch,
trackedEntityBoundingSphereScratch,
);
if (state === BoundingSphereState.PENDING) {
return;
Expand All @@ -1599,7 +1603,9 @@ function updateTrackedEntity(widget) {
}

const bs =
state !== BoundingSphereState.FAILED ? boundingSphereScratch : undefined;
state !== BoundingSphereState.FAILED
? trackedEntityBoundingSphereScratch
: undefined;
widget._entityView = new EntityView(trackedEntity, scene, scene.ellipsoid);
widget._entityView.update(currentTime, bs);
widget._needTrackedEntityUpdate = false;
Expand Down
20 changes: 10 additions & 10 deletions packages/engine/Specs/DataSources/DataSourceDisplaySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ describe(
});
});

it("ready is true when datasources are ready", function () {
it("ready is true once datasources are ready and stays true", async function () {
const source1 = new MockDataSource();
const source2 = new MockDataSource();

Expand All @@ -372,19 +372,19 @@ describe(
dataSourceCollection: dataSourceCollection,
visualizersCallback: visualizersCallback,
});
expect(display.ready).toBe(false);
expect(display.ready).withContext("before adding sources").toBe(false);

return Promise.all([
await Promise.all([
dataSourceCollection.add(source1),
dataSourceCollection.add(source2),
]).then(function () {
display.update(Iso8601.MINIMUM_VALUE);
expect(display.ready).toBe(true);
]);

spyOn(MockVisualizer.prototype, "update").and.returnValue(false);
display.update(Iso8601.MINIMUM_VALUE);
expect(display.ready).toBe(false);
});
display.update(Iso8601.MINIMUM_VALUE);
expect(display.ready).withContext("after adding sources").toBe(true);

spyOn(MockVisualizer.prototype, "update").and.returnValue(false);
display.update(Iso8601.MINIMUM_VALUE);
expect(display.ready).withContext("after updating again").toBe(true);
});

it("triggers a rendering when the data source becomes ready", function () {
Expand Down
Loading