Skip to content

Commit

Permalink
Clone offset in adjustBoundingSphereOffset
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jhnwsk committed Dec 5, 2019
1 parent afff7a4 commit fd010a9
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
6 changes: 3 additions & 3 deletions Source/Scene/Camera.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
31 changes: 31 additions & 0 deletions Specs/Scene/CameraSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit fd010a9

Please sign in to comment.