From ce5dd9348d3823716ff1b6ec3429177b23d1eabf Mon Sep 17 00:00:00 2001 From: Harry Morris Date: Tue, 21 Jul 2026 21:32:04 +1000 Subject: [PATCH 1/4] fix: render terrain-clamped billboards and labels in 2D and Columbus View --- CHANGES.md | 1 + packages/engine/Source/Scene/Billboard.js | 81 +++++++++++-------- .../engine/Source/Scene/QuadtreePrimitive.js | 57 +++++++++---- .../engine/Source/Scene/SceneTransforms.js | 10 +++ .../Source/Shaders/BillboardCollectionVS.glsl | 4 +- .../Specs/Scene/BillboardCollectionSpec.js | 44 ++++++++++ .../Specs/Scene/QuadtreePrimitiveSpec.js | 34 ++++---- 7 files changed, 164 insertions(+), 67 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 86aaf51407ad..36710f228f89 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,7 @@ - Fixed incorrect JSDoc description for `offCenterFrustum` in `OrthographicFrustum` and `PerspectiveFrustum`, which was copied from `projectionMatrix` and incorrectly described the property as returning a projection matrix. [#13570](https://github.com/CesiumGS/cesium/pull/13570) - Auto-normalize non-unit `alignedAxis` in `BillboardCollection` instead of silently ignoring it. [#6596](https://github.com/CesiumGS/cesium/issues/6596) - Fixed SPZ-compressed Gaussian splat loading to read the compressed payload from the buffer view declared by `KHR_gaussian_splatting_compression_spz_2`, preventing incorrect cache reuse for assets with SPZ payloads in different buffer views. [#12847](https://github.com/CesiumGS/cesium/issues/12847) +- Fixed terrain-clamped billboards, labels, and points being mispositioned and not properly rendering in 2D/Columbus view. [#5042](https://github.com/CesiumGS/cesium/issues/5042) [#12531](https://github.com/CesiumGS/cesium/issues/12531) ## 1.143 - 2026-07-01 diff --git a/packages/engine/Source/Scene/Billboard.js b/packages/engine/Source/Scene/Billboard.js index cdb7d3a3a57f..141a47ce9ee2 100644 --- a/packages/engine/Source/Scene/Billboard.js +++ b/packages/engine/Source/Scene/Billboard.js @@ -1163,30 +1163,22 @@ Billboard._updateClamping = function (collection, owner) { return; } - function updateFunction(clampedPosition) { - const updatedClampedPosition = ellipsoid.cartographicToCartesian( - clampedPosition, - owner._clampedPosition, - ); - + function handleSceneUpdateHeight(clampedPositionCartographic) { + // Apply the height offset in cartographic space if (isHeightReferenceRelative(owner._heightReference)) { - if (owner._mode === SceneMode.SCENE3D) { - clampedPosition.height += position.height; - ellipsoid.cartographicToCartesian( - clampedPosition, - updatedClampedPosition, - ); - } else { - updatedClampedPosition.x += position.height; - } + clampedPositionCartographic.height += position.height; } - owner._clampedPosition = updatedClampedPosition; + // Assign via the setter, as the setter marks the position as dirty. + owner._clampedPosition = ellipsoid.cartographicToCartesian( + clampedPositionCartographic, + owner._clampedPosition, + ); } owner._removeCallbackFunc = scene.updateHeight( position, - updateFunction, + handleSceneUpdateHeight, owner._heightReference, ); @@ -1196,7 +1188,7 @@ Billboard._updateClamping = function (collection, owner) { scratchCartographic.height = height; } - updateFunction(scratchCartographic); + handleSceneUpdateHeight(scratchCartographic); }; /** @@ -1351,14 +1343,28 @@ Billboard.prototype._setTranslate = function (value) { } }; +// `_getActualPosition` is distinct in meaning from `this._actualPosition` +// _getActualPosition is expected to return the _exact_ position in the current render state's projection +// _actualPosition holds the position in the current render frame; its *projected* form is only meaningful in 2D/Columbus View (in 3D it is just a copy of _position) Billboard.prototype._getActualPosition = function () { - return defined(this._clampedPosition) - ? this._clampedPosition - : this._actualPosition; + if (defined(this._clampedPosition)) { + return this._mode === SceneMode.SCENE3D + ? this._clampedPosition + : this._actualPosition; + } + return this._actualPosition; }; Billboard.prototype._setActualPosition = function (value) { - if (!defined(this._clampedPosition)) { + // Store the render-frame position. For clamped billboards in 3D the ECEF + // _clampedPosition is used directly by _getActualPosition, so _actualPosition + // is only needed (and only updated) outside 3D — but it MUST be updated there + // so 2D/Columbus View get the projected position instead of a stale ECEF one. + + // We only update the actual position when one of: + // - Scene mode is not 3D (to capture mode changes) + // - We don't have a clamped position + if (!defined(this._clampedPosition) || this._mode !== SceneMode.SCENE3D) { Cartesian3.clone(value, this._actualPosition); } makeDirty(this, POSITION_INDEX); @@ -1375,7 +1381,17 @@ Billboard._computeActualPosition = function ( if (frameState.mode !== billboard._mode) { billboard._updateClamping(); } - return billboard._clampedPosition; + + // clampedPosition is always ECEF, so safe to return direct in 3D. + if (frameState.mode === SceneMode.SCENE3D) { + return billboard._clampedPosition; + } + + // in 2D and Columbus View project the coordinate into the current map frame. + return SceneTransforms.computeActualEllipsoidPosition( + frameState, + billboard._clampedPosition, + ); } else if (frameState.mode === SceneMode.SCENE3D) { return position; } @@ -1462,20 +1478,17 @@ Billboard.prototype.computeScreenSpacePosition = function (scene, result) { Cartesian2.clone(this._pixelOffset, scratchPixelOffset); Cartesian2.add(scratchPixelOffset, this._translate, scratchPixelOffset); + // If _clampedPosition is set, that is what we favour. + const position = this._clampedPosition ?? this._position; + let modelMatrix = billboardCollection.modelMatrix; - let position = this._position; - if (defined(this._clampedPosition)) { - position = this._clampedPosition; - if (scene.mode !== SceneMode.SCENE3D) { - // position needs to be in world coordinates - const projection = scene.mapProjection; - const ellipsoid = projection.ellipsoid; - const cart = projection.unproject(position, scratchCartographic); - position = ellipsoid.cartographicToCartesian(cart, scratchCartesian3); - modelMatrix = Matrix4.IDENTITY; - } + + if (this._clampedPosition && scene.mode !== SceneMode.SCENE3D) { + // The model matrix isn't applied when rendering clamped in 2D/CV. + modelMatrix = Matrix4.IDENTITY; } + // _computeScreenSpacePosition always expects ECEF position, so no unprojection required. const windowCoordinates = Billboard._computeScreenSpacePosition( modelMatrix, position, diff --git a/packages/engine/Source/Scene/QuadtreePrimitive.js b/packages/engine/Source/Scene/QuadtreePrimitive.js index 21942777b948..912616941032 100644 --- a/packages/engine/Source/Scene/QuadtreePrimitive.js +++ b/packages/engine/Source/Scene/QuadtreePrimitive.js @@ -1444,15 +1444,18 @@ function updateHeights(primitive, frameState) { defined(terrainData) && terrainData.wasCreatedByUpsampling(); if (tile.level > data.level && !upsampledGeometryFromParent) { - let position; - // find cached entry + let positionCarto; + // Cached as a cartographic (converted below). A cartographic is + // inherently mode-independent, so a single entry is valid in every + // scene mode (the raw pick is ECEF in 3D but projected in 2D/CV). const cachedData = tile.getPositionCacheEntry( data.positionCartographic, primitive.maximumScreenSpaceError, ); if (defined(cachedData)) { - // cache hit - position = cachedData; + // cache hit; clone into a scratch so the callback can never mutate + // the cached entry (which would corrupt it for every other tile). + positionCarto = Cartographic.clone(cachedData, scratchCartographic); } else { if (!defined(data.positionOnEllipsoidSurface)) { // cartesian has to be on the ellipsoid surface for `ellipsoid.geodeticSurfaceNormal` @@ -1518,7 +1521,7 @@ function updateHeights(primitive, frameState) { Cartesian3.clone(Cartesian3.UNIT_X, scratchRay.direction); } - position = tile.data.pick( + const position = tile.data.pick( scratchRay, mode, projection, @@ -1527,21 +1530,41 @@ function updateHeights(primitive, frameState) { ); if (defined(position)) { - // `pick` wrote into the module-level `scratchPosition`, so `position` - // aliases it — clone before caching or the next pick mutates every entry. - tile.setPositionCacheEntry( - data.positionCartographic, - primitive.maximumScreenSpaceError, - Cartesian3.clone(position), - ); + // Convert the mode-frame pick result to a mode-independent + // cartographic before caching, so a cached entry is valid in every + // scene mode. + if (mode === SceneMode.SCENE3D) { + // In 3D the pick result is already ECEF. + positionCarto = ellipsoid.cartesianToCartographic( + position, + scratchCartographic, + ); + } else { + // In 2D and Columbus View the pick result is in the projected map frame, laid out as + // (height, easting, northing). Un-swizzle it back to the projection's native + // (easting, northing, height) layout and unproject to recover the true cartographic. + const projected = Cartesian3.fromElements( + position.y, + position.z, + position.x, + scratchPosition, + ); + positionCarto = projection.unproject( + projected, + scratchCartographic, + ); + } + if (defined(positionCarto)) { + tile.setPositionCacheEntry( + data.positionCartographic, + primitive.maximumScreenSpaceError, + Cartographic.clone(positionCarto), + ); + } } } - if (defined(position)) { + if (defined(positionCarto)) { if (defined(data.callback)) { - const positionCarto = ellipsoid.cartesianToCartographic( - position, - scratchCartographic, - ); data.callback(positionCarto); } data.level = tile.level; diff --git a/packages/engine/Source/Scene/SceneTransforms.js b/packages/engine/Source/Scene/SceneTransforms.js index d982dbd8308e..f02c4c0166d1 100644 --- a/packages/engine/Source/Scene/SceneTransforms.js +++ b/packages/engine/Source/Scene/SceneTransforms.js @@ -296,6 +296,16 @@ const projectedPosition = new Cartesian3(); const positionInCartographic = new Cartographic(); /** + * Transforms a position in world coordinates to the coordinate frame used to render the current scene mode. + * + * In 3D this is the same as the input ECEF coordinate. + * In 2D and Columbus View this is the projected map frame laid out as (height, easting, northing). + * In 2D the height is flattened to 0. + * + * @param {FrameState} frameState + * @param {Cartesian3} position The world-space (ECEF) position. + * @param {Cartesian3} [result] + * @returns {Cartesian3|undefined} * @private */ SceneTransforms.computeActualEllipsoidPosition = function ( diff --git a/packages/engine/Source/Shaders/BillboardCollectionVS.glsl b/packages/engine/Source/Shaders/BillboardCollectionVS.glsl index 7d642a7a1207..3caf7ea8c44b 100644 --- a/packages/engine/Source/Shaders/BillboardCollectionVS.glsl +++ b/packages/engine/Source/Shaders/BillboardCollectionVS.glsl @@ -292,7 +292,9 @@ void main() v_compressed.y = enableDepthCheck; #ifdef VS_THREE_POINT_DEPTH_CHECK -if (lengthSq < (u_threePointDepthTestDistance * u_threePointDepthTestDistance) && (enableDepthCheck == 1.0)) { +// In 2D, the globe and billboards are coplanar, but the depth reconstruction from getGlobeDepth is imprecise +// It suffered error greater than the depthsilon of 10.0, causing the verts to be discarded for clamped billboards. +if (czm_sceneMode != czm_sceneMode2D && lengthSq < (u_threePointDepthTestDistance * u_threePointDepthTestDistance) && (enableDepthCheck == 1.0)) { float depthsilon = 10.0; vec2 depthOrigin; // Horizontal origin for labels comes from a special attribute. If that value is 0, this is a billboard - use the regular origin. diff --git a/packages/engine/Specs/Scene/BillboardCollectionSpec.js b/packages/engine/Specs/Scene/BillboardCollectionSpec.js index f124b5f6483e..46c4333d6cde 100644 --- a/packages/engine/Specs/Scene/BillboardCollectionSpec.js +++ b/packages/engine/Specs/Scene/BillboardCollectionSpec.js @@ -20,6 +20,7 @@ import { BlendOption, HeightReference, HorizontalOrigin, + SceneMode, TextureAtlas, VerticalOrigin, SplitDirection, @@ -2705,6 +2706,49 @@ describe("Scene/BillboardCollection", function () { expect(b._clampedPosition).toBeUndefined(); }); + it("uses a projected position (not the ECEF clamped position) in 2D", function () { + // Regression: in 2D a clamped billboard kept its Earth-centered + // _clampedPosition as the render-frame position and was drawn off the + // map. In 2D/CV the render frame is the projected map position. + spyOn(scene, "updateHeight"); + + const position = Cartesian3.fromDegrees(-72.0, 40.0); + const b = billboardsWithHeight.add({ + heightReference: HeightReference.CLAMP_TO_GROUND, + position: position, + }); + + // In 3D the render-frame position is the ECEF clamped position itself. + scene.renderForSpecs(); + expect(b._clampedPosition).toBeDefined(); + expect(b._getActualPosition()).toEqual(b._clampedPosition); + + scene.morphTo2D(0.0); + return pollToPromise(function () { + scene.renderForSpecs(); + return scene.mode === SceneMode.SCENE2D; + }).then(function () { + const projection = scene.mapProjection; + const carto = projection.ellipsoid.cartesianToCartographic( + b._clampedPosition, + ); + const projected = projection.project(carto); + // 2D flattens the height to the map datum: (0, easting, northing). + const expected = new Cartesian3(0.0, projected.x, projected.y); + + const actual = b._getActualPosition(); + expect(actual).toEqualEpsilon(expected, CesiumMath.EPSILON6); + // And crucially NOT the raw ECEF clamped position. + expect( + Cartesian3.equalsEpsilon( + actual, + b._clampedPosition, + CesiumMath.EPSILON3, + ), + ).toBe(false); + }); + }); + it("removes callback after disableDepthTest", function () { const removeCallback = jasmine.createSpy(); spyOn(scene, "updateHeight").and.returnValue(removeCallback); diff --git a/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js b/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js index c1bb43dab8df..5a98bba8f5ef 100644 --- a/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js +++ b/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js @@ -1086,18 +1086,18 @@ describe("Scene/QuadtreePrimitive", function () { expect(position1).toEqual(position2); }); - it("position cache stores a new Cartesian3 per entry to prevent unintended mutation", function () { + it("position cache stores a distinct cartographic per entry to prevent unintended mutation", function () { // Previously, every pick() inside `updateHeights` passed the same module-level `scratchPosition`, - // so every cached entry ended up aliasing a single shared Cartesian3. + // so every cached entry ended up aliasing a single shared object. // The next pick() mutated every prior entry across every tile. // Observable symptom is clamped billboards jumping to heights from neighbouring tiles // (CesiumGS/cesium#12602). - // To test, we drive two `updateHeight` registrations whose picks return different cartesians, - // then assert that every cached cartesian is a distinct object - // AND that none of them were mutated to match the last pick's cartesian after the fact. - // Prior to fix, all cache entries would reference the same object - // with value {x:4000, y:5000, z:6000}, i.e. the final pick. + // The pick result is converted to a mode-independent cartographic before caching. + // We drive two `updateHeight` registrations whose picks return different positions, + // then assert that every cached cartographic is a distinct object + // AND that none of them were mutated to match the last pick after the fact. + // Prior to fix, all cache entries would reference the same object, i.e. the final pick. const tileProvider = createSpyTileProvider(); tileProvider.getReady.and.returnValue(true); @@ -1110,8 +1110,12 @@ describe("Scene/QuadtreePrimitive", function () { }, }; - const positionA = new Cartesian3(1000, 2000, 3000); - const positionB = new Cartesian3(4000, 5000, 6000); + // Realistic surface ECEF positions so `cartesianToCartographic` succeeds. + const positionA = Cartesian3.fromDegrees(-72.0, 40.0, 100.0); + const positionB = Cartesian3.fromDegrees(10.0, -20.0, 250.0); + const ellipsoid = Ellipsoid.WGS84; + const cartoA = ellipsoid.cartesianToCartographic(positionA); + const cartoB = ellipsoid.cartesianToCartographic(positionB); let pickCount = 0; tileProvider.loadTile.and.callFake(function (frameState, tile) { @@ -1170,18 +1174,18 @@ describe("Scene/QuadtreePrimitive", function () { expect(pickCount).toBeGreaterThanOrEqual(2); expect(storedValues.length).toBeGreaterThanOrEqual(2); - // Every stored value must be a distinct Cartesian3 instance. - // With the alias bug, two stored entries would reference the same `scratchPosition`. + // Every stored value must be a distinct object instance. + // With the alias bug, two stored entries would reference the same scratch. const uniqueRefs = new Set(storedValues); expect(uniqueRefs.size).toBe(storedValues.length); - // The stored positions must equal the values pick() produced at the time they were cached. - // With the bug, every entry would now equal whichever position was cached last. + // The stored cartographics must equal the values pick() produced at the time they + // were cached. With the bug, every entry would now equal whichever was cached last. const hasPositionA = storedValues.some((v) => - Cartesian3.equalsEpsilon(v, positionA, CesiumMath.EPSILON10), + Cartographic.equalsEpsilon(v, cartoA, CesiumMath.EPSILON10), ); const hasPositionB = storedValues.some((v) => - Cartesian3.equalsEpsilon(v, positionB, CesiumMath.EPSILON10), + Cartographic.equalsEpsilon(v, cartoB, CesiumMath.EPSILON10), ); expect(hasPositionA).toBe(true); expect(hasPositionB).toBe(true); From 5b762b7cae9cefcb5c7cb5652133a725a94cee0f Mon Sep 17 00:00:00 2001 From: Harry Morris Date: Tue, 21 Jul 2026 22:16:30 +1000 Subject: [PATCH 2/4] refactor: simplify clamped billboard render-position accessors --- packages/engine/Source/Scene/Billboard.js | 34 ++++++------------- .../Specs/Scene/BillboardCollectionSpec.js | 1 + 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/packages/engine/Source/Scene/Billboard.js b/packages/engine/Source/Scene/Billboard.js index 141a47ce9ee2..510668dfd69a 100644 --- a/packages/engine/Source/Scene/Billboard.js +++ b/packages/engine/Source/Scene/Billboard.js @@ -1017,6 +1017,10 @@ Object.defineProperties(Billboard.prototype, { value, this._actualClampedPosition, ); + if (this._mode === SceneMode.SCENE3D) { + // In 3D mode the actual (rendered) position is the same as clamped position (ECEF). + Cartesian3.clone(value, this._actualPosition); + } makeDirty(this, POSITION_INDEX); }, }, @@ -1343,30 +1347,15 @@ Billboard.prototype._setTranslate = function (value) { } }; -// `_getActualPosition` is distinct in meaning from `this._actualPosition` -// _getActualPosition is expected to return the _exact_ position in the current render state's projection -// _actualPosition holds the position in the current render frame; its *projected* form is only meaningful in 2D/Columbus View (in 3D it is just a copy of _position) +// `_actualPosition` is the billboard's position in the current render frame: +// ECEF in 3D, and the projected map coordinate in 2D/Columbus View. +// It is kept current by `recomputeActualPositions` Billboard.prototype._getActualPosition = function () { - if (defined(this._clampedPosition)) { - return this._mode === SceneMode.SCENE3D - ? this._clampedPosition - : this._actualPosition; - } return this._actualPosition; }; Billboard.prototype._setActualPosition = function (value) { - // Store the render-frame position. For clamped billboards in 3D the ECEF - // _clampedPosition is used directly by _getActualPosition, so _actualPosition - // is only needed (and only updated) outside 3D — but it MUST be updated there - // so 2D/Columbus View get the projected position instead of a stale ECEF one. - - // We only update the actual position when one of: - // - Scene mode is not 3D (to capture mode changes) - // - We don't have a clamped position - if (!defined(this._clampedPosition) || this._mode !== SceneMode.SCENE3D) { - Cartesian3.clone(value, this._actualPosition); - } + Cartesian3.clone(value, this._actualPosition); makeDirty(this, POSITION_INDEX); }; @@ -1382,12 +1371,12 @@ Billboard._computeActualPosition = function ( billboard._updateClamping(); } - // clampedPosition is always ECEF, so safe to return direct in 3D. + // Clamped position is already our rendering position when in 3D if (frameState.mode === SceneMode.SCENE3D) { return billboard._clampedPosition; } - // in 2D and Columbus View project the coordinate into the current map frame. + // in 2D and Columbus View we instead project the ECEF coordinate into the current map frame. return SceneTransforms.computeActualEllipsoidPosition( frameState, billboard._clampedPosition, @@ -1478,13 +1467,12 @@ Billboard.prototype.computeScreenSpacePosition = function (scene, result) { Cartesian2.clone(this._pixelOffset, scratchPixelOffset); Cartesian2.add(scratchPixelOffset, this._translate, scratchPixelOffset); - // If _clampedPosition is set, that is what we favour. const position = this._clampedPosition ?? this._position; let modelMatrix = billboardCollection.modelMatrix; if (this._clampedPosition && scene.mode !== SceneMode.SCENE3D) { - // The model matrix isn't applied when rendering clamped in 2D/CV. + // The model matrix isn't applied when rendering clamped in 2D/CV (see BillboardCollection#update) modelMatrix = Matrix4.IDENTITY; } diff --git a/packages/engine/Specs/Scene/BillboardCollectionSpec.js b/packages/engine/Specs/Scene/BillboardCollectionSpec.js index 46c4333d6cde..4609324de1f4 100644 --- a/packages/engine/Specs/Scene/BillboardCollectionSpec.js +++ b/packages/engine/Specs/Scene/BillboardCollectionSpec.js @@ -2724,6 +2724,7 @@ describe("Scene/BillboardCollection", function () { expect(b._getActualPosition()).toEqual(b._clampedPosition); scene.morphTo2D(0.0); + scene.camera.setView({ destination: Rectangle.MAX_VALUE }); return pollToPromise(function () { scene.renderForSpecs(); return scene.mode === SceneMode.SCENE2D; From 7f49a35aa8af7193b224a2ef18b550ce75e35159 Mon Sep 17 00:00:00 2001 From: Harry Morris Date: Tue, 21 Jul 2026 23:39:55 +1000 Subject: [PATCH 3/4] docs: clarify how _actualPosition is kept current across scene modes --- packages/engine/Source/Scene/Billboard.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/engine/Source/Scene/Billboard.js b/packages/engine/Source/Scene/Billboard.js index 510668dfd69a..85d0f7098afa 100644 --- a/packages/engine/Source/Scene/Billboard.js +++ b/packages/engine/Source/Scene/Billboard.js @@ -1349,7 +1349,9 @@ Billboard.prototype._setTranslate = function (value) { // `_actualPosition` is the billboard's position in the current render frame: // ECEF in 3D, and the projected map coordinate in 2D/Columbus View. -// It is kept current by `recomputeActualPositions` +// It is kept current by `recomputeActualPositions` in 2D/Columbus View, and — +// for clamped billboards in 3D — by the `_clampedPosition` setter (3D has no +// per-frame actual-position recompute). Billboard.prototype._getActualPosition = function () { return this._actualPosition; }; From 372754005b3db2230bdb7dd3a9daa1623cdca287 Mon Sep 17 00:00:00 2001 From: Harry Morris Date: Wed, 22 Jul 2026 00:19:31 +1000 Subject: [PATCH 4/4] test: cover clamped billboard positioning in 2D and Columbus View Add coverage for the two subtlest parts of the fix: that updateHeights un-projects a tile pick result back to the correct cartographic in 2D and Columbus View, and that a clamped billboard reports the projected map position (with height preserved) in Columbus View. --- .../Specs/Scene/BillboardCollectionSpec.js | 49 +++++++++++ .../Specs/Scene/QuadtreePrimitiveSpec.js | 84 +++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/packages/engine/Specs/Scene/BillboardCollectionSpec.js b/packages/engine/Specs/Scene/BillboardCollectionSpec.js index 4609324de1f4..4bf8d97e33aa 100644 --- a/packages/engine/Specs/Scene/BillboardCollectionSpec.js +++ b/packages/engine/Specs/Scene/BillboardCollectionSpec.js @@ -2750,6 +2750,55 @@ describe("Scene/BillboardCollection", function () { }); }); + it("uses a projected position (not the ECEF clamped position) in Columbus View", function () { + // Columbus View keeps the real height, unlike 2D which flattens it, + // so the render-frame position is (height, easting, northing). It must + // still be the projected map position, not the raw ECEF clamped one. + spyOn(scene, "updateHeight"); + spyOn(scene, "getHeight").and.returnValue(500.0); + + const position = Cartesian3.fromDegrees(-72.0, 40.0); + const b = billboardsWithHeight.add({ + heightReference: HeightReference.CLAMP_TO_GROUND, + position: position, + }); + + scene.renderForSpecs(); + expect(b._clampedPosition).toBeDefined(); + + scene.morphToColumbusView(0.0); + scene.camera.setView({ destination: Rectangle.MAX_VALUE }); + return pollToPromise(function () { + scene.renderForSpecs(); + return scene.mode === SceneMode.COLUMBUS_VIEW; + }).then(function () { + const projection = scene.mapProjection; + const carto = projection.ellipsoid.cartesianToCartographic( + b._clampedPosition, + ); + const projected = projection.project(carto); + // Columbus View preserves the height: (height, easting, northing). + const expected = new Cartesian3( + projected.z, + projected.x, + projected.y, + ); + + const actual = b._getActualPosition(); + expect(actual).toEqualEpsilon(expected, CesiumMath.EPSILON6); + // The height must be preserved (not flattened to 0 like 2D). + expect(actual.x).toBeGreaterThan(0.0); + // And crucially NOT the raw ECEF clamped position. + expect( + Cartesian3.equalsEpsilon( + actual, + b._clampedPosition, + CesiumMath.EPSILON3, + ), + ).toBe(false); + }); + }); + it("removes callback after disableDepthTest", function () { const removeCallback = jasmine.createSpy(); spyOn(scene, "updateHeight").and.returnValue(removeCallback); diff --git a/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js b/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js index 5a98bba8f5ef..a9fefacc4b5d 100644 --- a/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js +++ b/packages/engine/Specs/Scene/QuadtreePrimitiveSpec.js @@ -1191,6 +1191,90 @@ describe("Scene/QuadtreePrimitive", function () { expect(hasPositionB).toBe(true); }); + it("unprojects the pick result to a cartographic in 2D and Columbus View", function () { + // In 2D and Columbus View, GlobeSurfaceTile.pick returns the hit in the + // projected map frame (swizzled to (height, easting, northing)), not + // ECEF. updateHeights must un-swizzle and unproject that back into a + // cartographic for the callback; running cartesianToCartographic on the + // projected point (as the code once did) would yield a garbage position. + const projection = scene.mapProjection; + const originalMode = scene.frameState.mode; + + [SceneMode.SCENE2D, SceneMode.COLUMBUS_VIEW].forEach(function (mode) { + const tileProvider = createSpyTileProvider(); + tileProvider.getReady.and.returnValue(true); + tileProvider.computeTileVisibility.and.returnValue(Visibility.FULL); + tileProvider.computeDistanceToTile.and.returnValue(1e-15); + tileProvider.terrainProvider = { + getTileDataAvailable: function () { + return true; + }, + }; + + // The terrain point the pick "finds" at the registered location. + const expected = Cartographic.fromDegrees(-72.0, 40.0, 123.0); + + // Reproduce a GlobeSurfaceTile.pick result in the current mode's frame: + // project to (easting, northing, height), then swizzle to (height, easting, northing). + const projected = projection.project(expected); + const pickInModeFrame = new Cartesian3( + projected.z, + projected.x, + projected.y, + ); + + tileProvider.loadTile.and.callFake(function (frameState, tile) { + tile.state = QuadtreeTileLoadState.DONE; + tile.renderable = true; + tile.data = { + pick: function ( + ray, + pickMode, + pickProjection, + cullBackFaces, + result, + ) { + return Cartesian3.clone(pickInModeFrame, result); + }, + mesh: {}, + }; + }); + + const quadtree = new QuadtreePrimitive({ + tileProvider: tileProvider, + }); + + let received; + quadtree.updateHeight( + Cartographic.fromDegrees(-72.0, 40.0), + function (carto) { + received = Cartographic.clone(carto, received); + }, + ); + + scene.frameState.mode = mode; + try { + for (let i = 0; i < 3; ++i) { + // Advance the frame so tile selection reruns each cycle. + ++scene.frameState.frameNumber; + quadtree.update(scene.frameState); + quadtree.beginFrame(scene.frameState); + quadtree.render(scene.frameState); + quadtree.endFrame(scene.frameState); + } + } finally { + scene.frameState.mode = originalMode; + } + + // The callback must receive the true cartographic (mode-independent), + // not cartesianToCartographic of the projected point. + expect(received).toBeDefined(); + expect( + Cartographic.equalsEpsilon(received, expected, CesiumMath.EPSILON7), + ).toBe(true); + }); + }); + it("gives correct priority to tile loads", function () { const tileProvider = createSpyTileProvider(); tileProvider.getReady.and.returnValue(true);