diff --git a/apps/desktop-gpui/src/app_windows.rs b/apps/desktop-gpui/src/app_windows.rs index 576102be830..92c8f1bf1d6 100644 --- a/apps/desktop-gpui/src/app_windows.rs +++ b/apps/desktop-gpui/src/app_windows.rs @@ -6622,10 +6622,14 @@ pub fn screenshot_finished(captured: Option, cx: &mut App) { .ok(); let behaviour = crate::store::GeneralSettings::load().post_screenshot_behaviour; - if behaviour.copies() - && let Err(error) = platform::copy_image_to_clipboard(&png, cx) - { - tracing::error!("copying the screenshot to the clipboard failed: {error}"); + if behaviour.copies() { + let copied = platform::copy_image_to_clipboard(png.clone(), cx); + cx.spawn(async move |_| { + if let Err(error) = copied.await { + tracing::error!("copying the screenshot to the clipboard failed: {error}"); + } + }) + .detach(); } if behaviour.opens_editor() { diff --git a/apps/desktop-gpui/src/main_window.rs b/apps/desktop-gpui/src/main_window.rs index 70796b56899..a9f5eb29de1 100644 --- a/apps/desktop-gpui/src/main_window.rs +++ b/apps/desktop-gpui/src/main_window.rs @@ -4785,13 +4785,14 @@ impl MainWindow { self.library_action(("lib-ss-copy", index), "icons/copy.svg", { let png = png.clone(); move |_, _, cx| { - if let Err(error) = crate::platform::copy_image_to_clipboard(&png, cx) { - tracing::warn!("copying screenshot failed: {error}"); - cx.spawn(async move |_| { + let copied = crate::platform::copy_image_to_clipboard(png.clone(), cx); + cx.spawn(async move |_| { + if let Err(error) = copied.await { + tracing::warn!("copying screenshot failed: {error}"); crate::platform::alert_dialog("Could not copy screenshot", &error); - }) - .detach(); - } + } + }) + .detach(); } }), ) diff --git a/apps/desktop-gpui/src/platform.rs b/apps/desktop-gpui/src/platform.rs index 7c05c4f6bba..e9c874c0672 100644 --- a/apps/desktop-gpui/src/platform.rs +++ b/apps/desktop-gpui/src/platform.rs @@ -558,10 +558,54 @@ fn clipboard_file_path(path: &std::path::Path) -> Result Result<(), String> { - let bytes = std::fs::read(path) - .map_err(|error| format!("Could not read the clipboard image: {error}"))?; - copy_image_bytes_to_clipboard(&bytes, cx) +// Prepares finish in completion order, not click order, so only the newest +// request may write: an older copy finishing late must not replace a newer one. +struct ClipboardWrites { + latest: std::sync::atomic::AtomicU64, +} + +impl ClipboardWrites { + const fn new() -> Self { + Self { + latest: std::sync::atomic::AtomicU64::new(0), + } + } + + fn request(&self) -> u64 { + self.latest + .fetch_add(1, std::sync::atomic::Ordering::SeqCst) + + 1 + } + + fn is_current(&self, id: u64) -> bool { + self.latest.load(std::sync::atomic::Ordering::SeqCst) == id + } +} + +static CLIPBOARD_WRITES: ClipboardWrites = ClipboardWrites::new(); + +pub fn copy_image_to_clipboard( + path: std::path::PathBuf, + cx: &gpui::App, +) -> gpui::Task> { + let id = CLIPBOARD_WRITES.request(); + cx.spawn(async move |cx| { + let image = cx + .background_executor() + .spawn(async move { + let bytes = std::fs::read(&path) + .map_err(|error| format!("Could not read the clipboard image: {error}"))?; + prepare_clipboard_image(bytes) + }) + .await?; + cx.update(|cx| { + if !CLIPBOARD_WRITES.is_current(id) { + tracing::debug!("skipping a superseded clipboard image write"); + return Ok(()); + } + write_clipboard_image(image, cx) + }) + }) } #[cfg(any(not(target_os = "macos"), test))] @@ -2102,7 +2146,13 @@ mod mac { }) } - pub fn copy_image_bytes_to_clipboard(bytes: &[u8], _cx: &gpui::App) -> Result<(), String> { + pub struct ClipboardImage(Vec); + + pub fn prepare_clipboard_image(bytes: Vec) -> Result { + Ok(ClipboardImage(bytes)) + } + + pub fn write_clipboard_image(image: ClipboardImage, _cx: &gpui::App) -> Result<(), String> { use objc2::rc::autoreleasepool; use objc2::runtime::Bool; use objc2::{class, msg_send}; @@ -2113,7 +2163,7 @@ mod mac { if pasteboard.is_null() { return Err("Clipboard unavailable".into()); } - let data = NSData::with_bytes(bytes); + let data = NSData::with_bytes(&image.0); let image: *mut AnyObject = msg_send![class!(NSImage), alloc]; let image: *mut AnyObject = msg_send![image, initWithData: &*data]; let image = Id::from_raw(image).ok_or("Failed to load image")?; @@ -2817,8 +2867,10 @@ mod stub { .map_err(|error| format!("Could not copy the file to the clipboard: {error}")) } - pub fn copy_image_bytes_to_clipboard(bytes: &[u8], cx: &gpui::App) -> Result<(), String> { - let format = match image::guess_format(bytes).map_err(|error| error.to_string())? { + pub struct ClipboardImage(gpui::ClipboardItem); + + pub fn prepare_clipboard_image(bytes: Vec) -> Result { + let format = match image::guess_format(&bytes).map_err(|error| error.to_string())? { image::ImageFormat::Png => gpui::ImageFormat::Png, image::ImageFormat::Jpeg => gpui::ImageFormat::Jpeg, image::ImageFormat::WebP => gpui::ImageFormat::Webp, @@ -2827,8 +2879,14 @@ mod stub { image::ImageFormat::Tiff => gpui::ImageFormat::Tiff, _ => return Err("Unsupported clipboard image format".into()), }; - let image = gpui::Image::from_bytes(format, bytes.to_vec()); - cx.try_write_to_clipboard(gpui::ClipboardItem::new_image(&image)) + let image = gpui::Image::from_bytes(format, bytes); + Ok(ClipboardImage(gpui::ClipboardItem { + entries: vec![gpui::ClipboardEntry::Image(image)], + })) + } + + pub fn write_clipboard_image(image: ClipboardImage, cx: &gpui::App) -> Result<(), String> { + cx.try_write_to_clipboard(image.0) .map_err(|error| format!("Could not copy the image to the clipboard: {error}")) } pub fn desktop_picture_path() -> Option { @@ -2931,6 +2989,52 @@ mod tests { assert!(!requests.request()); } + #[test] + fn newest_clipboard_request_wins() { + let writes = super::ClipboardWrites::new(); + let a = writes.request(); + let b = writes.request(); + assert!(!writes.is_current(a)); + assert!(writes.is_current(b)); + } + + #[test] + fn completed_clipboard_write_does_not_invalidate_a_later_request() { + let writes = super::ClipboardWrites::new(); + let a = writes.request(); + assert!(writes.is_current(a)); + let b = writes.request(); + assert!(writes.is_current(b)); + assert!(!writes.is_current(a)); + } + + #[test] + fn concurrent_clipboard_requests_get_distinct_ids_and_only_the_newest_is_current() { + let writes = std::sync::Arc::new(super::ClipboardWrites::new()); + let threads: Vec<_> = (0..8) + .map(|_| { + let writes = writes.clone(); + std::thread::spawn(move || writes.request()) + }) + .collect(); + let mut ids: Vec = threads + .into_iter() + .map(|thread| thread.join().unwrap()) + .collect(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(ids, (1..=8).collect::>()); + let current: Vec<_> = ids.iter().filter(|id| writes.is_current(**id)).collect(); + assert_eq!(current, [&8]); + } + + #[test] + fn clipboard_request_ids_increase_from_one() { + let writes = super::ClipboardWrites::new(); + let ids: Vec<_> = (0..4).map(|_| writes.request()).collect(); + assert_eq!(ids, [1, 2, 3, 4]); + } + #[test] fn confirmation_accepts_native_ok_and_matching_custom_label_only() { let accept = "Install update"; diff --git a/apps/desktop-gpui/src/platform/windows.rs b/apps/desktop-gpui/src/platform/windows.rs index 407ed3dc8fd..a0468465b38 100644 --- a/apps/desktop-gpui/src/platform/windows.rs +++ b/apps/desktop-gpui/src/platform/windows.rs @@ -647,15 +647,23 @@ pub fn copy_file_to_clipboard(path: &Path, _cx: &gpui::App) -> Result<(), String .map_err(|error| format!("Could not copy the file to the clipboard: {error}")) } -pub fn copy_image_bytes_to_clipboard(bytes: &[u8], _cx: &gpui::App) -> Result<(), String> { +pub struct ClipboardImage(clipboard_rs::RustImageData); + +pub fn prepare_clipboard_image(bytes: Vec) -> Result { + use clipboard_rs::RustImageData; use clipboard_rs::common::RustImage; - use clipboard_rs::{Clipboard, ClipboardContext, RustImageData}; - let image = RustImageData::from_bytes(bytes) - .map_err(|error| format!("Could not load the clipboard image: {error}"))?; + RustImageData::from_bytes(&bytes) + .map(ClipboardImage) + .map_err(|error| format!("Could not load the clipboard image: {error}")) +} + +pub fn write_clipboard_image(image: ClipboardImage, _cx: &gpui::App) -> Result<(), String> { + use clipboard_rs::{Clipboard, ClipboardContext}; + ClipboardContext::new() .map_err(|error| format!("Clipboard unavailable: {error}"))? - .set_image(image) + .set_image(image.0) .map_err(|error| format!("Could not copy the image to the clipboard: {error}")) } diff --git a/apps/desktop-gpui/src/screenshot_editor.rs b/apps/desktop-gpui/src/screenshot_editor.rs index 316da677dad..96489a861d4 100644 --- a/apps/desktop-gpui/src/screenshot_editor.rs +++ b/apps/desktop-gpui/src/screenshot_editor.rs @@ -1982,8 +1982,14 @@ impl ScreenshotEditorWindow { match result { Ok(bytes) => match destination { ExportDestination::Clipboard => { + let image = cx + .background_executor() + .spawn(async move { crate::platform::prepare_clipboard_image(bytes) }) + .await; this.update_in(cx, |this, window, cx| { - match crate::platform::copy_image_bytes_to_clipboard(&bytes, cx) { + match image + .and_then(|image| crate::platform::write_clipboard_image(image, cx)) + { Ok(()) => { this.toast_success( "Screenshot copied to clipboard!", diff --git a/apps/desktop-gpui/src/settings_window.rs b/apps/desktop-gpui/src/settings_window.rs index 37e5b466d03..19b355b8ca2 100644 --- a/apps/desktop-gpui/src/settings_window.rs +++ b/apps/desktop-gpui/src/settings_window.rs @@ -2392,13 +2392,14 @@ impl SettingsWindow { { let png = png.clone(); move |_this, _window, cx| { - if let Err(error) = crate::platform::copy_image_to_clipboard(&png, cx) { - tracing::warn!("copying screenshot failed: {error}"); - cx.spawn(async move |_, _| { + let copied = crate::platform::copy_image_to_clipboard(png.clone(), cx); + cx.spawn(async move |_, _| { + if let Err(error) = copied.await { + tracing::warn!("copying screenshot failed: {error}"); crate::platform::alert_dialog("Could not copy screenshot", &error); - }) - .detach(); - } + } + }) + .detach(); } }, ))