Compare commits

...
Author SHA1 Message Date
enricobuehler 8abdd74a62 fix(client/desktop): the Deck keeps its trackpad, and a pad stops buzzing at exit
apple / swift (pull_request) Successful in 1m23s
apple / screenshots (pull_request) Skipped
ci / rust-arm64 (pull_request) Successful in 1m55s
ci / web (pull_request) Successful in 1m14s
ci / docs-site (pull_request) Successful in 1m15s
android / android (pull_request) Successful in 8m52s
ci / rust (pull_request) Successful in 12m50s
windows / build (aarch64-pc-windows-msvc) (pull_request) Successful in 1m7s
windows / build (x86_64-pc-windows-msvc) (pull_request) Successful in 2m2s
Three faults in the desktop session's gamepad path.

The Steam Deck lost its built-in trackpad-mouse at the start of every session.
SDL's Valve HIDAPI driver clears the pad's digital mappings during
*enumeration*, which is part of bringing the gamepad subsystem up — so holding
the drivers off from inside GamepadService::pumped could never work: receiving
a GamepadSubsystem means the enumeration has already happened. The hint set
there detached a driver that had already done the damage, and lizard mode only
came back seconds later when the firmware watchdog restored it. The presenter
now disables them with its other pre-SDL_Init hints. The threaded worker always
had this right; only the caller-pumped path was wrong, and it could not fix
itself, hence a separate entry point its callers can place correctly.

Player LEDs did nothing at all on any pad that is not a DualSense. The match
arm handled the DualSense raw-effects path and let everything else fall through
a bare `_`, though SDL exposes set_player_index and owns the per-device
pattern. The wire carries a positional bitmask rather than an index, and the
bridge is the popcount: every convention that reaches this wire spells "player
N" as N lit LEDs — the DualSense patterns 0x04/0x0A/0x15/0x1B/0x1F and the
Switch/XInput run 0x01/0x03/0x07/0x0F alike — so counting them works for both,
where reading a bit position would only ever suit one. No lit LED means no
player, not player 0. The remaining unhandled variants are now named rather
than swept up by `_`, so a new one cannot join them silently.

A forwarded pad could be left buzzing when the session ended. detach() only
posts Ctl::Detach; the close that flushes the pad, tells the host to remove it
and explicitly zeroes the motors runs when the pump next drains that message.
Single mode broke out of the loop immediately after detaching and Event::Quit
never detached at all, so both skipped it entirely. The teardown now sits where
every exit converges instead of on the individual breaks. That still leaves the
several paths that leave by `?` on a fatal overlay or present error, so the
pump also silences its slots on Drop — the explicit call stays, because a pad
should go quiet before a long teardown rather than after it. Drop closes the
slots directly rather than draining the queue that would have done it: same
physical outcome, and it touches no lock, where draining reaches an unwrap on a
Mutex that would abort the process if it panicked mid-unwind.
2026-08-04 19:10:45 +02:00
2 changed files with 144 additions and 4 deletions
+130 -4
View File
@@ -285,6 +285,21 @@ fn set_valve_hidapi(enabled: bool) {
sdl3::hint::set("SDL_JOYSTICK_HIDAPI_STEAM", v);
}
/// Disable the Valve HIDAPI drivers **before SDL exists** — call this alongside the other
/// pre-`SDL_Init` hints, not after a subsystem is up.
///
/// The damage these drivers do happens at *enumeration*, which is part of initialising the
/// joystick/gamepad subsystem. Setting the hint afterwards does detach the driver, but only after
/// it has already sent the Deck its `ID_CLEAR_DIGITAL_MAPPINGS` + `TRACKPAD_NONE` — so the
/// built-in trackpad-mouse dies system-wide and stays dead until the firmware watchdog restores
/// lizard mode seconds later. The threaded worker ([`run`]) has always done this in the right
/// order; the caller-pumped path could not, because by the time it receives a
/// [`sdl3::GamepadSubsystem`] the enumeration has already happened. Hence a separate entry point
/// its callers can put in the right place.
pub fn preinit_disable_valve_hidapi() {
set_valve_hidapi(false);
}
/// Map the SDL-reported controller type to the virtual pad we'd ask the host to create.
fn pref_for_type(t: sdl3::gamepad::GamepadType) -> GamepadPref {
use sdl3::gamepad::GamepadType as T;
@@ -393,9 +408,12 @@ impl GamepadService {
/// and calls [`GamepadPump::tick`] once per loop iteration (the threaded worker's
/// per-wakeup work: ctl drain, chord-hold check, menu repeat, feedback).
///
/// Like the threaded worker, this disables the Valve HIDAPI drivers up front (their
/// mere enumeration kills the Deck's trackpad-mouse system-wide); they are enabled
/// for the duration of an attached session only.
/// The Valve HIDAPI drivers are held off here too, but this is **too late to be the only
/// place it happens**: the `subsystem` argument means enumeration is already done, and that
/// is when the Deck driver kills the trackpad-mouse. The caller must also call
/// [`preinit_disable_valve_hidapi`] with its other pre-`SDL_Init` hints. This call still
/// earns its place — it re-asserts "off" for a process that ran a session earlier — but on
/// its own it only detaches a driver that has already done the damage.
pub fn pumped(subsystem: sdl3::GamepadSubsystem) -> (GamepadService, GamepadPump) {
set_valve_hidapi(false);
let pads = Arc::new(Mutex::new(Vec::new()));
@@ -556,6 +574,38 @@ impl GamepadPump {
self.worker.menu_poll();
self.worker.render_feedback();
}
/// Close every forwarded slot — flush its held wire state, tell the host to remove the pad,
/// and physically silence it. Call once on the way out of the caller's event loop.
///
/// [`GamepadService::detach`] only *posts* `Ctl::Detach`; the close — the flush, the host-side
/// `GamepadRemove`, and the explicit `set_rumble(0, 0)` backstop in `close_slot_at` — happens
/// when the pump next drains it. An exit path that detached and then left the loop without
/// another [`tick`](Self::tick) therefore skipped all of it, and nothing else would: the slots
/// hold no `Drop` that silences them. A pad left mid-buzz stayed buzzing.
///
/// This closes the slots directly rather than draining the queued `Ctl::Detach` that would
/// have done it. Same physical outcome by a shorter path, and deliberately so: this also runs
/// from `Drop`, and `drain_ctl` reaches `Mutex::lock().unwrap()`, which on a poisoned lock
/// would panic — during an unwind that aborts the process. Closing a slot touches no lock.
///
/// Idempotent, and safe with nothing attached.
pub fn shutdown(&mut self) {
self.worker.close_all_slots();
}
}
/// The silence backstop of last resort. A caller's loop can also leave by `?` on a fatal overlay
/// or present error — several paths do — and those would skip an explicit
/// [`shutdown`](GamepadPump::shutdown) entirely, leaving a forwarded pad buzzing on the way out.
///
/// Callers should still call `shutdown` at their normal exit rather than lean on this: the pad
/// wants to go quiet *before* a long teardown (session join, `vkDeviceWaitIdle`), not after it.
/// Doing both is free — `shutdown` is idempotent.
impl Drop for GamepadPump {
fn drop(&mut self) {
self.shutdown();
}
}
/// The lowest wire pad index (0..[`MAX_PADS`](punktfunk_core::input::MAX_PADS)) not already held
@@ -1626,6 +1676,11 @@ impl Worker {
HidOutput::PlayerLeds { bits, .. } if is_ds => {
let _ = slot.pad.send_effect(&Ds5Feedback::player_packet(bits));
}
// Every other pad with player LEDs gets them through SDL, which owns the
// per-device pattern. This used to fall through and do nothing at all.
HidOutput::PlayerLeds { bits, .. } => {
let _ = set_player_leds(&slot.pad, bits);
}
HidOutput::Trigger {
which, ref effect, ..
} if is_ds => {
@@ -1633,12 +1688,43 @@ impl Worker {
.pad
.send_effect(&Ds5Feedback::trigger_packet(which, effect));
}
_ => {}
// Deliberately unhandled, listed rather than left to a bare `_` so a new
// variant cannot join them silently: adaptive triggers exist only on a
// DualSense, and the trackpad-haptic / raw-passthrough planes are DS-specific
// and carried by `send_effect` above when the pad is one.
HidOutput::Trigger { .. }
| HidOutput::TrackpadHaptic { .. }
| HidOutput::HidRaw { .. } => {}
}
}
}
}
/// The SDL player index for the wire's positional player-LED `bits`, or `None` for "no player".
///
/// The wire carries a bitmask — one bit per LED, low 5 — while SDL wants a player *index* and owns
/// the per-device pattern. The count bridges them: every convention that reaches this wire spells
/// "player N" as N lit LEDs, both the DualSense patterns (`0x04`, `0x0A`, `0x15`, `0x1B`, `0x1F`)
/// and the Switch/XInput run of low bits (`0x01`, `0x03`, `0x07`, `0x0F`). SDL's index is 0-based,
/// so player 1 is index 0; no lit LED means *no* player rather than player 0.
///
/// Split out from [`set_player_leds`] so the mapping is testable — an `sdl3::Gamepad` needs a real
/// device, so nothing that takes one can be.
fn player_index_from_bits(bits: u8) -> Option<u16> {
match (bits & 0x1F).count_ones() {
0 => None,
n => Some((n - 1) as u16),
}
}
/// Drive a non-DualSense pad's player LEDs from the wire's positional `bits`.
fn set_player_leds(pad: &sdl3::gamepad::Gamepad, bits: u8) -> Result<(), sdl3::Error> {
match player_index_from_bits(bits) {
None => pad.unset_player_index(),
Some(i) => pad.set_player_index(i),
}
}
/// The wire pad index a [`HidOutput`] is addressed to (every variant carries `pad`).
fn hidout_pad(h: &HidOutput) -> u8 {
match h {
@@ -2008,3 +2094,43 @@ mod slot_tests {
);
}
}
#[cfg(test)]
mod player_led_tests {
use super::*;
/// Both conventions that reach this wire spell "player N" as N lit LEDs, so the count is the
/// player number regardless of WHICH bits a given pad lights. Pinned because the mapping is
/// otherwise only obvious once you have seen both patterns side by side.
#[test]
fn player_index_counts_lit_leds_for_both_conventions() {
// DualSense / hid-playstation patterns — non-contiguous, symmetric about the centre LED.
assert_eq!(player_index_from_bits(0x04), Some(0)); // player 1
assert_eq!(player_index_from_bits(0x0A), Some(1)); // player 2
assert_eq!(player_index_from_bits(0x15), Some(2)); // player 3
assert_eq!(player_index_from_bits(0x1B), Some(3)); // player 4
assert_eq!(player_index_from_bits(0x1F), Some(4)); // player 5
// Switch/XInput style — a contiguous run of low bits, the same count each time.
assert_eq!(player_index_from_bits(0x01), Some(0));
assert_eq!(player_index_from_bits(0x03), Some(1));
assert_eq!(player_index_from_bits(0x07), Some(2));
assert_eq!(player_index_from_bits(0x0F), Some(3));
}
/// No lit LED is "no player", NOT player 0 — the difference between LEDs off and player 1 lit.
#[test]
fn no_lit_led_is_no_player() {
assert_eq!(player_index_from_bits(0x00), None);
// Only the low 5 bits are player LEDs; junk above them must not invent a player.
assert_eq!(player_index_from_bits(0xE0), None);
}
/// The mask is applied before counting, so out-of-range bits cannot inflate the index past
/// the 5 real LEDs.
#[test]
fn high_bits_are_masked_off_before_counting() {
assert_eq!(player_index_from_bits(0xFF), Some(4)); // 0x1F worth of LEDs, not 8
assert_eq!(player_index_from_bits(0xE4), Some(0)); // 0x04 with junk on top
}
}
+14
View File
@@ -466,6 +466,13 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
#[cfg(windows)]
crate::win32::set_app_user_model_id();
sdl3::hint::set("SDL_JOYSTICK_THREAD", "1");
// Hold SDL's Valve HIDAPI drivers off BEFORE SDL_Init: the Deck driver clears the pad's
// digital mappings at *enumeration*, which is part of bringing the gamepad subsystem up, so a
// hint set after `sdl.gamepad()` — where this used to live, inside GamepadService::pumped —
// only detached a driver that had already killed the built-in trackpad-mouse system-wide. The
// symptom was the Deck losing its trackpad cursor at the start of every session until the
// firmware watchdog restored lizard mode. They are still enabled for an attached session.
pf_client_core::gamepad::preinit_disable_valve_hidapi();
// A touchscreen (the Deck's glass) is forwarded as REAL touch passthrough below — so
// suppress SDL's default synthesis of mouse events from touch. Left on, every touch
// ALSO warps a synthetic mouse to the touch point, which under the stream's relative
@@ -1895,6 +1902,13 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
}
};
// Every exit from the loop above converges here, which is why the gamepad teardown belongs
// here and not on the individual `break`s. `gamepad.detach()` only queues the detach; the
// close — flush, host-side GamepadRemove, and the explicit rumble-stop backstop — runs when
// the pump drains it. Single mode broke out of the loop immediately after detaching and
// Event::Quit never detached at all, so both left forwarded pads unflushed and, if the game
// was rumbling at the time, still buzzing.
pump.shutdown();
// Join the pump BEFORE the device-wide idle: its decode submissions on the shared
// device would race vkDeviceWaitIdle otherwise.
if let Some(st) = stream.take() {