Fix race condition in Entity batching.

Fixes #2514 (and other related issues as discussed [on the forum](https://groups.google.com/d/msg/cesium-dev/JOUCjnqeFKg/q5aAZb2faAMJ)). The problem was that when static entities were updated, we would create a primitive to batch them. However, if the entities were updated again before the primitive was ready, we didn't remove it.
CesiumGS · Mar 2, 2015 · 304e270 · 304e270
1 parent 1d581d9
commit 304e270
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 22 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -25,6 +25,7 @@ Change Log
 * Fixed a bug in imagery loading that could cause some or all of the globe to be missing when using an imagery layer that does not cover the entire globe.
 * Fixed a bug that caused `ElipseOutlineGeometry` and `CircleOutlineGeometry` to be extruded to the ground when they should have instead been drawn at height. [#2499](https://github.com/AnalyticalGraphicsInc/cesium/issues/2499).
 * Fixed a bug that prevented per-vertex colors from working with `PolylineGeometry` and `SimplePolylineGeometry` when used asynchronously. [#2516](https://github.com/AnalyticalGraphicsInc/cesium/issues/2516)
+* Fixed a bug that would caused duplicate graphics if non-time-dynamic `Entity` objects were modified in quick succession. [#2514](https://github.com/AnalyticalGraphicsInc/cesium/issues/2514).  
 * Fixed some styling issues with `InfoBox` and `BaseLayerPicker` caused by using Bootstrap with Cesium. [#2487](https://github.com/AnalyticalGraphicsInc/cesium/issues/2479)
 * Added support for rendering a water effect on Quantized-Mesh terrain tiles.
 * Added `pack` and `unpack` functions to `Matrix2` and `Matrix3`.

diff --git a/Source/DataSources/StaticGeometryColorBatch.js b/Source/DataSources/StaticGeometryColorBatch.js
@@ -52,21 +52,19 @@ define([
     };
 
     Batch.prototype.update = function(time) {
-        var show = true;
         var isUpdated = true;
         var removedCount = 0;
         var primitive = this.primitive;
         var primitives = this.primitives;
         if (this.createPrimitive) {
-            this.attributes.removeAll();
             if (defined(primitive)) {
-                if (primitive.ready) {
+                if (!defined(this.oldPrimitive)) {
                     this.oldPrimitive = primitive;
                 } else {
                     primitives.remove(primitive);
                 }
-                show = false;
             }
+            this.attributes.removeAll();
             var geometry = this.geometry.values;
             if (geometry.length > 0) {
                 primitive = new Primitive({
@@ -77,7 +75,6 @@ define([
                         closed : this.closed
                     })
                 });
-                primitive.show = show;
                 primitives.add(primitive);
                 isUpdated = false;
             }
@@ -87,9 +84,7 @@ define([
             if (defined(this.oldPrimitive)) {
                 primitives.remove(this.oldPrimitive);
                 this.oldPrimitive = undefined;
-                primitive.show = true;
             }
-
             var updatersWithAttributes = this.updatersWithAttributes.values;
             var length = updatersWithAttributes.length;
             for (var i = 0; i < length; i++) {
@@ -115,7 +110,7 @@ define([
                 }
 
                 if (!updater.hasConstantFill) {
-                    show = updater.isFilled(time);
+                    var show = updater.isFilled(time);
                     var currentShow = attributes.show[0] === 1;
                     if (show !== currentShow) {
                         attributes.show = ShowGeometryInstanceAttribute.toValue(show, attributes.show);
@@ -148,13 +143,21 @@ define([
     };
 
     Batch.prototype.removeAllPrimitives = function() {
+        var primitives = this.primitives;
+
         var primitive = this.primitive;
         if (defined(primitive)) {
-            this.primitives.remove(primitive);
+            primitives.remove(primitive);
             this.primitive = undefined;
             this.geometry.removeAll();
             this.updaters.removeAll();
         }
+
+        var oldPrimitive = this.oldPrimitive;
+        if (defined(oldPrimitive)) {
+            primitives.remove(oldPrimitive);
+            this.oldPrimitive = undefined;
+        }
     };
 
     /**

diff --git a/Source/DataSources/StaticGeometryPerMaterialBatch.js b/Source/DataSources/StaticGeometryPerMaterialBatch.js
@@ -67,19 +67,17 @@ define([
     };
 
     Batch.prototype.update = function(time) {
-        var show = true;
         var isUpdated = true;
         var primitive = this.primitive;
         var primitives = this.primitives;
         var geometries = this.geometry.values;
         if (this.createPrimitive) {
             if (defined(primitive)) {
-                if (primitive.ready) {
+                if (!defined(this.oldPrimitive)) {
                     this.oldPrimitive = primitive;
                 } else {
                     primitives.remove(primitive);
                 }
-                show = false;
             }
             if (geometries.length > 0) {
                 this.material = MaterialProperty.getValue(time, this.materialProperty, this.material);
@@ -93,7 +91,6 @@ define([
                     })
                 });
 
-                primitive.show = show;
                 primitives.add(primitive);
                 isUpdated = false;
             }
@@ -103,7 +100,6 @@ define([
             if (defined(this.oldPrimitive)) {
                 primitives.remove(this.oldPrimitive);
                 this.oldPrimitive = undefined;
-                primitive.show = true;
             }
 
             this.material = MaterialProperty.getValue(time, this.materialProperty, this.material);
@@ -122,7 +118,7 @@ define([
                 }
 
                 if (!updater.hasConstantFill) {
-                    show = updater.isFilled(time);
+                    var show = updater.isFilled(time);
                     var currentShow = attributes.show[0] === 1;
                     if (show !== currentShow) {
                         attributes.show = ShowGeometryInstanceAttribute.toValue(show, attributes.show);
@@ -159,6 +155,10 @@ define([
         if (defined(primitive)) {
             primitives.remove(primitive);
         }
+        var oldPrimitive = this.oldPrimitive;
+        if (defined(oldPrimitive)) {
+            primitives.remove(oldPrimitive);
+        }
         this.removeMaterialSubscription();
     };
 

diff --git a/Source/DataSources/StaticOutlineGeometryBatch.js b/Source/DataSources/StaticOutlineGeometryBatch.js
@@ -52,20 +52,18 @@ define([
 
     var colorScratch = new Color();
     Batch.prototype.update = function(time) {
-        var show = true;
         var isUpdated = true;
         var removedCount = 0;
         var primitive = this.primitive;
         var primitives = this.primitives;
         if (this.createPrimitive) {
             this.attributes.removeAll();
             if (defined(primitive)) {
-                if (primitive.ready) {
+                if (!defined(this.oldPrimitive)) {
                     this.oldPrimitive = primitive;
                 } else {
                     primitives.remove(primitive);
                 }
-                show = false;
             }
             var geometry = this.geometry.values;
             if (geometry.length > 0) {
@@ -83,15 +81,13 @@ define([
 
                 primitives.add(primitive);
                 isUpdated = false;
-                primitive.show = show;
             }
             this.primitive = primitive;
             this.createPrimitive = false;
         } else if (defined(primitive) && primitive.ready) {
             if (defined(this.oldPrimitive)) {
                 primitives.remove(this.oldPrimitive);
                 this.oldPrimitive = undefined;
-                primitive.show = true;
             }
 
             var updatersWithAttributes = this.updatersWithAttributes.values;
@@ -119,7 +115,7 @@ define([
                 }
 
                 if (!updater.hasConstantOutline) {
-                    show = updater.isOutlineVisible(time);
+                    var show = updater.isOutlineVisible(time);
                     var currentShow = attributes.show[0] === 1;
                     if (show !== currentShow) {
                         attributes.show = ShowGeometryInstanceAttribute.toValue(show, attributes.show);
@@ -153,13 +149,21 @@ define([
     };
 
     Batch.prototype.removeAllPrimitives = function() {
+        var primitives = this.primitives;
+
         var primitive = this.primitive;
         if (defined(primitive)) {
-            this.primitives.remove(primitive);
+            primitives.remove(primitive);
             this.primitive = undefined;
             this.geometry.removeAll();
             this.updaters.removeAll();
         }
+
+        var oldPrimitive = this.oldPrimitive;
+        if (defined(oldPrimitive)) {
+            primitives.remove(oldPrimitive);
+            this.oldPrimitive = undefined;
+        }
     };
 
     /**