diff --git a/apps/vectorcraft/src/clipboard.rs b/apps/vectorcraft/src/clipboard.rs index bf3562ac0..1d8eb5aff 100644 --- a/apps/vectorcraft/src/clipboard.rs +++ b/apps/vectorcraft/src/clipboard.rs @@ -343,6 +343,8 @@ mod win { allow(dead_code) )] mod loads { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc; use std::time::{Duration, Instant}; @@ -353,11 +355,15 @@ mod loads { /// Requests of type `R` answered with `A` on a worker thread. pub(super) struct Loads { - requests: mpsc::Sender, - answers: mpsc::Receiver, - /// When a read that timed out and hasn't answered yet was sent: until it answers, reads - /// don't queue behind it. - pending: Option, + /// Each request goes with the channel its answer is sent on. + requests: mpsc::Sender<(R, mpsc::Sender)>, + /// Requests sent and not answered yet, counted down by the worker: while one is open, + /// reads don't queue behind it. + open: Arc, + /// When the last request was sent. + sent: Option, + /// Disconnected once the worker has let go of what its `load` holds. + done: mpsc::Receiver<()>, /// How long after it was sent dropping `Loads` waits for an unanswered read to end. finish: Duration, } @@ -374,42 +380,55 @@ mod loads { /// waits up to `finish` after an unanswered read was sent for that read to end. None when the /// worker can't start. pub(super) fn spawn(name: &str, finish: Duration, mut load: impl FnMut(R) -> A + Send + 'static) -> Option { - let (requests, asked) = mpsc::channel::(); - let (answer, answers) = mpsc::channel(); + let (requests, asked) = mpsc::channel::<(R, mpsc::Sender)>(); + let (alive, done) = mpsc::channel::<()>(); + let open = Arc::new(AtomicUsize::new(0)); + let answered = open.clone(); let work = move || { - while let Ok(request) = asked.recv() { - if answer.send(load(request)).is_err() { - break; - } + while let Ok((request, reply)) = asked.recv() { + let answer = load(request); + // Counted down before the answer goes, so its reader can ask again at once. + answered.fetch_sub(1, Ordering::SeqCst); + // A read that timed out no longer waits: its late answer is dropped. + let _ = reply.send(answer); } - // `load` and the clipboard it holds go first: dropping `Loads` waits for `answer`. + // `load` and the clipboard it holds go first: dropping `Loads` waits for `alive`. drop(load); - drop(answer); + drop(alive); }; std::thread::Builder::new().name(name.into()).spawn(work).ok()?; - Some(Self { requests, answers, pending: None, finish }) + Some(Self { requests, open, sent: None, done, finish }) + } + + /// `request` sent to the worker; its answer comes on the channel returned, for + /// [`wait_for_answer`] here or on another thread. Err when an earlier request is still + /// unanswered or the worker has stopped. + pub(super) fn ask(&mut self, request: R) -> Result, String> { + if self.open.load(Ordering::SeqCst) > 0 { + return Err("the clipboard's owner hasn't answered an earlier read".into()); + } + let (reply, answer) = mpsc::channel(); + self.open.fetch_add(1, Ordering::SeqCst); + self.sent = Some(Instant::now()); + if self.requests.send((request, reply)).is_err() { + self.open.fetch_sub(1, Ordering::SeqCst); + return Err(GONE.into()); + } + Ok(answer) } /// The answer to `request`, or why there is none, after `wait` at most. pub(super) fn get(&mut self, request: R, wait: Duration) -> Result { - if self.pending.is_some() { - match self.answers.try_recv() { - // The late answer of the read that timed out: stale by now. - Ok(_) => self.pending = None, - Err(mpsc::TryRecvError::Empty) => return Err("the clipboard's owner hasn't answered an earlier read".into()), - Err(mpsc::TryRecvError::Disconnected) => return Err(GONE.into()), - } - } - let sent = Instant::now(); - self.requests.send(request).map_err(|_| GONE.to_string())?; - match self.answers.recv_timeout(wait) { - Ok(answer) => Ok(answer), - Err(mpsc::RecvTimeoutError::Timeout) => { - self.pending = Some(sent); - Err(format!("the clipboard's owner didn't send its data within {wait:?}")) - } - Err(mpsc::RecvTimeoutError::Disconnected) => Err(GONE.into()), - } + wait_for_answer(&self.ask(request)?, wait) + } + } + + /// The answer that comes on `answer` ([`Loads::ask`]), or why there is none, after `wait` at most. + pub(super) fn wait_for_answer(answer: &mpsc::Receiver, wait: Duration) -> Result { + match answer.recv_timeout(wait) { + Ok(answer) => Ok(answer), + Err(mpsc::RecvTimeoutError::Timeout) => Err(format!("the clipboard's owner didn't send its data within {wait:?}")), + Err(mpsc::RecvTimeoutError::Disconnected) => Err(GONE.into()), } } @@ -420,8 +439,10 @@ mod loads { fn drop(&mut self) { let (stop, _) = mpsc::channel(); drop(std::mem::replace(&mut self.requests, stop)); - let end = stop_by(self.pending, Instant::now(), self.finish); - while self.answers.recv_timeout(end.saturating_duration_since(Instant::now())).is_ok() {} + let pending = self.sent.filter(|_| self.open.load(Ordering::SeqCst) > 0); + let end = stop_by(pending, Instant::now(), self.finish); + // Nothing is sent on `done`: this ends when the worker drops `alive`, or at `end`. + let _ = self.done.recv_timeout(end.saturating_duration_since(Instant::now())); } } @@ -473,12 +494,32 @@ mod loads { #[test] fn a_read_after_the_worker_stops_says_so() { let mut loads = Loads::spawn("test", Duration::ZERO, |()| String::new()).unwrap(); - // The worker ends when its answers can't be sent: drop them by hand. - let (_, dead) = mpsc::channel(); - drop(std::mem::replace(&mut loads.answers, dead)); + // The worker ends when its requests stop: cut them off by hand. + let (dead, _) = mpsc::channel(); + drop(std::mem::replace(&mut loads.requests, dead)); assert_eq!(loads.get((), Duration::from_millis(50)), Err(GONE.to_string())); } + #[test] + fn an_answer_waited_for_on_another_thread_holds_back_the_next_read() { + let (release, gate) = mpsc::channel::<()>(); + let mut loads = Loads::spawn("test", Duration::ZERO, move |n: u32| { + if n == 1 { + let _ = gate.recv(); + } + n + }) + .unwrap(); + let answer = loads.ask(1).unwrap(); + // The caller doesn't wait: the answer is waited for wherever the channel goes. + let waiter = std::thread::spawn(move || wait_for_answer(&answer, Duration::from_secs(600))); + // Until it comes, a read fails at once instead of queuing behind it. + assert!(loads.get(2, Duration::from_secs(600)).is_err()); + release.send(()).unwrap(); + assert_eq!(waiter.join().ok(), Some(Ok(1))); + assert_eq!(loads.get(3, Duration::from_secs(600)), Ok(3)); + } + #[test] fn dropping_the_reads_waits_until_the_worker_lets_go_of_its_clipboard() { let clipboard = Arc::new(()); @@ -590,9 +631,9 @@ mod portable { reads.get(request, wait).unwrap_or_else(|why| Answer::failed(request, why)) } - /// The longest a bitmap read of arboard's waits for the app that copied: the bitmap's transfer - /// and its PNG decoding happen within the read. Text and file lists wait - /// [`LOAD_WAIT`](super::loads::LOAD_WAIT). + /// The longest a paste's bitmap read of arboard's waits for the app that copied, off the UI + /// thread: the bitmap's transfer and its PNG decoding happen within the read. Text, file lists + /// and the UI thread's bitmap check wait [`LOAD_WAIT`](super::loads::LOAD_WAIT). #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] const IMAGE_WAIT: Duration = Duration::from_secs(10); @@ -628,7 +669,7 @@ mod portable { /// arboard's clipboard, with `C` for writes (arboard's handle; a stand-in in tests). On Linux /// and the BSDs its reads run on a worker with a handle of its own and wait - /// [`LOAD_WAIT`](super::loads::LOAD_WAIT) at most, a bitmap 10 s: arboard's Wayland + /// [`LOAD_WAIT`](super::loads::LOAD_WAIT) at most, a paste's bitmap 10 s off the UI thread: arboard's Wayland /// data-control read waits for the app that copied to send its data with no limit, and its X11 /// read waits up to 4 s for each format it tries. struct Arboard { @@ -667,8 +708,24 @@ mod portable { fn read(&mut self, request: Request) -> Answer { read_within(&mut self.reads, request, wait_for(request)) } + + /// A bitmap read on the worker that waits [`LOAD_WAIT`](super::loads::LOAD_WAIT) at most, for + /// a check on the UI thread. + fn image_now(&mut self) -> Result, String> { + read_within(&mut self.reads, Request::Image, super::loads::LOAD_WAIT).image() + } + + /// A bitmap read sent to the worker now; the work returned waits [`IMAGE_WAIT`] at most for + /// it, so the caller can wait off the UI thread. Err when the read can't be sent. + fn image_later(&mut self) -> Result { + let answer = self.reads.ask(Request::Image)?; + Ok(Box::new(move || super::loads::wait_for_answer(&answer, IMAGE_WAIT).and_then(Answer::image))) + } } + /// A bitmap read whose answer is waited for when it runs. + type ImageWork = Box Result, String> + Send>; + /// An open clipboard connection. enum Backend { Arboard(Arboard), @@ -726,14 +783,35 @@ mod portable { } } - fn get_image(&mut self) -> Result, String> { + /// The bitmap, for a check on the UI thread: on Linux and the BSDs it waits + /// [`LOAD_WAIT`](super::loads::LOAD_WAIT) at most, as text does. + fn image_now(&mut self) -> Result, String> { match self { + #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] + Self::Arboard(cb) => cb.image_now(), + #[cfg(not(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd")))] Self::Arboard(cb) => cb.read(Request::Image).image(), #[cfg(target_os = "linux")] Self::Wayland(_) => Err(TEXT_ONLY.into()), } } + /// The bitmap as work for another thread: on Linux and the BSDs the wait for the app + /// that copied happens when the work runs, elsewhere the bitmap is read here. + fn image_later(&mut self) -> Result { + match self { + #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] + Self::Arboard(cb) => cb.image_later(), + #[cfg(not(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd")))] + Self::Arboard(cb) => { + let image = cb.read(Request::Image).image()?; + Ok(Box::new(move || Ok(image))) + } + #[cfg(target_os = "linux")] + Self::Wayland(_) => Err(TEXT_ONLY.into()), + } + } + fn set_image(&mut self, img: arboard::ImageData<'_>) -> Result<(), String> { match self { Self::Arboard(cb) => cb.cb.set_image(img).map_err(|e| e.to_string()), @@ -913,7 +991,7 @@ mod portable { let Some(cb) = self.cb.opened() else { return false }; match &self.ours { Some(Ours::Text(t)) => cb.get_text().is_ok_and(|now| now == *t), - Some(Ours::Image(w, h)) => cb.get_image().is_ok_and(|now| (now.width, now.height) == (*w, *h)), + Some(Ours::Image(w, h)) => cb.image_now().is_ok_and(|now| (now.width, now.height) == (*w, *h)), None => false, } } @@ -936,10 +1014,12 @@ mod portable { match *m { TEXT => cb.get_text().ok().filter(|t| !t.is_empty()).map(|t| ClipboardRead::Now(Flavour { mime: TEXT, data: t.into_bytes() })), BITMAP => { - let img = cb.get_image().ok()?; - let (width, height) = (u32::try_from(img.width).ok()?, u32::try_from(img.height).ok()?); - let bytes = img.bytes.into_owned(); - Some(ClipboardRead::Later(Box::new(move || png_flavour(&image::RgbaImage::from_raw(width, height, bytes)?)))) + let read = cb.image_later().ok()?; + Some(ClipboardRead::Later(Box::new(move || { + let img = read().ok()?; + let (width, height) = (u32::try_from(img.width).ok()?, u32::try_from(img.height).ok()?); + png_flavour(&image::RgbaImage::from_raw(width, height, img.bytes.into_owned())?) + }))) } _ => None, } @@ -1137,6 +1217,29 @@ mod portable { assert_eq!(finish(true), Duration::ZERO); } + #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd", target_os = "openbsd"))] + #[test] + fn a_bitmap_paste_waits_for_the_owner_off_the_ui_thread() { + let (release, gate) = std::sync::mpsc::channel::<()>(); + // The app that copied sends its bitmap only when released. + let reads = Loads::spawn("test", Duration::ZERO, move |request| match request { + Request::Image => { + let _ = gate.recv(); + Answer::Image(Ok(arboard::ImageData { width: 2, height: 1, bytes: Cow::Owned(vec![0; 8]) })) + } + _ => Answer::failed(request, "not asked".into()), + }) + .unwrap(); + let mut cb = Arboard { reads, cb: () }; + // The read is sent and the UI thread goes on without its answer. + let work = cb.image_later().unwrap(); + // A check on the UI thread meanwhile fails at once instead of waiting behind it. + assert!(cb.read(Request::Text).text().is_err()); + let paste = std::thread::spawn(work); + release.send(()).unwrap(); + assert_eq!(paste.join().ok().and_then(Result::ok).map(|i| (i.width, i.height)), Some((2, 1))); + } + #[test] fn one_outage_is_logged_once_across_handles() { static LOGGED: AtomicBool = AtomicBool::new(false); diff --git a/docs/development.md b/docs/development.md index 78e620f7e..766b493c5 100644 --- a/docs/development.md +++ b/docs/development.md @@ -123,7 +123,7 @@ On GNOME/Wayland, enable **Preferences › User Interface › System Title Bar** The window runs natively on Wayland (eframe's `wayland` feature) and on X11. On machines with two GPUs it renders on the one the desktop runs on unless Preferences say otherwise; see [Desktop graphics processor](#desktop-graphics-processor) (#502). The system clipboard (`apps/vectorcraft/src/clipboard.rs`) is arboard with its `wayland-data-control` feature: - **Clipboard:** under Wayland arboard talks to the compositor through the data-control protocol (wlroots compositors such as Sway and Hyprland, KDE Plasma), so bitmaps (`image/png`), SVG markup and text copied in any app paste (#398). Where the compositor lacks the protocol (GNOME's Mutter, [arboard#223](https://github.com/1Password/arboard/issues/223)), arboard falls back to the X11 clipboard through XWayland, which only holds what X11 apps copied. Where there is no X11 either (the Flatpak on a Wayland session: `--socket=fallback-x11` gives it none), Copy and Paste use the compositor's core data-device protocol on the window's own connection through smithay-clipboard: text only (a copy's text or its SVG markup), and only while the window has focus. smithay-clipboard waits for the owner's data with no limit, so its reads run on a worker thread and a Paste waits 2 s at most: an owner that never sends its data costs one Paste 2 s, and later reads return at once until it answers. A clipboard neither backend opens is tried again after 1 s, doubling to a minute (a Copy or Paste of the user's at most once a second), and logged once when it goes and once when it comes back: before, the background Paste check reconnected and logged a warning every 250 ms. egui's own text paste into fields goes through smithay-clipboard and works either way. -- **Clipboard owners that never answer (#510):** reading the clipboard waits on the app that owns it: arboard gives up on an X11 owner after 4 s for each format it tries, and a Wayland read waits for the owner to send its data. So on Linux the check that enables the Paste menu items runs on a background thread with a second clipboard handle (`crates/ui-egui/src/clipboard_probe.rs`, installed as `Services::clipboard_probe`), only while nothing is copied in the app and a document is open, and the window keeps drawing. Every arboard read on Linux and the BSDs, including the UI thread's (a paste, and a Copy or Cut checking what the clipboard held), runs on a worker thread with a handle of its own and waits 2 s at most, a bitmap 10 s: an owner that never sends its data costs one read that wait, and later reads fail at once until it answers. On X11, quitting within 25 s of a read that got no answer waits for that read to end, so the app's own handle is the last one and hands what it copied to the clipboard manager. Windows and macOS keep the check on the UI thread: Windows only asks which formats the clipboard holds, macOS asks the pasteboard server. +- **Clipboard owners that never answer (#510):** reading the clipboard waits on the app that owns it: arboard gives up on an X11 owner after 4 s for each format it tries, and a Wayland read waits for the owner to send its data. So on Linux the check that enables the Paste menu items runs on a background thread with a second clipboard handle (`crates/ui-egui/src/clipboard_probe.rs`, installed as `Services::clipboard_probe`), only while nothing is copied in the app and a document is open, and the window keeps drawing. Every arboard read on Linux and the BSDs, including the UI thread's (a paste, and a Copy or Cut checking what the clipboard held), runs on a worker thread with a handle of its own and waits 2 s at most: an owner that never sends its data costs one read that wait, and later reads fail at once until it answers. A paste's bitmap read waits up to 10 s on the paste's background thread, so the UI thread does not wait for it. On X11, quitting within 25 s of a read that got no answer waits for that read to end, so the app's own handle is the last one and hands what it copied to the clipboard manager. Windows and macOS keep the check on the UI thread: Windows only asks which formats the clipboard holds, macOS asks the pasteboard server. - **Copied files:** files copied in a file manager (`text/uri-list`; `CF_HDROP` on Windows, file URLs on macOS) paste as the first one that is art (SVG, PDF, EPS, EMF/WMF or a bitmap; a `.ai` or `.ait` file from the editing data it holds, else as the PDF or PostScript it holds), before the clipboard's other formats (which include the files' paths as text). - **Dropping files on the window does not work under Wayland:** winit 0.30, which eframe 0.37 runs on, has no Wayland drag and drop ([winit#1881](https://github.com/rust-windowing/winit/issues/1881), added in winit 0.31; [egui#1563](https://github.com/emilk/egui/issues/1563)), so no `dropped_files` arrive. Until eframe moves to winit 0.31: copy the files in the file manager and paste them, use File › Place or Open, or open the window through XWayland, where drops work: set **Preferences › Performance › Linux Display Server** to **X11 (XWayland)** and restart (#1151), or start the app with `WAYLAND_DISPLAY= vectorcraft`. The preference (`linuxDisplayServer`, read at startup in `apps/vectorcraft/src/main.rs`, shown on Linux only) has winit open the window on X11 (`EventLoopBuilderExtX11::with_x11`) when the session is Wayland, `DISPLAY` is set and the X11 libraries winit loads are installed (`apps/vectorcraft/src/xwayland.rs` reads `ldconfig -p` and the usual library directories); otherwise the window opens on Wayland and the log says why. Unlike clearing `WAYLAND_DISPLAY`, it leaves the clipboard on the compositor's own. - **Pens and pen displays under Wayland (#491, #2028):** winit 0.30 doesn't bind the Wayland tablet protocol (`zwp_tablet_v2`; tablet input arrives in winit 0.31, [winit#4318](https://github.com/rust-windowing/winit/pull/4318), [egui#7731](https://github.com/emilk/egui/pull/7731)). Since Plasma 6.3, KWin no longer turns tablet input into pointer input for such apps ([kwin!6336](https://invent.kde.org/plasma/kwin/-/merge_requests/6336), commit [25c65abd](https://invent.kde.org/plasma/kwin/-/commit/25c65abddd95eb87fb892a8594faccbf509ff62f)), and GNOME's Mutter does not turn it into pointer input either (that commit's message): on KWin the pen moves the compositor's cursor over the window, but the app gets no motion or presses and shows none of its own cursors. Until eframe moves to winit 0.31, the app runs under XWayland, which supports the tablet protocol and hands the pen to X11 apps as a pointer: Preferences › Performance › Linux Display Server's Automatic (the default) and X11 (XWayland) When a Pen Is Attached (`x11WithPen`) open the window through XWayland when `/proc/bus/input/devices` lists a pen as the app starts, a device with the `BTN_TOOL_PEN` key and the `ABS_X` and `ABS_Y` axes, and log `pen found: opening the window through XWayland`, as PhotoCraft does (#4967; `VECTORCRAFT_NATIVE_WAYLAND=1` keeps the window on Wayland). For a pen attached after the app started, either run the app under XWayland (Preferences › Performance › Linux Display Server › X11 (XWayland); `WAYLAND_DISPLAY= vectorcraft`, or `env -u WAYLAND_DISPLAY vectorcraft`; for a desktop launcher, put `env WAYLAND_DISPLAY=` before the command in its `Exec=` line), or turn KWin's deprecated emulation back on for the whole session by setting `KWIN_WAYLAND_EMULATE_TABLET=1` in KWin's environment (for example in a file under `~/.config/environment.d/` or in `/etc/environment`) and logging in again. The pen then works as a mouse. Under XWayland the app also reads the pen's pressure (next item); with KWin's emulation on native Wayland the pen has no pressure. Other compositors that don't emulate a pointer for tablets behave the same way.