mbucchia / VirtualDesktop-OpenXR

An implementation of the OpenXR standard for Virtual Desktop
MIT License
209 stars 7 forks source link

Fix controller and hand tracking pose validation #39

Closed Nordskog closed 3 months ago

Nordskog commented 3 months ago

What

Success check

In OpenXrRuntime::getControllerPose() the result of calling ovr_GetDevicePoses() is checked for success by using OVR_SUCCESS(). This success check assumes that any value >=0 is a success.

For some bizarre reason this includes ovrSuccess_DeviceUnavailable = 1002, which is pretty much the only non-0 result code ovr_GetDevicePoses() will ever return, and happens when:

Since the Success of the call is used to determine if the controller is being tracked, effectively any call to ovr_GetDevicePoses() is considered to be tracked, even when it is not and the pose is zeroed out.

Changed to use OVR_UNQUALIFIED_SUCCESS() instead, which only returns true if result is ovrSuccess, i.e. 0.

The other possible success-type return codes listed in OVR_ErrorCode.h do not seem relevant, so we can probably safely assume that only ovrSuccess should be considered a success for this call. I can't think of many situation where device-literally-unavailable should be considered a success, so other uses of OVR_SUCCESS() should probably also be scrutinized.

Hand Tracking fallout

Since OpenXrRuntime::xrLocateHandJointsEXT falls back to using the controller pose to emulate hand tracking, the above results in it thinking the returned controller pose is valid, and it proceeds to return what it thinks is a valid set of hand tracking joints to the caller.

RightHandActive check

OpenXrRuntime::xrLocateHandJointsEXT uses the cached BodyTracking::BodyStateV2::LeftHandActive and RightHandActive to determine if hand-tracking is active. Either side should obviously only be considered if it is the same side hand as the current XrHandTrackerEXT, but the RightHandActive lacks this check, resulting in both hands being considered active if the right hand is active.

Fixed by adding the same comparison used for the left side.

Thoughts

Controllers emulating hand tracking and hand tracking emulating controllers makes for a bit of a conundrum when you want to take specific actions only when hand-tracking is active, so the controller pose being unavailable in headless mode is a blessing in disguise in my case. Hopefully that issue will remain until XR_EXT_hand_tracking_data_source is implemented nudge nudge wink wink.

Environment

In case any of the behavior I am seeing is environment-specific:

mbucchia commented 3 months ago

Thanks for these changes!

Can you add yourself to README.md, create a "Contributors" section just before the "Donate" one?

XR_EXT_hand_tracking_data_source is planned, it just wasn't a priority for this last release, since no engine/app is using it today AFAICT.

mbucchia commented 3 months ago

Wildly untested XR_EXT_hand_tracking_data_source: 8cb439ddd3fc738e3b7f81eddcc3fbfb4873b04f

mbucchia commented 3 months ago

Thank you!

BTW you are going to get a build failure email - this is due to my code signing certificate being expired. Nothing to worry about.

Nordskog commented 3 months ago

Many thanks for implementing XR_EXT_hand_tracking_data_source!