From 3865120f7726fcf301a7a4d720ea85a66c60ee97 Mon Sep 17 00:00:00 2001 From: Dylan Deverill Date: Sun, 6 Sep 2026 13:48:49 -0700 Subject: [PATCH] Decide a trackpad tap on the lift instead of after a fixed wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The confirmation window added in the previous commit does not work, and the measurement that sized it says why. Over 148 presses the finger is down for a median of 59 frames before the click and sometimes thousands: a thumb is planted and then pressed. Suppressing a tap on every press would need a wait longer than the longest of those, so any window short enough for a tap to feel responsive still fires before nearly every press. Twelve frames would have caught well under a fifth of them. A tap is knowable exactly, just not at the moment of contact. It is a contact that ended without a click, was brief, and did not travel — all three known at the lift. So that is where it is decided now, with no window to tune and no latency to trade off. It reports as a short pulse rather than a hold, because the event is over by the time it is known to have happened; the row is labelled Trackpad Tap, which is what it now is. The same session validates the click haptic fix from two commits ago: the shortest gap held the click bit low for 7 frames against a 4-frame release window, so all 148 presses fire. The probe had been replaying the old area-based latch and reporting 132 of 148, which described code that no longer exists — its replay now mirrors the current constants, and its touch-to-click section explains the lift rather than recommending a window that cannot work. Co-Authored-By: Claude Opus 5 --- src/app/ControllerManager.cpp | 112 ++++++++++++++++++++------------ src/app/TrackpadConfig.h | 12 ++-- src/app/VirtualController.cpp | 8 +-- src/probe/TrackpadZoneProbe.cpp | 42 ++++++------ 4 files changed, 102 insertions(+), 72 deletions(-) diff --git a/src/app/ControllerManager.cpp b/src/app/ControllerManager.cpp index c140fd5..aa53f3c 100644 --- a/src/app/ControllerManager.cpp +++ b/src/app/ControllerManager.cpp @@ -79,31 +79,63 @@ struct ControllerManager::Slot { uint8_t prevLeftDirs = DirNone; uint8_t prevRightDirs = DirNone; - // Touch confirmation, and the previous answer for the same reason as the + // Tap detection, and the previous answer for the same reason as the // directions above. // // The report's touch bit means "pad active", which a press sets on its way // down — the finger lands, force builds, and only then does the click bit - // follow. So a touch binding dispatched off the raw bit fired on every + // follow. A binding dispatched off the raw bit therefore fired on every // press, before the press itself, in every mode. Touch is not an // independent signal: a click always implies one. // - // What makes touch its own thing is a contact that does NOT become a - // press. So a finger has to be down this long without clicking before - // touch is reported at all, and a contact that clicks first reports none — - // nor any later in that same contact, since it has stopped being a tap. + // Waiting a fixed time to see whether a click follows does not rescue it. + // Measured with TrackpadZoneProbe over 148 presses, the finger is down for + // a median of 59 frames before the click and sometimes thousands — a + // thumb is planted and then pressed. Any window short enough for a tap to + // feel responsive fires before nearly every press. // - // Nothing distinguishes the two at the moment of contact: contact area - // does not separate them (a thumb loading for a press sits around 2500, - // above a light rest and overlapping a grinding one), and the click that - // would settle it has not happened yet. Only waiting does. - static constexpr int kTouchConfirmFrames = 12; // ~48ms at 250Hz - int touchFramesLeft = 0; - int touchFramesRight = 0; - bool touchClickedLeft = false; - bool touchClickedRight = false; - bool prevLeftTouch = false; - bool prevRightTouch = false; + // So a tap is decided on the lift instead, where it is not a guess at all: + // a contact that ended without a click, was brief, and did not travel. + // That is what a tap is, and all three are known by then. It reports as a + // short pulse rather than a hold, because the event being reported is over + // by the time it is known to have happened. + static constexpr int kTapMaxFrames = 50; // ~200ms — a tap is quick + static constexpr int kTapMaxTravel = 3000; // and lands in one place + static constexpr int kTapPulseFrames = 12; // ~48ms, long enough to register + struct TapState { + int frames = 0; // frames the finger has been down + bool sawClick = false; + int16_t startX = 0; + int16_t startY = 0; + int travel = 0; // furthest it has strayed from where it landed + int pulse = 0; // frames left in the pulse a tap fires + + // Returns whether the tap binding should be held this frame. + bool Update(bool touching, bool clicked, int16_t x, int16_t y) { + if (!touching) { + if (frames > 0 && !sawClick + && frames <= kTapMaxFrames && travel <= kTapMaxTravel) + pulse = kTapPulseFrames; + frames = 0; + sawClick = false; + travel = 0; + } else { + if (frames == 0) { startX = x; startY = y; travel = 0; } + if (clicked) sawClick = true; + ++frames; + const int dx = x - startX, dy = y - startY; + const int d = static_cast(std::sqrt( + static_cast(dx) * dx + static_cast(dy) * dy)); + if (d > travel) travel = d; + } + if (pulse > 0) { --pulse; return true; } + return false; + } + }; + TapState leftTap; + TapState rightTap; + bool prevLeftTap = false; + bool prevRightTap = false; // Auto-repeat for held key bindings. When the next repeat is due, and the // gap to use after that — both captured at press time from the user's @@ -1026,29 +1058,28 @@ void ControllerManager::ReadLoop(Slot* slot) { // worse, a direction still held for a frame after release. slot->leftPad.Update(buf, n); slot->rightPad.Update(buf, n); - // Is this contact a tap, or the beginning of a press? See - // kTouchConfirmFrames — nothing but time tells the two apart. - auto confirmTouch = [](bool touching, bool clicked, - int& framesDown, bool& sawClick) -> bool { - if (!touching) { framesDown = 0; sawClick = false; return false; } - if (clicked) sawClick = true; - ++framesDown; - return !sawClick && framesDown >= Slot::kTouchConfirmFrames; - }; + // Was that contact a tap? Only the lift can say — see Slot::TapState. const uint8_t tb2 = n > 4 ? buf[4] : 0; const uint8_t tb3 = n > 5 ? buf[5] : 0; + int16_t tlx = 0, tly = 0, trx = 0, try_ = 0; + if (n >= 28) { + memcpy(&tlx, buf + 18, 2); + memcpy(&tly, buf + 20, 2); + memcpy(&trx, buf + 24, 2); + memcpy(&try_, buf + 26, 2); + } const PadDigital resolved{ slot->leftPad.Directions(), slot->rightPad.Directions(), slot->leftPad.ClickInCentre(), slot->rightPad.ClickInCentre(), - confirmTouch((tb3 & SteamController::BTN_TP_LT) != 0, - (tb3 & SteamController::BTN_TP_LT_CLICK) != 0, - slot->touchFramesLeft, slot->touchClickedLeft), - confirmTouch((tb2 & SteamController::BTN_TP_RT) != 0, - (tb2 & SteamController::BTN_TP_RT_CLICK) != 0, - slot->touchFramesRight, slot->touchClickedRight), + slot->leftTap.Update((tb3 & SteamController::BTN_TP_LT) != 0, + (tb3 & SteamController::BTN_TP_LT_CLICK) != 0, + tlx, tly), + slot->rightTap.Update((tb2 & SteamController::BTN_TP_RT) != 0, + (tb2 & SteamController::BTN_TP_RT_CLICK) != 0, + trx, try_), }; if (slot->vc) slot->vc->Update(buf, n, m_profile, resolved); @@ -1316,14 +1347,13 @@ void ControllerManager::ReadLoop(Slot* slot) { { n>3 && (buf[3]&SteamController::BTN_R5)!=0, (prevBuf[3]&SteamController::BTN_R5)!=0, m_profile.back.r5 }, { n>5 && (buf[5]&SteamController::BTN_TP_LT_CLICK)!=0 && leftClickInCentre, (prevBuf[5]&SteamController::BTN_TP_LT_CLICK)!=0 && leftClickInCentre, leftPadClick }, { n>4 && (buf[4]&SteamController::BTN_TP_RT_CLICK)!=0 && rightClickInCentre, (prevBuf[4]&SteamController::BTN_TP_RT_CLICK)!=0 && rightClickInCentre, rightPadClick }, - // Touch is not zoned — it cannot collide with a direction, + // A tap is not zoned — it cannot collide with a direction, // which comes from the click, and a thumb crosses zones - // constantly while sliding. It is confirmed rather than read - // from the report, though: the raw bit is also set by a press - // on its way down, so the previous answer is carried on the - // slot the same way the directions are. - { resolved.leftTouch, slot->prevLeftTouch, lPad.EffectiveTouch() }, - { resolved.rightTouch, slot->prevRightTouch, rPad.EffectiveTouch() }, + // constantly while sliding. It is decided on the lift rather + // than read from the report, though, so like the directions + // its previous answer is carried on the slot. + { resolved.leftTap, slot->prevLeftTap, lPad.EffectiveTouch() }, + { resolved.rightTap, slot->prevRightTap, rPad.EffectiveTouch() }, // Directions are resolved rather than read, so their previous // state is the one carried on the slot, not a bit in prevBuf. { (lDirs&DirUp)!=0, (lPrevDirs&DirUp)!=0, lPad.EffectiveDirection(DirUp) }, @@ -1394,8 +1424,8 @@ void ControllerManager::ReadLoop(Slot* slot) { // it on any frame would stand a direction up or down for good. slot->prevLeftDirs = resolved.leftDirs; slot->prevRightDirs = resolved.rightDirs; - slot->prevLeftTouch = resolved.leftTouch; - slot->prevRightTouch = resolved.rightTouch; + slot->prevLeftTap = resolved.leftTap; + slot->prevRightTap = resolved.rightTap; memcpy(prevBuf, buf, n); hasPrev = true; diff --git a/src/app/TrackpadConfig.h b/src/app/TrackpadConfig.h index 5c2201e..00edf4c 100644 --- a/src/app/TrackpadConfig.h +++ b/src/app/TrackpadConfig.h @@ -122,11 +122,13 @@ struct PadDigital { // click binding on this without asking what mode the pad is in. bool leftClickInCentre = true; bool rightClickInCentre = true; - // A finger resting on the pad without pressing it. Not the report's touch - // bit, which a press also sets on its way down — see kTouchConfirmFrames - // in ControllerManager for why a touch binding cannot use the raw bit. - bool leftTouch = false; - bool rightTouch = false; + // A tap just happened: a contact that ended without ever clicking. Held + // true for a few frames so it dispatches as an ordinary press and release + // — the event is over by the time it is known to have happened. Not the + // report's touch bit, which a press also sets on its way down; see + // Slot::TapState in ControllerManager for why that bit cannot be bound. + bool leftTap = false; + bool rightTap = false; }; // Where a click landed on a pad, which is what keeps a directional pad's three diff --git a/src/app/VirtualController.cpp b/src/app/VirtualController.cpp index 187d6f3..9523b1f 100644 --- a/src/app/VirtualController.cpp +++ b/src/app/VirtualController.cpp @@ -298,21 +298,21 @@ void VirtualController::Update(const uint8_t* buf, size_t n, struct PadState { const TrackpadSettings& pad; - bool touching; // already narrowed to a tap, not a press + bool tapped; // a contact that ended without ever clicking bool clicked; // already narrowed to a centre press uint8_t dirs; }; const PadState pads[] = { - { profile.leftPad, resolved.leftTouch, + { profile.leftPad, resolved.leftTap, (b3 & SteamController::BTN_TP_LT_CLICK) != 0 && resolved.leftClickInCentre, resolved.leftDirs }, - { profile.rightPad, resolved.rightTouch, + { profile.rightPad, resolved.rightTap, (b2 & SteamController::BTN_TP_RT_CLICK) != 0 && resolved.rightClickInCentre, resolved.rightDirs }, }; for (const PadState& p : pads) { - if (p.touching) apply(p.pad.EffectiveTouch()); + if (p.tapped) apply(p.pad.EffectiveTouch()); if (p.clicked) apply(p.pad.EffectiveClick()); for (uint8_t d : { DirUp, DirDown, DirLeft, DirRight }) if (p.dirs & d) apply(p.pad.EffectiveDirection(static_cast(d))); diff --git a/src/probe/TrackpadZoneProbe.cpp b/src/probe/TrackpadZoneProbe.cpp index b227464..214a028 100644 --- a/src/probe/TrackpadZoneProbe.cpp +++ b/src/probe/TrackpadZoneProbe.cpp @@ -317,9 +317,8 @@ void PrintSummary() { // The live constants from ControllerManager's click haptic latch, mirrored so // the session can be replayed through the real state machine rather than // through a description of it. Keep in step with Slot. -constexpr int kPressConfirmFrames = 2; -constexpr float kReleaseIdleArea = 1000.0f; -constexpr int kAreaLowConfirmFrames = 8; +constexpr int kPressConfirmFrames = 2; +constexpr int kReleaseConfirmFrames = 4; struct Tap { uint16_t areaAtPress = 0; @@ -358,7 +357,7 @@ struct TapTracker { enum class State { WaitingForPress, WaitingForRelease }; State state = State::WaitingForPress; int clickTrue = 0; - int areaLow = 0; + int clickLow = 0; int hapticsFired = 0; // presses and releases both pulse // Frames the finger has been down for, or -1 when it is not down. Starts @@ -403,22 +402,16 @@ struct TapTracker { // --- what the latch would have done --- clickTrue = click ? clickTrue + 1 : 0; + clickLow = click ? 0 : clickLow + 1; if (state == State::WaitingForPress) { if (clickTrue >= kPressConfirmFrames) { - state = State::WaitingForRelease; - areaLow = 0; + state = State::WaitingForRelease; ++hapticsFired; if (!taps.empty()) taps.back().haptic = true; } - } else { - if (!click && static_cast(area) <= kReleaseIdleArea) { - if (++areaLow >= kAreaLowConfirmFrames) { - state = State::WaitingForPress; - ++hapticsFired; - } - } else { - areaLow = 0; - } + } else if (clickLow >= kReleaseConfirmFrames) { + state = State::WaitingForPress; + ++hapticsFired; } prevClick = click; @@ -490,14 +483,17 @@ void PrintTapSummary() { } if (!gapAreas.empty()) { - printf("\nCONTACT AREA BETWEEN PRESSES (the latch re-arms at <= %.0f)\n", - kReleaseIdleArea); + // Kept because it is the evidence for why the latch stopped consulting + // it: the release used to wait for this to fall to 1000, which a thumb + // that stays on the pad never does. + printf("\nCONTACT AREA BETWEEN PRESSES (what the latch used to wait on)\n"); printf(" min %.0f p05 %.0f p50 %.0f max %.0f\n", Percentile(gapAreas, 0.0), Percentile(gapAreas, 0.05), Percentile(gapAreas, 0.50), Percentile(gapAreas, 1.0)); int under = 0; - for (double a : gapAreas) if (a <= kReleaseIdleArea) ++under; - printf(" %d of %zu gaps reach it\n", under, gapAreas.size()); + for (double a : gapAreas) if (a <= 1000.0) ++under; + printf(" %d of %zu gaps would have reached the old 1000 threshold\n", + under, gapAreas.size()); } // The other window that matters: how long a press spends touching before @@ -509,9 +505,11 @@ void PrintTapSummary() { printf(" min %.0f p05 %.0f p50 %.0f max %.0f (n=%zu)\n", Percentile(toClick, 0.0), Percentile(toClick, 0.05), Percentile(toClick, 0.50), Percentile(toClick, 1.0), toClick.size()); - printf(" kTouchConfirmFrames must exceed the shortest of these (%.0f) or a\n" - " quick press still fires the tap binding on its way down. It is\n" - " currently 12.\n", Percentile(toClick, 0.0)); + printf(" This is why a tap is decided on the lift rather than after a\n" + " fixed wait. Suppressing a tap on every press would need a wait\n" + " longer than the LONGEST of these (%.0f frames), which is no wait\n" + " at all — a thumb gets planted and then pressed.\n", + Percentile(toClick, 1.0)); } if (!lowRuns.empty()) {