CesiumGS · lilleyse · Feb 26, 2020 · Feb 25, 2020 · Feb 26, 2020 · Feb 26, 2020
diff --git a/CHANGES.md b/CHANGES.md
@@ -7,6 +7,7 @@ Change Log
 
 * Added `Entity.tileset` for loading a 3D Tiles tileset via the Entity API using the new `Cesium3DTilesetGraphics` class.
 * Added `tileset.uri`, `tileset.show`, and `tileset.maximumScreenSpaceError` properties to CZML processing for loading 3D Tiles.
+* Massively improved performance of clamped Entity ground geometry with dynamic colors. [#8630](https://github.com/CesiumGS/cesium/pull/8630)
 * Added `Color.lerp` for linearly interpolating between two RGB colors. [#8607](https://github.com/CesiumGS/cesium/pull/8607)
 * `CesiumTerrainProvider` now supports terrain tiles using a `WebMercatorTilingScheme` by specifying `"projection": "EPSG:3857"` in `layer.json`. It also now supports numbering tiles from the North instead of the South by specifying `"scheme": "slippyMap"` in `layer.json`. [#8563](https://github.com/CesiumGS/cesium/pull/8563)
 * Added basic support for `isNaN`, `isFinite`, `null`, and `undefined` in the 3D Tiles styling GLSL backend for point clouds. [#8621](https://github.com/CesiumGS/cesium/pull/8621)

diff --git a/Source/DataSources/CorridorGeometryUpdater.js b/Source/DataSources/CorridorGeometryUpdater.js
@@ -174,7 +174,7 @@ import Property from './Property.js';
                !Property.isConstant(corridor.outlineWidth) || //
                !Property.isConstant(corridor.cornerType) || //
                !Property.isConstant(corridor.zIndex) || //
-               (this._onTerrain && !Property.isConstant(this._materialProperty));
+               (this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
     };
 
     CorridorGeometryUpdater.prototype._setStaticOptions = function(entity, corridor) {

diff --git a/Source/DataSources/EllipseGeometryUpdater.js b/Source/DataSources/EllipseGeometryUpdater.js
@@ -179,7 +179,7 @@ import Property from './Property.js';
                !Property.isConstant(ellipse.outlineWidth) || //
                !Property.isConstant(ellipse.numberOfVerticalLines) || //
                !Property.isConstant(ellipse.zIndex) || //
-               (this._onTerrain && !Property.isConstant(this._materialProperty));
+               (this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
     };
 
     EllipseGeometryUpdater.prototype._setStaticOptions = function(entity, ellipse) {

diff --git a/Source/DataSources/GeometryVisualizer.js b/Source/DataSources/GeometryVisualizer.js
@@ -141,7 +141,7 @@ import WallGeometryUpdater from './WallGeometryUpdater.js';
         if (supportsMaterialsforEntitiesOnTerrain) {
             for (i = 0; i < numberOfClassificationTypes; ++i) {
                 groundMaterialBatches.push(new StaticGroundGeometryPerMaterialBatch(groundPrimitives, i, MaterialAppearance));
-                groundColorBatches[i] = new StaticGroundGeometryPerMaterialBatch(groundPrimitives, i, PerInstanceColorAppearance);
+                groundColorBatches[i] = new StaticGroundGeometryColorBatch(groundPrimitives, i);
             }
         } else {
             for (i = 0; i < numberOfClassificationTypes; ++i) {

diff --git a/Source/DataSources/PolygonGeometryUpdater.js b/Source/DataSources/PolygonGeometryUpdater.js
@@ -240,7 +240,7 @@ import Property from './Property.js';
                !Property.isConstant(polygon.closeBottom) || //
                !Property.isConstant(polygon.zIndex) || //
                !Property.isConstant(polygon.arcType) || //
-               (this._onTerrain && !Property.isConstant(this._materialProperty));
+               (this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
     };
 
     PolygonGeometryUpdater.prototype._setStaticOptions = function(entity, polygon) {

diff --git a/Source/DataSources/RectangleGeometryUpdater.js b/Source/DataSources/RectangleGeometryUpdater.js
@@ -179,7 +179,7 @@ import Property from './Property.js';
                !Property.isConstant(rectangle.rotation) || //
                !Property.isConstant(rectangle.outlineWidth) || //
                !Property.isConstant(rectangle.zIndex) || //
-               (this._onTerrain && !Property.isConstant(this._materialProperty));
+               (this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
     };
 
     RectangleGeometryUpdater.prototype._setStaticOptions = function(entity, rectangle) {

diff --git a/Source/DataSources/StaticGroundGeometryColorBatch.js b/Source/DataSources/StaticGroundGeometryColorBatch.js
@@ -1,5 +1,6 @@
 import AssociativeArray from '../Core/AssociativeArray.js';
 import Color from '../Core/Color.js';
+import ColorGeometryInstanceAttribute from '../Core/ColorGeometryInstanceAttribute.js';
 import defined from '../Core/defined.js';
 import DistanceDisplayCondition from '../Core/DistanceDisplayCondition.js';
 import DistanceDisplayConditionGeometryInstanceAttribute from '../Core/DistanceDisplayConditionGeometryInstanceAttribute.js';
@@ -65,8 +66,6 @@ import Property from './Property.js';
         return false;
     };
 
-    var scratchArray = new Array(4);
-
     Batch.prototype.update = function(time) {
         var isUpdated = true;
         var removedCount = 0;
@@ -135,12 +134,7 @@ import Property from './Property.js';
 
                     if (!Color.equals(attributes._lastColor, fillColor)) {
                         attributes._lastColor = Color.clone(fillColor, attributes._lastColor);
-                        var color = this.color;
-                        var newColor = fillColor.toBytes(scratchArray);
-                        if (color[0] !== newColor[0] || color[1] !== newColor[1] ||
-                            color[2] !== newColor[2] || color[3] !== newColor[3]) {
-                           this.itemsToRemove[removedCount++] = updater;
-                        }
+                        attributes.color = ColorGeometryInstanceAttribute.toValue(fillColor, attributes.color);
                     }
                 }
 
@@ -241,9 +235,9 @@ import Property from './Property.js';
     StaticGroundGeometryColorBatch.prototype.add = function(time, updater) {
         var instance = updater.createFillGeometryInstance(time);
         var batches = this._batches;
-        // color and zIndex are batch breakers, so we'll use that for the key
+        // zIndex is a batch breaker, so we'll use that for the key
         var zIndex = Property.getValueOrDefault(updater.zIndex, 0);
-        var batchKey = new Uint32Array(instance.attributes.color.value.buffer)[0] + ':' + zIndex;
+        var batchKey = zIndex;
         var batch;
         if (batches.contains(batchKey)) {
             batch = batches.get(batchKey);

diff --git a/Source/Scene/ClassificationPrimitive.js b/Source/Scene/ClassificationPrimitive.js
@@ -26,8 +26,6 @@ import StencilConstants from './StencilConstants.js';
 import StencilFunction from './StencilFunction.js';
 import StencilOperation from './StencilOperation.js';
 
-    var ClassificationPrimitiveReadOnlyInstanceAttributes = ['color'];
-
     /**
      * A classification primitive represents a volume enclosing geometry in the {@link Scene} to be highlighted.
      * <p>
@@ -172,11 +170,6 @@ import StencilOperation from './StencilOperation.js';
 
         this.appearance = options.appearance;
 
-        var readOnlyAttributes;
-        if (defined(geometryInstances) && isArray(geometryInstances) && geometryInstances.length > 1) {
-            readOnlyAttributes = ClassificationPrimitiveReadOnlyInstanceAttributes;
-        }
-
         this._createBoundingVolumeFunction = options._createBoundingVolumeFunction;
         this._updateAndQueueCommandsFunction = options._updateAndQueueCommandsFunction;
 
@@ -191,7 +184,6 @@ import StencilOperation from './StencilOperation.js';
             allowPicking : defaultValue(options.allowPicking, true),
             asynchronous : defaultValue(options.asynchronous, true),
             compressVertices : defaultValue(options.compressVertices, true),
-            _readOnlyInstanceAttributes : readOnlyAttributes,
             _createBoundingVolumeFunction : undefined,
             _createRenderStatesFunction : undefined,
             _createShaderProgramFunction : undefined,

diff --git a/Source/Scene/Primitive.js b/Source/Scene/Primitive.js
@@ -323,8 +323,6 @@ import ShadowMode from './ShadowMode.js';
         this._colorCommands = [];
         this._pickCommands = [];
 
-        this._readOnlyInstanceAttributes = options._readOnlyInstanceAttributes;
-
         this._createBoundingVolumeFunction = options._createBoundingVolumeFunction;
         this._createRenderStatesFunction = options._createRenderStatesFunction;
         this._createShaderProgramFunction = options._createShaderProgramFunction;
@@ -2072,24 +2070,9 @@ import ShadowMode from './ShadowMode.js';
             if (perInstanceAttributeIndices.hasOwnProperty(name)) {
                 var attributeIndex = perInstanceAttributeIndices[name];
                 properties[name] = {
-                    get : createGetFunction(batchTable, index, attributeIndex)
+                    get : createGetFunction(batchTable, index, attributeIndex),
+                    set : createSetFunction(batchTable, index, attributeIndex, this, name)
                 };
-
-                var createSetter = true;
-                var readOnlyAttributes = this._readOnlyInstanceAttributes;
-                if (createSetter && defined(readOnlyAttributes)) {
-                    length = readOnlyAttributes.length;
-                    for (var k = 0; k < length; ++k) {
-                        if (name === readOnlyAttributes[k]) {
-                            createSetter = false;
-                            break;
-                        }
-                    }
-                }
-
-                if (createSetter) {
-                    properties[name].set = createSetFunction(batchTable, index, attributeIndex, this, name);
-                }
             }
         }
 

diff --git a/Specs/DataSources/GeometryVisualizerSpec.js b/Specs/DataSources/GeometryVisualizerSpec.js
@@ -708,57 +708,6 @@ describe('DataSources/GeometryVisualizer', function() {
         });
     });
 
-    it('batches ground entities by identical color if ground entity materials are not supported', function() {
-        spyOn(GroundPrimitive, 'supportsMaterials').and.callFake(function() {
-            return false;
-        });
-        var entities = new EntityCollection();
-        var visualizer = new GeometryVisualizer(scene, entities, scene.primitives, scene.groundPrimitives);
-
-        var blueColor = Color.BLUE.withAlpha(0.5);
-        entities.add({
-            position : new Cartesian3(1, 2, 3),
-            ellipse : {
-                semiMajorAxis : 2,
-                semiMinorAxis : 1,
-                material : blueColor
-            }
-        });
-
-        return visualizerUpdated(visualizer).then(function() {
-            expect(scene.groundPrimitives.length).toEqual(1);
-
-            entities.add({
-                position : new Cartesian3(12, 34, 45),
-                ellipse : {
-                    semiMajorAxis : 2,
-                    semiMinorAxis : 1,
-                    material : blueColor
-                }
-            });
-
-            return visualizerUpdated(visualizer);
-        }).then(function() {
-            expect(scene.groundPrimitives.length).toEqual(1);
-
-            entities.add({
-                position : new Cartesian3(123, 456, 789),
-                ellipse : {
-                    semiMajorAxis : 2,
-                    semiMinorAxis : 1,
-                    material : Color.BLUE.withAlpha(0.6)
-                }
-            });
-
-            return visualizerUpdated(visualizer);
-        }).then(function() {
-            expect(scene.groundPrimitives.length).toEqual(2);
-
-            entities.removeAll();
-            visualizer.destroy();
-        });
-    });
-
     it('batches ground entities by material if ground entity materials is supported', function() {
         if (!GroundPrimitive.isSupported(scene) || !GroundPrimitive.supportsMaterials(scene)) {
             return;

diff --git a/Specs/DataSources/StaticGroundGeometryColorBatchSpec.js b/Specs/DataSources/StaticGroundGeometryColorBatchSpec.js
@@ -37,11 +37,8 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
         ApproximateTerrainHeights._terrainHeights = undefined;
     });
 
-    function computeKey(color, zIndex) {
-        var ui8 = new Uint8Array(color);
-        var ui32 = new Uint32Array(ui8.buffer);
-        zIndex = defaultValue(zIndex, 0);
-        return ui32[0] + ':' + zIndex;
+    function computeKey(zIndex) {
+        return defaultValue(zIndex, 0);
     }
 
     it('updates color attribute after rebuilding primitive', function() {
@@ -75,7 +72,7 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
             var primitive = scene.groundPrimitives.get(0);
             var attributes = primitive.getGeometryInstanceAttributes(entity);
             var red = [255, 0, 0, 255];
-            var redKey = computeKey(red);
+            var redKey = computeKey();
             expect(attributes.color).toEqual(red);
 
             // Verify we have 1 batch with the key for red
@@ -97,7 +94,7 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
                 var primitive = scene.groundPrimitives.get(0);
                 var attributes = primitive.getGeometryInstanceAttributes(entity);
                 var green = [0, 128, 0, 255];
-                var greenKey = computeKey(green);
+                var greenKey = computeKey();
                 expect(attributes.color).toEqual(green);
 
                 // Verify we have 1 batch with the key for green

diff --git a/Specs/createGeometryUpdaterGroundGeometrySpecs.js b/Specs/createGeometryUpdaterGroundGeometrySpecs.js
@@ -27,14 +27,14 @@ import { PrimitiveCollection } from '../Source/Cesium.js';
             expect(updater.zIndex.getValue()).toBe(22);
         });
 
-        it('A time-varying color causes ground geometry to be dynamic', function() {
+        it('A time-varying color does not cause ground geometry to be dynamic', function() {
             var entity = createEntity();
             var color = new SampledProperty(Color);
             color.addSample(time, Color.WHITE);
             entity[geometryPropertyName].material = new ColorMaterialProperty(color);
             var updater = new Updater(entity, getScene());
 
-            expect(updater.isDynamic).toBe(true);
+            expect(updater.isDynamic).toBe(false);
         });
 
         it('Checks that an entity without height and extrudedHeight is on terrain', function() {