Skip to content

Commit

Permalink
Reduce sensitivity required by controller trigger
Browse files Browse the repository at this point in the history
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
  • Loading branch information
svillar committed Jan 14, 2025
1 parent 38044a8 commit fd37627
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 13 deletions.
4 changes: 1 addition & 3 deletions app/src/main/cpp/BrowserWorld.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/cpp/DeviceDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand Down
4 changes: 2 additions & 2 deletions app/src/openxr/cpp/DeviceDelegateOpenXR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/src/openxr/cpp/DeviceDelegateOpenXR.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions app/src/openxr/cpp/OpenXRHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions app/src/openxr/cpp/OpenXRInput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions app/src/openxr/cpp/OpenXRInput.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 12 additions & 2 deletions app/src/openxr/cpp/OpenXRInputSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -31,6 +38,7 @@ OpenXRInputSource::OpenXRInputSource(XrInstance instance, XrSession session, Ope
, mIndex(index)
{
elbow = ElbowModel::Create();
mClickThreshold = kControllerClickThreshold;
}

OpenXRInputSource::~OpenXRInputSource()
Expand Down Expand Up @@ -267,7 +275,7 @@ std::optional<OpenXRInputSource::OpenXRButtonState> 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;
}

Expand Down Expand Up @@ -815,6 +823,7 @@ void OpenXRInputSource::Update(const XrFrameState& frameState, XrSpace localSpac
std::vector<float> 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;
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions app/src/openxr/cpp/OpenXRInputSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class OpenXRInputSource {
vrb::Vector mControllerPositionOnScrollStart;
vrb::Matrix mEyeGazeTransformOnPinchStart;
XrTime mEyeTrackingPinchStartTime { 0 };
float mClickThreshold;

struct HandMeshMSFT {
XrSpace space = XR_NULL_HANDLE;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit fd37627

Please sign in to comment.