Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw if Plane normal is not normalized #5304

Merged
merged 4 commits into from
May 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Source/Core/Plane.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ define([
'./Cartesian3',
'./defined',
'./DeveloperError',
'./Math',
'./freezeObject'
], function(
Cartesian3,
defined,
DeveloperError,
CesiumMath,
freezeObject) {
'use strict';

Expand All @@ -33,12 +35,17 @@ define([
* @example
* // The plane x=0
* var plane = new Cesium.Plane(Cesium.Cartesian3.UNIT_X, 0.0);
*
* @exception {DeveloperError} Normal must be normalized
*/
function Plane(normal, distance) {
//>>includeStart('debug', pragmas.debug);
if (!defined(normal)) {
throw new DeveloperError('normal is required.');
}
if (!CesiumMath.equalsEpsilon(Cartesian3.magnitude(normal), 1.0, CesiumMath.EPSILON6)) {
throw new DeveloperError('normal must be normalized.');
}
if (!defined(distance)) {
throw new DeveloperError('distance is required.');
}
Expand Down Expand Up @@ -75,6 +82,8 @@ define([
* var point = Cesium.Cartesian3.fromDegrees(-72.0, 40.0);
* var normal = ellipsoid.geodeticSurfaceNormal(point);
* var tangentPlane = Cesium.Plane.fromPointNormal(point, normal);
*
* @exception {DeveloperError} Normal must be normalized
*/
Plane.fromPointNormal = function(point, normal, result) {
//>>includeStart('debug', pragmas.debug);
Expand All @@ -84,6 +93,9 @@ define([
if (!defined(normal)) {
throw new DeveloperError('normal is required.');
}
if (!CesiumMath.equalsEpsilon(Cartesian3.magnitude(normal), 1.0, CesiumMath.EPSILON6)) {
throw new DeveloperError('normal must be normalized.');
}
//>>includeEnd('debug');

var distance = -Cartesian3.dot(normal, point);
Expand All @@ -104,6 +116,8 @@ define([
* @param {Cartesian4} coefficients The plane's normal (normalized).
* @param {Plane} [result] The object onto which to store the result.
* @returns {Plane} A new plane instance or the modified result parameter.
*
* @exception {DeveloperError} Normal must be normalized
*/
Plane.fromCartesian4 = function(coefficients, result) {
//>>includeStart('debug', pragmas.debug);
Expand All @@ -115,6 +129,12 @@ define([
var normal = Cartesian3.fromCartesian4(coefficients, scratchNormal);
var distance = coefficients.w;

//>>includeStart('debug', pragmas.debug);
if (!CesiumMath.equalsEpsilon(Cartesian3.magnitude(normal), 1.0, CesiumMath.EPSILON6)) {
throw new DeveloperError('normal must be normalized.');
}
//>>includeEnd('debug');

if (!defined(result)) {
return new Plane(normal, distance);
} else {
Expand Down
4 changes: 2 additions & 2 deletions Source/Core/PolylinePipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ define([
var wrapLongitudeInversMatrix = new Matrix4();
var wrapLongitudeOrigin = new Cartesian3();
var wrapLongitudeXZNormal = new Cartesian3();
var wrapLongitudeXZPlane = new Plane(Cartesian3.ZERO, 0.0);
var wrapLongitudeXZPlane = new Plane(Cartesian3.UNIT_X, 0.0);
var wrapLongitudeYZNormal = new Cartesian3();
var wrapLongitudeYZPlane = new Plane(Cartesian3.ZERO, 0.0);
var wrapLongitudeYZPlane = new Plane(Cartesian3.UNIT_X, 0.0);
var wrapLongitudeIntersection = new Cartesian3();
var wrapLongitudeOffset = new Cartesian3();

Expand Down
2 changes: 1 addition & 1 deletion Source/Scene/CullingVolume.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ define([

var scratchPlaneCenter = new Cartesian3();
var scratchPlaneNormal = new Cartesian3();
var scratchPlane = new Plane(new Cartesian3(), 0.0);
var scratchPlane = new Plane(new Cartesian3(1.0, 0.0, 0.0), 0.0);

/**
* Constructs a culling volume from a bounding sphere. Creates six planes that create a box containing the sphere.
Expand Down
6 changes: 3 additions & 3 deletions Source/Scene/ScreenSpaceCameraController.js
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ define([
var translateCVEndPos = new Cartesian3();
var translatCVDifference = new Cartesian3();
var translateCVOrigin = new Cartesian3();
var translateCVPlane = new Plane(Cartesian3.ZERO, 0.0);
var translateCVPlane = new Plane(Cartesian3.UNIT_X, 0.0);
var translateCVStartMouse = new Cartesian2();
var translateCVEndMouse = new Cartesian2();

Expand Down Expand Up @@ -920,7 +920,7 @@ define([
var rotateCVTransform = new Matrix4();
var rotateCVVerticalTransform = new Matrix4();
var rotateCVOrigin = new Cartesian3();
var rotateCVPlane = new Plane(Cartesian3.ZERO, 0.0);
var rotateCVPlane = new Plane(Cartesian3.UNIT_X, 0.0);
var rotateCVCartesian3 = new Cartesian3();
var rotateCVCart = new Cartographic();
var rotateCVOldTransform = new Matrix4();
Expand Down Expand Up @@ -1242,7 +1242,7 @@ define([
}

var scratchStrafeRay = new Ray();
var scratchStrafePlane = new Plane(Cartesian3.ZERO, 0.0);
var scratchStrafePlane = new Plane(Cartesian3.UNIT_X, 0.0);
var scratchStrafeIntersection = new Cartesian3();
var scratchStrafeDirection = new Cartesian3();

Expand Down
2 changes: 1 addition & 1 deletion Source/Scene/TileBoundingRegion.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ define([
var westernMidpointScratch = new Cartesian3();
var easternMidpointScratch = new Cartesian3();
var cartographicScratch = new Cartographic();
var planeScratch = new Plane(Cartesian3.ZERO, 0.0);
var planeScratch = new Plane(Cartesian3.UNIT_X, 0.0);
var rayScratch = new Ray();

function computeBox(tileBB, rectangle, ellipsoid) {
Expand Down
24 changes: 23 additions & 1 deletion Specs/Core/PlaneSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ defineSuite([
}).toThrowDeveloperError();
});

it('constructor throws if normal is not normalized', function() {
expect(function() {
return new Plane(new Cartesian3(1.0, 2.0, 3.0), 0.0);
}).toThrowDeveloperError();
});

it('constructor throws without a distance', function() {
expect(function() {
return new Plane(Cartesian3.UNIT_X, undefined);
Expand All @@ -31,6 +37,7 @@ defineSuite([

it('constructs from a point and a normal', function() {
var normal = new Cartesian3(1.0, 2.0, 3.0);
normal = Cartesian3.normalize(normal, normal);
var point = new Cartesian3(4.0, 5.0, 6.0);
var plane = Plane.fromPointNormal(point, normal);
expect(plane.normal).toEqual(normal);
Expand All @@ -39,6 +46,7 @@ defineSuite([

it('constructs from a point and a normal with result', function() {
var normal = new Cartesian3(1.0, 2.0, 3.0);
normal = Cartesian3.normalize(normal, normal);
var point = new Cartesian3(4.0, 5.0, 6.0);

var plane = new Plane(Cartesian3.UNIT_X, 0.0);
Expand Down Expand Up @@ -75,14 +83,28 @@ defineSuite([
}).toThrowDeveloperError();
});

it('fromPointNormal throws if normal is not normalized', function() {
expect(function() {
return Plane.fromPointNormal(Cartesian3.ZERO, Cartesian3.ZERO);
}).toThrowDeveloperError();
});

it('fromCartesian4 throws without coefficients', function() {
expect(function() {
return Plane.fromCartesian4(undefined);
}).toThrowDeveloperError();
});

it('fromCartesian4 throws if normal is not normalized', function() {
expect(function() {
return Plane.fromCartesian4(new Cartesian4(1.0, 2.0, 3.0, 4.0));
}).toThrowDeveloperError();
});

it('gets the distance to a point', function() {
var plane = new Plane(new Cartesian3(1.0, 2.0, 3.0), 12.34);
var normal = new Cartesian3(1.0, 2.0, 3.0);
normal = Cartesian3.normalize(normal, normal);
var plane = new Plane(normal, 12.34);
var point = new Cartesian3(4.0, 5.0, 6.0);

expect(Plane.getPointDistance(plane, point)).toEqual(Cartesian3.dot(plane.normal, point) + plane.distance);
Expand Down