Fix ground primtives with dynamic color

Ground primitives with dynamic color exposed a bug in `StaticGroundGeometryColorBatch` where we were not removing ground primitives if the batch size went to 0. Fixing that bug exposed another bug which is the fact that dynamic colors for ground primitives need to be treated as dynamic geometry beause of the lack of per instance coloring for batched ground primitives. Fixes CesiumGS#4433
abwood · Oct 19, 2016 · 203bdf3 · 203bdf3
1 parent c384bf8
commit 203bdf3
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 7 deletions.
diff --git a/Source/DataSources/CorridorGeometryUpdater.js b/Source/DataSources/CorridorGeometryUpdater.js
@@ -527,7 +527,8 @@ define([
             !Property.isConstant(granularity) || //
             !Property.isConstant(width) || //
             !Property.isConstant(outlineWidth) || //
-            !Property.isConstant(cornerType)) {
+            !Property.isConstant(cornerType) || //
+            (onTerrain && !Property.isConstant(material))) {
             if (!this._dynamic) {
                 this._dynamic = true;
                 this._geometryChanged.raiseEvent(this);

diff --git a/Source/DataSources/EllipseGeometryUpdater.js b/Source/DataSources/EllipseGeometryUpdater.js
@@ -538,7 +538,8 @@ define([
             !Property.isConstant(granularity) || //
             !Property.isConstant(stRotation) || //
             !Property.isConstant(outlineWidth) || //
-            !Property.isConstant(numberOfVerticalLines)) {
+            !Property.isConstant(numberOfVerticalLines) || //
+            (onTerrain && !Property.isConstant(material))) {
             if (!this._dynamic) {
                 this._dynamic = true;
                 this._geometryChanged.raiseEvent(this);

diff --git a/Source/DataSources/PolygonGeometryUpdater.js b/Source/DataSources/PolygonGeometryUpdater.js
@@ -542,7 +542,8 @@ define([
             !Property.isConstant(perPositionHeightProperty) || //
             !Property.isConstant(perPositionHeight) || //
             !Property.isConstant(closeTop) || //
-            !Property.isConstant(closeBottom)) {
+            !Property.isConstant(closeBottom) || //
+            (onTerrain && !Property.isConstant(material))) {
 
             if (!this._dynamic) {
                 this._dynamic = true;

diff --git a/Source/DataSources/RectangleGeometryUpdater.js b/Source/DataSources/RectangleGeometryUpdater.js
@@ -534,7 +534,8 @@ define([
             !Property.isConstant(rotation) || //
             !Property.isConstant(outlineWidth) || //
             !Property.isConstant(closeBottom) || //
-            !Property.isConstant(closeTop)) {
+            !Property.isConstant(closeTop) || //
+            (onTerrain && !Property.isConstant(material))) {
             if (!this._dynamic) {
                 this._dynamic = true;
                 this._geometryChanged.raiseEvent(this);

diff --git a/Source/DataSources/StaticGroundGeometryColorBatch.js b/Source/DataSources/StaticGroundGeometryColorBatch.js
@@ -316,12 +316,13 @@ define([
         var batchesCopyCount = batchesArrayCopy.length;
         for (i = 0; i < batchesCopyCount; ++i) {
             var batch = batchesArrayCopy[i];
-            if (batch.geometry.length === 0) {
-                batches.remove(batch.key);
-            } else if (batch.isDirty) {
+            if (batch.isDirty) {
                 isUpdated = batchesArrayCopy[i].update(time) && isUpdated;
                 batch.isDirty = false;
             }
+            if (batch.geometry.length === 0) {
+                batches.remove(batch.key);
+            }
         }
 
         return isUpdated;

diff --git a/Specs/DataSources/CorridorGeometryUpdaterSpec.js b/Specs/DataSources/CorridorGeometryUpdaterSpec.js
@@ -242,6 +242,15 @@ defineSuite([
         expect(updater.isDynamic).toBe(true);
     });
 
+    it('A time-varying color causes ground geometry to be dynamic', function() {
+        var entity = createBasicCorridorWithoutHeight();
+        var updater = new CorridorGeometryUpdater(entity, scene);
+        var color = new SampledProperty(Color);
+        color.addSample(time, Color.WHITE);
+        entity.corridor.material = new ColorMaterialProperty(color);
+        expect(updater.isDynamic).toBe(true);
+    });
+
     function validateGeometryInstance(options) {
         var entity = createBasicCorridor();
 

diff --git a/Specs/DataSources/EllipseGeometryUpdaterSpec.js b/Specs/DataSources/EllipseGeometryUpdaterSpec.js
@@ -268,6 +268,15 @@ defineSuite([
         expect(updater.isDynamic).toBe(true);
     });
 
+    it('A time-varying color causes ground geometry to be dynamic', function() {
+        var entity = createBasicEllipseWithoutHeight();
+        var updater = new EllipseGeometryUpdater(entity, scene);
+        var color = new SampledProperty(Color);
+        color.addSample(time, Color.WHITE);
+        entity.ellipse.material = new ColorMaterialProperty(color);
+        expect(updater.isDynamic).toBe(true);
+    });
+
     function validateGeometryInstance(options) {
         var entity = new Entity();
         entity.position = new ConstantPositionProperty(options.center);

diff --git a/Specs/DataSources/PolygonGeometryUpdaterSpec.js b/Specs/DataSources/PolygonGeometryUpdaterSpec.js
@@ -253,6 +253,15 @@ defineSuite([
         expect(updater.isDynamic).toBe(true);
     });
 
+    it('A time-varying color causes ground geometry to be dynamic', function() {
+        var entity = createBasicPolygonWithoutHeight();
+        var updater = new PolygonGeometryUpdater(entity, scene);
+        var color = new SampledProperty(Color);
+        color.addSample(time, Color.WHITE);
+        entity.polygon.material = new ColorMaterialProperty(color);
+        expect(updater.isDynamic).toBe(true);
+    });
+
     function validateGeometryInstance(options) {
         var entity = createBasicPolygon();
 

diff --git a/Specs/DataSources/RectangleGeometryUpdaterSpec.js b/Specs/DataSources/RectangleGeometryUpdaterSpec.js
@@ -239,6 +239,15 @@ defineSuite([
         expect(updater.isDynamic).toBe(true);
     });
 
+    it('A time-varying color causes ground geometry to be dynamic', function() {
+        var entity = createBasicRectangleWithoutHeight();
+        var updater = new RectangleGeometryUpdater(entity, scene);
+        var color = new SampledProperty(Color);
+        color.addSample(time, Color.WHITE);
+        entity.rectangle.material = new ColorMaterialProperty(color);
+        expect(updater.isDynamic).toBe(true);
+    });
+
     function validateGeometryInstance(options) {
         var entity = createBasicRectangle();