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.

Sign up for GitHub

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

removed the deprecations for planes in clipping plane collection and related items #6498

Merged
merged 4 commits into from
Apr 27, 2018
Merged

removed the deprecations for planes in clipping plane collection and related items #6498

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
13ab40c
removed the deprecations and related items
Apr 25, 2018
e7c2269
deleted failing test because was only relevant for deprecations
Apr 25, 2018
6c61852
Merge branch 'master' into plane-clippingplanecollection-deprecations
Apr 27, 2018
345b443
comment change and update for CHANGES.md
Apr 27, 2018
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
36 changes: 10 additions & 26 deletions Source/Scene/ClippingPlaneCollection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,13 @@ define([
options = defaultValue(options, defaultValue.EMPTY_OBJECT);

this._planes = [];
this._containsUntrackablePlanes = false;

// Do partial texture updates if just one plane is dirty.
// If many planes are dirty, refresh the entire texture.
this._dirtyIndex = -1;
this._multipleDirtyPlanes = false;

// Add each plane to check if it's actually a Plane object instead of a ClippingPlane.
// Use of Plane objects will be deprecated.
// Add each ClippingPlane object.
var planes = options.planes;
if (defined(planes)) {
var planesLength = planes.length;
Expand Down Expand Up @@ -256,16 +254,13 @@ define([
*/
ClippingPlaneCollection.prototype.add = function(plane) {
var newPlaneIndex = this._planes.length;
if (plane instanceof ClippingPlane) {
var that = this;
plane.onChangeCallback = function(index) {
setIndexDirty(that, index);
};
plane.index = newPlaneIndex;
} else {
deprecationWarning('ClippingPlaneCollection.add', 'Ability to use Plane objects with ClippingPlaneCollection.add is deprecated and will be removed in Cesium 1.45. Please use ClippingPlane objects instead.');
this._containsUntrackablePlanes = true;
}

var that = this;
plane.onChangeCallback = function(index) {
setIndexDirty(that, index);
};
plane.index = newPlaneIndex;

setIndexDirty(this, newPlaneIndex);
this._planes.push(plane);
};
Expand Down Expand Up @@ -502,8 +497,8 @@ define([
}

// Use of Plane objects will be deprecated.
// But until then, we have no way of telling if they changed since last frame, so we have to do a full udpate.
var refreshFullTexture = this._multipleDirtyPlanes || this._containsUntrackablePlanes;
// But until then, we have no way of telling if they changed since last frame, so we have to do a full update.
var refreshFullTexture = this._multipleDirtyPlanes;
Copy link
Contributor

Choose a reason for hiding this comment

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

When more than one ClippingPlane is updated in between rendered frames, we refresh the entire clipping plane texture instead of just selectively updating a few adjacent pixels.
Looks like with the new changes though we can replace all uses of refreshFullTexture below with just direct access to this._multipleDirtyPlanes.

var dirtyIndex = this._dirtyIndex;

if (!refreshFullTexture && dirtyIndex === -1) {
Expand Down Expand Up @@ -664,17 +659,6 @@ define([
}
};

/**
* Determines if rendering with clipping planes is supported.
*
* @returns {Boolean} <code>true</code> if ClippingPlaneCollections are supported
* @deprecated
*/
ClippingPlaneCollection.isSupported = function() {
deprecationWarning('ClippingPlaneCollection.isSupported', 'ClippingPlaneCollection.isSupported is deprecated and will be removed in Cesium 1.45. Clipping Planes are now supported on all platforms capable of running Cesium.');
return true;
};

/**
* Function for checking if the context will allow clipping planes with floating point textures.
*
Expand Down
25 changes: 0 additions & 25 deletions Specs/Scene/ClippingPlaneCollectionSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,31 +419,6 @@ defineSuite([
scene.destroyForSpecs();
});

it('detects if a standard Plane has been added and always performs texture updates', function() {
var scene = createScene();

var gl = scene.frameState.context._gl;
spyOn(gl, 'texImage2D').and.callThrough();
spyOn(gl, 'texSubImage2D').and.callThrough();

clippingPlanes = new ClippingPlaneCollection({
planes : [new Plane(Cartesian3.UNIT_X, 1.0)],
enabled : false,
edgeColor : Color.RED,
modelMatrix : transform
});
expect(gl.texImage2D.calls.count()).toEqual(0);

clippingPlanes.update(scene.frameState);
expect(gl.texImage2D.calls.count()).toEqual(2);

clippingPlanes.update(scene.frameState);
expect(gl.texSubImage2D.calls.count()).toEqual(1);

clippingPlanes.destroy();
scene.destroyForSpecs();
});

it('provides a function for attaching the ClippingPlaneCollection to objects', function() {
var clippedObject1 = {
clippingPlanes : undefined
Expand Down