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

Treat frustum changes as camera changes in Scene#render #6821

Merged
merged 10 commits into from
Jul 30, 2018
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Change Log
* Added `CoplanarPolygonGeometry` and `CoplanarPolygonGeometryOutline` for drawing polygons composed of coplanar positions that are not necessarily on the ellipsoid surface. [#6769](https://github.com/AnalyticalGraphicsInc/cesium/pull/6769)
* Improved support for polygon entities using `perPositionHeight`, including supporting vertical polygons. This also improves KML compatibility. [#6791](https://github.com/AnalyticalGraphicsInc/cesium/pull/6791)
* Added `Cartesian3.midpoint` to compute the midpoint between two `Cartesian3` positions [#6836](https://github.com/AnalyticalGraphicsInc/cesium/pull/6836)
* Added `equalsEpsilon` methods to `OrthographicFrustum`, `PerspectiveFrustum`, `OrthographicOffCenterFrustum` and `PerspectiveOffCenterFrustum`.

##### Deprecated :hourglass_flowing_sand:
* Support for 3D Tiles `content.url` is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301). Use `content.uri instead`. Support for `content.url` will remain for backwards compatibility. [#6744](https://github.com/AnalyticalGraphicsInc/cesium/pull/6744)
Expand All @@ -31,6 +32,7 @@ Change Log
* Fixed a bug that caused billboard positions to be set incorrectly when using a `CallbackProperty`. [#6815](https://github.com/AnalyticalGraphicsInc/cesium/pull/6815)
* Improved support for generating a TypeScript typings file using `tsd-jsdoc` [#6767](https://github.com/AnalyticalGraphicsInc/cesium/pull/6767)
* Updated viewBoundingSphere to use correct zoomOptions [#6848](https://github.com/AnalyticalGraphicsInc/cesium/issues/6848)
* Fixed a bug that caused the scene to continuously render after resizing the viewer when `requestRenderMode` was enabled. [#6812](https://github.com/AnalyticalGraphicsInc/cesium/issues/6812)

### 1.47 - 2018-07-02

Expand Down
27 changes: 25 additions & 2 deletions Source/Core/OrthographicFrustum.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ define([
'./defined',
'./defineProperties',
'./DeveloperError',
'./Math',
'./OrthographicOffCenterFrustum'
], function(
Check,
defaultValue,
defined,
defineProperties,
DeveloperError,
CesiumMath,
OrthographicOffCenterFrustum) {
'use strict';

Expand Down Expand Up @@ -268,10 +270,31 @@ define([

return (this.width === other.width &&
this.aspectRatio === other.aspectRatio &&
this.near === other.near &&
this.far === other.far &&
this._offCenterFrustum.equals(other._offCenterFrustum));
};

/**
* Compares the provided OrthographicFrustum componentwise and returns
* <code>true</code> if they pass an absolute or relative tolerance test,
* <code>false</code> otherwise.
*
* @param {OrthographicFrustum} other The right hand side OrthographicFrustum.
* @param {Number} relativeEpsilon The relative epsilon tolerance to use for equality testing.
* @param {Number} [absoluteEpsilon=relativeEpsilon] The absolute epsilon tolerance to use for equality testing.
* @returns {Boolean} <code>true</code> if this and other are within the provided epsilon, <code>false</code> otherwise.
*/
OrthographicFrustum.prototype.equalsEpsilon = function(other, relativeEpsilon, absoluteEpsilon) {
if (!defined(other) || !(other instanceof OrthographicFrustum)) {
return false;
}

update(this);
update(other);

return (CesiumMath.equalsEpsilon(this.width, other.width, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.aspectRatio, other.aspectRatio, relativeEpsilon, absoluteEpsilon) &&
this._offCenterFrustum.equalsEpsilon(other._offCenterFrustum, relativeEpsilon, absoluteEpsilon));
};

return OrthographicFrustum;
});
24 changes: 24 additions & 0 deletions Source/Core/OrthographicOffCenterFrustum.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ define([
'./defined',
'./defineProperties',
'./DeveloperError',
'./Math',
'./Matrix4'
], function(
Cartesian3,
Expand All @@ -15,6 +16,7 @@ define([
defined,
defineProperties,
DeveloperError,
CesiumMath,
Matrix4) {
'use strict';

Expand Down Expand Up @@ -370,5 +372,27 @@ define([
this.far === other.far);
};

/**
* Compares the provided OrthographicOffCenterFrustum componentwise and returns
* <code>true</code> if they pass an absolute or relative tolerance test,
* <code>false</code> otherwise.
*
* @param {OrthographicOffCenterFrustum} other The right hand side OrthographicOffCenterFrustum.
* @param {Number} relativeEpsilon The relative epsilon tolerance to use for equality testing.
* @param {Number} [absoluteEpsilon=relativeEpsilon] The absolute epsilon tolerance to use for equality testing.
* @returns {Boolean} <code>true</code> if this and other are within the provided epsilon, <code>false</code> otherwise.
*/
OrthographicOffCenterFrustum.prototype.equalsEpsilon = function(other, relativeEpsilon, absoluteEpsilon) {
return (other === this) ||
(defined(other) &&
other instanceof OrthographicOffCenterFrustum &&
CesiumMath.equalsEpsilon(this.right, other.right, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.left, other.left, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.top, other.top, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.bottom, other.bottom, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.near, other.near, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.far, other.far, relativeEpsilon, absoluteEpsilon));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bagnell Does it make sense to use the same epsilon for all these values?

};

return OrthographicOffCenterFrustum;
});
27 changes: 25 additions & 2 deletions Source/Core/PerspectiveFrustum.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ define([
'./defined',
'./defineProperties',
'./DeveloperError',
'./Math',
'./PerspectiveOffCenterFrustum'
], function(
Check,
defaultValue,
defined,
defineProperties,
DeveloperError,
CesiumMath,
PerspectiveOffCenterFrustum) {
'use strict';

Expand Down Expand Up @@ -363,10 +365,31 @@ define([

return (this.fov === other.fov &&
this.aspectRatio === other.aspectRatio &&
this.near === other.near &&
this.far === other.far &&
this._offCenterFrustum.equals(other._offCenterFrustum));
};

/**
* Compares the provided PerspectiveFrustum componentwise and returns
* <code>true</code> if they pass an absolute or relative tolerance test,
* <code>false</code> otherwise.
*
* @param {PerspectiveFrustum} other The right hand side PerspectiveFrustum.
* @param {Number} relativeEpsilon The relative epsilon tolerance to use for equality testing.
* @param {Number} [absoluteEpsilon=relativeEpsilon] The absolute epsilon tolerance to use for equality testing.
* @returns {Boolean} <code>true</code> if this and other are within the provided epsilon, <code>false</code> otherwise.
*/
PerspectiveFrustum.prototype.equalsEpsilon = function(other, relativeEpsilon, absoluteEpsilon) {
if (!defined(other) || !(other instanceof PerspectiveFrustum)) {
return false;
}

update(this);
update(other);

return (CesiumMath.equalsEpsilon(this.fov, other.fov, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.aspectRatio, other.aspectRatio, relativeEpsilon, absoluteEpsilon) &&
this._offCenterFrustum.equalsEpsilon(other._offCenterFrustum, relativeEpsilon, absoluteEpsilon));
};

return PerspectiveFrustum;
});
24 changes: 24 additions & 0 deletions Source/Core/PerspectiveOffCenterFrustum.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ define([
'./defined',
'./defineProperties',
'./DeveloperError',
'./Math',
'./Matrix4'
], function(
Cartesian3,
Expand All @@ -15,6 +16,7 @@ define([
defined,
defineProperties,
DeveloperError,
CesiumMath,
Matrix4) {
'use strict';

Expand Down Expand Up @@ -421,5 +423,27 @@ define([
this.far === other.far);
};

/**
* Compares the provided PerspectiveOffCenterFrustum componentwise and returns
* <code>true</code> if they pass an absolute or relative tolerance test,
* <code>false</code> otherwise.
*
* @param {PerspectiveOffCenterFrustum} other The right hand side PerspectiveOffCenterFrustum.
* @param {Number} relativeEpsilon The relative epsilon tolerance to use for equality testing.
* @param {Number} [absoluteEpsilon=relativeEpsilon] The absolute epsilon tolerance to use for equality testing.
* @returns {Boolean} <code>true</code> if this and other are within the provided epsilon, <code>false</code> otherwise.
*/
PerspectiveOffCenterFrustum.prototype.equalsEpsilon = function(other, relativeEpsilon, absoluteEpsilon) {
return (other === this) ||
(defined(other) &&
other instanceof PerspectiveOffCenterFrustum &&
CesiumMath.equalsEpsilon(this.right, other.right, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.left, other.left, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.top, other.top, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.bottom, other.bottom, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.near, other.near, relativeEpsilon, absoluteEpsilon) &&
CesiumMath.equalsEpsilon(this.far, other.far, relativeEpsilon, absoluteEpsilon));
};

return PerspectiveOffCenterFrustum;
});
15 changes: 9 additions & 6 deletions Source/Scene/Scene.js
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,8 @@ define([
Cartesian3.equalsEpsilon(camera0.direction, camera1.direction, epsilon) &&
Cartesian3.equalsEpsilon(camera0.up, camera1.up, epsilon) &&
Cartesian3.equalsEpsilon(camera0.right, camera1.right, epsilon) &&
Matrix4.equalsEpsilon(camera0.transform, camera1.transform, epsilon);
Matrix4.equalsEpsilon(camera0.transform, camera1.transform, epsilon) &&
camera0.frustum.equalsEpsilon(camera1.frustum, epsilon);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add (or modify) a test to the Scene request render specs to test if this is being accounted for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble getting the test to fail on master. I'll try again tomorrow but do you have any tips / ideas?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at the other requestRender tests, if you haven't already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, the newest commit has a test that failed before and passes with these changes.

}

function updateDerivedCommands(scene, command) {
Expand Down Expand Up @@ -3172,13 +3173,17 @@ define([

function checkForCameraUpdates(scene) {
var camera = scene._camera;
if (!cameraEqual(camera, scene._cameraClone, CesiumMath.EPSILON15)) {
var cameraClone = scene._cameraClone;

scene._frustumChanged = !camera.frustum.equals(cameraClone.frustum);

if (!cameraEqual(camera, cameraClone, CesiumMath.EPSILON15)) {
if (!scene._cameraStartFired) {
camera.moveStart.raiseEvent();
scene._cameraStartFired = true;
}
scene._cameraMovedTime = getTimestamp();
Camera.clone(camera, scene._cameraClone);
Camera.clone(camera, cameraClone);

return true;
}
Expand Down Expand Up @@ -3310,10 +3315,8 @@ define([
tryAndCatchError(this, time, update);
this._postUpdate.raiseEvent(this, time);

this._frustumChanged = !this._camera.frustum.equals(this._cameraClone.frustum);

var cameraChanged = checkForCameraUpdates(this);
var shouldRender = !this.requestRenderMode || this._renderRequested || cameraChanged || this._frustumChanged || this._logDepthBufferDirty || (this.mode === SceneMode.MORPHING);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the only place scene._frustumChanged is used? If so, we can probably remove it entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used on Line 1513. It didn't look like cameraChanged (or something like it) could be a drop-in replacement so I didn't change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's fine to leave.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha

var shouldRender = !this.requestRenderMode || this._renderRequested || cameraChanged || this._logDepthBufferDirty || (this.mode === SceneMode.MORPHING);
if (!shouldRender && defined(this.maximumRenderTimeChange) && defined(this._lastRenderTime)) {
var difference = Math.abs(JulianDate.secondsDifference(this._lastRenderTime, time));
shouldRender = shouldRender || difference > this.maximumRenderTimeChange;
Expand Down
36 changes: 36 additions & 0 deletions Specs/Core/OrthographicFrustumSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,42 @@ defineSuite([
expect(pixelSize.y).toEqual(2.0);
});

it('equals', function() {
var frustum2 = new OrthographicFrustum();
frustum2.near = 1.0;
frustum2.far = 3.0;
frustum2.width = 2.0;
frustum2.aspectRatio = 1.0;
expect(frustum.equals(frustum2)).toEqual(true);
});

it('equals epsilon', function() {
var frustum2 = new OrthographicFrustum();
frustum2.near = 1.0;
frustum2.far = 3.0;
frustum2.width = 2.0;
frustum2.aspectRatio = 1.0;
expect(frustum.equalsEpsilon(frustum2, CesiumMath.EPSILON7)).toEqual(true);

var frustum3 = new OrthographicFrustum();
frustum3.near = 1.01;
frustum3.far = 3.01;
frustum3.width = 2.01;
frustum3.aspectRatio = 1.01;
expect(frustum.equalsEpsilon(frustum3, CesiumMath.EPSILON1)).toEqual(true);

var frustum4 = new OrthographicFrustum();
frustum4.near = 1.0;
frustum4.far = 3.0;
frustum4.width = 2.0;
frustum4.aspectRatio = 1.1;
expect(frustum.equalsEpsilon(frustum4, CesiumMath.EPSILON2)).toEqual(false);
});

it('equals undefined', function() {
expect(frustum.equals()).toEqual(false);
});

it('throws with undefined frustum parameters', function() {
var frustum = new OrthographicFrustum();
expect(function() {
Expand Down
45 changes: 45 additions & 0 deletions Specs/Core/OrthographicOffCenterFrustumSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,51 @@ defineSuite([
expect(pixelSize.y).toEqual(2.0);
});

it('equals', function() {
var frustum2 = new OrthographicOffCenterFrustum();
frustum2.near = 1.0;
frustum2.far = 3.0;
frustum2.right = 1.0;
frustum2.left = -1.0;
frustum2.top = 1.0;
frustum2.bottom = -1.0;

expect(frustum).toEqual(frustum2);
});

it('equals epsilon', function() {
var frustum2 = new OrthographicOffCenterFrustum();
frustum2.near = 1.0;
frustum2.far = 3.0;
frustum2.right = 1.0;
frustum2.left = -1.0;
frustum2.top = 1.0;
frustum2.bottom = -1.0;
expect(frustum.equalsEpsilon(frustum2, CesiumMath.EPSILON7)).toEqual(true);

var frustum3 = new OrthographicOffCenterFrustum();
frustum3.near = 1.01;
frustum3.far = 2.98;
frustum3.right = 1.02;
frustum3.left = -0.99;
frustum3.top = 0.99;
frustum3.bottom = -1.05;
expect(frustum.equalsEpsilon(frustum3, CesiumMath.EPSILON1)).toEqual(true);

var frustum4 = new OrthographicOffCenterFrustum();
frustum4.near = 1.1;
frustum4.far = 2.9;
frustum4.right = 0.0;
frustum4.left = -1.02;
frustum4.top = 1.02;
frustum4.bottom = -1.005;
expect(frustum.equalsEpsilon(frustum4, CesiumMath.EPSILON2)).toEqual(false);
});

it('equals undefined', function() {
expect(frustum.equals()).toEqual(false);
});

it('throws with undefined frustum parameters', function() {
var frustum = new OrthographicOffCenterFrustum();
expect(function() {
Expand Down
23 changes: 23 additions & 0 deletions Specs/Core/PerspectiveFrustumSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,29 @@ defineSuite([
expect(frustum.equals(frustum2)).toEqual(true);
});

it('equals epsilon', function() {
var frustum2 = new PerspectiveFrustum();
frustum2.near = 1.0;
frustum2.far = 2.0;
frustum2.fov = (Math.PI) / 3.0;
frustum2.aspectRatio = 1.0;
expect(frustum.equalsEpsilon(frustum2, CesiumMath.EPSILON7)).toEqual(true);

var frustum3 = new PerspectiveFrustum();
frustum3.near = 1.01;
frustum3.far = 2.01;
frustum3.fov = ((Math.PI) / 3.0) + 0.01;
frustum3.aspectRatio = 1.01;
expect(frustum.equalsEpsilon(frustum3, CesiumMath.EPSILON1)).toEqual(true);

var frustum4 = new PerspectiveFrustum();
frustum4.near = 1.0;
frustum4.far = 2.0;
frustum4.fov = (Math.PI) / 3.0;
frustum4.aspectRatio = 1.1;
expect(frustum.equalsEpsilon(frustum4, CesiumMath.EPSILON2)).toEqual(false);
});

it('equals undefined', function() {
expect(frustum.equals()).toEqual(false);
});
Expand Down
Loading