From e9a5452f40dd52b02167a1ceb7162de0ed49ac6c Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Tue, 9 Jun 2020 13:49:39 -0400 Subject: [PATCH 1/2] Fix geometry creation in TypeScript None of the `XXXGeometry` classes are actually `Geometry` instances. They are instead utility classes that create geometries via their `createGeometry` implementation. `GeometryInstance` can take either "type" but since JS doesn't have types we never really defined what the "utility" type is, so the TypeScript definition for `GeometryInstance` specifies that currently only specifies `Geometry`. This means that this valid JS code is a compiler error in TypeScript ``` const geometryInstance = new GeometryInstance({ geometry: new PolylineGeometry({ positions: [], }), }); ``` To fix this, I introduced a `GeometryFactory` base class like we have elsewhere in the code and changed `GeometryInstance` to take either type. This is the only place where we actually base "non-geometry Geometry" in the API. Happy to consider other names, like `GeometryCreator` if we don't like factory for some reason, but I want to get this in sooner rather than later for 1.70.1 fixes. Also fixed an issue with tsconfig.json I introduced in my last change which was failing to actually catch TS compile errors because it wasn't including the Cesium.d.ts. --- Source/Core/GeometryFactory.js | 22 ++ Source/Core/GeometryInstance.js | 2 +- Source/Core/PlaneGeometry.js | 2 +- Source/Core/SimplePolylineGeometry.js | 2 +- Source/Core/SphereGeometry.js | 2 +- Source/Core/SphereOutlineGeometry.js | 2 +- Specs/TypeScript/index.ts | 315 ++++++++++++++++++++++---- Specs/TypeScript/tsconfig.json | 5 +- 8 files changed, 297 insertions(+), 55 deletions(-) create mode 100644 Source/Core/GeometryFactory.js diff --git a/Source/Core/GeometryFactory.js b/Source/Core/GeometryFactory.js new file mode 100644 index 00000000000..35ffcaab8b5 --- /dev/null +++ b/Source/Core/GeometryFactory.js @@ -0,0 +1,22 @@ +import DeveloperError from "../Core/DeveloperError.js"; + +/** + * @constructor + * @class + * @abstract + */ +function GeometryFactory() { + DeveloperError.throwInstantiationError(); +} + +/** + * Returns a geometry. + * + * @param {GeometryFactory} geometryFactory A description of the circle. + * @returns {Geometry|undefined} The computed vertices and indices. + */ +GeometryFactory.createGeometry = function (geometryFactory) { + DeveloperError.throwInstantiationError(); +}; + +export default GeometryFactory; diff --git a/Source/Core/GeometryInstance.js b/Source/Core/GeometryInstance.js index 5b845ac4f83..195e61cf4eb 100644 --- a/Source/Core/GeometryInstance.js +++ b/Source/Core/GeometryInstance.js @@ -13,7 +13,7 @@ import Matrix4 from "./Matrix4.js"; * @constructor * * @param {Object} options Object with the following properties: - * @param {Geometry} options.geometry The geometry to instance. + * @param {Geometry|GeometryFactory} options.geometry The geometry to instance. * @param {Matrix4} [options.modelMatrix=Matrix4.IDENTITY] The model matrix that transforms to transform the geometry from model to world coordinates. * @param {Object} [options.id] A user-defined object to return when the instance is picked with {@link Scene#pick} or get/set per-instance attributes with {@link Primitive#getGeometryInstanceAttributes}. * @param {Object} [options.attributes] Per-instance attributes like a show or color attribute shown in the example below. diff --git a/Source/Core/PlaneGeometry.js b/Source/Core/PlaneGeometry.js index 2987229952e..200c09bc185 100644 --- a/Source/Core/PlaneGeometry.js +++ b/Source/Core/PlaneGeometry.js @@ -16,7 +16,7 @@ import VertexFormat from "./VertexFormat.js"; * @alias PlaneGeometry * @constructor * - * @param {Object} options Object with the following properties: + * @param {Object} [options] Object with the following properties: * @param {VertexFormat} [options.vertexFormat=VertexFormat.DEFAULT] The vertex attributes to be computed. * * @example diff --git a/Source/Core/SimplePolylineGeometry.js b/Source/Core/SimplePolylineGeometry.js index 08ba7e6f1e8..08fcb5bd134 100644 --- a/Source/Core/SimplePolylineGeometry.js +++ b/Source/Core/SimplePolylineGeometry.js @@ -251,7 +251,7 @@ var generateArcOptionsScratch = { * Computes the geometric representation of a simple polyline, including its vertices, indices, and a bounding sphere. * * @param {SimplePolylineGeometry} simplePolylineGeometry A description of the polyline. - * @returns {Geometry} The computed vertices and indices. + * @returns {Geometry|undefined} The computed vertices and indices. */ SimplePolylineGeometry.createGeometry = function (simplePolylineGeometry) { var positions = simplePolylineGeometry._positions; diff --git a/Source/Core/SphereGeometry.js b/Source/Core/SphereGeometry.js index b47c4dfbb9e..9a3e46c9754 100644 --- a/Source/Core/SphereGeometry.js +++ b/Source/Core/SphereGeometry.js @@ -110,7 +110,7 @@ SphereGeometry.unpack = function (array, startingIndex, result) { * Computes the geometric representation of a sphere, including its vertices, indices, and a bounding sphere. * * @param {SphereGeometry} sphereGeometry A description of the sphere. - * @returns {Geometry} The computed vertices and indices. + * @returns {Geometry|undefined} The computed vertices and indices. */ SphereGeometry.createGeometry = function (sphereGeometry) { return EllipsoidGeometry.createGeometry(sphereGeometry._ellipsoidGeometry); diff --git a/Source/Core/SphereOutlineGeometry.js b/Source/Core/SphereOutlineGeometry.js index 2d4133775e1..dbb3d1b1fe3 100644 --- a/Source/Core/SphereOutlineGeometry.js +++ b/Source/Core/SphereOutlineGeometry.js @@ -110,7 +110,7 @@ SphereOutlineGeometry.unpack = function (array, startingIndex, result) { * Computes the geometric representation of an outline of a sphere, including its vertices, indices, and a bounding sphere. * * @param {SphereOutlineGeometry} sphereGeometry A description of the sphere outline. - * @returns {Geometry} The computed vertices and indices. + * @returns {Geometry|undefined} The computed vertices and indices. */ SphereOutlineGeometry.createGeometry = function (sphereGeometry) { return EllipsoidOutlineGeometry.createGeometry( diff --git a/Specs/TypeScript/index.ts b/Specs/TypeScript/index.ts index 4e07cba7180..97f13e0a3e3 100644 --- a/Specs/TypeScript/index.ts +++ b/Specs/TypeScript/index.ts @@ -1,65 +1,101 @@ import { ArcGisMapServerImageryProvider, + ArcGISTiledElevationTerrainProvider, BingMapsImageryProvider, - ImageryProvider, - WebMapServiceImageryProvider, - OpenStreetMapImageryProvider, - TileMapServiceImageryProvider, + BoxGeometry, + BoxOutlineGeometry, + CallbackProperty, + Camera, + Cartesian3, + CesiumTerrainProvider, + CheckerboardMaterialProperty, + CircleGeometry, + CircleOutlineGeometry, + ColorMaterialProperty, + CompositeMaterialProperty, + CompositePositionProperty, + CompositeProperty, + ConstantPositionProperty, + ConstantProperty, + CoplanarPolygonGeometry, + CoplanarPolygonOutlineGeometry, + CorridorGeometry, + CorridorOutlineGeometry, + CustomDataSource, + CylinderGeometry, + CylinderOutlineGeometry, + CzmlDataSource, + DataSource, + EllipseGeometry, + EllipseOutlineGeometry, + EllipsoidGeometry, + EllipsoidOutlineGeometry, + EllipsoidTerrainProvider, + EntityCollection, + FrustumGeometry, + FrustumOutlineGeometry, + GeoJsonDataSource, + GeometryInstance, GoogleEarthEnterpriseImageryProvider, GoogleEarthEnterpriseMapsProvider, + GoogleEarthEnterpriseMetadata, + GoogleEarthEnterpriseTerrainProvider, GridImageryProvider, + GridMaterialProperty, + GroundPolylineGeometry, + ImageMaterialProperty, + ImageryProvider, IonImageryProvider, + KmlDataSource, MapboxImageryProvider, MapboxStyleImageryProvider, - SingleTileImageryProvider, - TileCoordinatesImageryProvider, - UrlTemplateImageryProvider, - WebMapTileServiceImageryProvider, - GoogleEarthEnterpriseMetadata, - TerrainProvider, - ArcGISTiledElevationTerrainProvider, - CesiumTerrainProvider, - EllipsoidTerrainProvider, - GoogleEarthEnterpriseTerrainProvider, - VRTheWorldTerrainProvider, - DataSource, - CzmlDataSource, - GeoJsonDataSource, - KmlDataSource, - CustomDataSource, - Camera, - Scene, - Property, - PropertyBag, - ConstantProperty, - SampledProperty, - PositionProperty, - SampledPositionProperty, - TimeIntervalCollectionProperty, - CompositeProperty, - CompositePositionProperty, - Cartesian3, - ReferenceProperty, MaterialProperty, - EntityCollection, - CallbackProperty, - ConstantPositionProperty, - TimeIntervalCollectionPositionProperty, - ColorMaterialProperty, - CompositeMaterialProperty, - GridMaterialProperty, - ImageMaterialProperty, + NodeTransformationProperty, + OpenStreetMapImageryProvider, + OrthographicFrustum, + PlaneGeometry, + PlaneOutlineGeometry, + PolygonGeometry, + PolygonHierarchy, + PolygonOutlineGeometry, + PolylineArrowMaterialProperty, + PolylineDashMaterialProperty, + PolylineGeometry, PolylineGlowMaterialProperty, PolylineOutlineMaterialProperty, + PolylineVolumeGeometry, + PolylineVolumeOutlineGeometry, + PositionProperty, + PositionPropertyArray, + Property, + PropertyArray, + PropertyBag, + Quaternion, + Rectangle, + RectangleGeometry, + RectangleOutlineGeometry, + ReferenceProperty, + SampledPositionProperty, + SampledProperty, + Scene, + SimplePolylineGeometry, + SingleTileImageryProvider, + SphereGeometry, + SphereOutlineGeometry, StripeMaterialProperty, - CheckerboardMaterialProperty, - PolylineDashMaterialProperty, - VelocityVectorProperty, + TerrainProvider, + TileCoordinatesImageryProvider, + TileMapServiceImageryProvider, + TimeIntervalCollectionPositionProperty, + TimeIntervalCollectionProperty, + UrlTemplateImageryProvider, VelocityOrientationProperty, - PropertyArray, - PositionPropertyArray, - PolylineArrowMaterialProperty, - NodeTransformationProperty, + VelocityVectorProperty, + VRTheWorldTerrainProvider, + WallGeometry, + WallOutlineGeometry, + WebMapServiceImageryProvider, + WebMapTileServiceImageryProvider, } from "cesium"; // Verify ImageryProvider instances conform to the expected interface @@ -158,3 +194,186 @@ property = materialProperty = new StripeMaterialProperty(); property = materialProperty = new CheckerboardMaterialProperty(); property = materialProperty = new PolylineDashMaterialProperty(); property = materialProperty = new PolylineArrowMaterialProperty(); + +// Verify GeometryInstance can be take XXXGeometry objects. +let geometryInstance: GeometryInstance; + +geometryInstance = new GeometryInstance({ + geometry: new BoxGeometry({ + minimum: new Cartesian3(0, 0, 0), + maximum: new Cartesian3(1, 1, 1), + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new BoxOutlineGeometry({ + minimum: new Cartesian3(0, 0, 0), + maximum: new Cartesian3(1, 1, 1), + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CircleGeometry({ + center: new Cartesian3(0, 0, 0), + radius: 10, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CircleOutlineGeometry({ + center: new Cartesian3(0, 0, 0), + radius: 10, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CoplanarPolygonGeometry({ + polygonHierarchy: new PolygonHierarchy(), + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CoplanarPolygonOutlineGeometry({ + polygonHierarchy: new PolygonHierarchy(), + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CorridorGeometry({ positions: [], width: 1 }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CorridorOutlineGeometry({ positions: [], width: 1 }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CylinderGeometry({ + bottomRadius: 10, + length: 10, + topRadius: 10, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new CylinderOutlineGeometry({ + bottomRadius: 10, + length: 10, + topRadius: 10, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new EllipseGeometry({ + center: Cartesian3.ZERO, + semiMajorAxis: 1, + semiMinorAxis: 10, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new EllipseOutlineGeometry({ + center: Cartesian3.ZERO, + semiMajorAxis: 1, + semiMinorAxis: 10, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new EllipsoidGeometry(), +}); + +geometryInstance = new GeometryInstance({ + geometry: new EllipsoidOutlineGeometry(), +}); + +geometryInstance = new GeometryInstance({ + geometry: new FrustumGeometry({ + frustum: new OrthographicFrustum(), + orientation: new Quaternion(), + origin: Cartesian3.ZERO, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new FrustumOutlineGeometry({ + frustum: new OrthographicFrustum(), + orientation: new Quaternion(), + origin: Cartesian3.ZERO, + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new GroundPolylineGeometry({ positions: [] }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PlaneGeometry(), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PlaneOutlineGeometry(), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PolygonGeometry({ polygonHierarchy: new PolygonHierarchy() }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PolygonOutlineGeometry({ + polygonHierarchy: new PolygonHierarchy(), + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PolylineGeometry({ + positions: [], + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PolylineVolumeGeometry({ + polylinePositions: [], + shapePositions: [], + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new PolylineVolumeOutlineGeometry({ + polylinePositions: [], + shapePositions: [], + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new RectangleGeometry({ rectangle: Rectangle.MAX_VALUE }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new RectangleOutlineGeometry({ rectangle: Rectangle.MAX_VALUE }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new SimplePolylineGeometry({ + positions: [], + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new SphereGeometry(), +}); + +geometryInstance = new GeometryInstance({ + geometry: new SphereOutlineGeometry(), +}); + +geometryInstance = new GeometryInstance({ + geometry: new WallGeometry({ + positions: [], + }), +}); + +geometryInstance = new GeometryInstance({ + geometry: new WallOutlineGeometry({ + positions: [], + }), +}); diff --git a/Specs/TypeScript/tsconfig.json b/Specs/TypeScript/tsconfig.json index dae62585a12..1b1b0d54157 100644 --- a/Specs/TypeScript/tsconfig.json +++ b/Specs/TypeScript/tsconfig.json @@ -5,6 +5,7 @@ "strict": true }, "include": [ - "../../Source" - ], + "index.ts", + "../../Source/Cesium.d.ts" + ] } \ No newline at end of file From a6d0a2c7fcaa0f4842bf5a7ff22c5f7067b8c1c4 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Tue, 9 Jun 2020 14:01:44 -0400 Subject: [PATCH 2/2] Update doc/CHANGES --- CHANGES.md | 1 + Source/Core/GeometryFactory.js | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index d88ad1932e8..a17efbc2b40 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,7 @@ - Fixed JSDoc and TypeScript type definitions for `EllipsoidTangentPlane.fromPoints`, which takes an array of `Cartesian3`, not a single instance. [#8928](https://github.com/CesiumGS/cesium/pull/8928) - Fixed JSDoc and TypeScript type definitions for `EntityCollection.getById` and `CompositeEntityCollection.getById`, which can both return undefined. [#8928](https://github.com/CesiumGS/cesium/pull/8928) - Fixed JSDoc and TypeScript type definitions for `Viewer` options parameter, which was incorrectly listed as required. +- Fixed TypeScript type definitions to allow the creation of `GeometryInstance` instances using `XXXGeometry` classes. [#8941](https://github.com/CesiumGS/cesium/pull/8941). - Fixed a memory leak where some 3D Tiles requests were being unintentionally retained after the requests were cancelled. [#8843](https://github.com/CesiumGS/cesium/pull/8843) ### 1.70.0 - 2020-06-01 diff --git a/Source/Core/GeometryFactory.js b/Source/Core/GeometryFactory.js index 35ffcaab8b5..10ee22d3cf1 100644 --- a/Source/Core/GeometryFactory.js +++ b/Source/Core/GeometryFactory.js @@ -1,6 +1,9 @@ import DeveloperError from "../Core/DeveloperError.js"; /** + * Base class for all geometry creation utility classes that can be passed to {@link GeometryInstance} + * for asynchronous geometry creation. + * * @constructor * @class * @abstract