From 4e134e549d206fa7d49e09675c39c705fafb0497 Mon Sep 17 00:00:00 2001 From: Bill Doyle Date: Thu, 31 Oct 2024 15:49:29 -0400 Subject: [PATCH 1/3] Remove unnecessary hand tracker tracking These variables are unused for anything except clearing them later, and prevent switching between controller sets at runtime, like when switching from controllers to hand tracking and back. --- src/open_vr/openvr_data.cpp | 22 ++-------------------- src/open_vr/openvr_data.h | 4 ---- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/open_vr/openvr_data.cpp b/src/open_vr/openvr_data.cpp index 7d9c3ed..fc641b6 100644 --- a/src/open_vr/openvr_data.cpp +++ b/src/open_vr/openvr_data.cpp @@ -341,10 +341,6 @@ bool openvr_data::initialise() { tracked_devices[i].tracker = Ref(); } - device_hands_are_available = false; - left_hand_device = vr::k_unTrackedDeviceIndexInvalid; - right_hand_device = vr::k_unTrackedDeviceIndexInvalid; - // find any already attached devices for (uint32_t i = vr::k_unTrackedDeviceIndex_Hmd; i < vr::k_unMaxTrackedDeviceCount; i++) { if (is_tracked_device_connected(i)) { @@ -878,17 +874,8 @@ void openvr_data::attach_device(uint32_t p_device_index) { int32_t controllerRole = get_controller_role(p_device_index); if (controllerRole == vr::TrackedControllerRole_RightHand) { hand = 2; - device_hands_are_available = true; } else if (controllerRole == vr::TrackedControllerRole_LeftHand) { hand = 1; - device_hands_are_available = true; - } else if (!device_hands_are_available) { - // this definately needs to improve, if we haven't got hand information, our first controller becomes left and our second becomes right - if (left_hand_device == vr::k_unTrackedDeviceIndexInvalid) { - hand = 1; - } else if (right_hand_device == vr::k_unTrackedDeviceIndexInvalid) { - hand = 2; - } } } else { Array arr; @@ -905,17 +892,12 @@ void openvr_data::attach_device(uint32_t p_device_index) { new_tracker->set_tracker_desc(device_name); new_tracker->set_tracker_hand(XRPositionalTracker::TrackerHand(hand)); - // remember our primary left and right hand devices - if ((hand == 1) && (left_hand_device == vr::k_unTrackedDeviceIndexInvalid)) { + if (hand == 1) { new_tracker->set_tracker_name("left_hand"); - vr::VRInput()->GetInputSourceHandle("/user/hand/left", &device->source_handle); - left_hand_device = p_device_index; - } else if ((hand == 2) && (right_hand_device == vr::k_unTrackedDeviceIndexInvalid)) { + } else if (hand == 2) { new_tracker->set_tracker_name("right_hand"); - vr::VRInput()->GetInputSourceHandle("/user/hand/right", &device->source_handle); - right_hand_device = p_device_index; } else { // other devices don't have source handles... sprintf(device_name, "controller_%i", p_device_index); diff --git a/src/open_vr/openvr_data.h b/src/open_vr/openvr_data.h index bba549f..97ed57d 100644 --- a/src/open_vr/openvr_data.h +++ b/src/open_vr/openvr_data.h @@ -121,10 +121,6 @@ class openvr_data { vr::VRInputValueHandle_t source_handle; }; - bool device_hands_are_available; - uint32_t left_hand_device; - uint32_t right_hand_device; - tracked_device tracked_devices[vr::k_unMaxTrackedDeviceCount]; void attach_device(uint32_t p_device_index); From 3d728d4aa5ff1996f36a65e952f87d5e1c6c2554 Mon Sep 17 00:00:00 2001 From: Bill Doyle Date: Thu, 31 Oct 2024 15:44:45 -0400 Subject: [PATCH 2/3] Avoid removing the wrong trackers when detaching When detaching a tracker which has become inactive, verify that it still owns its name in the XRServer. If a newer tracker has replaced it, for example when switching between controllers and hand tracking, attempting to remove it will accidentally remove the newer one instead. This should be fixed upstream, but for now this workaround avoids the problem and will be forward compatible. --- src/open_vr/openvr_data.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/open_vr/openvr_data.cpp b/src/open_vr/openvr_data.cpp index fc641b6..aebbcb6 100644 --- a/src/open_vr/openvr_data.cpp +++ b/src/open_vr/openvr_data.cpp @@ -914,7 +914,7 @@ void openvr_data::attach_device(uint32_t p_device_index) { } //////////////////////////////////////////////////////////////// -// Called when we loose tracked device, cleanup +// Called when we lose tracked device, cleanup void openvr_data::detach_device(uint32_t p_device_index) { tracked_device *device = &tracked_devices[p_device_index]; @@ -923,16 +923,20 @@ void openvr_data::detach_device(uint32_t p_device_index) { } else if (device->tracker.is_valid()) { XRServer *xr_server = XRServer::get_singleton(); if (xr_server != nullptr) { - xr_server->remove_tracker(device->tracker); + // XXX: Work around a design issue with XRServer: removing a tracker happens by + // name, instead of removing the exact object you pass. This means that if a + // tracker has been replaced and then goes inactive, we will remove the wrong one. + Ref existing_tracker = xr_server->get_tracker( + device->tracker->get_tracker_name()); + if (existing_tracker == device->tracker) { + xr_server->remove_tracker(device->tracker); + } else { + Array arr; + arr.push_back(device->tracker->get_tracker_name()); + UtilityFunctions::push_warning(String("Not removing tracker {0}, already replaced").format(arr)); + } } device->tracker.unref(); - - // unset left/right hand devices - if (left_hand_device == p_device_index) { - left_hand_device = vr::k_unTrackedDeviceIndexInvalid; - } else if (right_hand_device == p_device_index) { - right_hand_device = vr::k_unTrackedDeviceIndexInvalid; - } } } From d74c7484e3c047d95fdd84d1c55ab91f0b8a7734 Mon Sep 17 00:00:00 2001 From: Bill Doyle Date: Thu, 31 Oct 2024 15:53:12 -0400 Subject: [PATCH 3/3] Fix potential out of bounds with invalid device index --- src/open_vr/openvr_data.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/open_vr/openvr_data.cpp b/src/open_vr/openvr_data.cpp index aebbcb6..6697732 100644 --- a/src/open_vr/openvr_data.cpp +++ b/src/open_vr/openvr_data.cpp @@ -840,11 +840,12 @@ void openvr_data::remove_pose_action(const char *p_action) { //////////////////////////////////////////////////////////////// // Called when we detect a new device, set it up void openvr_data::attach_device(uint32_t p_device_index) { + if (p_device_index == vr::k_unTrackedDeviceIndexInvalid) { + return; + } tracked_device *device = &tracked_devices[p_device_index]; - if (p_device_index == vr::k_unTrackedDeviceIndexInvalid) { - // really?! - } else if (device->tracker.is_null()) { + if (device->tracker.is_null()) { char device_name[256]; strcpy(device_name, get_device_name(p_device_index, 255)); @@ -916,11 +917,12 @@ void openvr_data::attach_device(uint32_t p_device_index) { //////////////////////////////////////////////////////////////// // Called when we lose tracked device, cleanup void openvr_data::detach_device(uint32_t p_device_index) { + if (p_device_index == vr::k_unTrackedDeviceIndexInvalid) { + return; + } tracked_device *device = &tracked_devices[p_device_index]; - if (p_device_index == vr::k_unTrackedDeviceIndexInvalid) { - // really?! - } else if (device->tracker.is_valid()) { + if (device->tracker.is_valid()) { XRServer *xr_server = XRServer::get_singleton(); if (xr_server != nullptr) { // XXX: Work around a design issue with XRServer: removing a tracker happens by