From 1bef48db870227e4a2bae03941d8feaba6c6e266 Mon Sep 17 00:00:00 2001 From: Khushal Bhardwaj Date: Tue, 20 Oct 2026 08:45:07 +0000 Subject: [PATCH 1/2] perf(desktop-gpui): read and decode the screenshot clipboard image off the UI thread --- apps/desktop-gpui/src/app_windows.rs | 12 ++++-- apps/desktop-gpui/src/main_window.rs | 13 ++++--- apps/desktop-gpui/src/platform.rs | 45 +++++++++++++++++----- apps/desktop-gpui/src/platform/windows.rs | 18 ++++++--- apps/desktop-gpui/src/screenshot_editor.rs | 8 +++- apps/desktop-gpui/src/settings_window.rs | 13 ++++--- 6 files changed, 77 insertions(+), 32 deletions(-) diff --git a/apps/desktop-gpui/src/app_windows.rs b/apps/desktop-gpui/src/app_windows.rs index 61619c06965..3a65dfc8a48 100644 --- a/apps/desktop-gpui/src/app_windows.rs +++ b/apps/desktop-gpui/src/app_windows.rs @@ -6549,10 +6549,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 cabac8bdd14..e075602c305 100644 --- a/apps/desktop-gpui/src/main_window.rs +++ b/apps/desktop-gpui/src/main_window.rs @@ -4712,13 +4712,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 6189355ea70..e71377936d9 100644 --- a/apps/desktop-gpui/src/platform.rs +++ b/apps/desktop-gpui/src/platform.rs @@ -558,10 +558,21 @@ 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) +pub fn copy_image_to_clipboard( + path: std::path::PathBuf, + cx: &gpui::App, +) -> gpui::Task> { + 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| write_clipboard_image(image, cx)) + }) } #[cfg(any(not(target_os = "macos"), test))] @@ -2102,7 +2113,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 +2130,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")?; @@ -2820,8 +2837,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, @@ -2830,8 +2849,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 { diff --git a/apps/desktop-gpui/src/platform/windows.rs b/apps/desktop-gpui/src/platform/windows.rs index 60ef53662d6..ae2e823b85c 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 8e123c37854..acac013d4cd 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 52d2a2fd3c7..ef9a410b553 100644 --- a/apps/desktop-gpui/src/settings_window.rs +++ b/apps/desktop-gpui/src/settings_window.rs @@ -2375,13 +2375,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(); } }, )) From 779c03cf044bf257bdf8a2c78064b08ad51c3780 Mon Sep 17 00:00:00 2001 From: Khushal Bhardwaj Date: Wed, 21 Oct 2026 07:16:57 +0000 Subject: [PATCH 2/2] fix(desktop-gpui): let the newest clipboard copy win when prepares finish out of order --- apps/desktop-gpui/src/platform.rs | 81 ++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/apps/desktop-gpui/src/platform.rs b/apps/desktop-gpui/src/platform.rs index e71377936d9..9117c707c78 100644 --- a/apps/desktop-gpui/src/platform.rs +++ b/apps/desktop-gpui/src/platform.rs @@ -558,10 +558,37 @@ fn clipboard_file_path(path: &std::path::Path) -> Result 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() @@ -571,7 +598,13 @@ pub fn copy_image_to_clipboard( prepare_clipboard_image(bytes) }) .await?; - cx.update(|cx| write_clipboard_image(image, cx)) + cx.update(|cx| { + if !CLIPBOARD_WRITES.is_current(id) { + tracing::debug!("skipping a superseded clipboard image write"); + return Ok(()); + } + write_clipboard_image(image, cx) + }) }) } @@ -2959,6 +2992,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";