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

Fix 1.67 ground polygon regression #8694

Merged
merged 2 commits into from
Mar 19, 2020
+118 −69
Merged

Fix 1.67 ground polygon regression #8694

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1be7513
Fix 1.67 ground polygon regression
mramato Mar 19, 2020
806bb17
Update CHANGES
mramato Mar 19, 2020
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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@ Change Log
* `TileMapServiceImageryProvider` will now force `minimumLevel` to 0 if the `tilemapresource.xml` metadata request fails and the `rectangle` is too large for the given detail level [#8448](https://github.com/AnalyticalGraphicsInc/cesium/pull/8448)
* Fixed ground atmosphere rendering when using a samller ellipsoid. [#8683](https://github.com/CesiumGS/cesium/issues/8683)
* Fixed globe incorrectly occluding objects when using a smaller ellipsoid. [#7124](https://github.com/CesiumGS/cesium/issues/7124)
* Fixed a regression introduced in 1.67 which caused overlapping colored ground geometry to have visual artifacts. [#8694](https://github.com/CesiumGS/cesium/pull/8694)

### 1.67.0 - 2020-03-02

69 changes: 40 additions & 29 deletions Source/DataSources/StaticGroundGeometryColorBatch.js
View file
Original file line number Diff line number Diff line change
@@ -8,17 +8,17 @@ import ShowGeometryInstanceAttribute from '../Core/ShowGeometryInstanceAttribute
import GroundPrimitive from '../Scene/GroundPrimitive.js';
import BoundingSphereState from './BoundingSphereState.js';
import Property from './Property.js';
import RectangleCollisionChecker from '../Core/RectangleCollisionChecker.js';

var colorScratch = new Color();
var distanceDisplayConditionScratch = new DistanceDisplayCondition();
var defaultDistanceDisplayCondition = new DistanceDisplayCondition();

function Batch(primitives, classificationType, color, key, zIndex) {
function Batch(primitives, classificationType, color, zIndex) {
this.primitives = primitives;
this.zIndex = zIndex;
this.classificationType = classificationType;
this.color = color;
this.key = key;
this.createPrimitive = false;
this.waitingOnCreate = false;
this.primitive = undefined;
@@ -31,13 +31,19 @@ import Property from './Property.js';
this.showsUpdated = new AssociativeArray();
this.itemsToRemove = [];
this.isDirty = false;
this.rectangleCollisionCheck = new RectangleCollisionChecker();
}

Batch.prototype.overlapping = function(rectangle) {
return this.rectangleCollisionCheck.collides(rectangle);
};

Batch.prototype.add = function(updater, instance) {
var id = updater.id;
this.createPrimitive = true;
this.geometry.set(id, instance);
this.updaters.set(id, updater);
this.rectangleCollisionCheck.insert(id, instance.geometry.rectangle);
if (!updater.hasConstantFill || !updater.fillMaterialProperty.isConstant || !Property.isConstant(updater.distanceDisplayConditionProperty)) {
this.updatersWithAttributes.set(id, updater);
} else {
@@ -52,8 +58,10 @@ import Property from './Property.js';

Batch.prototype.remove = function(updater) {
var id = updater.id;
var geometryInstance = this.geometry.get(id);
this.createPrimitive = this.geometry.remove(id) || this.createPrimitive;
if (this.updaters.remove(id)) {
this.rectangleCollisionCheck.remove(id, geometryInstance.geometry.rectangle);
this.updatersWithAttributes.remove(id);
var unsubscribe = this.subscriptions.get(id);
if (defined(unsubscribe)) {
@@ -227,33 +235,39 @@ import Property from './Property.js';
* @private
*/
function StaticGroundGeometryColorBatch(primitives, classificationType) {
this._batches = new AssociativeArray();
this._batches = [];
this._primitives = primitives;
this._classificationType = classificationType;
}

StaticGroundGeometryColorBatch.prototype.add = function(time, updater) {
var instance = updater.createFillGeometryInstance(time);
var batches = this._batches;
// zIndex is a batch breaker, so we'll use that for the key
var zIndex = Property.getValueOrDefault(updater.zIndex, 0);
var batchKey = zIndex;
var batch;
if (batches.contains(batchKey)) {
batch = batches.get(batchKey);
} else {
batch = new Batch(this._primitives, this._classificationType, instance.attributes.color.value, batchKey, zIndex);
batches.set(batchKey, batch);
var length = batches.length;
for (var i = 0; i < length; ++i) {
var item = batches[i];
if (item.zIndex === zIndex &&
!item.overlapping(instance.geometry.rectangle)) {
batch = item;
break;
}
}

if (!defined(batch)) {
batch = new Batch(this._primitives, this._classificationType, instance.attributes.color.value, zIndex);
batches.push(batch);
}
batch.add(updater, instance);
return batch;
};

StaticGroundGeometryColorBatch.prototype.remove = function(updater) {
var batchesArray = this._batches.values;
var count = batchesArray.length;
var batches = this._batches;
var count = batches.length;
for (var i = 0; i < count; ++i) {
if (batchesArray[i].remove(updater)) {
if (batches[i].remove(updater)) {
return;
}
}
@@ -266,15 +280,14 @@ import Property from './Property.js';
//Perform initial update
var isUpdated = true;
var batches = this._batches;
var batchesArray = batches.values;
var batchCount = batchesArray.length;
var batchCount = batches.length;
for (i = 0; i < batchCount; ++i) {
isUpdated = batchesArray[i].update(time) && isUpdated;
isUpdated = batches[i].update(time) && isUpdated;
}

//If any items swapped between batches we need to move them
for (i = 0; i < batchCount; ++i) {
var oldBatch = batchesArray[i];
var oldBatch = batches[i];
var itemsToRemove = oldBatch.itemsToRemove;
var itemsToMoveLength = itemsToRemove.length;
for (var j = 0; j < itemsToMoveLength; j++) {
@@ -287,27 +300,25 @@ import Property from './Property.js';
}

//If we moved anything around, we need to re-build the primitive and remove empty batches
var batchesArrayCopy = batchesArray.slice();
var batchesCopyCount = batchesArrayCopy.length;
for (i = 0; i < batchesCopyCount; ++i) {
var batch = batchesArrayCopy[i];
for (i = batchCount - 1; i >= 0; --i) {
var batch = batches[i];
if (batch.isDirty) {
isUpdated = batchesArrayCopy[i].update(time) && isUpdated;
isUpdated = batches[i].update(time) && isUpdated;
batch.isDirty = false;
}
if (batch.geometry.length === 0) {
batches.remove(batch.key);
batches.splice(i, 1);
}
}

return isUpdated;
};

StaticGroundGeometryColorBatch.prototype.getBoundingSphere = function(updater, result) {
var batchesArray = this._batches.values;
var batchCount = batchesArray.length;
var batches = this._batches;
var batchCount = batches.length;
for (var i = 0; i < batchCount; ++i) {
var batch = batchesArray[i];
var batch = batches[i];
if (batch.contains(updater)) {
return batch.getBoundingSphere(updater, result);
}
@@ -317,10 +328,10 @@ import Property from './Property.js';
};

StaticGroundGeometryColorBatch.prototype.removeAllPrimitives = function() {
var batchesArray = this._batches.values;
var batchCount = batchesArray.length;
var batches = this._batches;
var batchCount = batches.length;
for (var i = 0; i < batchCount; ++i) {
batchesArray[i].removeAllPrimitives();
batches[i].removeAllPrimitives();
}
};
export default StaticGroundGeometryColorBatch;
117 changes: 77 additions & 40 deletions Specs/DataSources/StaticGroundGeometryColorBatchSpec.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { ApproximateTerrainHeights } from '../../Source/Cesium.js';
import { Cartesian3 } from '../../Source/Cesium.js';
import { Color } from '../../Source/Cesium.js';
import { defaultValue } from '../../Source/Cesium.js';
import { DistanceDisplayCondition } from '../../Source/Cesium.js';
import { JulianDate } from '../../Source/Cesium.js';
import { Math as CesiumMath } from '../../Source/Cesium.js';
@@ -37,10 +36,6 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
ApproximateTerrainHeights._terrainHeights = undefined;
});

function computeKey(zIndex) {
return defaultValue(zIndex, 0);
}

it('updates color attribute after rebuilding primitive', function() {
if (!GroundPrimitive.isSupported(scene)) {
return;
@@ -72,13 +67,10 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
var primitive = scene.groundPrimitives.get(0);
var attributes = primitive.getGeometryInstanceAttributes(entity);
var red = [255, 0, 0, 255];
var redKey = computeKey();
expect(attributes.color).toEqual(red);

// Verify we have 1 batch with the key for red
// Verify we have 1 batch
expect(batch._batches.length).toEqual(1);
expect(batch._batches.contains(redKey)).toBe(true);
expect(batch._batches.get(redKey).key).toEqual(redKey);

entity.ellipse.material = Color.GREEN;
updater._onEntityPropertyChanged(entity, 'ellipse');
@@ -94,19 +86,55 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
var primitive = scene.groundPrimitives.get(0);
var attributes = primitive.getGeometryInstanceAttributes(entity);
var green = [0, 128, 0, 255];
var greenKey = computeKey();
expect(attributes.color).toEqual(green);

// Verify we have 1 batch with the key for green
expect(batch._batches.length).toEqual(1);
expect(batch._batches.contains(greenKey)).toBe(true);
expect(batch._batches.get(greenKey).key).toEqual(greenKey);

batch.removeAllPrimitives();
});
});
});

it('batches overlapping geometry separately', function() {
if (!GroundPrimitive.isSupported(scene)) {
return;
}

var batch = new StaticGroundGeometryColorBatch(scene.groundPrimitives, ClassificationType.BOTH);
var entity = new Entity({
position : new Cartesian3(1234, 5678, 9101112),
ellipse : {
semiMajorAxis : 2,
semiMinorAxis : 1,
show : new CallbackProperty(function() {
return true;
}, false),
material : Color.RED
}
});

var entity2 = new Entity({
position : new Cartesian3(1234, 5678, 9101112),
ellipse : {
semiMajorAxis : 2,
semiMinorAxis : 1,
show : new CallbackProperty(function() {
return true;
}, false),
material : Color.RED
}
});

var updater = new EllipseGeometryUpdater(entity, scene);
batch.add(time, updater);

var updater2 = new EllipseGeometryUpdater(entity2, scene);
batch.add(time, updater2);

expect(batch._batches.length).toEqual(2);
});

it('updates with sampled distance display condition out of range', function() {
var validTime = JulianDate.fromIso8601('2018-02-14T04:10:00+1100');
var outOfRangeTime = JulianDate.fromIso8601('2018-02-14T04:20:00+1100');
@@ -205,18 +233,6 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
}

var batch = new StaticGroundGeometryColorBatch(scene.groundPrimitives, ClassificationType.BOTH);
function buildEntity() {
return new Entity({
position : new Cartesian3(1234, 5678, 9101112),
ellipse : {
semiMajorAxis : 2,
semiMinorAxis : 1,
height : 0,
outline : true,
outlineColor : Color.RED.withAlpha(0.5)
}
});
}

function renderScene() {
scene.initializeFrame();
@@ -225,8 +241,25 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
return isUpdated;
}

var entity1 = buildEntity();
var entity2 = buildEntity();
var entity1 = new Entity({
position : new Cartesian3(1234, 5678, 9101112),
ellipse : {
semiMajorAxis : 0.2,
semiMinorAxis : 0.1,
outline : true,
outlineColor : Color.RED.withAlpha(0.5)
}
});

var entity2 = new Entity({
position : new Cartesian3(1234, 4678, 9101112),
ellipse : {
semiMajorAxis : 0.2,
semiMinorAxis : 0.1,
outline : true,
outlineColor : Color.RED.withAlpha(0.5)
}
});

var updater1 = new EllipseGeometryUpdater(entity1, scene);
var updater2 = new EllipseGeometryUpdater(entity2, scene);
@@ -272,18 +305,6 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
}

var batch = new StaticGroundGeometryColorBatch(scene.groundPrimitives, ClassificationType.BOTH);
function buildEntity() {
return new Entity({
position : new Cartesian3(1234, 5678, 9101112),
ellipse : {
semiMajorAxis : 2,
semiMinorAxis : 1,
height : 0,
outline : true,
outlineColor : Color.RED.withAlpha(0.5)
}
});
}

function renderScene() {
scene.initializeFrame();
@@ -292,11 +313,27 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
return isUpdated;
}

var entity1 = buildEntity();
var entity1 = new Entity({
position : new Cartesian3(1234, 5678, 9101112),
ellipse : {
semiMajorAxis : 0.2,
semiMinorAxis : 0.1,
outline : true,
outlineColor : Color.RED.withAlpha(0.5)
}
});
var updater1 = new EllipseGeometryUpdater(entity1, scene);
batch.add(time, updater1);

var entity2 = buildEntity();
var entity2 = new Entity({
position : new Cartesian3(1234, 4678, 9101112),
ellipse : {
semiMajorAxis : 0.2,
semiMinorAxis : 0.1,
outline : true,
outlineColor : Color.RED.withAlpha(0.5)
}
});
var updater2 = new EllipseGeometryUpdater(entity2, scene);

return pollToPromise(renderScene)