Skip to content

Commit

Permalink
fix(troika-3d): prevent tree getting in bad state due to removal of o…
Browse files Browse the repository at this point in the history
…rphanable 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.
  • Loading branch information
lojjic committed Oct 29, 2020
1 parent 04c819d commit 8121425
Showing 1 changed file with 28 additions and 21 deletions.
49 changes: 28 additions & 21 deletions packages/troika-3d/src/facade/Object3DFacade.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

/**
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 8121425

Please sign in to comment.