perf(desktop-gpui): read and decode the screenshot clipboard image off the UI thread

apps/desktop-gpui/src/app_windows.rs

@@ -6622,10 +6622,14 @@ pub fn screenshot_finished(captured: Option<PathBuf>, 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() {

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();
                     }
                 }),
             )

apps/desktop-gpui/src/platform.rs

@@ -558,10 +558,54 @@ fn clipboard_file_path(path: &std::path::Path) -> Result<std::path::PathBuf, Str
     Ok(path)
 }
 
-pub fn copy_image_to_clipboard(path: &std::path::Path, cx: &gpui::App) -> 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<Result<(), String>> {
+    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<u8>);
+
+    pub fn prepare_clipboard_image(bytes: Vec<u8>) -> Result<ClipboardImage, String> {
+        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<u8>) -> Result<ClipboardImage, String> {
+        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<std::path::PathBuf> {
@@ -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<u64> = threads
+            .into_iter()
+            .map(|thread| thread.join().unwrap())
+            .collect();
+        ids.sort_unstable();
+        ids.dedup();
+        assert_eq!(ids, (1..=8).collect::<Vec<_>>());
+        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";

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<u8>) -> Result<ClipboardImage, String> {
+    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}"))
 }
 

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!",

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();
                     }
                 },
             ))