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

Add hand capsule data to XRHandTracker #89289

Closed
wants to merge 1 commit into from

Conversation

devloglogan
Copy link
Contributor

This PR makes it possible to provide XRHandTracker with capsule data that could then be used to construct meshes or collision shapes.

Discussion has taken place that a vendor-neutral way of populating/consuming hand capsule data may be desirable in GodotVR/godot_openxr_vendors#88. The conversation boils down to: OpenXR extension XR_FB_hand_tracking_capsules provides capsule data, however similar capsule data could also be constructed via joint data from the vendor-neutral XR_EXT_hand_tracking. This is the main motivation for this PR.

@devloglogan devloglogan requested review from a team as code owners March 8, 2024 17:11
@AThousandShips AThousandShips added this to the 4.x milestone Mar 8, 2024
@dsnopek dsnopek changed the title Add hand capsule data to XRHandTracker Add hand capsule data to XRHandTracker Mar 8, 2024
@dsnopek
Copy link
Contributor

dsnopek commented Mar 8, 2024

Thanks! The implementation looks great to me :-)

I think the main things we'll need to discuss among the @godotengine/xr team is:

  1. Does it make sense to add this sort vendor-neutral storage to Godot now? There's only Meta that provides capsules with their hand tracking data. Alternatively, we could make that data available in a vendor-specific way in 'godot_openxr_vendors' for now, and come back to this later when more vendors support it? Although, we can also make our own implementation that derives the capsules using a custom algorithm, so then we'd have at least two implementations.
  2. Is this the way we think the data should be organized? The data from Meta can provide multiple capsules per joint (which it does for the wrist joint) and that's why the data here is organized to allow for multiple capsules per joint. Having only the one source of capsule data, it's hard to say for sure if this structure will be sufficient to hold the data from future sources. Does it seem like what's in this PR is OK?

@Malcolmnixon
Copy link
Contributor

This is slightly off topic, but Meta's XR_FB_hand_tracking_capsules extension certainly seems a little rough around the edges.

I "assumed" they would have named the capsules - so someone could tell what finger was colliding with a button... but no! The openxr header file has #define XR_HAND_TRACKING_CAPSULE_COUNT_FB 19 which strongly implies the 19 capsules correspond to the 19 finger bones (including metacarpals), but there's no guarantee of that. As such all your getting is a swarm of capsules which roughly cover the hand.

It's just mildly annoying they decided to separate bones/joints from colliders, considering that game engines (including Godot) have mechanisms for creating physics bones (https://docs.godotengine.org/en/stable/classes/class_physicalbone3d.html) associated with skeleton bones - which allow for bones to interact with physics and to allow for advanced effects such as ragdoll.

We already get the finger joints and tip locations - so we could actually get the lengths of every bone in the hand (including knowing which bone it is). The only thing "missing" is the radius of the bone capsule. We could get that from either:

  • Attempting to correlate the capsule data with the finger data
  • Use a fixed lookup table for each bone - not ideal as hopefully Meta is actually measuring how pudgy the fingers really are.

In an ideal world what would have been nice would have been:

  • XRHandTracker.has_bone(joint : HandJoint) -> bool
  • XRHandTracker.get_bone_length(joint : HandJoint) ->float
  • XRHandTracker.get_bone_radius(joint : HandJoint) -> float

Alas with Meta coming out of the gate decoupling bones and colliders I'm not sure we should design an API with them combined back.

@dsnopek
Copy link
Contributor

dsnopek commented Mar 9, 2024

I "assumed" they would have named the capsules - so someone could tell what finger was colliding with a button... but no!

They do associate each capsule with a joint, but it's only defined as the joint "that drives this capsule’s transform". And there can be (and are!) multiple capsules to a single joint. So, unfortunately, they don't relate directly to bones.

If I had designed the data, like you, I would also have been inclined to associate each capsule with a bone.

Alas with Meta coming out of the gate decoupling bones and colliders I'm not sure we should design an API with them combined back.

I agree. Any attempt to "normalize" their data to correlate each capsule to a bone wouldn't be guaranteed to continue working per the spec.

@BastiaanOlij
Copy link
Contributor

Hmmm, the thing is, when we look at the current core solution, no additions are needed. All information is already in XRHandTracker as the current assumption is that you use the normal bone data together with the already provided radiuses to set up collision shapes around the fingers. Though equally there are platforms where you use a fixed size hand regardless of the actual size of the players hand, and thus use colliders added to the hand asset (this is generally true for VR games, while the other approach is more common in AR).

The hand capsule approach Meta has seems a solution that attempts to do the same thing OpenXR already solves, and its still unclear to me as to why? What does this offer what we don't already have the ability to do with what core OpenXR offers us, and wouldn't we be better off to support the core functionality so all platforms benefit from the work?

I'm not aware of any other vendor going down this path, so I don't think we should be changing anything in XRHandTracker. This just seems like adding an alternative structure for something we're already recording data for. Unless more vendors adopt a similar approach, I don't think this is something we should do today.

I'm not against having the logic in the vendor plugin if we must have a Meta specific implementation, that is where it belongs. It just seems, uhm, double.
The real problem with the implementation in the vendor plugin was that implementing hands that properly react to physics is a lot more difficult than adding some static colliders to the fingers. That's also the things we're working on solving in XR tools but in a platform agnostic way.

@devloglogan
Copy link
Contributor Author

I have no qualms with closing this PR, updating the vendor repo PR to expose the data like this, and removing the physics aspects if we find that preferable! I agree that it does feel slightly "double", and because of that if feels off in adding anything related to it to Godot itself. But I don't think there's anything wrong with exposing the data via the vendor repo.

I will plan on updating the vendor repo PR, and if there's consensus on this being the agreed solution we can close this PR.

@devloglogan
Copy link
Contributor Author

Closing, a PR in the vendor repo has been merged that exposes the data for XR_FB_hand_tracking_capsules. (GodotVR/godot_openxr_vendors#116)

@AThousandShips AThousandShips removed this from the 4.x milestone Apr 19, 2024
@devloglogan devloglogan deleted the xr-hand-capsules branch April 19, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants