From fd37627bb33d7a31c3b6d630408880abdc60737c Mon Sep 17 00:00:00 2001 From: Sergio Villar Senin Date: Tue, 14 Jan 2025 13:15:16 +0100 Subject: [PATCH] Reduce sensitivity required by controller trigger We've been traditionally using a high value for clicking threshold (0.91) in order not to generate false positives specially when using hand tracking. However, that does not make much sense when using physical controllers because they're very precise and false positives are not a real problem there. We can actually lower it to just 25% of the full pressed position without any accuracy loss, and indeed, the experience is much better. It's also good for a11y. Fixes #1487 --- app/src/main/cpp/BrowserWorld.cpp | 4 +--- app/src/main/cpp/DeviceDelegate.h | 2 +- app/src/openxr/cpp/DeviceDelegateOpenXR.cpp | 4 ++-- app/src/openxr/cpp/DeviceDelegateOpenXR.h | 2 +- app/src/openxr/cpp/OpenXRHelpers.h | 4 ---- app/src/openxr/cpp/OpenXRInput.cpp | 6 ++++++ app/src/openxr/cpp/OpenXRInput.h | 1 + app/src/openxr/cpp/OpenXRInputSource.cpp | 14 ++++++++++++-- app/src/openxr/cpp/OpenXRInputSource.h | 2 ++ 9 files changed, 26 insertions(+), 13 deletions(-) diff --git a/app/src/main/cpp/BrowserWorld.cpp b/app/src/main/cpp/BrowserWorld.cpp index 3ba169297c7..4cbdd72c90e 100644 --- a/app/src/main/cpp/BrowserWorld.cpp +++ b/app/src/main/cpp/BrowserWorld.cpp @@ -215,7 +215,6 @@ struct BrowserWorld::State { bool isApplicationEntitled = false; #endif TrackedKeyboardRendererPtr trackedKeyboardRenderer; - float selectThreshold; State() : paused(true), glInitialized(false), modelsLoaded(false), env(nullptr), cylinderDensity(0.0f), nearClip(0.1f), farClip(300.0f), activity(nullptr), windowsInitialized(false), exitImmersiveRequested(false), loaderDelay(0) { @@ -529,7 +528,7 @@ BrowserWorld::State::UpdateControllers(bool& aRelayoutWidgets) { const float scale = (hitPoint - device->GetHeadTransform().MultiplyPosition(vrb::Vector(0.0f, 0.0f, 0.0f))).Magnitude(); controller.pointer->SetScale(scale + kPointerSize - controller.selectFactor * kPointerSize); - if (controller.selectFactor >= selectThreshold) + if (controller.selectFactor >= device->GetSelectThreshold(controller.index)) controller.pointer->SetPointerColor(kPointerColorSelected); else controller.pointer->SetPointerColor(VRBrowser::GetPointerColor()); @@ -934,7 +933,6 @@ BrowserWorld::RegisterDeviceDelegate(DeviceDelegatePtr aDelegate) { m.device->SetControllerDelegate(delegate); m.device->SetReorientClient(this); m.gestures = m.device->GetGestureDelegate(); - m.selectThreshold = m.device->GetSelectThreshold(); } else if (previousDevice) { m.leftCamera = m.rightCamera = nullptr; m.controllers->Reset(); diff --git a/app/src/main/cpp/DeviceDelegate.h b/app/src/main/cpp/DeviceDelegate.h index 8ece589e680..89cb8782596 100644 --- a/app/src/main/cpp/DeviceDelegate.h +++ b/app/src/main/cpp/DeviceDelegate.h @@ -138,7 +138,7 @@ class DeviceDelegate { virtual void SetImmersiveBlendMode(device::BlendMode) {}; virtual bool PopulateTrackedKeyboardInfo(TrackedKeyboardInfo& keyboardInfo) { return false; }; virtual void SetHandTrackingEnabled(bool value) {}; - virtual float GetSelectThreshold() { return 1.f; }; + virtual float GetSelectThreshold(int32_t controllerIndex) { return 1.f; }; protected: DeviceDelegate() {} diff --git a/app/src/openxr/cpp/DeviceDelegateOpenXR.cpp b/app/src/openxr/cpp/DeviceDelegateOpenXR.cpp index 8020ab89795..a2595a30c4c 100644 --- a/app/src/openxr/cpp/DeviceDelegateOpenXR.cpp +++ b/app/src/openxr/cpp/DeviceDelegateOpenXR.cpp @@ -1751,8 +1751,8 @@ void DeviceDelegateOpenXR::SetHandTrackingEnabled(bool value) { m.handTrackingEnabled = value; } -float DeviceDelegateOpenXR::GetSelectThreshold() { - return kClickThreshold; +float DeviceDelegateOpenXR::GetSelectThreshold(int32_t aControllerIndex) { + return m.input->GetSelectThreshold(aControllerIndex); } void diff --git a/app/src/openxr/cpp/DeviceDelegateOpenXR.h b/app/src/openxr/cpp/DeviceDelegateOpenXR.h index 3eaf63afb4a..37f2056127a 100644 --- a/app/src/openxr/cpp/DeviceDelegateOpenXR.h +++ b/app/src/openxr/cpp/DeviceDelegateOpenXR.h @@ -80,7 +80,7 @@ class DeviceDelegateOpenXR : public DeviceDelegate { bool ExitApp(); bool ShouldExitRenderLoop() const; void SetImmersiveBlendMode(device::BlendMode) override; - float GetSelectThreshold() override; + float GetSelectThreshold(int32_t controllerIndex) override; protected: struct State; diff --git a/app/src/openxr/cpp/OpenXRHelpers.h b/app/src/openxr/cpp/OpenXRHelpers.h index 7bc8481df3f..6924c3418f8 100644 --- a/app/src/openxr/cpp/OpenXRHelpers.h +++ b/app/src/openxr/cpp/OpenXRHelpers.h @@ -14,10 +14,6 @@ namespace crow { -// Threshold to consider a trigger value as a click -// Used when devices don't map the click value for triggers; -const float kClickThreshold = 0.91f; - #if defined(HVR) const vrb::Vector kAverageHeight(0.0f, 1.6f, 0.0f); #else diff --git a/app/src/openxr/cpp/OpenXRInput.cpp b/app/src/openxr/cpp/OpenXRInput.cpp index a30db57ffd7..4dc8acacb65 100644 --- a/app/src/openxr/cpp/OpenXRInput.cpp +++ b/app/src/openxr/cpp/OpenXRInput.cpp @@ -380,4 +380,10 @@ bool OpenXRInput::updateEyeGaze(XrFrameState frameState, const vrb::Matrix& head return true; } + +float OpenXRInput::GetSelectThreshold(int32_t aControllerIndex) const { + ASSERT(mInputSources.at(aControllerIndex)); + return mInputSources.at(aControllerIndex)->GetSelectThreshold(); +} + } // namespace crow diff --git a/app/src/openxr/cpp/OpenXRInput.h b/app/src/openxr/cpp/OpenXRInput.h index 4338a679b20..d27568d89e1 100644 --- a/app/src/openxr/cpp/OpenXRInput.h +++ b/app/src/openxr/cpp/OpenXRInput.h @@ -76,6 +76,7 @@ class OpenXRInput { void InitializeEyeGaze(ControllerDelegate&); void InitializeEyeGazeSpaces(); bool updateEyeGaze(XrFrameState, const vrb::Matrix& head, ControllerDelegate&); + float GetSelectThreshold(int32_t aControllerIndex) const; }; } // namespace crow \ No newline at end of file diff --git a/app/src/openxr/cpp/OpenXRInputSource.cpp b/app/src/openxr/cpp/OpenXRInputSource.cpp index 42badcc936a..b80d1a5603b 100644 --- a/app/src/openxr/cpp/OpenXRInputSource.cpp +++ b/app/src/openxr/cpp/OpenXRInputSource.cpp @@ -14,6 +14,13 @@ namespace crow { // Otherwise single (slow) clicks would easily trigger scrolling. const XrDuration kEyeTrackingScrollThreshold = 250000000; +// Threshold to consider a trigger value as a click +// Used when devices don't map the click value for triggers; +const float kControllerClickThreshold = 0.25f; +// Threshold to consider a pinch as a click. Hand tracking is not as precise as controllers, that's +// why we need to be extra careful in order not to generate false positives. +const float kPinchThreshold = 0.91f; + OpenXRInputSourcePtr OpenXRInputSource::Create(XrInstance instance, XrSession session, OpenXRActionSet& actionSet, const XrSystemProperties& properties, OpenXRHandFlags handeness, int index) { OpenXRInputSourcePtr input(new OpenXRInputSource(instance, session, actionSet, properties, handeness, index)); @@ -31,6 +38,7 @@ OpenXRInputSource::OpenXRInputSource(XrInstance instance, XrSession session, Ope , mIndex(index) { elbow = ElbowModel::Create(); + mClickThreshold = kControllerClickThreshold; } OpenXRInputSource::~OpenXRInputSource() @@ -267,7 +275,7 @@ std::optional OpenXRInputSource::GetButton queryActionState(button.flags & OpenXRButtonFlags::Value, actions.value, result.value, result.clicked ? 1.0 : 0.0); queryActionState(button.flags & OpenXRButtonFlags::Ready, actions.ready, result.ready, true); - if (!clickedHasValue && result.value > kClickThreshold) { + if (!clickedHasValue && result.value > mClickThreshold) { result.clicked = true; } @@ -815,6 +823,7 @@ void OpenXRInputSource::Update(const XrFrameState& frameState, XrSpace localSpac std::vector jointRadii; PopulateHandJointLocations(renderMode, jointTransforms, jointRadii); if (!mIsHandInteractionSupported) { + mClickThreshold = kPinchThreshold; EmulateControllerFromHand(renderMode, frameState.predictedDisplayTime, head, jointTransforms[HAND_JOINT_FOR_AIM], pointerMode, usingEyeTracking, eyeTrackingTransform, delegate); delegate.SetHandJointLocations(mIndex, std::move(jointTransforms), std::move(jointRadii)); return; @@ -831,6 +840,7 @@ void OpenXRInputSource::Update(const XrFrameState& frameState, XrSpace localSpac bool usingTrackedPointer = pointerMode == DeviceDelegate::PointerMode::TRACKED_POINTER; bool hasAim = isPoseActive && (poseLocation.locationFlags & XR_SPACE_LOCATION_ORIENTATION_VALID_BIT); + mClickThreshold = mUsingHandInteractionProfile ? kPinchThreshold : kControllerClickThreshold; // When using hand interaction profiles we still get valid aim even if the hand is facing // the head like when performing system gestures. In that case we don't want to show the beam. @@ -937,7 +947,7 @@ void OpenXRInputSource::Update(const XrFrameState& frameState, XrSpace localSpac auto immersiveButton = GetImmersiveButton(button); if (isHandActionEnabled && button.type == OpenXRButtonType::Trigger) { - delegate.SetButtonState(mIndex, ControllerDelegate::BUTTON_APP, -1, state->value >= kClickThreshold, + delegate.SetButtonState(mIndex, ControllerDelegate::BUTTON_APP, -1, state->value >= mClickThreshold, state->value > 0, 1.0); } else { delegate.SetButtonState(mIndex, browserButton, immersiveButton.has_value() ? immersiveButton.value() : -1, diff --git a/app/src/openxr/cpp/OpenXRInputSource.h b/app/src/openxr/cpp/OpenXRInputSource.h index f65b4d5c7b1..52ed8fd69a4 100644 --- a/app/src/openxr/cpp/OpenXRInputSource.h +++ b/app/src/openxr/cpp/OpenXRInputSource.h @@ -88,6 +88,7 @@ class OpenXRInputSource { vrb::Vector mControllerPositionOnScrollStart; vrb::Matrix mEyeGazeTransformOnPinchStart; XrTime mEyeTrackingPinchStartTime { 0 }; + float mClickThreshold; struct HandMeshMSFT { XrSpace space = XR_NULL_HANDLE; @@ -122,6 +123,7 @@ class OpenXRInputSource { bool HasPhysicalControllersAvailable() const; void SetHandMeshBufferSizes(const uint32_t indexCount, const uint32_t vertexCount); HandMeshBufferPtr GetNextHandMeshBuffer(); + float GetSelectThreshold() const { return mClickThreshold; } }; } // namespace crow