From fd010a9284af2ae25fe14f52cba1291af43fcdfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20W=C4=85sak?= Date: Thu, 5 Dec 2019 15:45:16 +0100 Subject: [PATCH] Clone offset in adjustBoundingSphereOffset Cloning the offset parameter of adjustBoundingSphereOffset results in no argument reassignment in functions that use it. Because this reassignment is not document and therefore unintentional it prevents confusion when using the Camer api. --- CONTRIBUTORS.md | 1 + Source/Scene/Camera.js | 6 +++--- Specs/Scene/CameraSpec.js | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 6346f2b35a77..ce085e672fb3 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -239,3 +239,4 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for details on how to contribute to Cesiu * [Tinco Andringa](https://github.com/tinco) * [André Borud](https://github.com/andreborud) * [Nathan Schulte](https://github.com/nmschulte) +* [Jan Wąsak](https://github.com/jhnwsk) diff --git a/Source/Scene/Camera.js b/Source/Scene/Camera.js index ec9266cbe678..80748eb40716 100644 --- a/Source/Scene/Camera.js +++ b/Source/Scene/Camera.js @@ -2939,9 +2939,7 @@ import SceneMode from './SceneMode.js'; var MINIMUM_ZOOM = 100.0; function adjustBoundingSphereOffset(camera, boundingSphere, offset) { - if (!defined(offset)) { - offset = HeadingPitchRange.clone(Camera.DEFAULT_OFFSET); - } + offset = HeadingPitchRange.clone(defined(offset) ? offset : Camera.DEFAULT_OFFSET); var minimumZoom = camera._scene.screenSpaceCameraController.minimumZoomDistance; var maximumZoom = camera._scene.screenSpaceCameraController.maximumZoomDistance; @@ -3038,7 +3036,9 @@ import SceneMode from './SceneMode.js'; options = defaultValue(options, defaultValue.EMPTY_OBJECT); var scene2D = this._mode === SceneMode.SCENE2D || this._mode === SceneMode.COLUMBUS_VIEW; this._setTransform(Matrix4.IDENTITY); + console.log(options.offset); var offset = adjustBoundingSphereOffset(this, boundingSphere, options.offset); + console.log(options.offset); var position; if (scene2D) { diff --git a/Specs/Scene/CameraSpec.js b/Specs/Scene/CameraSpec.js index 581363eab74a..9a0d4750c675 100644 --- a/Specs/Scene/CameraSpec.js +++ b/Specs/Scene/CameraSpec.js @@ -2633,6 +2633,23 @@ describe('Scene/Camera', function() { expect(camera.pitch).toEqualEpsilon(pitch, CesiumMath.EPSILON5); }); + it('viewBoundingSphere does not modify offset.range when it is zero', function() { + scene.mode = SceneMode.SCENE3D; + + var heading = CesiumMath.toRadians(45.0); + var pitch = CesiumMath.toRadians(-45.0); + var range = 0.0; + var offset = new HeadingPitchRange(heading, pitch, range); + + var sphere = new BoundingSphere(Cartesian3.fromDegrees(-117.16, 32.71, 0.0), 10.0); + camera.viewBoundingSphere(sphere, offset); + camera._setTransform(Matrix4.IDENTITY); + + expect(offset.heading).toEqual(CesiumMath.toRadians(45.0)); + expect(offset.pitch).toEqual(CesiumMath.toRadians(-45.0)); + expect(offset.range).toEqual(0.0); + }); + it('viewBoundingSphere in 2D', function() { var frustum = new OrthographicOffCenterFrustum(); frustum.left = -10.0; @@ -2748,6 +2765,20 @@ describe('Scene/Camera', function() { expect(CesiumMath.equalsEpsilon(distance, maxValue, 0.1)).toBe(true); }); + it('flyToBoundingSphere does not modify options.offset range if it is zero ', function() { + scene.mode = SceneMode.SCENE3D; + var options = { + offset: new HeadingPitchRange(0.0, -1.5, 0.0) + }; + + var sphere = new BoundingSphere(Cartesian3.fromDegrees(-117.16, 32.71, 0.0), 100000.0); + camera.flyToBoundingSphere(sphere, options); + + expect(options.offset.heading).toEqual(0.0); + expect(options.offset.pitch).toEqual(-1.5); + expect(options.offset.range).toEqual(0.0); + }); + it('distanceToBoundingSphere', function() { scene.mode = SceneMode.SCENE3D;