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

Expose OpenXR raw hand tracking data #78032

Merged

Conversation

BastiaanOlij
Copy link
Contributor

This PR adds methods to OpenXRInterface that retrieve the raw tracking data from OpenXRs hand tracking. This has been requested by a few people who wish to apply the tracking data to full body avatars or who just want to use the data outside of the skeleton implementation.

@BastiaanOlij BastiaanOlij added this to the 4.2 milestone Jun 9, 2023
@BastiaanOlij BastiaanOlij self-assigned this Jun 9, 2023
@BastiaanOlij BastiaanOlij marked this pull request as ready for review June 20, 2023 00:26
@BastiaanOlij BastiaanOlij force-pushed the expose_openxr_hand_tracking_data branch from f7f5681 to 72131e8 Compare July 17, 2023 04:20
@lyuma
Copy link
Contributor

lyuma commented Aug 1, 2023

Hey, since a few friends were asking me about this I wanted take a look. Overall seems pretty straightforward.

I'm a little uncertain about the Array-of-Dictionary return value. I know there are some existing APIs which do this, but there's a lot of allocations needed and no auto-complete.

Since it's only 2 (velocity) or 3 (position) values each, I'm wondering if it would be possible to separate each value into its own function call and return a TypedArray or PackedArray for the Vector3 values and TypedArray for the rotations?

Something like

	Vector<Vector3> get_hand_joint_positions(Hand p_hand);
	TypedArray<Quaternion> get_hand_joint_rotations(Hand p_hand);
	Vector<float> get_hand_joint_radii(Hand p_hand);

	Vector<Vector3> get_hand_joint_linear_velocities(Hand p_hand);
	Vector<Vector3> get_hand_joint_angular_velocities(Hand p_hand);

Another approach would be for example having a getters which pass in a joint index for each:

	Vector3 get_hand_joint_position(Hand p_hand, Hand::HandJoints p_joint);
	Quaternion get_hand_joint_rotation(Hand p_hand, Hand::HandJoints p_joint);
	float get_hand_joint_radius(Hand p_hand, Hand::HandJoints p_joint);

	Vector3 get_hand_joint_linear_velocity(Hand p_hand, Hand::HandJoints p_joint);
	Vector3 get_hand_joint_angular_velocity(Hand p_hand, Hand::HandJoints p_joint);

@Faolan-Rad
Copy link
Contributor

Another thing that should be added is the ability to set_motion_range, and also I would prefer for it not to use a dictionary due to the function most likely being called every frame. Also, I encountered a problem where the left hand is the right hand and the right hand cannot be accessed.

@Faolan-Rad
Copy link
Contributor

	Vector3 get_hand_joint_position(Hand p_hand, Hand::HandJoints p_joint);
	Quaternion get_hand_joint_rotation(Hand p_hand, Hand::HandJoints p_joint);
	float get_hand_joint_radius(Hand p_hand, Hand::HandJoints p_joint);

	Vector3 get_hand_joint_linear_velocity(Hand p_hand, Hand::HandJoints p_joint);
	Vector3 get_hand_joint_angular_velocity(Hand p_hand, Hand::HandJoints p_joint);

This solution would be better do to no memory allocations every frame

@BastiaanOlij
Copy link
Contributor Author

@lyuma good suggestions, that shouldn't be a hard change and indeed would probably be more efficient memory wise.

@Faolan-Rad shouldn't be hard to also expose motion range, we already implement it in the API, just need to expose it to GDScript

@BastiaanOlij BastiaanOlij force-pushed the expose_openxr_hand_tracking_data branch from 72131e8 to f4a4d92 Compare August 28, 2023 06:13
@BastiaanOlij
Copy link
Contributor Author

@lyuma @Faolan-Rad I've made the requested changes, indeed neater this way!

Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

It looks good to me! Thanks

modules/openxr/scene/openxr_hand.h Outdated Show resolved Hide resolved
modules/openxr/scene/openxr_hand.h Outdated Show resolved Hide resolved
modules/openxr/openxr_interface.cpp Outdated Show resolved Hide resolved
@akien-mga
Copy link
Member

@BastiaanOlij Can you do the requested style fixes?

@BastiaanOlij BastiaanOlij force-pushed the expose_openxr_hand_tracking_data branch from f4a4d92 to 58df9bd Compare September 7, 2023 12:55
@BastiaanOlij
Copy link
Contributor Author

@BastiaanOlij Can you do the requested style fixes?

ah thank for the poke, done!

@akien-mga akien-mga merged commit 0f38fdf into godotengine:master Sep 7, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@BastiaanOlij BastiaanOlij deleted the expose_openxr_hand_tracking_data branch September 7, 2023 23:31
@goatchurchprime
Copy link

This is perfect (in terms of accessing the hand tracking data so that I can map it to whatever non-OpenXR compatible hand shape that I usually have to hand (eg where the bones are aligned with the Y-axis as is the convention from Blender models)), except for one small issue...

We can't tell whether the hand positions are valid without using a completely separate OpenXRHand node and checking its visible flag.
https://github.com/godotengine/godot/blob/master/modules/openxr/scene/openxr_hand.cpp#L276

Is it possible to add in some kind of hand_activated and/or confidence rating into this interface? These might be more applicable than the finger bone velocities (though I might experiment with those at some stage to see what happens when you project the it forwards by 0.01seconds).

@BastiaanOlij
Copy link
Contributor Author

Thanks @goatchurchprime , sorry had forgotten this was already merged when I asked you to comment. I'll find a moment next week to add in the confidence information, makes perfect sense we expose that too.

@goatchurchprime
Copy link

This might be higher priority than I thought since I can't make the OpenXRHand node (needed only for the active/tracking_confidence flag) work when I use it in the Godot XR Tools Demos.

The docs file say that it needs to be a direct child of the XROrigin3D, and there are two origins when running the demos. I've tried every combination of putting these nodes in either of them, in and out of their base scenes but the visible flag is always false even when the hands are tracking.

I know that OpenXRHand works in my other simpler project (no instanced scenes, no dynamically loaded second scenes), so it is mind-boggling I can't reproduce a working version by hacking the XR Tools Demo. Since I can't properly state the bug in a way that would mean it would be debuggable, I don't want to report it.

However, an active/tracking_confidence function would mean that the OpenXRHand object would not need to be used.

[I'd go further to say that OpenXRHand could be deprecated since OpenXR-compatible hand skeletons are as rare as hen's teeth]

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.

6 participants