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

Add extensibility to various classes #11859

Merged
merged 6 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
9 changes: 6 additions & 3 deletions packages/engine/Source/Core/IntersectionTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,10 @@ function addWithCancellationCheck(left, right, tolerance) {
return difference;
}

function quadraticVectorExpression(A, b, c, x, w) {
/**
* @private
*/
IntersectionTests.quadraticVectorExpression = function (A, b, c, x, w) {
const xSquared = x * x;
const wSquared = w * w;

Expand Down Expand Up @@ -634,7 +637,7 @@ function quadraticVectorExpression(A, b, c, x, w) {
}

return solutions;
}
};

const firstAxisScratch = new Cartesian3();
const secondAxisScratch = new Cartesian3();
Expand Down Expand Up @@ -740,7 +743,7 @@ IntersectionTests.grazingAltitudeLocation = function (ray, ellipsoid) {
const b = Matrix3.multiplyByVector(temp, position, bCart);

// Solve for the solutions to the expression in standard form:
const solutions = quadraticVectorExpression(
const solutions = IntersectionTests.quadraticVectorExpression(
A,
Cartesian3.negate(b, firstAxisScratch),
0.0,
Expand Down
74 changes: 51 additions & 23 deletions packages/engine/Source/DataSources/CzmlDataSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ import VelocityOrientationProperty from "./VelocityOrientationProperty.js";
import VelocityVectorProperty from "./VelocityVectorProperty.js";
import WallGraphics from "./WallGraphics.js";
import Cesium3DTilesetGraphics from "./Cesium3DTilesetGraphics.js";
import SensorVolumePortionToDisplay from "../Scene/SensorVolumePortionToDisplay.js";

// A marker type to distinguish CZML properties where we need to end up with a unit vector.
// The data is still loaded into Cartesian3 objects but they are normalized.
Expand Down Expand Up @@ -576,6 +577,10 @@ function unwrapInterval(type, czmlInterval, sourceUri) {
return unwrapQuaternionInterval(czmlInterval);
case Rotation:
return defaultValue(czmlInterval.number, czmlInterval);
case SensorVolumePortionToDisplay:
return SensorVolumePortionToDisplay[
defaultValue(czmlInterval.portionToDisplay, czmlInterval)
];
case ShadowMode:
return ShadowMode[
defaultValue(
Expand All @@ -598,7 +603,7 @@ function unwrapInterval(type, czmlInterval, sourceUri) {
defaultValue(czmlInterval.verticalOrigin, czmlInterval)
];
default:
throw new RuntimeError(type);
throw new RuntimeError(`Unknown CzmlDataSource interval type: ${type}`);
}
}

Expand Down Expand Up @@ -5006,31 +5011,54 @@ Object.defineProperties(CzmlDataSource.prototype, {
* @type {CzmlDataSource.UpdaterFunction[]}
*/
CzmlDataSource.updaters = [
processBillboard, //
processBox, //
processCorridor, //
processCylinder, //
processEllipse, //
processEllipsoid, //
processLabel, //
processModel, //
processName, //
processDescription, //
processPath, //
processPoint, //
processPolygon, //
processPolyline, //
processPolylineVolume, //
processProperties, //
processRectangle, //
processPosition, //
processTileset, //
processViewFrom, //
processWall, //
processOrientation, //
processBillboard,
processBox,
processCorridor,
processCylinder,
processEllipse,
processEllipsoid,
processLabel,
processModel,
processName,
processDescription,
processPath,
processPoint,
processPolygon,
processPolyline,
processPolylineVolume,
processProperties,
processRectangle,
processPosition,
processTileset,
processViewFrom,
processWall,
processOrientation,
processAvailability,
];

/**
* Add the provided updater to the list of updaters if not already included
* @private
* @param {CzmlDataSource.UpdaterFunction} updater
*/
CzmlDataSource.registerUpdater = function (updater) {
if (!CzmlDataSource.updaters.includes(updater)) {
CzmlDataSource.updaters.push(updater);
}
};

/**
* Remove the provided updater from the list of updaters if already included
* @private
* @param {CzmlDataSource.UpdaterFunction} updater
*/
CzmlDataSource.unregisterUpdater = function (updater) {
if (CzmlDataSource.updaters.includes(updater)) {
const index = CzmlDataSource.updaters.indexOf(updater);
CzmlDataSource.updaters.splice(index, 1);
}
};

/**
* Processes the provided url or CZML object without clearing any existing data.
*
Expand Down
25 changes: 25 additions & 0 deletions packages/engine/Source/DataSources/DataSourceDisplay.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,30 @@ function DataSourceDisplay(options) {
this._ready = false;
}

const ExtraVisualizers = [];
/**
* Add the provided Visualizer to the default visualizers callback if not already included
* @private
* @param {Visualizer} visualizer
*/
DataSourceDisplay.registerVisualizer = function (visualizer) {
if (!ExtraVisualizers.includes(visualizer)) {
ExtraVisualizers.push(visualizer);
}
};

/**
* Remove the provided Visualizer from the default visualizers callback if it's included
* @private
* @param {Visualizer} visualizer
*/
DataSourceDisplay.unregisterVisualizer = function (visualizer) {
if (ExtraVisualizers.includes(visualizer)) {
const index = ExtraVisualizers.indexOf(visualizer);
ExtraVisualizers.splice(index, 1);
}
};

/**
* Gets or sets the default function which creates an array of visualizers used for visualization.
* By default, this function uses all standard visualizers.
Expand Down Expand Up @@ -151,6 +175,7 @@ DataSourceDisplay.defaultVisualizersCallback = function (
dataSource._primitives,
dataSource._groundPrimitives
),
...ExtraVisualizers.map((Visualizer) => new Visualizer(scene, entities)),
jjspace marked this conversation as resolved.
Show resolved Hide resolved
];
};

Expand Down
22 changes: 20 additions & 2 deletions packages/engine/Source/DataSources/Entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import WallGraphics from "./WallGraphics.js";

const cartoScratch = new Cartographic();

const ExtraPropertyNames = [];

function createConstantPositionProperty(value) {
return new ConstantPositionProperty(value);
}
Expand Down Expand Up @@ -127,7 +129,7 @@ function Entity(options) {
"corridor",
"cylinder",
"description",
"ellipse", //
"ellipse",
"ellipsoid",
"label",
"model",
Expand All @@ -136,14 +138,15 @@ function Entity(options) {
"path",
"plane",
"point",
"polygon", //
"polygon",
"polyline",
"polylineVolume",
"position",
"properties",
"rectangle",
"viewFrom",
"wall",
...ExtraPropertyNames,
];

this._billboard = undefined;
Expand Down Expand Up @@ -494,6 +497,21 @@ Object.defineProperties(Entity.prototype, {
wall: createPropertyTypeDescriptor("wall", WallGraphics),
});

/**
* Add the specified type and construct the properties for it in the Entity class
* @private
* @param {string} propertyName
* @param {any} Type
Copy link
Contributor

Choose a reason for hiding this comment

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

Strictly speaking this would be

Suggested change
* @param {any} Type
* @param {object.constructor} Type

right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is marked as private, but typically we do add descriptions for each parameter. That applies here and throughout this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More and more often I go down rabbit holes dug by JSDoc being old and not maintained.
object.constructor is not a type. After a quick google search I came across this issue jsdoc/jsdoc#1349 which, paired with other JSDoc knowledge, seems to indicate there's no way to really specify "any class" as a type. The best I think we can do is specify a type of {{ constructor: function }} to try and only accept objects with a constructor function?

*/
Entity.registerEntityType = function (propertyName, Type) {
Object.defineProperties(Entity.prototype, {
[propertyName]: createPropertyTypeDescriptor(propertyName, Type),
});
Comment on lines +507 to +509
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I'm not entirely sure how to "undo" this change. The only way I can tell it would be possible is to make this property configurable: true which then lets me delete Entity.prototype.propertyName. But I'm not entirely sure we want to make the property fully configurable?
    • This may point to a deeper issue of needing a better way to create and manage entity type properties.
  2. I'm also not really sure how to even test and validate the correct properties are set up with the right getter/setter. They'd default to undefined so I couldn't find a nice way to check. Also the privateName and subscriptionName are not always defined so using Object.keys to check doesn't seem like a 100% complete solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline: Given we'd have to jump through hoops here to make an "unsubscribe" function, we'll omit.

Depending on if we need additional Entity flexibility in the future, we could consider how the types are managed under the hood. But for the scope of this PR, its not needed.

if (!ExtraPropertyNames.includes(propertyName)) {
ExtraPropertyNames.push(propertyName);
}
};

/**
* Given a time, returns true if this object should have data during that time.
*
Expand Down
114 changes: 114 additions & 0 deletions packages/engine/Source/DataSources/GeometryUpdaterSet.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import destroyObject from "../Core/destroyObject.js";
import Event from "../Core/Event.js";
import EventHelper from "../Core/EventHelper.js";
import BoxGeometryUpdater from "./BoxGeometryUpdater.js";
import CorridorGeometryUpdater from "./CorridorGeometryUpdater.js";
import CylinderGeometryUpdater from "./CylinderGeometryUpdater.js";
import EllipseGeometryUpdater from "./EllipseGeometryUpdater.js";
import EllipsoidGeometryUpdater from "./EllipsoidGeometryUpdater.js";
import PlaneGeometryUpdater from "./PlaneGeometryUpdater.js";
import PolygonGeometryUpdater from "./PolygonGeometryUpdater.js";
import PolylineVolumeGeometryUpdater from "./PolylineVolumeGeometryUpdater.js";
import RectangleGeometryUpdater from "./RectangleGeometryUpdater.js";
import WallGeometryUpdater from "./WallGeometryUpdater.js";

/** @type {GeometryUpdater[]} */
const geometryUpdaters = [
BoxGeometryUpdater,
CylinderGeometryUpdater,
CorridorGeometryUpdater,
EllipseGeometryUpdater,
EllipsoidGeometryUpdater,
PlaneGeometryUpdater,
PolygonGeometryUpdater,
PolylineVolumeGeometryUpdater,
RectangleGeometryUpdater,
WallGeometryUpdater,
];

/**
* @private
jjspace marked this conversation as resolved.
Show resolved Hide resolved
* @param {Entity} entity
* @param {Scene} scene
*/
function GeometryUpdaterSet(entity, scene) {
this.entity = entity;
this.scene = scene;
const updaters = new Array(geometryUpdaters.length);
const geometryChanged = new Event();
function raiseEvent(geometry) {
jjspace marked this conversation as resolved.
Show resolved Hide resolved
geometryChanged.raiseEvent(geometry);
}
const eventHelper = new EventHelper();
for (let i = 0; i < updaters.length; i++) {
const updater = new geometryUpdaters[i](entity, scene);
eventHelper.add(updater.geometryChanged, raiseEvent);
updaters[i] = updater;
}
this.updaters = updaters;
this.geometryChanged = geometryChanged;
this.eventHelper = eventHelper;

this._removeEntitySubscription = entity.definitionChanged.addEventListener(
GeometryUpdaterSet.prototype._onEntityPropertyChanged,
this
);
}

GeometryUpdaterSet.prototype._onEntityPropertyChanged = function (
entity,
propertyName,
newValue,
oldValue
) {
const updaters = this.updaters;
for (let i = 0; i < updaters.length; i++) {
updaters[i]._onEntityPropertyChanged(
entity,
propertyName,
newValue,
oldValue
);
}
};

GeometryUpdaterSet.prototype.forEach = function (callback) {
const updaters = this.updaters;
for (let i = 0; i < updaters.length; i++) {
callback(updaters[i]);
}
};

GeometryUpdaterSet.prototype.destroy = function () {
this.eventHelper.removeAll();
const updaters = this.updaters;
for (let i = 0; i < updaters.length; i++) {
updaters[i].destroy();
}
this._removeEntitySubscription();
destroyObject(this);
};

/**
* Add the provided updater to the default list of updaters if not already included
* @param {GeometryUpdater} updater
*/
GeometryUpdaterSet.registerUpdater = function (updater) {
if (!geometryUpdaters.includes(updater)) {
console.log("registerUpdater", updater);
jjspace marked this conversation as resolved.
Show resolved Hide resolved
geometryUpdaters.push(updater);
}
};

/**
* Remove the provided updater from the default list of updaters if included
* @param {GeometryUpdater} updater
*/
GeometryUpdaterSet.unregisterUpdater = function (updater) {
if (geometryUpdaters.includes(updater)) {
const index = geometryUpdaters.indexOf(updater);
geometryUpdaters.splice(index, 1);
}
};

export default GeometryUpdaterSet;
Loading
Loading