Skip to content

Commit

Permalink
changed the order of distance and pixelRatio args, and added deprecat…
Browse files Browse the repository at this point in the history
…ed warnings
  • Loading branch information
IanLilleyT committed Oct 14, 2019
1 parent 11e4cfa commit a573f75
Show file tree
Hide file tree
Showing 13 changed files with 84 additions and 52 deletions.
2 changes: 1 addition & 1 deletion Apps/Sandcastle/gallery/Star Burst.html
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
var diff = Cesium.Cartesian3.subtract(entityPosition, camera.positionWC, new Cesium.Cartesian3());
var distance = Cesium.Cartesian3.dot(camera.directionWC, diff);

var dimensions = camera.frustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, new Cesium.Cartesian2());
var dimensions = camera.frustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, new Cesium.Cartesian2());
Cesium.Cartesian2.multiplyByScalar(offset, Cesium.Cartesian2.maximumComponent(dimensions), offset);

var labelOffset;
Expand Down
5 changes: 4 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ Change Log

### 1.63 - 2019-11-01

##### Deprecated :hourglass_flowing_sand:
* `OrthographicFrustum.getPixelDimensions`, `OrthographicOffCenterFrustum.getPixelDimensions`, `PerspectiveFrustum.getPixelDimensions`, and `PerspectiveOffCenterFrustum.getPixelDimensions` now take a `pixelRatio` argument before the `result` argument. The previous function definition will no longer work in 1.65. [#8237](https://github.com/AnalyticalGraphicsInc/cesium/pull/8237)

##### Additions :tada:
* Added `pixelRatio` parameter to `OrthographicFrustum.getPixelDimensions`, `OrthographicOffCenterFrustum.getPixelDimensions`, `PerspectiveFrustum.getPixelDimensions`, and `PerspectiveOffCenterFrustum.getPixelDimensions`. Pass in `scene.pixelRatio` for dimensions in CSS pixel units or `1.0` for dimensions in native device pixel units.
* Added `pixelRatio` parameter to `OrthographicFrustum.getPixelDimensions`, `OrthographicOffCenterFrustum.getPixelDimensions`, `PerspectiveFrustum.getPixelDimensions`, and `PerspectiveOffCenterFrustum.getPixelDimensions`. Pass in `scene.pixelRatio` for dimensions in CSS pixel units or `1.0` for dimensions in native device pixel units. [#8237](https://github.com/AnalyticalGraphicsInc/cesium/pull/8237)

##### Fixes :wrench:
* Fixed css pixel usage for polylines, point clouds, models, primitives, and post-processing. [#8113](https://github.com/AnalyticalGraphicsInc/cesium/issues/8113)
Expand Down
15 changes: 11 additions & 4 deletions Source/Core/OrthographicFrustum.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Cartesian2 from './Cartesian2.js';
import Check from './Check.js';
import defaultValue from './defaultValue.js';
import defined from './defined.js';
Expand Down Expand Up @@ -199,8 +200,8 @@ import OrthographicOffCenterFrustum from './OrthographicOffCenterFrustum.js';
*
* @param {Number} drawingBufferWidth The width of the drawing buffer.
* @param {Number} drawingBufferHeight The height of the drawing buffer.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Number} distance The distance to the near plane in meters.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Cartesian2} result The object onto which to store the result.
* @returns {Cartesian2} The modified result parameter or a new instance of {@link Cartesian2} with the pixel's width and height in the x and y properties, respectively.
*
Expand All @@ -211,11 +212,17 @@ import OrthographicOffCenterFrustum from './OrthographicOffCenterFrustum.js';
* @example
* // Example 1
* // Get the width and height of a pixel.
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, scene.pixelRatio, 0.0, new Cesium.Cartesian2());
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, 0.0, scene.pixelRatio, new Cesium.Cartesian2());
*/
OrthographicFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, result) {
OrthographicFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, result) {
update(this);
return this._offCenterFrustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, result);

if (pixelRatio instanceof Cartesian2) {
result = pixelRatio;
pixelRatio = 1.0;
deprecationWarning('getPixelDimensions-parameter-change', 'getPixelDimensions now takes a pixelRatio argument before the result argument in Cesium 1.63. The previous function definition will no longer work in 1.65.');
}
return this._offCenterFrustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, result);
};

/**
Expand Down
19 changes: 13 additions & 6 deletions Source/Core/OrthographicOffCenterFrustum.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Cartesian2 from './Cartesian2.js';
import Cartesian3 from './Cartesian3.js';
import Cartesian4 from './Cartesian4.js';
import CullingVolume from './CullingVolume.js';
Expand Down Expand Up @@ -271,8 +272,8 @@ import Matrix4 from './Matrix4.js';
*
* @param {Number} drawingBufferWidth The width of the drawing buffer.
* @param {Number} drawingBufferHeight The height of the drawing buffer.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Number} distance The distance to the near plane in meters.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Cartesian2} result The object onto which to store the result.
* @returns {Cartesian2} The modified result parameter or a new instance of {@link Cartesian2} with the pixel's width and height in the x and y properties, respectively.
*
Expand All @@ -283,11 +284,17 @@ import Matrix4 from './Matrix4.js';
* @example
* // Example 1
* // Get the width and height of a pixel.
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, scene.pixelRatio, 0.0, new Cesium.Cartesian2());
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, 0.0, scene.pixelRatio, new Cesium.Cartesian2());
*/
OrthographicOffCenterFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, result) {
OrthographicOffCenterFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, result) {
update(this);

if (pixelRatio instanceof Cartesian2) {
result = pixelRatio;
pixelRatio = 1.0;
deprecationWarning('getPixelDimensions-parameter-change', 'getPixelDimensions now takes a pixelRatio argument before the result argument in Cesium 1.63. The previous function definition will no longer work in 1.65.');
}

//>>includeStart('debug', pragmas.debug);
if (!defined(drawingBufferWidth) || !defined(drawingBufferHeight)) {
throw new DeveloperError('Both drawingBufferWidth and drawingBufferHeight are required.');
Expand All @@ -298,15 +305,15 @@ import Matrix4 from './Matrix4.js';
if (drawingBufferHeight <= 0) {
throw new DeveloperError('drawingBufferHeight must be greater than zero.');
}
if (!defined(distance)) {
throw new DeveloperError('distance is required.');
}
if (!defined(pixelRatio)) {
throw new DeveloperError('pixelRatio is required.');
}
if (pixelRatio <= 0) {
throw new DeveloperError('pixelRatio must be greater than zero.');
}
if (!defined(distance)) {
throw new DeveloperError('distance is required.');
}
if (!defined(result)) {
throw new DeveloperError('A result object is required.');
}
Expand Down
18 changes: 13 additions & 5 deletions Source/Core/PerspectiveFrustum.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Cartesian2 from './Cartesian2.js';
import Check from './Check.js';
import defaultValue from './defaultValue.js';
import defined from './defined.js';
Expand Down Expand Up @@ -283,8 +284,8 @@ import PerspectiveOffCenterFrustum from './PerspectiveOffCenterFrustum.js';
*
* @param {Number} drawingBufferWidth The width of the drawing buffer.
* @param {Number} drawingBufferHeight The height of the drawing buffer.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Number} distance The distance to the near plane in meters.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Cartesian2} result The object onto which to store the result.
* @returns {Cartesian2} The modified result parameter or a new instance of {@link Cartesian2} with the pixel's width and height in the x and y properties, respectively.
*
Expand All @@ -295,7 +296,7 @@ import PerspectiveOffCenterFrustum from './PerspectiveOffCenterFrustum.js';
* @example
* // Example 1
* // Get the width and height of a pixel.
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, scene.pixelRatio, 1.0, new Cesium.Cartesian2());
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, 1.0, scene.pixelRatio, new Cesium.Cartesian2());
*
* @example
* // Example 2
Expand All @@ -306,11 +307,18 @@ import PerspectiveOffCenterFrustum from './PerspectiveOffCenterFrustum.js';
* var toCenter = Cesium.Cartesian3.subtract(primitive.boundingVolume.center, position, new Cesium.Cartesian3()); // vector from camera to a primitive
* var toCenterProj = Cesium.Cartesian3.multiplyByScalar(direction, Cesium.Cartesian3.dot(direction, toCenter), new Cesium.Cartesian3()); // project vector onto camera direction vector
* var distance = Cesium.Cartesian3.magnitude(toCenterProj);
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, scene.pixelRatio, distance, new Cesium.Cartesian2());
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, distance, scene.pixelRatio, new Cesium.Cartesian2());
*/
PerspectiveFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, result) {
PerspectiveFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, result) {
update(this);
return this._offCenterFrustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, result);

if (pixelRatio instanceof Cartesian2) {
result = pixelRatio;
pixelRatio = 1.0;
deprecationWarning('getPixelDimensions-parameter-change', 'getPixelDimensions now takes a pixelRatio argument before the result argument in Cesium 1.63. The previous function definition will no longer work in 1.65.');
}

return this._offCenterFrustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, result);
};

/**
Expand Down
21 changes: 14 additions & 7 deletions Source/Core/PerspectiveOffCenterFrustum.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Cartesian2 from './Cartesian2.js';
import Cartesian3 from './Cartesian3.js';
import Cartesian4 from './Cartesian4.js';
import CullingVolume from './CullingVolume.js';
Expand Down Expand Up @@ -310,8 +311,8 @@ import Matrix4 from './Matrix4.js';
*
* @param {Number} drawingBufferWidth The width of the drawing buffer.
* @param {Number} drawingBufferHeight The height of the drawing buffer.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Number} distance The distance to the near plane in meters.
* @param {Number} pixelRatio The scaling factor from pixel space to coordinate space.
* @param {Cartesian2} result The object onto which to store the result.
* @returns {Cartesian2} The modified result parameter or a new instance of {@link Cartesian2} with the pixel's width and height in the x and y properties, respectively.
*
Expand All @@ -322,7 +323,7 @@ import Matrix4 from './Matrix4.js';
* @example
* // Example 1
* // Get the width and height of a pixel.
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, scene.pixelRatio, 1.0, new Cesium.Cartesian2());
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, 1.0, scene.pixelRatio, new Cesium.Cartesian2());
*
* @example
* // Example 2
Expand All @@ -333,11 +334,17 @@ import Matrix4 from './Matrix4.js';
* var toCenter = Cesium.Cartesian3.subtract(primitive.boundingVolume.center, position, new Cesium.Cartesian3()); // vector from camera to a primitive
* var toCenterProj = Cesium.Cartesian3.multiplyByScalar(direction, Cesium.Cartesian3.dot(direction, toCenter), new Cesium.Cartesian3()); // project vector onto camera direction vector
* var distance = Cesium.Cartesian3.magnitude(toCenterProj);
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, scene.pixelRatio, distance, new Cesium.Cartesian2());
* var pixelSize = camera.frustum.getPixelDimensions(scene.drawingBufferWidth, scene.drawingBufferHeight, distance, scene.pixelRatio, new Cesium.Cartesian2());
*/
PerspectiveOffCenterFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, pixelRatio, distance, result) {
PerspectiveOffCenterFrustum.prototype.getPixelDimensions = function(drawingBufferWidth, drawingBufferHeight, distance, pixelRatio, result) {
update(this);

if (pixelRatio instanceof Cartesian2) {
result = pixelRatio;
pixelRatio = 1.0;
deprecationWarning('getPixelDimensions-parameter-change', 'getPixelDimensions now takes a pixelRatio argument before the result argument in Cesium 1.63. The previous function definition will no longer work in 1.65.');
}

//>>includeStart('debug', pragmas.debug);
if (!defined(drawingBufferWidth) || !defined(drawingBufferHeight)) {
throw new DeveloperError('Both drawingBufferWidth and drawingBufferHeight are required.');
Expand All @@ -348,15 +355,15 @@ import Matrix4 from './Matrix4.js';
if (drawingBufferHeight <= 0) {
throw new DeveloperError('drawingBufferHeight must be greater than zero.');
}
if (!defined(distance)) {
throw new DeveloperError('distance is required.');
}
if (!defined(pixelRatio)) {
throw new DeveloperError('pixelRatio is required');
}
if (pixelRatio <= 0) {
throw new DeveloperError('pixelRatio must be greater than zero.');
}
if (!defined(distance)) {
throw new DeveloperError('distance is required.');
}
if (!defined(result)) {
throw new DeveloperError('A result object is required.');
}
Expand Down
2 changes: 1 addition & 1 deletion Source/Scene/Camera.js
Original file line number Diff line number Diff line change
Expand Up @@ -2652,7 +2652,7 @@ import SceneMode from './SceneMode.js';
//>>includeEnd('debug');

var distance = this.distanceToBoundingSphere(boundingSphere);
var pixelSize = this.frustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, this._scene.pixelRatio, distance, scratchPixelSize);
var pixelSize = this.frustum.getPixelDimensions(drawingBufferWidth, drawingBufferHeight, distance, this._scene.pixelRatio, scratchPixelSize);
return Math.max(pixelSize.x, pixelSize.y);
};

Expand Down
18 changes: 9 additions & 9 deletions Specs/Core/OrthographicFrustumSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,31 +139,31 @@ describe('Core/OrthographicFrustum', function() {

it('get pixel dimensions throws without canvas height', function() {
expect(function() {
return frustum.getPixelDimensions(1.0, undefined, 1.0, 0.0, new Cartesian2());
return frustum.getPixelDimensions(1.0, undefined, 0.0, 1.0, new Cartesian2());
}).toThrowDeveloperError();
});

it('get pixel dimensions throws without canvas width', function() {
expect(function() {
return frustum.getPixelDimensions(undefined, 1.0, 1.0, 0.0, new Cartesian2());
return frustum.getPixelDimensions(undefined, 1.0, 0.0, 1.0, new Cartesian2());
}).toThrowDeveloperError();
});

it('get pixel dimensions throws with canvas width less than or equal to zero', function() {
expect(function() {
return frustum.getPixelDimensions(0.0, 1.0, 1.0, 0.0, new Cartesian2());
return frustum.getPixelDimensions(0.0, 1.0, 0.0, 1.0, new Cartesian2());
}).toThrowDeveloperError();
});

it('get pixel dimensions throws with canvas height less than or equal to zero', function() {
expect(function() {
return frustum.getPixelDimensions(1.0, 0.0, 1.0, 0.0, new Cartesian2());
return frustum.getPixelDimensions(1.0, 0.0, 0.0, 1.0, new Cartesian2());
}).toThrowDeveloperError();
});

it('get pixel dimensions throws without pixel ratio', function() {
expect(function() {
return frustum.getPixelDimensions(1.0, 1.0, undefined, 0.0, new Cartesian2());
return frustum.getPixelDimensions(1.0, 1.0, 0.0, undefined, new Cartesian2());
}).toThrowDeveloperError();
});

Expand All @@ -177,8 +177,8 @@ describe('Core/OrthographicFrustum', function() {
var dimensions = new Cartesian2(1.0, 1.0);
var pixelRatio = 1.0;
var distance = 1.0;
var pixelSize = frustum.getPixelDimensions(dimensions.x, dimensions.y, pixelRatio, distance, new Cartesian2());
var expected = frustum._offCenterFrustum.getPixelDimensions(dimensions.x, dimensions.y, pixelRatio, distance, new Cartesian2());
var pixelSize = frustum.getPixelDimensions(dimensions.x, dimensions.y, distance, pixelRatio, new Cartesian2());
var expected = frustum._offCenterFrustum.getPixelDimensions(dimensions.x, dimensions.y, distance, pixelRatio, new Cartesian2());
expect(pixelSize.x).toEqual(expected.x);
expect(pixelSize.y).toEqual(expected.y);
});
Expand All @@ -187,8 +187,8 @@ describe('Core/OrthographicFrustum', function() {
var dimensions = new Cartesian2(1.0, 1.0);
var pixelRatio = 2.0;
var distance = 1.0;
var pixelSize = frustum.getPixelDimensions(dimensions.x, dimensions.y, pixelRatio, distance, new Cartesian2());
var expected = frustum._offCenterFrustum.getPixelDimensions(dimensions.x, dimensions.y, pixelRatio, distance, new Cartesian2());
var pixelSize = frustum.getPixelDimensions(dimensions.x, dimensions.y, distance, pixelRatio, new Cartesian2());
var expected = frustum._offCenterFrustum.getPixelDimensions(dimensions.x, dimensions.y, distance, pixelRatio, new Cartesian2());
expect(pixelSize.x).toEqual(expected.x);
expect(pixelSize.y).toEqual(expected.y);
});
Expand Down
Loading

0 comments on commit a573f75

Please sign in to comment.