From cf028dbd3ef1edf1d09027458d208821be7e8976 Mon Sep 17 00:00:00 2001 From: saberoueslati Date: Fri, 17 Jul 2026 09:29:17 +0100 Subject: [PATCH] gpui: Apply EXIF orientation when decoding static images (#61017) ## Context Static images decoded by GPUI ignored their EXIF `Orientation` tag, so JPEGs that rely on it (photos from phones and cameras) rendered rotated or flipped in image preview, Markdown preview, and other `ImageSource` consumers. The `image` crate only exposes orientation through `ImageDecoder::orientation()`, which must be read before the decoder is consumed; `DynamicImage::from_decoder` does not apply it. The old path went straight through `image::load_from_memory_with_format`, so the metadata was never read. Closes #60844. Video of manual test below : [Screencast from 2026-07-15 02-14-41.webm](https://github.com/user-attachments/assets/002b0324-44f8-4ddb-acbd-8c3e070d5248) ## How to Review **Cargo.toml** Raises the workspace `image` dependency minimum from `0.25.1` to `0.25.4`, the first release exposing `orientation()` and `apply_orientation()`. The lockfile already resolves to `0.25.10`, so no lock change is required. **crates/gpui/src/platform.rs** Adds two crate-private helpers. `decode_static_image_from_decoder` reads `orientation()`, decodes to a `DynamicImage`, applies the orientation, then converts RGBA to GPUI's BGRA. `decode_static_image` builds a format decoder from bytes and delegates to it. `Image::to_image_data` now routes its static formats through these helpers instead of `image::load_from_memory_with_format`. Adds `test_image_to_image_data_applies_exif_orientation`, which decodes a 16x32 fixture stored upside down with EXIF Orientation 3 and asserts the corner pixels were flipped upright; the test fails if orientation is dropped. **crates/gpui/src/elements/img.rs** Routes `ImageAssetLoader` (Markdown paths, URLs, embedded resources) through the same helpers, including the static WebP branch. GIF and animated-WebP frame handling are left unchanged, since animated orientation is out of scope. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the UI/UX checklist - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed static images ignoring their EXIF orientation, which caused some JPEGs to render rotated or flipped in image and Markdown preview. --- .../image/exif-orientation-rotate-180.jpg | Bin 0 -> 378 bytes crates/gpui/examples/image/image.rs | 6 +- crates/gpui/src/elements/img.rs | 26 ++---- crates/gpui/src/platform.rs | 75 ++++++++++++------ 4 files changed, 61 insertions(+), 46 deletions(-) create mode 100644 crates/gpui/examples/image/exif-orientation-rotate-180.jpg diff --git a/crates/gpui/examples/image/exif-orientation-rotate-180.jpg b/crates/gpui/examples/image/exif-orientation-rotate-180.jpg new file mode 100644 index 0000000000000000000000000000000000000000..9c095652a1697a63ce5b6eff0526baa4b51bafaf GIT binary patch literal 378 zcmex=M1EUawo9GqO-Km}U`7?>ED znVDFaSy@?FfU0YO@(e73tU`*0j%>n#iR?;+B1Vl97jh^&Z9FI%bn%0VaZ*teCzqJG zgrtbvx}>nyN9P&a7buactm7Wa!P7idPZheaY<=ec|~Pab4zPm zdq-#2q{&mJPMbbs=B!1Fmn>bje8tLDn>KIRx^4T8ox2VlK63Qf@e?OcUAlbb>b2`P zZr*zM=<$=M&z`?{`Re1R&tJZN`~KtSum86gI6y&b&+wnY{+Gt0{r4YrX{`T$698_d BZD;@h literal 0 HcmV?d00001 diff --git a/crates/gpui/examples/image/image.rs b/crates/gpui/examples/image/image.rs index 45fce26d04..688f881bc6 100644 --- a/crates/gpui/examples/image/image.rs +++ b/crates/gpui/examples/image/image.rs @@ -100,7 +100,7 @@ impl Render for ImageShowcase { .items_center() .gap_8() .child(ImageContainer::new( - "Image loaded from a local file", + "Image loaded from a local file with EXIF orientation", self.local_resource.clone(), )) .child(ImageContainer::new( @@ -202,7 +202,9 @@ fn run_example() { cx.open_window(window_options, |_, cx| { cx.new(|_| ImageShowcase { // Relative path to your root project path - local_resource: manifest_dir.join("examples/image/app-icon.png").into(), + local_resource: manifest_dir + .join("examples/image/exif-orientation-rotate-180.jpg") + .into(), remote_resource: "https://picsum.photos/800/400".into(), asset_resource: "image/color.svg".into(), }) diff --git a/crates/gpui/src/elements/img.rs b/crates/gpui/src/elements/img.rs index 4850d895b6..7e92ecba3d 100644 --- a/crates/gpui/src/elements/img.rs +++ b/crates/gpui/src/elements/img.rs @@ -2,14 +2,15 @@ use crate::{ AnyElement, AnyImageCache, App, Asset, AssetLogger, Bounds, DefiniteLength, Element, ElementId, Entity, GlobalElementId, Hitbox, Image, ImageCache, InspectorElementId, InteractiveElement, Interactivity, IntoElement, LayoutId, Length, ObjectFit, Pixels, RenderImage, Resource, - SharedString, SharedUri, StyleRefinement, Styled, Task, Window, px, + SharedString, SharedUri, StyleRefinement, Styled, Task, Window, decode_static_image, + decode_static_image_from_decoder, px, }; use anyhow::Result; use futures::Future; use gpui_util::ResultExt; use image::{ - AnimationDecoder, DynamicImage, Frame, ImageError, ImageFormat, Rgba, + AnimationDecoder, ImageError, ImageFormat, Rgba, codecs::{gif::GifDecoder, webp::WebPDecoder}, }; use scheduler::Instant; @@ -716,27 +717,10 @@ impl Asset for ImageAssetLoader { frames } else { - let mut data = DynamicImage::from_decoder(decoder)?.into_rgba8(); - - // Convert from RGBA to BGRA. - for pixel in data.chunks_exact_mut(4) { - pixel.swap(0, 2); - } - - SmallVec::from_elem(Frame::new(data), 1) + decode_static_image_from_decoder(decoder)? } } - _ => { - let mut data = - image::load_from_memory_with_format(&bytes, format)?.into_rgba8(); - - // Convert from RGBA to BGRA. - for pixel in data.chunks_exact_mut(4) { - pixel.swap(0, 2); - } - - SmallVec::from_elem(Frame::new(data), 1) - } + _ => decode_static_image(&bytes, format)?, }; Ok(Arc::new(RenderImage::new(data))) diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index df7973909c..3caf351fc8 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -42,15 +42,15 @@ use crate::{ RenderImageParams, RenderSvgParams, Scene, ShapedGlyph, ShapedRun, SharedString, Size, SvgRenderer, SystemWindowTab, Task, Window, WindowControlArea, hash, point, px, size, }; -use anyhow::Result; #[cfg(any(target_os = "linux", target_os = "freebsd"))] use anyhow::bail; +use anyhow::{Context as _, Result}; use async_task::Runnable; use futures::channel::oneshot; #[cfg(any(test, feature = "test-support"))] use image::RgbaImage; use image::codecs::gif::GifDecoder; -use image::{AnimationDecoder as _, Frame}; +use image::{AnimationDecoder as _, DynamicImage, Frame}; use raw_window_handle::{HasDisplayHandle, HasWindowHandle}; use scheduler::Instant; pub use scheduler::RunnableMeta; @@ -2368,6 +2368,33 @@ pub struct Image { pub id: u64, } +pub(crate) fn decode_static_image( + bytes: &[u8], + format: image::ImageFormat, +) -> Result> { + let decoder = image::ImageReader::with_format(Cursor::new(bytes), format) + .into_decoder() + .context("creating image decoder")?; + decode_static_image_from_decoder(decoder) +} + +pub(crate) fn decode_static_image_from_decoder( + mut decoder: impl image::ImageDecoder, +) -> Result> { + let orientation = decoder + .orientation() + .context("reading decoder's orientation")?; + let mut image = DynamicImage::from_decoder(decoder).context("decoding image")?; + image.apply_orientation(orientation); + + let mut data = image.into_rgba8(); + for pixel in data.chunks_exact_mut(4) { + pixel.swap(0, 2); + } + + Ok(SmallVec::from_elem(Frame::new(data), 1)) +} + impl Hash for Image { fn hash(&self, state: &mut H) { state.write_u64(self.id); @@ -2423,20 +2450,6 @@ impl Image { /// Convert the clipboard image to an `ImageData` object. pub fn to_image_data(&self, svg_renderer: SvgRenderer) -> Result> { - fn frames_for_image( - bytes: &[u8], - format: image::ImageFormat, - ) -> Result> { - let mut data = image::load_from_memory_with_format(bytes, format)?.into_rgba8(); - - // Convert from RGBA to BGRA. - for pixel in data.chunks_exact_mut(4) { - pixel.swap(0, 2); - } - - Ok(SmallVec::from_elem(Frame::new(data), 1)) - } - let frames = match self.format { ImageFormat::Gif => { let decoder = GifDecoder::new(Cursor::new(&self.bytes))?; @@ -2463,18 +2476,18 @@ impl Image { frames } - ImageFormat::Png => frames_for_image(&self.bytes, image::ImageFormat::Png)?, - ImageFormat::Jpeg => frames_for_image(&self.bytes, image::ImageFormat::Jpeg)?, - ImageFormat::Webp => frames_for_image(&self.bytes, image::ImageFormat::WebP)?, - ImageFormat::Bmp => frames_for_image(&self.bytes, image::ImageFormat::Bmp)?, - ImageFormat::Tiff => frames_for_image(&self.bytes, image::ImageFormat::Tiff)?, - ImageFormat::Ico => frames_for_image(&self.bytes, image::ImageFormat::Ico)?, + ImageFormat::Png => decode_static_image(&self.bytes, image::ImageFormat::Png)?, + ImageFormat::Jpeg => decode_static_image(&self.bytes, image::ImageFormat::Jpeg)?, + ImageFormat::Webp => decode_static_image(&self.bytes, image::ImageFormat::WebP)?, + ImageFormat::Bmp => decode_static_image(&self.bytes, image::ImageFormat::Bmp)?, + ImageFormat::Tiff => decode_static_image(&self.bytes, image::ImageFormat::Tiff)?, + ImageFormat::Ico => decode_static_image(&self.bytes, image::ImageFormat::Ico)?, ImageFormat::Svg => { return svg_renderer .render_single_frame(&self.bytes, 1.0) .map_err(Into::into); } - ImageFormat::Pnm => frames_for_image(&self.bytes, image::ImageFormat::Pnm)?, + ImageFormat::Pnm => decode_static_image(&self.bytes, image::ImageFormat::Pnm)?, }; Ok(Arc::new(RenderImage::new(frames))) @@ -2559,6 +2572,22 @@ mod image_tests { use super::*; use std::sync::Arc; + #[test] + fn test_image_to_image_data_applies_exif_orientation() { + let image = Image::from_bytes( + ImageFormat::Jpeg, + include_bytes!("../examples/image/exif-orientation-rotate-180.jpg").to_vec(), + ); + + let render_image = image.to_image_data(SvgRenderer::new(Arc::new(()))).unwrap(); + + assert_eq!(render_image.size(0), size(16.into(), 32.into())); + + let bytes = render_image.as_bytes(0).unwrap(); + assert_eq!(&bytes[..4], &[255, 255, 255, 255]); + assert_eq!(&bytes[(16 * 32 - 1) * 4..], &[0, 0, 0, 255]); + } + #[test] fn test_svg_image_to_image_data_converts_to_bgra() { let image = Image::from_bytes(