agent: Do not decode images during render (#56866)

Turns out we were creating an ImageDecoder on every frame (added in
#46167) when a tool returned an image as output, because we were trying
to get its dimensions. That is now cached on `ContentBlock::Image`.

Release Notes:

- N/A

crates/acp_thread/Cargo.toml

@@ -13,7 +13,7 @@ path = "src/acp_thread.rs"
 doctest = false
 
 [features]
-test-support = ["gpui/test-support", "project/test-support", "dep:parking_lot", "dep:image"]
+test-support = ["gpui/test-support", "project/test-support", "dep:parking_lot"]
 
 [dependencies]
 action_log.workspace = true
@@ -35,7 +35,7 @@ language_model.workspace = true
 log.workspace = true
 markdown.workspace = true
 parking_lot = { workspace = true, optional = true }
-image = { workspace = true, optional = true }
+image.workspace = true
 portable-pty.workspace = true
 project.workspace = true
 prompt_store.workspace = true

crates/acp_thread/src/acp_thread.rs

@@ -648,9 +648,16 @@ impl Display for ToolCallStatus {
 #[derive(Debug, PartialEq, Clone)]
 pub enum ContentBlock {
     Empty,
-    Markdown { markdown: Entity<Markdown> },
+    Markdown {
-    ResourceLink { resource_link: acp::ResourceLink },
+        markdown: Entity<Markdown>,
-    Image { image: Arc<gpui::Image> },
+    },
+    ResourceLink {
+        resource_link: acp::ResourceLink,
+    },
+    Image {
+        image: Arc<gpui::Image>,
+        dimensions: Option<gpui::Size<u32>>,
+    },
 }
 
 impl ContentBlock {
@@ -692,8 +699,8 @@ impl ContentBlock {
                 };
             }
             (ContentBlock::Empty, acp::ContentBlock::Image(image_content)) => {
-                if let Some(image) = Self::decode_image(image_content) {
+                if let Some((image, dimensions)) = Self::decode_image(image_content) {
-                    *self = ContentBlock::Image { image };
+                    *self = ContentBlock::Image { image, dimensions };
                 } else {
                     let new_content = Self::image_md(image_content);
                     *self = Self::create_markdown_block(new_content, language_registry, cx);
@@ -721,14 +728,36 @@ impl ContentBlock {
         }
     }
 
-    fn decode_image(image_content: &acp::ImageContent) -> Option<Arc<gpui::Image>> {
+    fn decode_image(
+        image_content: &acp::ImageContent,
+    ) -> Option<(Arc<gpui::Image>, Option<gpui::Size<u32>>)> {
         use base64::Engine as _;
 
         let bytes = base64::engine::general_purpose::STANDARD
             .decode(image_content.data.as_bytes())
             .ok()?;
         let format = gpui::ImageFormat::from_mime_type(&image_content.mime_type)?;
-        Some(Arc::new(gpui::Image::from_bytes(format, bytes)))
+        let dimensions = Self::image_dimensions(&bytes, format);
+        Some((Arc::new(gpui::Image::from_bytes(format, bytes)), dimensions))
+    }
+
+    fn image_dimensions(bytes: &[u8], format: gpui::ImageFormat) -> Option<gpui::Size<u32>> {
+        let format = match format {
+            gpui::ImageFormat::Png => image::ImageFormat::Png,
+            gpui::ImageFormat::Jpeg => image::ImageFormat::Jpeg,
+            gpui::ImageFormat::Webp => image::ImageFormat::WebP,
+            gpui::ImageFormat::Gif => image::ImageFormat::Gif,
+            gpui::ImageFormat::Svg => return None,
+            gpui::ImageFormat::Bmp => image::ImageFormat::Bmp,
+            gpui::ImageFormat::Tiff => image::ImageFormat::Tiff,
+            gpui::ImageFormat::Ico => image::ImageFormat::Ico,
+            gpui::ImageFormat::Pnm => image::ImageFormat::Pnm,
+        };
+
+        image::ImageReader::with_format(std::io::Cursor::new(bytes), format)
+            .into_dimensions()
+            .ok()
+            .map(|(width, height)| gpui::Size { width, height })
     }
 
     fn create_markdown_block(
@@ -808,9 +837,9 @@ impl ContentBlock {
         }
     }
 
-    pub fn image(&self) -> Option<&Arc<gpui::Image>> {
+    pub fn image(&self) -> Option<(&Arc<gpui::Image>, Option<gpui::Size<u32>>)> {
         match self {
-            ContentBlock::Image { image } => Some(image),
+            ContentBlock::Image { image, dimensions } => Some((image, *dimensions)),
             _ => None,
         }
     }
@@ -895,7 +924,7 @@ impl ToolCallContent {
         }
     }
 
-    pub fn image(&self) -> Option<&Arc<gpui::Image>> {
+    pub fn image(&self) -> Option<(&Arc<gpui::Image>, Option<gpui::Size<u32>>)> {
         match self {
             Self::ContentBlock(content) => content.image(),
             _ => None,

crates/agent_ui/src/conversation_view/thread_view.rs

@@ -6446,7 +6446,6 @@ impl ThreadView {
                                         content_ix,
                                         tool_call,
                                         use_card_layout,
-                                        has_image_content,
                                         failed_or_canceled,
                                         focus_handle,
                                         window,
@@ -6578,7 +6577,6 @@ impl ThreadView {
                                         content_ix,
                                         tool_call,
                                         use_card_layout,
-                                        has_image_content,
                                         failed_or_canceled,
                                         focus_handle,
                                         window,
@@ -7570,7 +7568,6 @@ impl ThreadView {
         context_ix: usize,
         tool_call: &ToolCall,
         card_layout: bool,
-        is_image_tool_call: bool,
         has_failed: bool,
         focus_handle: &FocusHandle,
         window: &Window,
@@ -7589,14 +7586,14 @@ impl ThreadView {
                         window,
                         cx,
                     )
-                } else if let Some(image) = content.image() {
+                } else if let Some((image, dimensions)) = content.image() {
                     let location = tool_call.locations.first().cloned();
                     self.render_image_output(
                         entry_ix,
                         image.clone(),
+                        dimensions,
                         location,
                         card_layout,
-                        is_image_tool_call,
                         cx,
                     )
                 } else {
@@ -7778,30 +7775,26 @@ impl ThreadView {
         &self,
         entry_ix: usize,
         image: Arc<gpui::Image>,
+        dimensions: Option<gpui::Size<u32>>,
         location: Option<acp::ToolCallLocation>,
         card_layout: bool,
-        show_dimensions: bool,
         cx: &Context<Self>,
     ) -> AnyElement {
-        let dimensions_label = if show_dimensions {
+        let format_name = match image.format() {
-            let format_name = match image.format() {
+            gpui::ImageFormat::Png => "PNG",
-                gpui::ImageFormat::Png => "PNG",
+            gpui::ImageFormat::Jpeg => "JPEG",
-                gpui::ImageFormat::Jpeg => "JPEG",
+            gpui::ImageFormat::Webp => "WebP",
-                gpui::ImageFormat::Webp => "WebP",
+            gpui::ImageFormat::Gif => "GIF",
-                gpui::ImageFormat::Gif => "GIF",
+            gpui::ImageFormat::Svg => "SVG",
-                gpui::ImageFormat::Svg => "SVG",
+            gpui::ImageFormat::Bmp => "BMP",
-                gpui::ImageFormat::Bmp => "BMP",
+            gpui::ImageFormat::Tiff => "TIFF",
-                gpui::ImageFormat::Tiff => "TIFF",
+            gpui::ImageFormat::Ico => "ICO",
-                gpui::ImageFormat::Ico => "ICO",
+            gpui::ImageFormat::Pnm => "PNM",
-                gpui::ImageFormat::Pnm => "PNM",
+        };
-            };
+        let dimensions_label = if let Some(size) = dimensions {
-            let dimensions = image::ImageReader::new(std::io::Cursor::new(image.bytes()))
+            format!("{}×{} {}", size.width, size.height, format_name)
-                .with_guessed_format()
-                .ok()
-                .and_then(|reader| reader.into_dimensions().ok());
-            dimensions.map(|(w, h)| format!("{}×{} {}", w, h, format_name))
         } else {
-            None
+            format_name.into()
         };
 
         v_flex()
@@ -7816,29 +7809,27 @@ impl ThreadView {
                         .border_color(self.tool_card_border_color(cx))
                 }
             })
-            .when(dimensions_label.is_some() || location.is_some(), |this| {
+            .child(
-                this.child(
+                h_flex()
-                    h_flex()
+                    .w_full()
-                        .w_full()
+                    .justify_between()
-                        .justify_between()
+                    .items_center()
-                        .items_center()
+                    .child(
-                        .children(dimensions_label.map(|label| {
+                        Label::new(dimensions_label)
-                            Label::new(label)
+                            .size(LabelSize::XSmall)
-                                .size(LabelSize::XSmall)
+                            .color(Color::Muted)
-                                .color(Color::Muted)
+                            .buffer_font(cx),
-                                .buffer_font(cx)
+                    )
-                        }))
+                    .when_some(location, |this, _loc| {
-                        .when_some(location, |this, _loc| {
+                        this.child(
-                            this.child(
+                            Button::new(("go-to-file", entry_ix), "Go to File")
-                                Button::new(("go-to-file", entry_ix), "Go to File")
+                                .label_size(LabelSize::Small)
-                                    .label_size(LabelSize::Small)
+                                .on_click(cx.listener(move |this, _, window, cx| {
-                                    .on_click(cx.listener(move |this, _, window, cx| {
+                                    this.open_tool_call_location(entry_ix, 0, window, cx);
-                                        this.open_tool_call_location(entry_ix, 0, window, cx);
+                                })),
-                                    })),
+                        )
-                            )
+                    }),
-                        }),
+            )
-                )
-            })
             .child(
                 img(image)
                     .max_w_96()