diff --git a/CHANGES.md b/CHANGES.md index 005c551baa9d..c2e88ad68e9f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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 diff --git a/Source/DataSources/StaticGroundGeometryColorBatch.js b/Source/DataSources/StaticGroundGeometryColorBatch.js index ea0ad1000393..df51704a3a67 100644 --- a/Source/DataSources/StaticGroundGeometryColorBatch.js +++ b/Source/DataSources/StaticGroundGeometryColorBatch.js @@ -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,7 +235,7 @@ import Property from './Property.js'; * @private */ function StaticGroundGeometryColorBatch(primitives, classificationType) { - this._batches = new AssociativeArray(); + this._batches = []; this._primitives = primitives; this._classificationType = classificationType; } @@ -235,25 +243,31 @@ import Property from './Property.js'; 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,16 +300,14 @@ 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); } } @@ -304,10 +315,10 @@ import Property from './Property.js'; }; 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; diff --git a/Specs/DataSources/StaticGroundGeometryColorBatchSpec.js b/Specs/DataSources/StaticGroundGeometryColorBatchSpec.js index 6199a4ec5310..726bac667232 100644 --- a/Specs/DataSources/StaticGroundGeometryColorBatchSpec.js +++ b/Specs/DataSources/StaticGroundGeometryColorBatchSpec.js @@ -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)