diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7eca1256b0..388b44f032 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,11 +85,11 @@ jobs: - name: Test if: runner.os != 'Linux' - run: cargo test --workspace -- --test-threads=1 + run: cargo test --workspace - name: Test (with craft-fonts) if: runner.os == 'Linux' - run: cargo test --workspace -- --test-threads=1 + run: cargo test --workspace - name: Layering if: runner.os == 'Linux' diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index cc91fef9e9..2b3366e9c2 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2840,7 +2840,16 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let editing_content = (app.left_open && app.left == crate::LeftPanel::Tool("edit")) || matches!(app.quick_tool, QuickTool::AddText | QuickTool::EditText); let text_style = app.text_style.clone(); - let can_paste_image = app.can_paste_image(); + // Asking means opening the system clipboard (and decoding any image on it), so it's asked + // once when a right-click opens a menu, not on every frame. + let can_paste_key = egui::Id::new("can-paste-image"); + let can_paste_image = if ui.input(|i| i.pointer.secondary_pressed()) { + let can = app.can_paste_image(); + ui.data_mut(|d| d.insert_temp(can_paste_key, can)); + can + } else { + ui.data(|d| d.get_temp(can_paste_key)).unwrap_or(false) + }; let view = &mut app.views[index]; // Opened without the owner password and something is restricted. let secured = doc.security_summary().is_some_and(|s| !(s.owner || (s.permissions.modify() && s.permissions.assemble()))); diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 61be108256..d0c3721eb2 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -824,7 +824,7 @@ impl crate::PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] if self.system_clipboard { let img = arboard::ImageData { width: w as usize, height: h as usize, bytes: std::borrow::Cow::Borrowed(&rgba) }; - arboard::Clipboard::new().and_then(|mut c| c.set_image(img)).map_err(|e| e.to_string())?; + crate::os_clipboard::with(|c| c.set_image(img)).map_err(|e| e.to_string())?; } #[cfg(target_arch = "wasm32")] if self.system_clipboard { @@ -897,10 +897,8 @@ impl crate::PdfCraftApp { pub fn can_paste_image(&self) -> bool { #[cfg(not(target_arch = "wasm32"))] { - if self.system_clipboard - && let Ok(mut cb) = arboard::Clipboard::new() - { - return cb.get_image().is_ok(); + if self.system_clipboard { + return crate::os_clipboard::with(|cb| cb.get_image().map(|_| ())).is_ok(); } } self.last_copied_image.is_some() || self.last_copied_file.is_some() @@ -920,11 +918,9 @@ impl crate::PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] let (has_system_cb, os_image) = if self.system_clipboard { - if let Ok(mut cb) = arboard::Clipboard::new() { - let img_opt = cb.get_image().ok().map(|img| (img.width, img.height, img.bytes.to_vec())); - (true, img_opt) - } else { - (false, None) + match crate::os_clipboard::with(|cb| Ok(cb.get_image().ok().map(|img| (img.width, img.height, img.bytes.into_owned())))) { + Ok(img_opt) => (true, img_opt), + Err(_) => (false, None), } } else { (false, None) diff --git a/crates/ui-egui/src/create_ui.rs b/crates/ui-egui/src/create_ui.rs index fb8f1b2e27..bf12378206 100644 --- a/crates/ui-egui/src/create_ui.rs +++ b/crates/ui-egui/src/create_ui.rs @@ -59,11 +59,14 @@ pub enum Clip { /// An image on the system clipboard, or else its text. #[cfg(not(target_arch = "wasm32"))] fn read_clipboard() -> Option { - let mut cb = arboard::Clipboard::new().ok()?; - if let Ok(img) = cb.get_image() { - return Some(Clip::Image { width: img.width, height: img.height, rgba: img.bytes.into_owned() }); - } - cb.get_text().ok().filter(|t| !t.trim().is_empty()).map(Clip::Text) + crate::os_clipboard::with(|cb| { + if let Ok(img) = cb.get_image() { + return Ok(Some(Clip::Image { width: img.width, height: img.height, rgba: img.bytes.into_owned() })); + } + Ok(cb.get_text().ok().filter(|t| !t.trim().is_empty()).map(Clip::Text)) + }) + .ok() + .flatten() } pub(crate) fn png_from_rgba(width: usize, height: usize, rgba: &[u8]) -> Result, String> { diff --git a/crates/ui-egui/src/image_pick.rs b/crates/ui-egui/src/image_pick.rs index 9e117395cd..0dc879d09d 100644 --- a/crates/ui-egui/src/image_pick.rs +++ b/crates/ui-egui/src/image_pick.rs @@ -208,7 +208,7 @@ impl PdfCraftApp { } let dir = clipboard_dir().ok_or("there is no folder for the copied file")?; let path = write_clipboard_file(&dir, name, bytes)?; - arboard::Clipboard::new().and_then(|mut c| c.set().file_list(&[path])).map_err(|e| e.to_string()) + crate::os_clipboard::with(|c| c.set().file_list(&[path])).map_err(|e| e.to_string()) } #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ba8fd6390d..083f65769d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -96,6 +96,8 @@ mod icon_data; pub mod icons; pub mod last_session; pub mod motion; +#[cfg(not(target_arch = "wasm32"))] +mod os_clipboard; mod page_selection_ui; mod pageboxes; mod palette; @@ -2951,11 +2953,8 @@ impl PdfCraftApp { } if paste { #[cfg(not(target_arch = "wasm32"))] - let system_has_text = if self.system_clipboard { - arboard::Clipboard::new().and_then(|mut cb| cb.get_text()).map(|t| !t.is_empty()).unwrap_or(false) - } else { - false - }; + let system_has_text = + if self.system_clipboard { crate::os_clipboard::with(|cb| cb.get_text()).map(|t| !t.is_empty()).unwrap_or(false) } else { false }; #[cfg(target_arch = "wasm32")] let system_has_text = false; diff --git a/crates/ui-egui/src/os_clipboard.rs b/crates/ui-egui/src/os_clipboard.rs new file mode 100644 index 0000000000..0160dc1fcc --- /dev/null +++ b/crates/ui-egui/src/os_clipboard.rs @@ -0,0 +1,18 @@ +//! The one way to the operating system's clipboard. +//! +//! The system clipboard is a single process-wide resource: on Windows, two threads that +//! open it at once can corrupt the heap. Every access goes through [`with`], which holds a +//! lock across the whole open, use and close, so parallel callers (test threads, workers) +//! take turns instead of racing. + +use std::sync::{Mutex, PoisonError}; + +static LOCK: Mutex<()> = Mutex::new(()); + +/// Opens the system clipboard, runs `f` on it and closes it, with no other thread inside. +pub(crate) fn with(f: impl FnOnce(&mut arboard::Clipboard) -> Result) -> Result { + let _turn = LOCK.lock().unwrap_or_else(PoisonError::into_inner); + // Declared after the guard, so it's closed before the lock is released. + let mut clipboard = arboard::Clipboard::new()?; + f(&mut clipboard) +} diff --git a/crates/ui-egui/src/zoom_snap.rs b/crates/ui-egui/src/zoom_snap.rs index eb2cc86727..caf21d8775 100644 --- a/crates/ui-egui/src/zoom_snap.rs +++ b/crates/ui-egui/src/zoom_snap.rs @@ -141,7 +141,7 @@ impl PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] if self.system_clipboard { let img = arboard::ImageData { width: w as usize, height: h as usize, bytes: std::borrow::Cow::Borrowed(&out.rgba) }; - arboard::Clipboard::new().and_then(|mut c| c.set_image(img)).map_err(|e| e.to_string())?; + crate::os_clipboard::with(|c| c.set_image(img)).map_err(|e| e.to_string())?; } self.last_snapshot = Some((w, h, out.rgba)); Ok((w, h)) diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index a9c3dca38a..6efab8114a 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -4977,20 +4977,16 @@ fn stale_clipboard_plain_text_does_not_paste_image() { // Put an image into internal cache h.state_mut().last_copied_image = Some((10, 10, vec![0; 400])); h.state_mut().last_copied_file = Some(("cached.png".to_string(), vec![1, 2, 3])); - h.state_mut().system_clipboard = true; - - // Set actual system clipboard to plain text - #[cfg(not(target_arch = "wasm32"))] - if let Ok(mut cb) = arboard::Clipboard::new() { - cb.set_text("just plain text").ok(); - } + // The text arrives in the paste event, as it does from the OS. The real system clipboard is + // shared by every test thread, so this test doesn't write it. + h.state_mut().system_clipboard = false; // A paste event containing plain text clears internal image cache and does not paste image h.event(egui::Event::Paste("just plain text".into())); h.run_steps(2); - // can_paste_image should now be false because the system clipboard has text, not an image! - assert!(!h.state().can_paste_image(), "cannot paste image when system clipboard has text"); + // can_paste_image should now be false because the clipboard has text, not an image! + assert!(!h.state().can_paste_image(), "cannot paste image when the clipboard has text"); // paste_image should fail and return false without pasting assert!(!h.state_mut().paste_image(0, None), "must not paste stale cached image");