-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Object3D: Add childadded
and childremoved
events.
#16934
Conversation
a631f4e
to
01fd0e1
Compare
In this case, #8368 sounds more appropriate, right? I mean this PR only allows to track a removal or addition of direct child objects. |
@Mugen87, do you think it has to work recursively? |
This PR is a lot simpler, introduces a useful new event notification and gives a lot of the same capability without the potential overhead of recursively firing events on every object in deep hierarchies.
It does but it can be used to track every object addition and removal to and from the scene (or sub hierarchy) pretty easily. I've hacked this event into my application and am tracking object addition and removal like so (this is a simplified example): const allObjects = new Set();
let addChildCb, removeChildCb;
addChildCb = function(e) {
const child = e.child;
child.traverse(c => {
c.addEventListener('childadded', addChildCb);
c.addEventListener('childremoved', removeChildCb);
allObjects.add(c);
});
};
removeChildCb = function(e) {
const child = e.child;
child.traverse(c => {
c.removeEventListener('childadded', addChildCb);
c.removeEventListener('childremoved', removeChildCb);
allObjects.delete(c);
});
};
addChildCb({ child: scene }); The only other way to really know this right now is to manually track the addition of every object or traverse the hierarchy every frame and look for differences. I think there's a lot of utility to easily and efficiently tracking which objects have been added to the world. |
Do the extra events provide anything that cannot be achieved by accessing the parent of the target object of the added/removed events? |
The example I showed in the comment above cannot be achieved with just the My request for global uniforms (#16922) would be addressed by this PR because all Meshes with materials could be tracked and have their uniforms updated before render (traverse is slow and iterates over a lot of non-mesh objects). I feel in a lot of ways this allows a lot of flexibility and for features to be transparently added without the need for modifying core. |
I am not sure I get it. Why can't you use the event
For any cases where you need access to the parent, it could perhaps be an idea to have a |
@EliasHasle Using just the const scene = new THREE.Scene();
allObjects.add(scene);
addCb({ target: scene });
scene.add(new THREE.Object3D());
console.log(Array.from(allObjects.values()));
// logs [Scene] but we expect [Scene, Object3D] And using the const scene = new THREE.Scene();
addChildCb({ child: scene });
scene.add(new THREE.Object3D());
console.log(Array.from(allObjects.values()));
// logs [Scene, Object3D] |
Sorry. You are totally right. Maybe the |
I have some code that horizontally or vertically positions children within a group based on sort order defined in userData. Without these events, I have to check if children.length has changed each frame to trigger if this positions need recalculating. |
Without this change, every Object3D must listen for added and removed events... class Thing extends Object3D {
constructor() {
super();
this.addEventListener('added', () => {
console.log('added', this.parent);
this.parent?.dispatchEvent({ type: 'childadded', child: this });
});
this.addEventListener('removed', () => {
console.log('removed', this.parent);
this.parent?.dispatchEvent({ type: 'childremoved', child: this });
});
}
}
const group = new Group();
group.addEventListener('childadded', (event: any) => { console.log('child added', event) })
group.addEventListener('childremoved', (event: any) => { console.log('child removed', event) })
const thing = new Thing();
group.add(thing);
group.remove(thing); Unfortunately, Object3D.remove sets parent to null before calling dispatchEvent, so the childremoved event never fires object.parent = null;
this.children.splice( index, 1 );
object.dispatchEvent( _removedEvent ); This can be worked around by storing parent during the add, but its such a hack constructor() {
super();
let parent: Object3D | undefined = undefined;
this.addEventListener('added', () => {
console.log('added', this.parent);
this.parent?.dispatchEvent({ type: 'childadded', child: this });
parent = this.parent;
});
this.addEventListener('removed', () => {
console.log('removed', this.parent);
parent?.dispatchEvent({ type: 'childremoved', child: this });
}); |
What is the prognosis for this making it in? It's a very safe and simple change, and I've had multiple instances where this functionality would be useful. |
+1 to this, I didn't see this PR and had created this issue outlining the same solution Issue 27563. With the childadded / removed events, we can then intercept the addition /removal of objects to / from a Scene (via a subclass or a wrapper) and recursively set the childadded / childremoved events on every object to dispatch some event on the scene (eg "scenegraph_child_added" / "scenegraph_child_removed"). Then we can observe these scenegraph_child_added / scenegraph_child_removed events to perform some logic (eg, inspecting the class of the added object / any child objects and, if they pass some test, adding them to a list of objects to track / update each render loop). |
An alternative approach would be to dispatch the old added / removed events globally via the document. If the new parent / old parent objects were added as parameters to this event, outside observers could listen for these events and then simply traverse the parent chain up to see if it ends on a Scene object in order to check if the scene graph has changed. This would be a breaking change though, as users would need to start monitoring the document for the added / removed events instead of a specific object. The benefit being it would stick to just using the one event instead of adding a new one. |
childadded
and childremoved
events.
@@ -357,6 +357,7 @@ Object3D.prototype = Object.assign( Object.create( EventDispatcher.prototype ), | |||
this.children.push( object ); | |||
|
|||
object.dispatchEvent( _addedEvent ); | |||
this.dispatchEvent( { type: 'childadded', child: object } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From #11459 (comment).
@gkjohnson I think it's okay creating an object per event but I also understand the concerns of devs who perform many add and remove operations. A single object would be more performant.
Since the listeners in dispatchEvent()
are processed in a sync fashion, I think it would be safe to have single event objects and just change the child
reference. Besides, other event types like added
or removed
have shared event objects as well. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good - we just have to set and then remove the objects from the cached event before and after dispatch. I'll merge this as is and then make a new PR with this.
Reviving some of the ideas from #11459 this PR adds
childadded
andchildremoved
events that get dispatched from the parent when a child is added or removed.I'd like to track when an object is added or removed from the scene anywhere in the hierarchy and there's currently no simple way to achieve that.
I can add the tests and docs if this looks merge-able.