Defers batch destruction until next update

Author: Jean Dumoulin

packages/engine/Source/DataSources/StaticGeometryColorBatch.js

@@ -47,23 +47,8 @@ function Batch(
   this.subscriptions = new AssociativeArray();
   this.showsUpdated = new AssociativeArray();
   this.itemsToRemove = [];
-  this.invalidated = false;
-
-  let removeMaterialSubscription;
-  if (defined(depthFailMaterialProperty)) {
-    removeMaterialSubscription =
-      depthFailMaterialProperty.definitionChanged.addEventListener(
-        Batch.prototype.onMaterialChanged,
-        this,
-      );
-  }
-  this.removeMaterialSubscription = removeMaterialSubscription;
 }
 
-Batch.prototype.onMaterialChanged = function () {
-  this.invalidated = true;
-};
-
 Batch.prototype.isMaterial = function (updater) {
   const material = this.depthFailMaterialProperty;
   const updaterMaterial = updater.depthFailMaterialProperty;
@@ -391,9 +376,6 @@ Batch.prototype.destroy = function () {
   if (defined(oldPrimitive)) {
     primitives.remove(oldPrimitive);
   }
-  if (defined(this.removeMaterialSubscription)) {
-    this.removeMaterialSubscription();
-  }
 };
 
 /**
@@ -451,12 +433,9 @@ StaticGeometryColorBatch.prototype.add = function (time, updater) {
 function removeItem(items, updater) {
   const length = items.length;
   for (let i = length - 1; i >= 0; i--) {
-    const item = items[i];
-    if (item.remove(updater)) {
-      if (item.updaters.length === 0) {
-        items.splice(i, 1);
-        item.destroy();
-      }
+    if (items[i].remove(updater)) {
+      // If the item is now empty, delete it (deferred until the next update,
+      // in case a new updater is added to the same item first).
       return true;
     }
   }
@@ -493,22 +472,13 @@ function updateItems(batch, items, time, isUpdated) {
   let i;
   for (i = length - 1; i >= 0; i--) {
     const item = items[i];
-    if (item.invalidated) {
+    if (item.updaters.length === 0) {
       items.splice(i, 1);
-      const updaters = item.updaters.values;
-      const updatersLength = updaters.length;
-      for (let h = 0; h < updatersLength; h++) {
-        batch.add(time, updaters[h]);
-      }
       item.destroy();
     }
   }
 
-  length = items.length;
-  for (i = 0; i < length; ++i) {
-    isUpdated = items[i].update(time) && isUpdated;
-  }
-  return isUpdated;
+  return items.every(item => item.update(time));
 }
 
 StaticGeometryColorBatch.prototype.update = function (time) {

packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js

@@ -43,20 +43,10 @@ function Batch(
   this.depthFailMaterial = undefined;
   this.updatersWithAttributes = new AssociativeArray();
   this.attributes = new AssociativeArray();
-  this.invalidated = false;
-  this.removeMaterialSubscription =
-    materialProperty.definitionChanged.addEventListener(
-      Batch.prototype.onMaterialChanged,
-      this,
-    );
   this.subscriptions = new AssociativeArray();
   this.showsUpdated = new AssociativeArray();
 }
 
-Batch.prototype.onMaterialChanged = function () {
-  this.invalidated = true;
-};
-
 Batch.prototype.isMaterial = function (updater) {
   const material = this.materialProperty;
   const updaterMaterial = updater.fillMaterialProperty;
@@ -378,7 +368,6 @@ Batch.prototype.destroy = function () {
   if (defined(oldPrimitive)) {
     primitives.remove(oldPrimitive);
   }
-  this.removeMaterialSubscription();
 };
 
 /**
@@ -426,40 +415,27 @@ StaticGeometryPerMaterialBatch.prototype.remove = function (updater) {
   const items = this._items;
   const length = items.length;
   for (let i = length - 1; i >= 0; i--) {
-    const item = items[i];
-    if (item.remove(updater)) {
-      if (item.updaters.length === 0) {
-        items.splice(i, 1);
-        item.destroy();
-      }
+    if (items[i].remove(updater)) {
+      // If the item is now empty, delete it (deferred until the next update,
+      // in case a new updater is added to the same item first).
       break;
     }
   }
 };
 
 StaticGeometryPerMaterialBatch.prototype.update = function (time) {
-  let i;
   const items = this._items;
   const length = items.length;
 
-  for (i = length - 1; i >= 0; i--) {
+  for (let i = length - 1; i >= 0; i--) {
     const item = items[i];
-    if (item.invalidated) {
+    if (item.updaters.length === 0) {
       items.splice(i, 1);
-      const updaters = item.updaters.values;
-      const updatersLength = updaters.length;
-      for (let h = 0; h < updatersLength; h++) {
-        this.add(time, updaters[h]);
-      }
       item.destroy();
     }
   }
 
-  let isUpdated = true;
-  for (i = 0; i < items.length; i++) {
-    isUpdated = items[i].update(time) && isUpdated;
-  }
-  return isUpdated;
+  return items.every(item => item.update(time));
 };
 
 StaticGeometryPerMaterialBatch.prototype.getBoundingSphere = function (

packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js

@@ -44,23 +44,13 @@ function Batch(
   this.material = undefined;
   this.updatersWithAttributes = new AssociativeArray();
   this.attributes = new AssociativeArray();
-  this.invalidated = false;
-  this.removeMaterialSubscription =
-    materialProperty.definitionChanged.addEventListener(
-      Batch.prototype.onMaterialChanged,
-      this,
-    );
   this.subscriptions = new AssociativeArray();
   this.showsUpdated = new AssociativeArray();
   this.zIndex = zIndex;
 
   this._asynchronous = asynchronous;
 }
 
-Batch.prototype.onMaterialChanged = function () {
-  this.invalidated = true;
-};
-
 // Check if the given updater's material is compatible with this batch
 Batch.prototype.isMaterial = function (updater) {
   const material = this.materialProperty;
@@ -325,7 +315,6 @@ Batch.prototype.destroy = function () {
   if (defined(oldPrimitive)) {
     orderedGroundPrimitives.remove(oldPrimitive);
   }
-  this.removeMaterialSubscription();
 };
 
 /**
@@ -371,12 +360,9 @@ StaticGroundPolylinePerMaterialBatch.prototype.remove = function (updater) {
   const items = this._items;
   const length = items.length;
   for (let i = length - 1; i >= 0; i--) {
-    const item = items[i];
-    if (item.remove(updater)) {
-      if (item.updaters.length === 0) {
-        items.splice(i, 1);
-        item.destroy();
-      }
+    if (items[i].remove(updater)) {
+      // If the item is now empty, delete it (deferred until the next update,
+      // in case a new updater is added to the same item first).
       break;
     }
   }
@@ -389,22 +375,13 @@ StaticGroundPolylinePerMaterialBatch.prototype.update = function (time) {
 
   for (i = length - 1; i >= 0; i--) {
     const item = items[i];
-    if (item.invalidated) {
+    if (item.updaters.length === 0) {
       items.splice(i, 1);
-      const updaters = item.updaters.values;
-      const updatersLength = updaters.length;
-      for (let h = 0; h < updatersLength; h++) {
-        this.add(time, updaters[h]);
-      }
       item.destroy();
     }
   }
 
-  let isUpdated = true;
-  for (i = 0; i < items.length; i++) {
-    isUpdated = items[i].update(time) && isUpdated;
-  }
-  return isUpdated;
+  return items.every(item => item.update(time));
 };
 
 StaticGroundPolylinePerMaterialBatch.prototype.getBoundingSphere = function (