Skip to content
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

Remove CSS2DObject from CSS2DRender when Object3D is Removed #25083

Closed
wants to merge 3 commits into from

Conversation

doppl3r
Copy link

@doppl3r doppl3r commented Dec 6, 2022

Description

The Problem: When Object3D objects are removed, their child CSS2DObject types are not removed from the CSS2DRenderer.

The Solution: CSS2DObject types will now be removed when the parent Object3D type is removed. The CSS2DObject "removed" event is safely invoked after it is added to the parent Object3D.

Use cases:

// The child CSS2DObject will now be removed
moon.removeFromParent();

// This will also remove the child CSS2DObject(s)
scene.clear(); 

@doppl3r doppl3r changed the title Add event to remove from parent Remove CSS2DObject from CSS2DRender when Object3D is Removed Dec 6, 2022
@@ -18,6 +18,17 @@ class CSS2DObject extends Object3D {
this.element.style.userSelect = 'none';

this.element.setAttribute( 'draggable', false );

const _this = this;
this.addEventListener( 'added', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right fix for the problem.

I wonder if it wouldn't be better to solve this issue on Object3D level. Meaning when a 3D object is added or removed to/from the scene graph, descendants receive an event as well since they are indirectly affected by this operation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe having this particular event logic in the CSS2DObject level provides a unique solution for objects that also contain HTML DOM references.

CSS2DObject types do not have descendants, so it makes sense to require event listeners to their parent rather than assigning events externally.

Copy link
Collaborator

@Mugen87 Mugen87 Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CSS2DObject types do not have descendants

Um, I don't think this assumptions is correct.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, my assumption was not true. I tried nesting a CSS2DObject within another CSS2DObject and they render HTML on top of each other (not pretty, but still optional). The solution I provided in this PR still work when ancestors invoke clear() or removeFromParent().

Do you have any pseudo code in mind for adding events to descendants within the Object3D class?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meaning when a 3D object is added or removed to/from the scene graph, descendants receive an event as well since they are indirectly affected by this operation.

Related issue: #22416
Related PR: #16934

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 9, 2022

Closing. This isn't the right fix for the issue. We should reconsider to thoughts raised in #22416.

The issue in CSS2DRenderer should automatically go away if added and removed events were propagated through the hierarchy.

@Mugen87 Mugen87 closed this Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants