From 120bbf46c1f659a6009ef80e7426c6b7cb9062af Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 17 Apr 2026 16:31:03 -0700 Subject: [PATCH] Update image resizing to fit 2048 square bounds (#18384) We don't have to downsize to 768 height. --- codex-rs/core/tests/suite/view_image.rs | 61 +++++++++--------- codex-rs/utils/image/src/lib.rs | 82 +++++++++++++++---------- 2 files changed, 79 insertions(+), 64 deletions(-) diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index f9d6d9402..47254ab17 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -1,5 +1,6 @@ #![cfg(not(target_os = "windows"))] +use anyhow::Context; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_exec_server::CreateDirectoryOptions; @@ -127,10 +128,10 @@ async fn write_workspace_png( write_workspace_file(test, rel_path, png_bytes(width, height, rgba)?).await } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { - skip_if_no_network!(Ok(())); - +async fn assert_user_turn_local_image_resizes_to( + original_dimensions: (u32, u32), + expected_dimensions: (u32, u32), +) -> anyhow::Result<()> { let server = start_mock_server().await; let mut builder = test_codex(); @@ -142,8 +143,7 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { .. } = &test; - let original_width = 2304; - let original_height = 864; + let (original_width, original_height) = original_dimensions; let local_image_dir = tempfile::tempdir()?; let abs_path = local_image_dir.path().join("example.png"); let image = ImageBuffer::from_pixel(original_width, original_height, Rgba([20u8, 40, 60, 255])); @@ -187,7 +187,7 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { let body = mock.single_request().body_json(); let image_message = - find_image_message(&body).expect("pending input image message not included in request"); + find_image_message(&body).context("pending input image message not included in request")?; let image_url = image_message .get("content") .and_then(Value::as_array) @@ -200,26 +200,37 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { } }) }) - .expect("image_url present"); + .context("image_url present")?; let (prefix, encoded) = image_url .split_once(',') - .expect("image url contains data prefix"); + .context("image url contains data prefix")?; assert_eq!(prefix, "data:image/png;base64"); let decoded = BASE64_STANDARD .decode(encoded) - .expect("image data decodes from base64 for request"); - let resized = load_from_memory(&decoded).expect("load resized image"); + .context("image data decodes from base64 for request")?; + let resized = load_from_memory(&decoded).context("load resized image")?; let (width, height) = resized.dimensions(); - assert!(width <= 2048); - assert!(height <= 768); - assert!(width < original_width); - assert!(height < original_height); + assert_eq!((width, height), expected_dimensions); Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + assert_user_turn_local_image_resizes_to((2304, 864), (2048, 768)).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_turn_with_vertical_local_image_resizes_to_square_bounds() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + assert_user_turn_local_image_resizes_to((1024, 4096), (512, 2048)).await +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); @@ -347,10 +358,7 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { .expect("image data decodes from base64 for request"); let resized = load_from_memory(&decoded).expect("load resized image"); let (resized_width, resized_height) = resized.dimensions(); - assert!(resized_width <= 2048); - assert!(resized_height <= 768); - assert!(resized_width < original_width); - assert!(resized_height < original_height); + assert_eq!((resized_width, resized_height), (2048, 768)); Ok(()) } @@ -637,10 +645,7 @@ async fn view_image_tool_treats_null_detail_as_omitted() -> anyhow::Result<()> { .expect("image data decodes from base64 for request"); let resized = load_from_memory(&decoded).expect("load resized image"); let (width, height) = resized.dimensions(); - assert!(width <= 2048); - assert!(height <= 768); - assert!(width < original_width); - assert!(height < original_height); + assert_eq!((width, height), (2048, 768)); Ok(()) } @@ -740,10 +745,7 @@ async fn view_image_tool_resizes_when_model_lacks_original_detail_support() -> a .expect("image data decodes from base64 for request"); let resized = load_from_memory(&decoded).expect("load resized image"); let (resized_width, resized_height) = resized.dimensions(); - assert!(resized_width <= 2048); - assert!(resized_height <= 768); - assert!(resized_width < original_width); - assert!(resized_height < original_height); + assert_eq!((resized_width, resized_height), (2048, 768)); Ok(()) } @@ -841,10 +843,7 @@ async fn view_image_tool_does_not_force_original_resolution_with_capability_only .expect("image data decodes from base64 for request"); let resized = load_from_memory(&decoded).expect("load resized image"); let (resized_width, resized_height) = resized.dimensions(); - assert!(resized_width <= 2048); - assert!(resized_height <= 768); - assert!(resized_width < original_width); - assert!(resized_height < original_height); + assert_eq!((resized_width, resized_height), (2048, 768)); Ok(()) } diff --git a/codex-rs/utils/image/src/lib.rs b/codex-rs/utils/image/src/lib.rs index 0c0a026e4..a3f595a18 100644 --- a/codex-rs/utils/image/src/lib.rs +++ b/codex-rs/utils/image/src/lib.rs @@ -15,10 +15,8 @@ use image::codecs::jpeg::JpegEncoder; use image::codecs::png::PngEncoder; use image::codecs::webp::WebPEncoder; use image::imageops::FilterType; -/// Maximum width used when resizing images before uploading. -pub const MAX_WIDTH: u32 = 2048; -/// Maximum height used when resizing images before uploading. -pub const MAX_HEIGHT: u32 = 768; +/// Maximum width or height used when resizing images before uploading. +pub const MAX_DIMENSION: u32 = 2048; pub mod error; @@ -80,40 +78,41 @@ pub fn load_for_prompt_bytes( let (width, height) = dynamic.dimensions(); - let encoded = - if mode == PromptImageMode::Original || (width <= MAX_WIDTH && height <= MAX_HEIGHT) { - if let Some(format) = format.filter(|format| can_preserve_source_bytes(*format)) { - let mime = format_to_mime(format); - EncodedImage { - bytes: file_bytes, - mime, - width, - height, - } - } else { - let (bytes, output_format) = encode_image(&dynamic, ImageFormat::Png)?; - let mime = format_to_mime(output_format); - EncodedImage { - bytes, - mime, - width, - height, - } + let encoded = if mode == PromptImageMode::Original + || (width <= MAX_DIMENSION && height <= MAX_DIMENSION) + { + if let Some(format) = format.filter(|format| can_preserve_source_bytes(*format)) { + let mime = format_to_mime(format); + EncodedImage { + bytes: file_bytes, + mime, + width, + height, } } else { - let resized = dynamic.resize(MAX_WIDTH, MAX_HEIGHT, FilterType::Triangle); - let target_format = format - .filter(|format| can_preserve_source_bytes(*format)) - .unwrap_or(ImageFormat::Png); - let (bytes, output_format) = encode_image(&resized, target_format)?; + let (bytes, output_format) = encode_image(&dynamic, ImageFormat::Png)?; let mime = format_to_mime(output_format); EncodedImage { bytes, mime, - width: resized.width(), - height: resized.height(), + width, + height, } - }; + } + } else { + let resized = dynamic.resize(MAX_DIMENSION, MAX_DIMENSION, FilterType::Triangle); + let target_format = format + .filter(|format| can_preserve_source_bytes(*format)) + .unwrap_or(ImageFormat::Png); + let (bytes, output_format) = encode_image(&resized, target_format)?; + let mime = format_to_mime(output_format); + EncodedImage { + bytes, + mime, + width: resized.width(), + height: resized.height(), + } + }; Ok(encoded) }) @@ -251,8 +250,8 @@ mod tests { ) .expect("process image"); - assert!(processed.width <= MAX_WIDTH); - assert!(processed.height <= MAX_HEIGHT); + assert!(processed.width <= MAX_DIMENSION); + assert!(processed.height <= MAX_DIMENSION); assert_eq!(processed.mime, mime); let detected_format = @@ -265,6 +264,23 @@ mod tests { } } + #[tokio::test(flavor = "multi_thread")] + async fn downscales_tall_image_to_fit_square_bounds() { + let image = ImageBuffer::from_pixel(1024, 4096, Rgba([200u8, 10, 10, 255])); + let original_bytes = image_bytes(&image, ImageFormat::Png); + + let processed = load_for_prompt_bytes( + Path::new("in-memory-image"), + original_bytes, + PromptImageMode::ResizeToFit, + ) + .expect("process image"); + + assert_eq!(processed.width, 512); + assert_eq!(processed.height, MAX_DIMENSION); + assert_eq!(processed.mime, "image/png"); + } + #[tokio::test(flavor = "multi_thread")] async fn preserves_large_image_in_original_mode() { let image = ImageBuffer::from_pixel(4096, 2048, Rgba([180u8, 30, 30, 255]));