Merge pull request #7163 from AnalyticalGraphicsInc/fix-batch-remove

Fix StaticGeometryMaterialBatch remove
CesiumGS · Oct 22, 2018 · 602acf1 · 602acf1
2 parents 23a95aa + 4361669
commit 602acf1
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 2 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -13,6 +13,7 @@ Change Log
 * Added `atmosphereHueShift`, `atmosphereSaturationShift`, and `atmosphereBrightnessShift` properties to `Globe` which shift the color of the ground atmosphere to match the hue, saturation, and brightness shifts of the sky atmosphere. [#4195](https://github.com/AnalyticalGraphicsInc/cesium/issues/4195)
 
 ##### Fixes :wrench:
+* Fixed issue removing geometry entities with different materials [#7163](https://github.com/AnalyticalGraphicsInc/cesium/pull/7163)
 * Fixed an issue where `pickPosition` would return incorrect results when called after `sampleHeight` or `clampToHeight`. [#7113](https://github.com/AnalyticalGraphicsInc/cesium/pull/7113)
 * Fixed crash when updating polyline attributes twice in one frame [#7155](https://github.com/AnalyticalGraphicsInc/cesium/pull/7155)
 * Fixed a crash when using `BingMapsGeocoderService` [#7143](https://github.com/AnalyticalGraphicsInc/cesium/issues/7143)

diff --git a/Source/DataSources/StaticGeometryColorBatch.js b/Source/DataSources/StaticGeometryColorBatch.js
@@ -109,7 +109,9 @@ define([
                 this.subscriptions.remove(id);
                 this.showsUpdated.remove(id);
             }
+            return true;
         }
+        return false;
     };
 
     Batch.prototype.update = function(time) {
@@ -353,8 +355,8 @@ define([
                 if (item.updaters.length === 0) {
                     items.splice(i, 1);
                     item.destroy();
-                    return true;
                 }
+                return true;
             }
         }
         return false;

diff --git a/Source/DataSources/StaticGeometryPerMaterialBatch.js b/Source/DataSources/StaticGeometryPerMaterialBatch.js
@@ -104,8 +104,9 @@ define([
                 this.subscriptions.remove(id);
                 this.showsUpdated.remove(id);
             }
+            return true;
         }
-        return this.createPrimitive;
+        return false;
     };
 
     var colorScratch = new Color();

diff --git a/Source/DataSources/StaticOutlineGeometryBatch.js b/Source/DataSources/StaticOutlineGeometryBatch.js
@@ -79,7 +79,9 @@ define([
                 this.subscriptions.remove(id);
                 this.showsUpdated.remove(id);
             }
+            return true;
         }
+        return false;
     };
 
     Batch.prototype.update = function(time) {

diff --git a/Specs/DataSources/StaticGeometryPerMaterialBatchSpec.js b/Specs/DataSources/StaticGeometryPerMaterialBatchSpec.js
@@ -8,6 +8,8 @@ defineSuite([
         'Core/Math',
         'Core/TimeInterval',
         'Core/TimeIntervalCollection',
+        'DataSources/BoxGeometryUpdater',
+        'DataSources/CheckerboardMaterialProperty',
         'DataSources/ColorMaterialProperty',
         'DataSources/ConstantPositionProperty',
         'DataSources/ConstantProperty',
@@ -18,6 +20,7 @@ defineSuite([
         'DataSources/PolylineArrowMaterialProperty',
         'DataSources/PolylineGeometryUpdater',
         'DataSources/PolylineGraphics',
+        'DataSources/StripeMaterialProperty',
         'DataSources/TimeIntervalCollectionProperty',
         'Scene/MaterialAppearance',
         'Scene/PolylineColorAppearance',
@@ -35,6 +38,8 @@ defineSuite([
         CesiumMath,
         TimeInterval,
         TimeIntervalCollection,
+        BoxGeometryUpdater,
+        CheckerboardMaterialProeprty,
         ColorMaterialProperty,
         ConstantPositionProperty,
         ConstantProperty,
@@ -45,6 +50,7 @@ defineSuite([
         PolylineArrowMaterialProperty,
         PolylineGeometryUpdater,
         PolylineGraphics,
+        StripeMaterialProperty,
         TimeIntervalCollectionProperty,
         MaterialAppearance,
         PolylineColorAppearance,
@@ -312,4 +318,64 @@ defineSuite([
             batch.removeAllPrimitives();
         });
     });
+
+    it('removes all updaters', function() {
+        var batch = new StaticGeometryPerMaterialBatch(scene.primitives, MaterialAppearance, undefined, false, ShadowMode.DISABLED);
+
+        function buildEntity(MaterialProperty) {
+            var material = new MaterialProperty({
+                evenColor : Color.YELLOW,
+                oddColor: Color.BLUE
+            });
+
+            return new Entity({
+                position : new Cartesian3(1234, 5678, 9101112),
+                ellipse : {
+                    semiMajorAxis : 2,
+                    semiMinorAxis : 1,
+                    height : 0,
+                    material: material
+                }
+            });
+        }
+
+        function renderScene() {
+            scene.initializeFrame();
+            var isUpdated = batch.update(time);
+            scene.render(time);
+            return isUpdated;
+        }
+
+        var entity1 = buildEntity(StripeMaterialProperty);
+        var entity2 = buildEntity(CheckerboardMaterialProeprty);
+        var entity3 = buildEntity(StripeMaterialProperty);
+        var entity4 = buildEntity(CheckerboardMaterialProeprty);
+
+        var updater1 = new EllipseGeometryUpdater(entity1, scene);
+        var updater2 = new EllipseGeometryUpdater(entity2, scene);
+        var updater3 = new EllipseGeometryUpdater(entity3, scene);
+        var updater4 = new EllipseGeometryUpdater(entity4, scene);
+        var emptyUpdater = new BoxGeometryUpdater(entity1, scene);
+
+        batch.add(time, updater1);
+        batch.add(time, updater2);
+        batch.add(time, updater3);
+        batch.add(time, updater4);
+        return pollToPromise(renderScene)
+            .then(function() {
+                expect(scene.primitives.length).toEqual(2);
+            })
+            .then(function() {
+                batch.remove(updater1);
+                batch.remove(updater2);
+                batch.remove(emptyUpdater);
+                batch.remove(updater3);
+                batch.remove(updater4);
+                return pollToPromise(renderScene);
+            })
+            .then(function() {
+                expect(scene.primitives.length).toEqual(0);
+                batch.removeAllPrimitives();
+            });
+    });
 });