From 81214255cba846c4e832ac3ada3225e1688c59bd Mon Sep 17 00:00:00 2001
From: Jason Johnston <jason.johnston@protectwise.com>
Date: Thu, 29 Oct 2020 16:09:37 -0600
Subject: [PATCH] fix(troika-3d): prevent tree getting in bad state due to
 removal of orphanable children

The optimization for removing non-renderable objects from the ThreeJS
tree could sometimes leave the tree in an incomplete state with objects
queued for removal but not flushed, if the ancestors weren't all in an
update pass. This change forces flushing the removals up the tree.
---
 .../troika-3d/src/facade/Object3DFacade.js    | 49 +++++++++++--------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/packages/troika-3d/src/facade/Object3DFacade.js b/packages/troika-3d/src/facade/Object3DFacade.js
index 3852986b..a5edd1d3 100644
--- a/packages/troika-3d/src/facade/Object3DFacade.js
+++ b/packages/troika-3d/src/facade/Object3DFacade.js
@@ -86,27 +86,7 @@ class Object3DFacade extends PointerEventTarget {
 
     // If any children were removed during the update, remove them from the threejs
     // object in a single batch; this avoids threejs's very expensive single-item remove.
-    if (this._removeChildIds) {
-      let threeObject = this.threeObject
-      let removeChildIds = this._removeChildIds
-      threeObject.children = threeObject.children.filter(child => {
-        if (child.id in removeChildIds) {
-          child.parent = null
-          child.dispatchEvent(removedEvent)
-          return false
-        }
-        return true
-      })
-
-      // If that resulted in a non-renderable object having no renderable children,
-      // queue it for removal it from the threejs object tree
-      let parentObj3D = this._parentObject3DFacade
-      if (canObjectBeOrphaned(threeObject) && parentObj3D && parentObj3D.threeObject === threeObject.parent) {
-        parentObj3D._queueRemoveChildObject3D(threeObject.id)
-      }
-
-      this._removeChildIds = null
-    }
+    this._flushQueuedChildRemovals()
   }
 
   /**
@@ -386,6 +366,33 @@ class Object3DFacade extends PointerEventTarget {
     removeChildIds[threeObjectId] = true
   }
 
+  _flushQueuedChildRemovals() {
+    // If any children were queued for removal, remove them from the threejs
+    // object in a single batch; this avoids threejs's very expensive single-item remove.
+    if (this._removeChildIds) {
+      let threeObject = this.threeObject
+      let removeChildIds = this._removeChildIds
+      threeObject.children = threeObject.children.filter(child => {
+        if (child.id in removeChildIds) {
+          child.parent = null
+          child.dispatchEvent(removedEvent)
+          return false
+        }
+        return true
+      })
+
+      // If that resulted in a non-renderable object having no renderable children,
+      // remove it from the threejs object tree, recursively upward.
+      let parentObj3D = this._parentObject3DFacade
+      if (canObjectBeOrphaned(threeObject) && parentObj3D && parentObj3D.threeObject === threeObject.parent) {
+        parentObj3D._queueRemoveChildObject3D(threeObject.id)
+        parentObj3D._flushQueuedChildRemovals() //if we don't force a parent flush, tree can get in a bad state
+      }
+
+      this._removeChildIds = null
+    }
+  }
+
   destructor() {
     this.notifyWorld('object3DRemoved')
     let parentObj3D = this._parentObject3DFacade