mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
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
This commit is contained in:
committed by
copyberry
parent
7761f0a594
commit
0598a96e93
Generated
+1
@@ -5260,6 +5260,7 @@ dependencies = [
|
||||
"itertools 0.14.0",
|
||||
"lazy_static",
|
||||
"libc",
|
||||
"objc2",
|
||||
"objc2-app-kit",
|
||||
"objc2-foundation",
|
||||
"opentelemetry_sdk",
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<u8>, 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<image::DynamicImage, PasteImageError> {
|
||||
// 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<u8> = 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.
|
||||
|
||||
@@ -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<DynamicImage> {
|
||||
objc2::rc::autoreleasepool(|_| {
|
||||
let items = NSPasteboard::generalPasteboard().pasteboardItems()?;
|
||||
first_image(&items)
|
||||
})
|
||||
}
|
||||
|
||||
fn first_image(items: &NSArray<NSPasteboardItem>) -> Option<DynamicImage> {
|
||||
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<DynamicImage> {
|
||||
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;
|
||||
@@ -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<NSPasteboardItem> {
|
||||
let item = NSPasteboardItem::new();
|
||||
assert!(item.setData_forType(&NSData::with_bytes(bytes), ty));
|
||||
item
|
||||
}
|
||||
|
||||
fn encoded(format: ImageFormat, width: u32) -> Vec<u8> {
|
||||
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))
|
||||
);
|
||||
}
|
||||
@@ -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<impl BufRead + Seek>,
|
||||
mut limits: Limits,
|
||||
) -> ImageResult<DynamicImage> {
|
||||
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;
|
||||
@@ -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<u8>) {
|
||||
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<u8>, 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(_))
|
||||
));
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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()))
|
||||
|
||||
Reference in New Issue
Block a user