diff --git a/CHANGES.md b/CHANGES.md index cff06df4ce3..09534ebabab 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -32,6 +32,8 @@ - Fix flickering issue caused by bounding sphere retrieval being blocked by the bounding sphere of another entity. [#12230](https://github.com/CesiumGS/cesium/pull/12230) - Fixed `ImageBasedLighting.imageBasedLightingFactor` not affecting lighting. [#12129](https://github.com/CesiumGS/cesium/pull/12129) +- Fix error with normalization of corner points for lines and corridors with collinear points. [#12255](https://github.com/CesiumGS/cesium/pull/12255) + ### 1.122 - 2024-10-01 #### @cesium/engine diff --git a/packages/engine/Source/Core/CorridorGeometryLibrary.js b/packages/engine/Source/Core/CorridorGeometryLibrary.js index 42bbc303148..0b72e825e1f 100644 --- a/packages/engine/Source/Core/CorridorGeometryLibrary.js +++ b/packages/engine/Source/Core/CorridorGeometryLibrary.js @@ -256,10 +256,6 @@ CorridorGeometryLibrary.computePositions = function (params) { Cartesian3.subtract(nextPosition, position, forward), forward, ); - cornerDirection = Cartesian3.normalize( - Cartesian3.add(forward, backward, cornerDirection), - cornerDirection, - ); const forwardProjection = Cartesian3.multiplyByScalar( normal, @@ -284,6 +280,10 @@ CorridorGeometryLibrary.computePositions = function (params) { ); if (doCorner) { + cornerDirection = Cartesian3.normalize( + Cartesian3.add(forward, backward, cornerDirection), + cornerDirection, + ); cornerDirection = Cartesian3.cross( cornerDirection, normal, diff --git a/packages/engine/Source/Core/PolylineVolumeGeometryLibrary.js b/packages/engine/Source/Core/PolylineVolumeGeometryLibrary.js index 6937caab062..1509263eec8 100644 --- a/packages/engine/Source/Core/PolylineVolumeGeometryLibrary.js +++ b/packages/engine/Source/Core/PolylineVolumeGeometryLibrary.js @@ -434,8 +434,6 @@ PolylineVolumeGeometryLibrary.computePositions = function ( } forward = Cartesian3.subtract(nextPosition, position, forward); forward = Cartesian3.normalize(forward, forward); - cornerDirection = Cartesian3.add(forward, backward, cornerDirection); - cornerDirection = Cartesian3.normalize(cornerDirection, cornerDirection); surfaceNormal = ellipsoid.geodeticSurfaceNormal(position, surfaceNormal); const forwardProjection = Cartesian3.multiplyByScalar( @@ -461,6 +459,8 @@ PolylineVolumeGeometryLibrary.computePositions = function ( ); if (doCorner) { + cornerDirection = Cartesian3.add(forward, backward, cornerDirection); + cornerDirection = Cartesian3.normalize(cornerDirection, cornerDirection); cornerDirection = Cartesian3.cross( cornerDirection, surfaceNormal, diff --git a/packages/engine/Specs/Core/CorridorGeometryLibrarySpec.js b/packages/engine/Specs/Core/CorridorGeometryLibrarySpec.js new file mode 100644 index 00000000000..6dc4887b306 --- /dev/null +++ b/packages/engine/Specs/Core/CorridorGeometryLibrarySpec.js @@ -0,0 +1,38 @@ +import { CorridorGeometryLibrary, Ellipsoid } from "../../index.js"; + +describe("Core/CorridorGeometryLibrary", () => { + describe("computePositions", () => { + it("should not compute corners for collinear points", () => { + const options = { + positions: [ + { x: 1, y: 1, z: 1 }, + { x: 2, y: 2, z: 2 }, + { x: 3, y: 3, z: 3 }, + { x: 4, y: 4, z: 4 }, + ], + width: 0.15, + ellipsoid: Ellipsoid.WGS84, + }; + + // The fact it doesn't throw an error also verifies the fix #12255 + const result = CorridorGeometryLibrary.computePositions(options); + expect(result.corners.length).toEqual(0); + }); + + it("should compute corners for non-collinear points", () => { + const options = { + positions: [ + { x: 0, y: 0, z: 1 }, + { x: 1, y: 0, z: 2 }, + { x: 1, y: 1, z: 3 }, + { x: 0, y: 1, z: 4 }, + ], + width: 0.15, + ellipsoid: Ellipsoid.WGS84, + }; + + const result = CorridorGeometryLibrary.computePositions(options); + expect(result.corners.length).toEqual(2); + }); + }); +}); diff --git a/packages/engine/Specs/Core/PolylineVolumeGeometryLibrarySpec.js b/packages/engine/Specs/Core/PolylineVolumeGeometryLibrarySpec.js new file mode 100644 index 00000000000..5e06779d900 --- /dev/null +++ b/packages/engine/Specs/Core/PolylineVolumeGeometryLibrarySpec.js @@ -0,0 +1,56 @@ +import { + Cartesian3, + Cartesian2, + BoundingRectangle, + PolylineVolumeGeometry, + Ellipsoid, + PolylineVolumeGeometryLibrary, +} from "../../index.js"; + +describe("Core/PolylineVolumeGeometryLibrary", () => { + describe("computePositions", () => { + // Tests the fix #12255 + it("should not throw error if positions are collinear after scaling to geodetic surface", () => { + const positions = [ + new Cartesian3(1, 1, 1), + new Cartesian3(2, 2, 2), + new Cartesian3(3, 3, 3), + new Cartesian3(4, 4, 4), + ]; + + const shape = [ + new Cartesian2(-0.15, -0.15), + new Cartesian2(0.15, -0.15), + new Cartesian2(0.15, 0.15), + new Cartesian2(-0.15, 0.15), + ]; + + const ellipsoidStub = new Ellipsoid( + 6378137.0, + 6378137.0, + 6356752.3142451793, + ); + // It's easier to stub the function than to predict the values to be collinear after calling the function + ellipsoidStub.scaleToGeodeticSurface = function (cartesian, result) { + return Cartesian3.clone(cartesian, result); + }; + + const boundingRectangle = new BoundingRectangle(-0.15, -0.15, 0.3, 0.3); + const geometry = new PolylineVolumeGeometry({ + polylinePositions: positions, + shapePositions: shape, + }); + geometry._ellipsoid = ellipsoidStub; + + expect(() => + PolylineVolumeGeometryLibrary.computePositions( + positions, + shape, + boundingRectangle, + geometry, + true, + ), + ).not.toThrowDeveloperError(); + }); + }); +});