-
Notifications
You must be signed in to change notification settings - Fork 28
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 XR_FB_hand_tracking_capsules extension wrapper and OpenXRFbHandTrackingCapsules node #88
Conversation
It's a good start, I've been planning on adding similar logic to XR tools for awhile now, seeing we can do most of this through normal hand tracking, this just adds some sizing info. One thing I would look into is not creating an Also don't make Next, don't just update the transform, trust me, I've been trying to make hands work properly since I was sent a leap motion years ago, I've run into all the problem cases, I've tried loads of solutions, I'm close but lacked the time to dedicate to it last year. edit watching my 5 year old video back I'm reminding I could actually push the cylinders through the table because of this, it totally breaks the physics engine. You'll see 4 become 3 by the end of the video :P The only way to properly exert forces on objects is to use
Doing this right, in a way that actually works for a user, requires a fairly complex implementation, with possible enhancements to the physics engine. We can get close, and we should attempt to get close and call it good enough, as the feedback out of this will help us convince the physics team of what XR needs need to be applied to the physics engine. Lastly have a look at: https://github.com/GodotVR/godot-xr-tools/blob/master/addons/godot-xr-tools/objects/force_body/force_body.gd and https://github.com/GodotVR/godot-xr-tools/blob/master/addons/godot-xr-tools/hands/collision_hand.gd These form the basis of the implementation that has most of the above things implemented (not all) but only for the palm of the hand. I'm hoping to add finger collisions to this soon but with a number of "this is good enough but far from perfect" concessions due to missing logic. |
Given all the issues that @BastiaanOlij mentions, and the fact that a number of them may have different solutions depending on how the developer wants the game to work, would it be better to simply expose the capsule positions and sizes and let the developer decide what to do with them? Maybe XR Tools could accept hand capsule information, and that could come either from the developer (or some defaults?), or pulled from this extension if available? |
The difficulty is not really knowing where this is going, something I totally forgot about and that we still need to expose is that The real problem however is not the capsule data itself. Its the physics interaction and that we're missing functionality in the physics engine to accurately move a the collision data for a hand through space, check for collisions and react to those. edit DUH, I stand corrected, when I added access to raw data I did actually add it, see |
@BastiaanOlij just looking for a little clarity. If I follow: you feel a simpler implementation of |
@devloglogan Can you bump the |
Once we implement the logic in something like XR Tools yes, but we'd need to verify the data we get from the core extension is indeed usable |
With PR godotengine/godot#88639, we could conceivably add capsules data to However, before we do that, it'd probably be best to do some experimentation to see if we can fully derive the capsules from the radii, OR if the capsule data from this extension provides us with something better or more accurate than we can derive. |
Sure! Happy to look into this and report back soon. |
4c3d90b
to
a0f7004
Compare
…ackingCapsules node
a0f7004
to
9f7d38d
Compare
if (y_dir.is_equal_approx(Vector3(0, 1, 0))) { | ||
x_dir = y_dir.cross(Vector3(1, 0, 0)).normalized(); | ||
} else { | ||
x_dir = y_dir.cross(Vector3(0, 1, 0)).normalized(); |
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.
We should have axis constants you can use instead
GDCLASS(OpenXRFbHandTrackingCapsules, Node3D) | ||
|
||
public: | ||
enum Hand { |
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.
Can we reuse the XRHandTracker::Hand
enum instead?
} | ||
|
||
void OpenXRFbHandTrackingCapsulesExtensionWrapper::_bind_methods() { | ||
ClassDB::bind_method(D_METHOD("is_enabled"), &OpenXRFbHandTrackingCapsulesExtensionWrapper::is_enabled); |
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.
You can remove this binding since we're not exposing this singleton.
|
||
void OpenXRFbHandTrackingCapsules::_ready() { | ||
for (int i = 0; i < XR_HAND_TRACKING_CAPSULE_COUNT_FB; i++) { | ||
AnimatableBody3D *animatable_body = memnew(AnimatableBody3D); |
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.
Do we have a destructor to clear those references?
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.
It's my understanding that if a node is added to the scene tree (which these would be immediately), it's not necessary to provide cleanup for it. That would fall to the user. @dsnopek I think we talked about this at one point?
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.
Ah, yep, we don't generally clean up child nodes, because they'll automatically get cleaned up when their parent node is freed.
@@ -96,8 +101,10 @@ void initialize_plugin_module(ModuleInitializationLevel p_level) { | |||
Engine::get_singleton()->register_singleton("OpenXRFbSpatialEntityContainerExtensionWrapper", OpenXRFbSpatialEntityContainerExtensionWrapper::get_singleton()); | |||
Engine::get_singleton()->register_singleton("OpenXRFbSceneExtensionWrapper", OpenXRFbSceneExtensionWrapper::get_singleton()); | |||
Engine::get_singleton()->register_singleton("OpenXRFbFaceTrackingExtensionWrapper", OpenXRFbFaceTrackingExtensionWrapper::get_singleton()); | |||
Engine::get_singleton()->register_singleton("OpenXRFbHandTrackingCapsulesExtensionWrapper", OpenXRFbHandTrackingCapsulesExtensionWrapper::get_singleton()); |
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.
You can remove this registration since the api is exposed by the added node class.
@dsnopek Finally got around to checking out how well the capsule data can be derived from the already integrated sphere data. I'm just averaging the radius/position between joints to create the capsules. Sphere-derived is left hand, XR_FB_hand_tracking_capsules is the right hand. capsules-from-spheres.mp4They're not identical when done this way. If we wanted to tweak it further so that it would be more identical to the right hand, I'm sure we could. But the left hand does feel more accurate to me, and I'd personally feel more inclined to use the left hand as displayed here. |
I've made a core PR for providing/consuming hand capsule data in Also, I realized that I was actually interpreting the hand capsule data incorrectly, and that godot's capsule height doesn't factor in capsule radius at all (as opposed to the data provided by the extension). The size of the hand tracked capsules makes a lot more sense to me with this fix added. :) |
Closing, as a different implementation has been merged (#116) |
Once again this PR only functions with Godot 4.3 / PR #87546
Creates an extension wrapper for the XR_FB_hand_tracking_capsules extension, which provides info on a bunch of capsule shapes that can be used while hand tracking.
This is implemented via an
OpenXRFbHandTrackingCapsules
node, which should be the child of anXRController3D
. The node instantiates anAnimatableBody3D
for each hand tracking capsule at runtime, and updates their transforms every physics frame. I've set theOpenXRFbHandTrackingCapsules
node up with duplicate properties ofAnimatableBody3D
(not all of them, just the ones that I think made sense), and updating those properties at runtime should also update the children accordingly.hand-tracking-capsules.mp4