input: fix hand tracking with no interaction profile and/or grip pose#261
input: fix hand tracking with no interaction profile and/or grip pose#261ImSapphire wants to merge 8 commits intoSupreeeme:mainfrom
Conversation
560abea to
562665a
Compare
6f35c5b to
eb3646e
Compare
21bb680 to
b836a4d
Compare
| Hand::Left => Vec3::from_array([0.09, -0.03, -0.09]), | ||
| Hand::Right => Vec3::from_array([-0.09, -0.03, -0.09]), |
There was a problem hiding this comment.
Where are these values coming from?
There was a problem hiding this comment.
they're pulled from OpenComposite
There was a problem hiding this comment.
These values derived from OC, are pulled from SteamVR's SteamLink for hands, so they should be spot on.
There was a problem hiding this comment.
This could use a comment or be a const or something.
| offset *= Mat4::from_euler( | ||
| glam::EulerRot::XYZ, | ||
| 0.0, | ||
| if data.hand == Hand::Left { | ||
| -FRAC_PI_4 | ||
| } else { | ||
| FRAC_PI_4 | ||
| }, | ||
| if data.hand == Hand::Left { | ||
| -FRAC_PI_2 | ||
| } else { | ||
| FRAC_PI_2 | ||
| }, | ||
| ); |
There was a problem hiding this comment.
What's up with this transform?
There was a problem hiding this comment.
it's from OC. without this transform, when the fingertips are facing forward and palms are facing down, in-game the palms will be facing inward and fingers are pointed 45deg down
There was a problem hiding this comment.
Most likely also derived from SteamLink ditto the above comment by me.
| self.interaction_profile.or_else(|| { | ||
| if matches!(self.device_type, TrackedDeviceType::Controller { .. }) | ||
| && self | ||
| .get_hand_skeleton(xr_data, session_data.tracking_space()) | ||
| .is_some() | ||
| { | ||
| Some(&VRLinkHand) | ||
| } else { | ||
| None | ||
| } | ||
| }) |
There was a problem hiding this comment.
Is hand tracking only something that can be dynamically switched to, and does the runtime change the interaction profiles when hand tracking is enabled? If the profile doesn't change this will be stuck with the old profile and never work.
There was a problem hiding this comment.
at least on Quest through WiVRn the interaction profile will change to NULL when switching to hand tracking.
i don’t think it’s critical that we return the VRLinkHand profile in all cases where hand tracking is active, i just did this so hand tracking works with no interaction profile
| TrackedDevice::new( | ||
| TrackedDeviceType::Controller(ControllerData { | ||
| hand: Hand::Left, | ||
| hand_tracker: create_hand_tracker(Hand::Left), | ||
| skeleton_cache: Mutex::new(Box::new(None)), | ||
| pose_is_synthesised: false.into(), | ||
| }), | ||
| None, | ||
| None, | ||
| ), | ||
| TrackedDevice::new( | ||
| TrackedDeviceType::Controller(ControllerData { | ||
| hand: Hand::Right, | ||
| hand_tracker: create_hand_tracker(Hand::Right), | ||
| skeleton_cache: Mutex::new(Box::new(None)), | ||
| pose_is_synthesised: false.into(), | ||
| }), | ||
| None, | ||
| None, | ||
| ), |
There was a problem hiding this comment.
Why are controllers being automatically created at startup?
There was a problem hiding this comment.
without this hand tracking will not work if controllers aren’t connected at least once
There was a problem hiding this comment.
Why? We should not report controllers if they don't actually exist. If I'm understanding the VRLinkHand profile properly, we just need to add bindings for the hand_interaction profile. In fact, couldn't we just do that instead of checking for the hand skeleton?
There was a problem hiding this comment.
If I'm understanding the VRLinkHand profile properly, we just need to add bindings for the hand_interaction profile. In fact, couldn't we just do that instead of checking for the hand skeleton?
It's possible to have XR_EXT_hand_tracking data available while XR_EXT_hand_interaction is not supported, I only added the VRLinkHand profile for the properties
There was a problem hiding this comment.
Fair enough, but that should be a comment on the profile, that's not really immediately obvious. In any case though, we should not create controllers when they don't exist, that's just confusing. There should be a way to check if hand tracking is available and try to pull a pose from it without this. XrSystemHandTrackingPropertiesEXT::supportsHandTracking might be useful.
There was a problem hiding this comment.
In any case though, we should not create controllers when they don't exist, that's just confusing
I do agree, but there's lots of functions that rely on the controllers existing, like IsTrackedDeviceConnected and Get*TrackedDeviceProperty, so I'm pretty sure we have to do this for hand tracking to work without having gotten a valid interaction profile for the hands, right?
There was a problem hiding this comment.
It would make more sense to take an approach similar to the info set used to determine if there are controllers before reporting them to the game - each frame, you could attempt to get a pose from the hand tracker, and when it succeeds you can add a controller. I don't have a real example on this but I'm pretty sure lying about controllers being connected can actually mess some games up. (should've written a test, rip)
src/input/devices.rs
Outdated
| pub skeleton_cache: Mutex<Box<Option<xr::HandJointLocations>>>, | ||
| pub pose_is_synthesised: AtomicBool, |
There was a problem hiding this comment.
You could instead have an enum like:
enum SkeletonPose {
Real(Box<Option<xr::HandJointLocations>>),
Synthesized(Box<Option<xr::HandJointLocations>>)
}
and then just have a field like:
skeleton_cache: Mutex<SkeletonPose>
Also, I don't think all of the fields in this struct need to be pub.
There was a problem hiding this comment.
synthesised pose doesn't really have anything to do with the openxr hand skeleton though, so i'm not sure it would make sense to wrap this in an enum here
b836a4d to
ad4b5c1
Compare
7339c29 to
b05146c
Compare
| Hand::Left => Vec3::from_array([0.09, -0.03, -0.09]), | ||
| Hand::Right => Vec3::from_array([-0.09, -0.03, -0.09]), |
There was a problem hiding this comment.
This could use a comment or be a const or something.
| offset *= Mat4::from_euler( | ||
| glam::EulerRot::XYZ, | ||
| 0.0, | ||
| if data.hand == Hand::Left { | ||
| -FRAC_PI_4 | ||
| } else { | ||
| FRAC_PI_4 | ||
| }, | ||
| if data.hand == Hand::Left { | ||
| -FRAC_PI_2 | ||
| } else { | ||
| FRAC_PI_2 | ||
| }, | ||
| ); |
| let pose_from_hand_tracking = || { | ||
| if let Some((joints, velocities)) = | ||
| controller.get_hand_skeleton(xr_data, session_data.tracking_space()) | ||
| { |
There was a problem hiding this comment.
This could be a method on ControllerData.
| pub fn get_hand_skeleton( | ||
| &self, | ||
| xr_data: &OpenXrData<impl crate::openxr_data::Compositor>, | ||
| base: &xr::Space, | ||
| ) -> Option<(xr::HandJointLocations, xr::HandJointVelocities)> { | ||
| let TrackedDeviceType::Controller(data) = self.get_type() else { | ||
| return None; | ||
| }; |
There was a problem hiding this comment.
This should just be a method on ControllerData.
| TrackedDevice::new( | ||
| TrackedDeviceType::Controller(ControllerData { | ||
| hand: Hand::Left, | ||
| hand_tracker: create_hand_tracker(Hand::Left), | ||
| skeleton_cache: Mutex::new(Box::new(None)), | ||
| pose_is_synthesised: false.into(), | ||
| }), | ||
| None, | ||
| None, | ||
| ), | ||
| TrackedDevice::new( | ||
| TrackedDeviceType::Controller(ControllerData { | ||
| hand: Hand::Right, | ||
| hand_tracker: create_hand_tracker(Hand::Right), | ||
| skeleton_cache: Mutex::new(Box::new(None)), | ||
| pose_is_synthesised: false.into(), | ||
| }), | ||
| None, | ||
| None, | ||
| ), |
There was a problem hiding this comment.
Why? We should not report controllers if they don't actually exist. If I'm understanding the VRLinkHand profile properly, we just need to add bindings for the hand_interaction profile. In fact, couldn't we just do that instead of checking for the hand skeleton?
| hand: Hand, | ||
| transforms: &mut [vr::VRBoneTransform_t], | ||
| ) { | ||
| ) -> Option<[vr::VRBoneTransform_t; Count as usize]> { |
b05146c to
a9ccf05
Compare
a9ccf05 to
dd94b89
Compare
Fixes hand tracking on WiVRn (WiVRn/WiVRn@1e2823c). Closes #165
Hand tracking is still pretty scuffed, we don't support the 'augmented' inputs from hand tracking that SteamVR supports (/input/[finger]_pinch trigger input, /input/index_point trigger input, /input/system left side button input, plus rebinds for basic trigger and grip pull).
I don't like the
pose_is_synthesisedhack, but this is what OC did and I'm not good enough at math to figure out how to fix the wrist pose when we're using a fake grip.Tested in VRChat and Resonite.