From 0598a96e9361485fd4cd02f9c287aca627ad6ee1 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Mon, 9 Nov 2026 20:59:01 +0000 Subject: [PATCH] Prefer native clipboard images on macOS and honor image orientation (#60455) ## Why The macOS clipboard reader requests TIFF even when PNG or JPEG data is available, causing clipboard owners to generate a large TIFF just for a paste. Pasted image files also need their orientation metadata applied before encoding attachments. ## What changed - Read native macOS pasteboard representations before falling back to `arboard`, preferring PNG within each item while respecting item order and JPEG/TIFF ordering. - Keep copied image files ahead of clipboard previews and apply orientation metadata when decoding files and native PNG/JPEG data. - Share orientation-aware decoding with transcript thumbnails while preserving allocation limits. ## Testing Add tests for pasteboard item ordering, all EXIF orientations in PNG and JPEG images, and rejection of images exceeding the allocation budget. GitOrigin-RevId: 088f37a3205e49c805a09a84bbbb00b74dd6cb78 --- codex-rs/Cargo.lock | 1 + codex-rs/tui/Cargo.toml | 5 +- codex-rs/tui/src/clipboard_paste.rs | 106 +++++++++--------- codex-rs/tui/src/clipboard_paste/macos.rs | 65 +++++++++++ .../tui/src/clipboard_paste/macos_tests.rs | 50 +++++++++ codex-rs/tui/src/image_decode.rs | 31 +++++ codex-rs/tui/src/image_decode_tests.rs | 76 +++++++++++++ codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/transcript_image.rs | 13 +-- 9 files changed, 281 insertions(+), 67 deletions(-) create mode 100644 codex-rs/tui/src/clipboard_paste/macos.rs create mode 100644 codex-rs/tui/src/clipboard_paste/macos_tests.rs create mode 100644 codex-rs/tui/src/image_decode.rs create mode 100644 codex-rs/tui/src/image_decode_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 68439ee505..7d3b08af27 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -5260,6 +5260,7 @@ dependencies = [ "itertools 0.14.0", "lazy_static", "libc", + "objc2", "objc2-app-kit", "objc2-foundation", "opentelemetry_sdk", diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 913ee8f8cb..19d2e1a7b2 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -144,8 +144,9 @@ libc = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.10" -objc2-app-kit = { version = "0.3.2", default-features = false, features = ["std", "NSAccessibility", "NSWorkspace"] } -objc2-foundation = { version = "0.3.2", default-features = false, features = ["std", "NSDateFormatter", "NSFormatter", "NSLocale", "NSString"] } +objc2 = "0.6.3" +objc2-app-kit = { version = "0.3.2", default-features = false, features = ["std", "NSAccessibility", "NSPasteboard", "NSPasteboardItem", "NSWorkspace"] } +objc2-foundation = { version = "0.3.2", default-features = false, features = ["std", "NSArray", "NSData", "NSDateFormatter", "NSEnumerator", "NSFormatter", "NSLocale", "NSString"] } [target.'cfg(target_os = "linux")'.dependencies] # Keep the same executor features as the credential store, which also uses zbus. diff --git a/codex-rs/tui/src/clipboard_paste.rs b/codex-rs/tui/src/clipboard_paste.rs index ff291801ea..d362d69b54 100644 --- a/codex-rs/tui/src/clipboard_paste.rs +++ b/codex-rs/tui/src/clipboard_paste.rs @@ -1,5 +1,7 @@ //! Read clipboard images and text on the TUI host. Text reads run on the clipboard worker. +#[cfg(target_os = "macos")] +mod macos; pub(crate) mod text; use std::path::Path; @@ -50,70 +52,66 @@ pub struct PastedImageInfo { pub encoded_format: EncodedImageFormat, // Always PNG for now. } -/// Capture image from system clipboard, encode to PNG, and return bytes + info. +/// Read the first clipboard image and encode it as a PNG attachment. #[cfg(not(target_os = "android"))] pub fn paste_image_as_png() -> Result<(Vec, PastedImageInfo), PasteImageError> { let _span = tracing::debug_span!("paste_image_as_png").entered(); tracing::debug!("attempting clipboard image read"); - let mut cb = arboard::Clipboard::new() + let mut clipboard = arboard::Clipboard::new() .map_err(|e| PasteImageError::ClipboardUnavailable(e.to_string()))?; - // Sometimes images on the clipboard come as files (e.g. when copy/pasting from - // Finder), sometimes they come as image data (e.g. when pasting from Chrome). - // Accept both, and prefer files if both are present. - let files = cb - .get() - .file_list() - .map_err(|e| PasteImageError::ClipboardUnavailable(e.to_string())); - let dyn_img = if let Some(img) = files - .unwrap_or_default() - .into_iter() - .find_map(|f| image::open(f).ok()) - { + let image = read_clipboard_image(&mut clipboard)?; + + let span = tracing::debug_span!("encode_image", byte_length = tracing::field::Empty).entered(); + let mut png = std::io::Cursor::new(Vec::new()); + image + .write_to(&mut png, image::ImageFormat::Png) + .map_err(|e| PasteImageError::EncodeFailed(e.to_string()))?; + span.record("byte_length", png.get_ref().len()); + let info = PastedImageInfo { + width: image.width(), + height: image.height(), + encoded_format: EncodedImageFormat::Png, + }; + Ok((png.into_inner(), info)) +} + +#[cfg(not(target_os = "android"))] +fn read_clipboard_image( + clipboard: &mut arboard::Clipboard, +) -> Result { + // A copied file takes precedence over its clipboard preview. + let files = clipboard.get().file_list().unwrap_or_default(); + if let Some(image) = files.into_iter().find_map(|path| { + let reader = image::ImageReader::open(path).ok()?; + crate::image_decode::decode_oriented(reader, image::Limits::default()).ok() + }) { tracing::debug!( "clipboard image opened from file: {}x{}", - img.width(), - img.height() + image.width(), + image.height() ); - img - } else { - let _span = tracing::debug_span!("get_image").entered(); - let img = cb.get_image().map_err(|e| { - if cfg!(windows) && !matches!(e, arboard::Error::ContentNotAvailable) { - PasteImageError::ClipboardUnavailable(e.to_string()) - } else { - PasteImageError::NoImage(e.to_string()) - } - })?; - let w = img.width as u32; - let h = img.height as u32; - tracing::debug!("clipboard image opened from image: {}x{}", w, h); - - let Some(rgba_img) = image::RgbaImage::from_raw(w, h, img.bytes.into_owned()) else { - return Err(PasteImageError::EncodeFailed("invalid RGBA buffer".into())); - }; - - image::DynamicImage::ImageRgba8(rgba_img) - }; - - let mut png: Vec = Vec::new(); - { - let span = - tracing::debug_span!("encode_image", byte_length = tracing::field::Empty).entered(); - let mut cursor = std::io::Cursor::new(&mut png); - dyn_img - .write_to(&mut cursor, image::ImageFormat::Png) - .map_err(|e| PasteImageError::EncodeFailed(e.to_string()))?; - span.record("byte_length", png.len()); + return Ok(image); } - Ok(( - png, - PastedImageInfo { - width: dyn_img.width(), - height: dyn_img.height(), - encoded_format: EncodedImageFormat::Png, - }, - )) + #[cfg(target_os = "macos")] + if let Some(image) = macos::clipboard_image() { + return Ok(image); + } + + let _span = tracing::debug_span!("get_image").entered(); + let bitmap = clipboard.get_image().map_err(|e| { + if cfg!(windows) && !matches!(e, arboard::Error::ContentNotAvailable) { + PasteImageError::ClipboardUnavailable(e.to_string()) + } else { + PasteImageError::NoImage(e.to_string()) + } + })?; + let width = bitmap.width as u32; + let height = bitmap.height as u32; + tracing::debug!("clipboard image opened from image: {width}x{height}"); + let rgba = image::RgbaImage::from_raw(width, height, bitmap.bytes.into_owned()) + .ok_or_else(|| PasteImageError::EncodeFailed("invalid RGBA buffer".into()))?; + Ok(image::DynamicImage::ImageRgba8(rgba)) } /// Android/Termux does not support arboard; return a clear error. diff --git a/codex-rs/tui/src/clipboard_paste/macos.rs b/codex-rs/tui/src/clipboard_paste/macos.rs new file mode 100644 index 0000000000..d4c310643e --- /dev/null +++ b/codex-rs/tui/src/clipboard_paste/macos.rs @@ -0,0 +1,65 @@ +//! Read native image representations in macOS pasteboard item order. + +use crate::image_decode::decode_oriented; +use image::DynamicImage; +use image::ImageFormat; +use image::ImageReader; +use image::Limits; +use objc2_app_kit::NSPasteboard; +use objc2_app_kit::NSPasteboardItem; +use objc2_foundation::NSArray; +use objc2_foundation::NSString; +use objc2_foundation::ns_string; +use std::io::Cursor; + +/// Decode an offered PNG or JPEG before falling back to [arboard's macOS image reader]. +/// That reader requests TIFF even when the pasteboard already holds the original PNG or +/// JPEG. Clipboard owners such as Vicinae then generate a large TIFF just for this paste. +/// +/// [arboard's macOS image reader]: https://github.com/1Password/arboard/blob/a3750c79a5d63f3987317e03c412b7d9dffdc2af/src/platform/osx.rs#L217-L231 +pub(super) fn clipboard_image() -> Option { + objc2::rc::autoreleasepool(|_| { + let items = NSPasteboard::generalPasteboard().pasteboardItems()?; + first_image(&items) + }) +} + +fn first_image(items: &NSArray) -> Option { + let png = ns_string!("public.png"); + for item in items { + let types = item.types(); + // Prefer lossless PNG, even when it isn't the first representation advertised. + if types.containsObject(png) + && let Some(image) = decode_representation(&item, png, ImageFormat::Png) + { + return Some(image); + } + + // For JPEG and TIFF, respect the owner's order. arboard reads the first TIFF. + for ty in types { + if *ty == *ns_string!("public.tiff") { + return None; + } + if *ty == *ns_string!("public.jpeg") + && let Some(image) = decode_representation(&item, &ty, ImageFormat::Jpeg) + { + return Some(image); + } + } + } + None +} + +fn decode_representation( + item: &NSPasteboardItem, + ty: &NSString, + format: ImageFormat, +) -> Option { + let bytes = item.dataForType(ty)?.to_vec(); + let reader = ImageReader::with_format(Cursor::new(bytes), format); + decode_oriented(reader, Limits::default()).ok() +} + +#[cfg(test)] +#[path = "macos_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/clipboard_paste/macos_tests.rs b/codex-rs/tui/src/clipboard_paste/macos_tests.rs new file mode 100644 index 0000000000..1faffd553f --- /dev/null +++ b/codex-rs/tui/src/clipboard_paste/macos_tests.rs @@ -0,0 +1,50 @@ +#![cfg(test)] + +use super::*; +use objc2::rc::Retained; +use objc2_foundation::NSData; +use objc2_foundation::NSString; +use pretty_assertions::assert_eq; + +fn item(ty: &NSString, bytes: &[u8]) -> Retained { + let item = NSPasteboardItem::new(); + assert!(item.setData_forType(&NSData::with_bytes(bytes), ty)); + item +} + +fn encoded(format: ImageFormat, width: u32) -> Vec { + let mut bytes = Cursor::new(Vec::new()); + DynamicImage::new_rgb8(width, /*height*/ 1) + .write_to(&mut bytes, format) + .unwrap(); + bytes.into_inner() +} + +#[test] +fn tiff_item_precedes_later_png() { + let tiff = item(ns_string!("public.tiff"), b"owned by arboard"); + let png = item( + ns_string!("public.png"), + &encoded(ImageFormat::Png, /*width*/ 2), + ); + let items = NSArray::from_retained_slice(&[tiff, png]); + assert_eq!(first_image(&items), None); +} + +#[test] +fn jpeg_item_precedes_later_png() { + let text = item(ns_string!("public.utf8-plain-text"), b"text"); + let jpeg = item( + ns_string!("public.jpeg"), + &encoded(ImageFormat::Jpeg, /*width*/ 3), + ); + let png = item( + ns_string!("public.png"), + &encoded(ImageFormat::Png, /*width*/ 2), + ); + let items = NSArray::from_retained_slice(&[text, jpeg, png]); + assert_eq!( + first_image(&items), + Some(DynamicImage::new_rgb8(/*width*/ 3, /*height*/ 1)) + ); +} diff --git a/codex-rs/tui/src/image_decode.rs b/codex-rs/tui/src/image_decode.rs new file mode 100644 index 0000000000..48fd044359 --- /dev/null +++ b/codex-rs/tui/src/image_decode.rs @@ -0,0 +1,31 @@ +//! Decode image pixels in their display orientation within the caller's allocation limits. + +use image::DynamicImage; +use image::ImageDecoder; +use image::ImageReader; +use image::ImageResult; +use image::Limits; +use std::io::BufRead; +use std::io::Seek; + +pub(crate) fn decode_oriented( + mut reader: ImageReader, + mut limits: Limits, +) -> ImageResult { + reader.limits(limits.clone()); + let mut decoder = reader.into_decoder()?; + // Match ImageReader::decode's allocation check before allocating the pixel buffer. + limits.reserve(decoder.total_bytes())?; + decoder.set_limits(limits)?; + + let orientation = decoder.orientation().ok(); + let mut image = DynamicImage::from_decoder(decoder)?; + if let Some(orientation) = orientation { + image.apply_orientation(orientation); + } + Ok(image) +} + +#[cfg(test)] +#[path = "image_decode_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/image_decode_tests.rs b/codex-rs/tui/src/image_decode_tests.rs new file mode 100644 index 0000000000..9ef8de6d48 --- /dev/null +++ b/codex-rs/tui/src/image_decode_tests.rs @@ -0,0 +1,76 @@ +#![cfg(test)] + +use super::decode_oriented; +use image::DynamicImage; +use image::ImageEncoder; +use image::ImageFormat; +use image::ImageReader; +use image::Limits; +use image::codecs::jpeg::JpegEncoder; +use image::codecs::png::PngEncoder; +use pretty_assertions::assert_eq; +use std::io::Cursor; + +fn write_fixture(mut encoder: impl ImageEncoder, orientation: Option) { + if let Some(orientation) = orientation { + // A little-endian EXIF directory with a single orientation entry. + let mut exif = b"II\x2a\0\x08\0\0\0\x01\0\x12\x01\x03\0\x01\0\0\0".to_vec(); + exif.extend_from_slice(&[orientation, 0, 0, 0, 0, 0, 0, 0]); + encoder.set_exif_metadata(exif).unwrap(); + } + encoder + .write_image( + &[0, 40, 80, 120, 160, 200], + /*width*/ 3, + /*height*/ 2, + image::ExtendedColorType::L8, + ) + .unwrap(); +} + +fn check_orientation(orientation: Option, display: fn(&DynamicImage) -> DynamicImage) { + let mut png = Vec::new(); + write_fixture(PngEncoder::new(&mut png), orientation); + let mut jpeg = Vec::new(); + write_fixture(JpegEncoder::new(&mut jpeg), orientation); + + for (data, format) in [(png, ImageFormat::Png), (jpeg, ImageFormat::Jpeg)] { + let raw = image::load_from_memory_with_format(&data, format).unwrap(); + let actual = decode_oriented( + ImageReader::with_format(Cursor::new(data), format), + Limits::default(), + ) + .unwrap(); + assert_eq!( + actual.to_rgb8(), + display(&raw).to_rgb8(), + "{format:?} {orientation:?}" + ); + } +} + +#[test] +fn decodes_images_in_display_orientation() { + check_orientation(/*orientation*/ None, DynamicImage::clone); + check_orientation(Some(1), DynamicImage::clone); + check_orientation(Some(2), DynamicImage::fliph); + check_orientation(Some(3), DynamicImage::rotate180); + check_orientation(Some(4), DynamicImage::flipv); + check_orientation(Some(5), |image| image.rotate90().fliph()); + check_orientation(Some(6), DynamicImage::rotate90); + check_orientation(Some(7), |image| image.rotate270().fliph()); + check_orientation(Some(8), DynamicImage::rotate270); +} + +#[test] +fn rejects_images_larger_than_the_pixel_budget() { + let mut jpeg = Vec::new(); + write_fixture(JpegEncoder::new(&mut jpeg), /*orientation*/ None); + let reader = ImageReader::with_format(Cursor::new(jpeg), ImageFormat::Jpeg); + let mut limits = Limits::default(); + limits.max_alloc = Some(5); // The fixture has six grayscale pixels. + assert!(matches!( + decode_oriented(reader, limits), + Err(image::ImageError::Limits(_)) + )); +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 115f0835c2..2d124a4f3b 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -136,6 +136,7 @@ mod copy_input_guard; pub(crate) mod custom_terminal; mod daybreak; mod experimental_features; +mod image_decode; mod markdown_copy; mod permission_discovery; mod pets; diff --git a/codex-rs/tui/src/transcript_image.rs b/codex-rs/tui/src/transcript_image.rs index 0e141e9fda..4a6d6950f1 100644 --- a/codex-rs/tui/src/transcript_image.rs +++ b/codex-rs/tui/src/transcript_image.rs @@ -15,7 +15,6 @@ use crossterm::cursor::RestorePosition; use crossterm::cursor::SavePosition; use crossterm::queue; use image::DynamicImage; -use image::ImageDecoder; use image::ImageFormat; use image::RgbaImage; use ratatui::layout::Rect; @@ -80,7 +79,7 @@ impl TranscriptImage { if max_width == 0 || !metadata.is_file() || metadata.len() > 32 * 1024 * 1024 { return None; } - let mut reader = image::ImageReader::open(path) + let reader = image::ImageReader::open(path) .ok()? .with_guessed_format() .ok()?; @@ -88,15 +87,7 @@ impl TranscriptImage { // screenshots usable, but never allocate hundreds of MB for an optional thumbnail. let mut limits = image::Limits::default(); limits.max_alloc = Some(32 * 1024 * 1024); - reader.limits(limits.clone()); - let mut decoder = reader.into_decoder().ok()?; - limits.reserve(decoder.total_bytes()).ok()?; - decoder.set_limits(limits).ok()?; - let orientation = decoder.orientation().ok(); - let mut image = DynamicImage::from_decoder(decoder).ok()?; - if let Some(orientation) = orientation { - image.apply_orientation(orientation); - } + let image = crate::image_decode::decode_oriented(reader, limits).ok()?; Some( image .thumbnail(max_width.min(image.width()), max_height.min(image.height()))