From c607fc9a1c68c34b4a9c80d27c5a4d08533720d5 Mon Sep 17 00:00:00 2001 From: Milad Alizadeh Date: Mon, 7 Sep 2026 11:47:41 +0100 Subject: [PATCH] Bound every wait before the overlay is interactive Running wl-pick twice left the second instance hung with no window at all and no way to end it but kill. The hang was in the capture phase, not the overlay. sway answers a capture request for a toplevel that another client is already capturing with silence: no frame, no failed, no stopped. captures_settled() waits for every tile to reach one of those three, so it waited forever, holding its buffers, having never created a layer surface. Any concurrent capture client will do this, not just a second wl-pick. So the phases before the overlay is interactive now have a deadline. pump_for polls the connection with one, and a tile that never arrives is drawn as a bare label, exactly as an outright capture failure already was. report_unsettled names those tiles on stderr, which is the diagnostic whose absence made this hard to find. The first instance had a second problem: sway hands the keyboard to the new overlay and sends the old one wl_keyboard.leave, but the dispatcher only handled Key, so the loser sat on screen holding a grab it no longer had, deaf to every key. That is the process that stays around. Losing the grab for good now ends the run, so a second wl-pick started from the same keybinding replaces the first rather than stranding it. For good, because sway also sends leave followed immediately by enter on the same surface -- microseconds apart -- as a focus refresh when the pointer crosses the overlay. Treating a bare leave as terminal made a lone instance quit itself after less than a second. Focus is tracked in the dispatcher and the main loop only gives up once it has failed to come back. pump had no callers left after that: every wait is now either budgeted or focus-aware. --- Cargo.toml | 2 +- README.md | 10 +++++ src/app.rs | 25 ++++++++++++ src/capture.rs | 2 - src/main.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++----- src/overlay.rs | 13 ++++-- 6 files changed, 142 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 367adba..467b43e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ wayland-client = "0.31" wayland-protocols = { version = "0.32", features = ["client", "staging", "unstable"] } wayland-protocols-wlr = { version = "0.3", features = ["client"] } memmap2 = "0.9" -rustix = { version = "1", features = ["fs", "mm", "shm"] } +rustix = { version = "1", features = ["event", "fs", "mm", "shm"] } swayipc = "4" cosmic-text = "0.19" diff --git a/README.md b/README.md index 8c47d7d..0b3b5b4 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,16 @@ chooser_type=simple chooser_cmd=wl-pick --format portal ``` +**Starting a second wl-pick replaces the first.** The new overlay takes the +keyboard grab, and the one that loses it exits without printing anything — so +hitting the keybinding twice leaves you with one overlay, not a stranded +process. The catch is that sway answers a capture request for a toplevel +another client is already capturing with silence — no frame, no failure — so +the replacement's thumbnails are mostly blank until the first instance has +gone. Every wait before the overlay is interactive is capped at two seconds +for that reason: a tile that never arrives is drawn as a bare label, and the +grid still works. + | key | | |---|---| | `→` `←` / `l` `h` / `Tab` `Shift+Tab` | next / previous tile | diff --git a/src/app.rs b/src/app.rs index f3e650d..f572296 100644 --- a/src/app.rs +++ b/src/app.rs @@ -109,6 +109,8 @@ pub struct App { pub(crate) output: String, pub(crate) ending: Ending, + /// Whether we hold the keyboard. Without it the overlay cannot be operated. + pub(crate) focused: bool, pub(crate) picked: Option, pub(crate) stats: Stats, } @@ -134,6 +136,8 @@ pub enum Ending { Picked, Cancelled, Closed, + /// The keyboard went to another surface, so we can no longer be operated. + Unfocused, } impl Ending { @@ -143,6 +147,7 @@ impl Ending { Ending::Picked => "picked", Ending::Cancelled => "cancelled", Ending::Closed => "the compositor closed the overlay", + Ending::Unfocused => "lost the keyboard to another surface", } } } @@ -196,6 +201,7 @@ impl App { configured: false, output, ending: Ending::Running, + focused: false, picked: None, stats: Stats::default(), }; @@ -272,6 +278,25 @@ impl App { pub fn captures_settled(&self) -> bool { self.tiles.iter().all(|t| t.settled) } + + /// Say which tiles the compositor went quiet on, and name the likeliest + /// reason: sway answers a capture request on a toplevel that another client + /// is already capturing with silence rather than with `failed`. + pub fn report_unsettled(&self) { + let stuck: Vec<&str> = self + .tiles + .iter() + .filter(|t| !t.settled) + .map(|t| t.target.title.as_str()) + .collect(); + eprintln!( + "wl-pick: no frame for {} of {} tiles ({}); \ + another capture client may hold these sources", + stuck.len(), + self.tiles.len(), + stuck.join(", ") + ); + } } // --- enumeration ---------------------------------------------------------- diff --git a/src/capture.rs b/src/capture.rs index 5249dcd..ebe94c4 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -398,8 +398,6 @@ impl Dispatch for App { app.frame_ready(i); } ext_image_copy_capture_frame_v1::Event::Failed { reason } => { - // Live mode just retries on the next tick; only a failure with no - // frame yet leaves the tile without a thumbnail. // Live mode retries on the next tick; only a failure with no // frame yet leaves the tile without a thumbnail. if tile.frames == 0 { diff --git a/src/main.rs b/src/main.rs index cf7d727..3556eba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,16 +36,27 @@ mod theme; use std::error::Error; use std::process::ExitCode; -use std::time::Instant; +use std::time::{Duration, Instant}; +use rustix::event::{PollFd, PollFlags, Timespec}; use wayland_client::globals::registry_queue_init; use wayland_client::{Connection, EventQueue}; -use app::App; +use app::{App, Ending}; use config::Config; use target::Target; use theme::Layout; +/// How long the phases before the overlay is interactive may take. Capture +/// measures ~90ms for fourteen windows, so this is a wide margin around +/// anything healthy, and only a stall reaches it. +const STARTUP_BUDGET: Duration = Duration::from_secs(2); + +/// How long a keyboard leave is given to turn out to be a focus refresh rather +/// than a real loss. sway's pair arrives microseconds apart; this is only long +/// enough to be sure, and short enough that a real handover looks instant. +const REFOCUS_GRACE: Duration = Duration::from_millis(150); + fn main() -> ExitCode { match run() { Ok(code) => code, @@ -121,7 +132,18 @@ fn run() -> Result> { phases.mark("constraints"); app.start_captures(&qh)?; - pump(&mut queue, &mut app, |a| a.captures_settled())?; + // Tiles that never delivered are shown as labels without a thumbnail, + // exactly as an outright capture failure is. Better a grid you can use + // than a process you have to hunt down. + if !pump_for( + &conn, + &mut queue, + &mut app, + |a| a.captures_settled(), + STARTUP_BUDGET, + )? { + app.report_unsettled(); + } phases.mark("capture"); if let Some(job) = labels { @@ -133,14 +155,44 @@ fn run() -> Result> { } app.show(&qh)?; - pump(&mut queue, &mut app, |a| a.configured)?; + if !pump_for( + &conn, + &mut queue, + &mut app, + |a| a.configured, + STARTUP_BUDGET, + )? { + return Err("the compositor never configured the overlay".into()); + } app.paint(); app.sync_tiles(&qh); app.arm_frame_callback(&qh); conn.flush()?; phases.mark("mapped"); - pump(&mut queue, &mut app, |a| a.finished())?; + // The keyboard grab is what makes the overlay usable, so losing it for + // good ends the run: that is how a second wl-pick, started from the same + // keybinding, replaces the first instead of leaving it stranded on screen. + // A leave only counts once it has failed to come back, because sway also + // cycles focus off and on in a single batch as the pointer crosses us. + loop { + queue.blocking_dispatch(&mut app)?; + if !app.finished() + && !app.focused + && !pump_for( + &conn, + &mut queue, + &mut app, + |a| a.focused || a.finished(), + REFOCUS_GRACE, + )? + { + app.ending = Ending::Unfocused; + } + if app.finished() { + break; + } + } if opts.verbose { app.report(start.elapsed()); } @@ -160,16 +212,49 @@ fn run() -> Result> { Ok(ExitCode::SUCCESS) } -/// Run the event loop until `done`. -fn pump( +/// Run the event loop until `done`, or until `limit` has passed. Returns +/// whether `done` came true in time. +/// +/// Every wait before the overlay is interactive is bounded, because a +/// compositor is entitled to simply never answer. sway does exactly that for a +/// capture request on a toplevel another client is already capturing: no frame, +/// no `failed`, no `stopped`, just silence — and an unbounded wait on that is a +/// picker with no window that has to be killed from another terminal. +fn pump_for( + conn: &Connection, queue: &mut EventQueue, app: &mut App, done: impl Fn(&App) -> bool, -) -> Result<(), Box> { - while !done(app) { - queue.blocking_dispatch(app)?; + limit: Duration, +) -> Result> { + let deadline = Instant::now() + limit; + loop { + queue.dispatch_pending(app)?; + if done(app) { + return Ok(true); + } + conn.flush()?; + // No guard means events arrived while we were asking; go read them. + let Some(guard) = conn.prepare_read() else { + continue; + }; + let Some(left) = deadline.checked_duration_since(Instant::now()) else { + return Ok(false); + }; + let fd = guard.connection_fd(); + let mut fds = [PollFd::new(&fd, PollFlags::IN)]; + let timeout = Timespec { + tv_sec: left.as_secs() as _, + tv_nsec: left.subsec_nanos() as _, + }; + match rustix::event::poll(&mut fds, Some(&timeout)) { + Ok(0) => return Ok(false), + // An interrupted poll has simply not waited its full time yet. + Ok(_) | Err(rustix::io::Errno::INTR) => {} + Err(e) => return Err(Box::new(e)), + } + guard.read()?; } - Ok(()) } /// Phase timings, printed with --verbose. Opening latency is the whole point of diff --git a/src/overlay.rs b/src/overlay.rs index 29b0703..fab55c9 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -376,8 +376,8 @@ impl Dispatch for App { _: &Connection, qh: &QueueHandle, ) { - if let wl_keyboard::Event::Key { key, state, .. } = event { - match state { + match event { + wl_keyboard::Event::Key { key, state, .. } => match state { WEnum::Value(wl_keyboard::KeyState::Pressed) => app.key(key, qh), WEnum::Value(wl_keyboard::KeyState::Released) if key == KEY_LEFTSHIFT || key == KEY_RIGHTSHIFT => @@ -385,7 +385,14 @@ impl Dispatch for App { app.shift = false } _ => {} - } + }, + // Focus is only tracked here. sway sends leave immediately + // followed by enter on the same surface when the pointer crosses + // it, so whether the grab is really gone is decided by the main + // loop, once the event batch has been dispatched. + wl_keyboard::Event::Enter { .. } => app.focused = true, + wl_keyboard::Event::Leave { .. } => app.focused = false, + _ => {} } } }