Skip to content

Commit cb981dc

Browse files
Beetixclaude
authored andcommitted
fix(linux): address CodeRabbit re-review of the click-capture path
- Reconnect on the same node path. `read_device` now removes its path from the shared `opened` set on every exit, so a device that unplugs and returns on the same `/dev/input/eventN` is re-adopted by the next scan instead of skipped for the rest of the recording. The set is an Arc<Mutex<HashSet>>; check-and-insert is one locked step so two scans can't both adopt a path. - Drop pre-stream presses by time, not just a flag. `PointerButton` and `StreamEvent::State` arrive on different channels, so a "Share"-button press read before streaming could be dequeued after a bare `streaming` flag flipped true. Replace the flag with `streaming_since: Option<u64>` and emit a press only when its read time is >= that instant — the older press is dropped regardless of delivery order. - Stop click capture on disconnect. `streaming_since` is cleared on the `unconnected` transition, so presses after the stream drops no longer emit click samples against a stale cursor position. - Update the installation capability table: Linux click effects now work on Wayland with the `input` group, matching the "Mouse clicks on Wayland" section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4ce9237 commit cb981dc

3 files changed

Lines changed: 61 additions & 36 deletions

File tree

electron/native/pipewire-capture/src/input.rs

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use std::collections::HashSet;
2424
use std::io::ErrorKind;
2525
use std::path::PathBuf;
2626
use std::sync::mpsc::Sender;
27+
use std::sync::{Arc, Mutex};
2728
use std::thread;
2829
use std::time::Duration;
2930

@@ -79,13 +80,13 @@ pub fn spawn_readers(sender: &Sender<Message>) -> ClickCapture {
7980
if std::env::var_os(DISABLE_ENV).is_some() {
8081
return ClickCapture::Disabled;
8182
}
82-
// Paths already given a reader. Only ever grows, so a device is never
83-
// double-read; the one case it misses is a device that unplugs and returns
84-
// on the SAME node path — a replug usually lands on a fresh `eventNN`, which
85-
// is not in the set and so is picked up.
86-
let mut opened: HashSet<PathBuf> = HashSet::new();
87-
scan_once(sender, &mut opened);
88-
let result = if opened.is_empty() {
83+
// Paths with a LIVE reader. Shared with the reader threads: each removes its
84+
// own path when it exits, so a device that unplugs and reconnects on the SAME
85+
// node path is adopted again by the next scan. A set that only grew skipped
86+
// such a replug for the rest of the recording.
87+
let opened: Arc<Mutex<HashSet<PathBuf>>> = Arc::new(Mutex::new(HashSet::new()));
88+
scan_once(sender, &opened);
89+
let result = if opened.lock().unwrap().is_empty() {
8990
ClickCapture::NoDevice
9091
} else {
9192
ClickCapture::Active
@@ -94,23 +95,28 @@ pub fn spawn_readers(sender: &Sender<Message>) -> ClickCapture {
9495
// so a daemon thread re-scans and starts readers for nodes it has not seen.
9596
// Detached, like the reader threads — it ends when the process does.
9697
let watch_sender = sender.clone();
98+
let watch_opened = Arc::clone(&opened);
9799
thread::spawn(move || loop {
98100
thread::sleep(RESCAN_INTERVAL);
99-
scan_once(&watch_sender, &mut opened);
101+
scan_once(&watch_sender, &watch_opened);
100102
});
101103
result
102104
}
103105

104-
/// Spawns a reader for every `BTN_LEFT` device not already in `opened`, recording
105-
/// each newly opened node's path. Shared by the initial scan and the watcher.
106-
fn scan_once(sender: &Sender<Message>, opened: &mut HashSet<PathBuf>) {
106+
/// Spawns a reader for every `BTN_LEFT` device not already being read, recording
107+
/// each newly opened node's path. Shared by the initial scan and the watcher; the
108+
/// check-and-insert is one locked step so two scans cannot both adopt one path.
109+
fn scan_once(sender: &Sender<Message>, opened: &Arc<Mutex<HashSet<PathBuf>>>) {
107110
for (path, device) in evdev::enumerate() {
108-
if opened.contains(&path) || !device_reports_left_button(&device) {
111+
if !device_reports_left_button(&device) {
109112
continue;
110113
}
111-
opened.insert(path);
114+
if !opened.lock().unwrap().insert(path.clone()) {
115+
continue; // already has a live reader
116+
}
112117
let forward = sender.clone();
113-
thread::spawn(move || read_device(device, forward));
118+
let owned = Arc::clone(opened);
119+
thread::spawn(move || read_device(device, path, forward, owned));
114120
}
115121
}
116122

@@ -132,7 +138,18 @@ fn device_reports_left_button(device: &Device) -> bool {
132138
/// next cursor sample. Returns when the device fails terminally (e.g. unplugged)
133139
/// or the loop's channel has closed, so the thread cannot outlive the recording
134140
/// it serves. A transient `EINTR` is retried, not mistaken for an unplug.
135-
fn read_device(mut device: Device, sender: Sender<Message>) {
141+
///
142+
/// On EVERY exit it drops `path` from `opened`, so a device reconnecting on the
143+
/// same node path is re-adopted by the next scan.
144+
fn read_device(
145+
mut device: Device,
146+
path: PathBuf,
147+
sender: Sender<Message>,
148+
opened: Arc<Mutex<HashSet<PathBuf>>>,
149+
) {
150+
let release = || {
151+
opened.lock().unwrap().remove(&path);
152+
};
136153
loop {
137154
let events = match device.fetch_events() {
138155
Ok(events) => events,
@@ -142,6 +159,7 @@ fn read_device(mut device: Device, sender: Sender<Message>) {
142159
// than ending silently, so click capture going quiet mid-recording
143160
// is answerable from the log; the watcher re-adopts it on a replug.
144161
Err(err) => {
162+
release();
145163
let _ = sender.send(Message::PointerDeviceLost(err.to_string()));
146164
return;
147165
}
@@ -150,6 +168,7 @@ fn read_device(mut device: Device, sender: Sender<Message>) {
150168
if is_left_button_press(event.event_type(), event.code(), event.value())
151169
&& sender.send(Message::PointerButton(timestamp_ms())).is_err()
152170
{
171+
release();
153172
return;
154173
}
155174
}

electron/native/pipewire-capture/src/main.rs

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -608,14 +608,17 @@ fn run<W: Write>(
608608
let mut cursor: Option<CursorState> = None;
609609
let mut known_assets: HashSet<String> = HashSet::new();
610610
let mut pending_asset: Option<CursorAsset> = None;
611-
// The pw_stream has reached `streaming`, i.e. mutter has actually started
612-
// handing us frames. Presses are ignored until then: before it, the only
613-
// thing on screen is the portal's own picker, and the click that dismisses
614-
// it (its "Share" button) would otherwise latch and ride out on the
615-
// recording's first sample as a phantom click at t≈0. mutter enables its
616-
// capture source on STREAMING, so this is the exact edge at which a press
617-
// starts landing on content the recording contains.
618-
let mut streaming = false;
611+
// Wall-clock ms at which the pw_stream last reached `streaming` (mutter began
612+
// handing us frames), or `None` when it is not streaming. A press counts only
613+
// if its own read time is at or after this: before streaming the only thing on
614+
// screen is the portal picker, and the click that dismisses it (its "Share"
615+
// button) would otherwise ride out on the first sample as a phantom click at
616+
// t≈0. Comparing timestamps rather than a bare flag closes two gaps: the press
617+
// and the stream-state arrive on DIFFERENT channels, so a pre-stream press can
618+
// be dequeued after the flag flips (it is still dropped, its time is older);
619+
// and clearing it on disconnect stops clicks emitting against a stale cursor
620+
// after capture has stopped.
621+
let mut streaming_since: Option<u64> = None;
619622
let mut reported_cursor_meta = false;
620623
// Allocated up front so the PipeWire callback has somewhere to put frames
621624
// from the very first buffer; `None` in cursor-only mode, which is also what
@@ -680,18 +683,13 @@ fn run<W: Write>(
680683
// stamped with the press time the reader captured: immediate so it is
681684
// not backdated to the next throttled sample, and one sample per press
682685
// so a rapid double-click reads as two clicks rather than collapsing
683-
// into one. Dropped before the stream is live (see `streaming`): a
684-
// press while the picker is still up is the click on its "Share"
685-
// button, not content.
686+
// into one. Counted only if the press happened at or after streaming
687+
// began (see `streaming_since`): a press while the picker is still up —
688+
// the click on its "Share" button — is older, so it is dropped even if
689+
// its message is delivered after the stream-state one.
686690
Ok(Message::PointerButton(press_ms)) => {
687-
if streaming {
688-
emit_sample(
689-
emitter,
690-
&cursor,
691-
size,
692-
&mut pending_asset,
693-
Some(press_ms),
694-
);
691+
if streaming_since.is_some_and(|since| press_ms >= since) {
692+
emit_sample(emitter, &cursor, size, &mut pending_asset, Some(press_ms));
695693
}
696694
}
697695

@@ -1116,8 +1114,16 @@ fn run<W: Write>(
11161114
});
11171115
// Once frames are flowing, presses land on recorded content; the
11181116
// picker (and the "Share" click that dismissed it) is behind us.
1117+
// Stamped so a press read before this instant is dropped by time,
1118+
// and cleared on disconnect so clicks stop emitting against a stale
1119+
// cursor after capture ends. Only set on the FIRST streaming edge so
1120+
// a transient renegotiation `paused`→`streaming` does not re-arm it.
11191121
if state == "streaming" {
1120-
streaming = true;
1122+
if streaming_since.is_none() {
1123+
streaming_since = Some(timestamp_ms());
1124+
}
1125+
} else if state == "unconnected" {
1126+
streaming_since = None;
11211127
}
11221128
if let Some(error) = error {
11231129
let _ = emitter.emit(&Event::Warning {

website/docs/installation.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ The editing tools are the same everywhere — zooms, backgrounds, crop/trim/spee
132132
| | macOS | Windows | Linux |
133133
|---|---|---|---|
134134
| Capture pipeline | Native (ScreenCaptureKit) | Native (Windows Graphics Capture) | Browser pipeline |
135-
| Custom cursor themes / click effects ||| ❌ (position-only, used for auto-zoom) |
135+
| Custom cursor themes / click effects ||| ✅ on Wayland — click capture needs the `input` group ([details](#mouse-clicks-on-wayland)) |
136136
| Webcam | Native capture | Native capture | Browser capture (still works as PiP) |
137137
| System audio | macOS 13+; permission prompt on 14.2+; not available on macOS 12 and below | Works out of the box | Needs PipeWire (default on Ubuntu 22.04+, Fedora 34+) |
138138
| MP4 export ||| ✅ (software encode) |

0 commit comments

Comments
 (0)