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

Inherit layers from parent object when creating hand and controller models #23279

Conversation

hybridherbst
Copy link
Contributor

Description

Since controller models and hand meshes are created asynchronously at runtime (e.g. fetched after controller connection), there's no good way to set layers on them (one would have to poll until they exist, or there would need to be an event for full creation being complete).

This PR makes them inherit layers from their parent objects on creation.

This contribution is funded by 🌵 needle.

@mrdoob
Copy link
Owner

mrdoob commented Jan 21, 2022

Do you mind giving some more context of why is this needed?
Why do controllers need to be aware of layers?

@hybridherbst
Copy link
Contributor Author

Sure - since layers control both visibility and raycasting, not being able to set the layers of loaded models is problematic when that is actually used. We've run into this issue with an app where layer 0 by default is raycastable (raycasting against the scene), and then the controller's rays are hitting themselves because we can't move them to another layer (since they're loaded async with no way to react to the loading being done).

What we had tried before this PR was to set the layer on the loader, but that didn't work because the loaded controller didn't inherit it - thus this PR. I guess the alternative would be to ensure that everything that dynamically loads models (e.g. the various XR factories) have proper callbacks to react to the loading being done.

(had to check the source code to see which of the loaders have async side effects and which directly load something)

@hybridherbst
Copy link
Contributor Author

@mrdoob did you have a chance to look at this again? Anything else unclear? :)

@hybridherbst
Copy link
Contributor Author

hybridherbst commented Sep 27, 2022

Rebased this on a recent dev (cleanly) in case there's still interest in merging this.
(we still need this / part of the reason why we're still using a three fork and not main)

@hybridherbst hybridherbst force-pushed the inherit-layers-on-controller-creation branch from 4c44f7a to c05c171 Compare November 24, 2022 19:43
@hybridherbst
Copy link
Contributor Author

Rebased again; still interested in getting this merged. As layers are used for both visibility and raycasting it's quite important to be able to control them reliably.

cc @mrdoob

@hybridherbst hybridherbst force-pushed the inherit-layers-on-controller-creation branch from c05c171 to da4564c Compare March 27, 2023 16:02
@hybridherbst
Copy link
Contributor Author

Rebased again, still useful, would love to see this mini improvement in the core 🙂
cc @mrdoob

@hybridherbst hybridherbst reopened this Mar 27, 2023
@hybridherbst hybridherbst force-pushed the inherit-layers-on-controller-creation branch from da4564c to cec3cad Compare May 23, 2023 21:37
@hybridherbst
Copy link
Contributor Author

Rebased again. With your recent interest in XR again maybe I can get your eyes on this @mrdoob? 🐰

@hybridherbst
Copy link
Contributor Author

This issue here is similar to the current discussion with WebXR cameras - in some cases it's necessary to have correct layer setup and that's what this PR ensures on creation. I don't think this has unintended consequences.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 15, 2024

Closing in favor of #27544.

@Mugen87 Mugen87 closed this Jan 15, 2024
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.

3 participants