From 26756aa44a111a9c4f975cb1bece99b31be76693 Mon Sep 17 00:00:00 2001 From: Jean Dumoulin Date: Tue, 19 Aug 2025 18:17:21 +0200 Subject: [PATCH 1/2] Defers batch destruction until next update --- .../DataSources/StaticGeometryColorBatch.js | 40 +++---------------- .../StaticGeometryPerMaterialBatch.js | 36 +++-------------- .../StaticGroundPolylinePerMaterialBatch.js | 33 +++------------ 3 files changed, 16 insertions(+), 93 deletions(-) diff --git a/packages/engine/Source/DataSources/StaticGeometryColorBatch.js b/packages/engine/Source/DataSources/StaticGeometryColorBatch.js index 4d65ea05dd8a..69434eaf04dd 100644 --- a/packages/engine/Source/DataSources/StaticGeometryColorBatch.js +++ b/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) { diff --git a/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js b/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js index 32c078c123fc..6898304f4750 100644 --- a/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js +++ b/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 ( diff --git a/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js b/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js index 9b946b750542..468fc102a7df 100644 --- a/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js +++ b/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js @@ -44,12 +44,6 @@ 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; @@ -57,10 +51,6 @@ function Batch( 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 ( From e04df47fc63cc739a9817a697014ddc9c94dbac3 Mon Sep 17 00:00:00 2001 From: Jean Dumoulin Date: Wed, 20 Aug 2025 11:19:18 +0200 Subject: [PATCH 2/2] Defers batch destruction until next update --- package.json | 2 +- .../Source/DataSources/StaticGeometryColorBatch.js | 9 +++++---- .../Source/DataSources/StaticGeometryPerMaterialBatch.js | 5 ++++- .../DataSources/StaticGroundPolylinePerMaterialBatch.js | 5 ++++- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 04b278537435..15566557843a 100644 --- a/package.json +++ b/package.json @@ -171,4 +171,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/engine/Source/DataSources/StaticGeometryColorBatch.js b/packages/engine/Source/DataSources/StaticGeometryColorBatch.js index 69434eaf04dd..63bcb60795c2 100644 --- a/packages/engine/Source/DataSources/StaticGeometryColorBatch.js +++ b/packages/engine/Source/DataSources/StaticGeometryColorBatch.js @@ -468,9 +468,7 @@ function moveItems(batch, items, time) { } function updateItems(batch, items, time, isUpdated) { - let length = items.length; - let i; - for (i = length - 1; i >= 0; i--) { + for (let i = items.length - 1; i >= 0; i--) { const item = items[i]; if (item.updaters.length === 0) { items.splice(i, 1); @@ -478,7 +476,10 @@ function updateItems(batch, items, time, isUpdated) { } } - return items.every(item => item.update(time)); + return items.reduce( + (isUpdated, item) => isUpdated && item.update(time), + isUpdated, + ); } StaticGeometryColorBatch.prototype.update = function (time) { diff --git a/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js b/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js index 6898304f4750..75375c573b1a 100644 --- a/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js +++ b/packages/engine/Source/DataSources/StaticGeometryPerMaterialBatch.js @@ -435,7 +435,10 @@ StaticGeometryPerMaterialBatch.prototype.update = function (time) { } } - return items.every(item => item.update(time)); + return items.reduce( + (isUpdated, item) => isUpdated && item.update(time), + false, + ); }; StaticGeometryPerMaterialBatch.prototype.getBoundingSphere = function ( diff --git a/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js b/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js index 468fc102a7df..51fd12167282 100644 --- a/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js +++ b/packages/engine/Source/DataSources/StaticGroundPolylinePerMaterialBatch.js @@ -381,7 +381,10 @@ StaticGroundPolylinePerMaterialBatch.prototype.update = function (time) { } } - return items.every(item => item.update(time)); + return items.reduce( + (isUpdated, item) => isUpdated && item.update(time), + false, + ); }; StaticGroundPolylinePerMaterialBatch.prototype.getBoundingSphere = function (