From 6f543ff16de1ca494ca0464b7dced2ef51c6ed3d Mon Sep 17 00:00:00 2001 From: Arnesh Banerjee Date: Mon, 27 Jul 2026 18:21:35 +0530 Subject: [PATCH] repl: Render SVG cell outputs SVG outputs (image/svg+xml) previously fell through to "Unsupported media type". Render them through GPUI's SVG renderer, reusing the existing ImageView via a from_svg constructor, and rank SVG above raster images so a kernel that offers both shows the scalable version. --- crates/gpui/src/svg_renderer.rs | 10 +- crates/repl/src/outputs.rs | 26 ++- crates/repl/src/outputs/image.rs | 341 ++++++++++++++++++++++++++++--- 3 files changed, 339 insertions(+), 38 deletions(-) diff --git a/crates/gpui/src/svg_renderer.rs b/crates/gpui/src/svg_renderer.rs index d06a4b0a135f5b..7b3a3ffcb45afd 100644 --- a/crates/gpui/src/svg_renderer.rs +++ b/crates/gpui/src/svg_renderer.rs @@ -1,5 +1,5 @@ use crate::{ - AssetSource, DevicePixels, IsZero, RenderImage, Result, SharedString, Size, + AssetSource, DevicePixels, IsZero, Pixels, RenderImage, Result, SharedString, Size, px, swap_rgba_pa_to_bgra, }; use image::Frame; @@ -102,6 +102,14 @@ pub struct SvgRenderer { /// scales should retain this value to avoid re-paying the parse cost. pub struct ParsedSvg(usvg::Tree); +impl ParsedSvg { + /// The intrinsic size of the document, in logical pixels. + pub fn size(&self) -> Size { + let size = self.0.size(); + crate::size(px(size.width()), px(size.height())) + } +} + /// The size in which to rasterize the SVG. #[derive(Clone, Copy)] pub enum SvgSize { diff --git a/crates/repl/src/outputs.rs b/crates/repl/src/outputs.rs index b4fb634d6931b6..a57d2eb3f4ab22 100644 --- a/crates/repl/src/outputs.rs +++ b/crates/repl/src/outputs.rs @@ -67,9 +67,12 @@ use settings::Settings; /// When deciding what to render from a collection of mediatypes, we need to rank them in order of importance fn rank_mime_type(mimetype: &MimeType) -> usize { match mimetype { - MimeType::DataTable(_) => 7, - MimeType::Html(_) => 6, - MimeType::Json(_) => 5, + MimeType::DataTable(_) => 8, + MimeType::Html(_) => 7, + MimeType::Json(_) => 6, + // Rank SVG above raster images so a kernel that offers both is shown + // as the scalable version. + MimeType::Svg(_) => 5, MimeType::Png(_) => 4, MimeType::Jpeg(_) => 3, MimeType::Markdown(_) => 2, @@ -423,6 +426,10 @@ impl Output { }, Err(error) => Output::Message(format!("Failed to load image: {}", error)), }, + Some(MimeType::Svg(data)) => Output::Image { + content: cx.new(|cx| ImageView::from_svg(data, cx)), + display_id, + }, Some(MimeType::DataTable(data)) => Output::Table { content: cx.new(|cx| TableView::new(data, window, cx)), display_id, @@ -859,14 +866,16 @@ mod tests { let data_table = MimeType::DataTable(Box::default()); let html = MimeType::Html(String::new()); let json = MimeType::Json(serde_json::json!({})); + let svg = MimeType::Svg(String::new()); let png = MimeType::Png(String::new()); let jpeg = MimeType::Jpeg(String::new()); let markdown = MimeType::Markdown(String::new()); let plain = MimeType::Plain(String::new()); - assert_eq!(rank_mime_type(&data_table), 7); - assert_eq!(rank_mime_type(&html), 6); - assert_eq!(rank_mime_type(&json), 5); + assert_eq!(rank_mime_type(&data_table), 8); + assert_eq!(rank_mime_type(&html), 7); + assert_eq!(rank_mime_type(&json), 6); + assert_eq!(rank_mime_type(&svg), 5); assert_eq!(rank_mime_type(&png), 4); assert_eq!(rank_mime_type(&jpeg), 3); assert_eq!(rank_mime_type(&markdown), 2); @@ -874,7 +883,8 @@ mod tests { assert!(rank_mime_type(&data_table) > rank_mime_type(&html)); assert!(rank_mime_type(&html) > rank_mime_type(&json)); - assert!(rank_mime_type(&json) > rank_mime_type(&png)); + assert!(rank_mime_type(&json) > rank_mime_type(&svg)); + assert!(rank_mime_type(&svg) > rank_mime_type(&png)); assert!(rank_mime_type(&png) > rank_mime_type(&jpeg)); assert!(rank_mime_type(&jpeg) > rank_mime_type(&markdown)); assert!(rank_mime_type(&markdown) > rank_mime_type(&plain)); @@ -882,10 +892,8 @@ mod tests { #[test] fn test_rank_mime_type_unsupported_returns_zero() { - let svg = MimeType::Svg(String::new()); let latex = MimeType::Latex(String::new()); - assert_eq!(rank_mime_type(&svg), 0); assert_eq!(rank_mime_type(&latex), 0); } diff --git a/crates/repl/src/outputs/image.rs b/crates/repl/src/outputs/image.rs index e5444be3d779c9..5a59cbbf73add0 100644 --- a/crates/repl/src/outputs/image.rs +++ b/crates/repl/src/outputs/image.rs @@ -3,9 +3,14 @@ use base64::{ Engine as _, alphabet, engine::{DecodePaddingMode, GeneralPurpose, GeneralPurposeConfig}, }; -use gpui::{App, ClipboardItem, Image, ImageFormat, Pixels, RenderImage, Window, img}; +use futures::{FutureExt as _, select_biased}; +use gpui::{ + App, ClipboardItem, DevicePixels, Empty, Image, ImageFormat, ParsedSvg, Pixels, RenderImage, + SharedString, Size, SvgSize, Task, Window, img, size, +}; use settings::Settings as _; use std::sync::Arc; +use std::time::Duration; use ui::{IntoElement, Styled, prelude::*}; use crate::outputs::{OutputContent, plain}; @@ -14,11 +19,40 @@ use crate::repl_settings::ReplSettings; /// ImageView renders an image inline in an editor, adapting to the line height to fit the image. pub struct ImageView { clipboard_image: Arc, - height: u32, - width: u32, - image: Arc, + source: ImageSource, + task: Option>, +} + +enum ImageSource { + Raster { + image: Arc, + size: Size, + }, + Svg(SvgImage), +} + +enum SvgImage { + Parsing { + /// Set once parsing has run long enough to be worth telling the user about. + slow: bool, + }, + Ready { + parsed: Arc, + /// The raster being displayed, and the device size it was made at. + raster: Option<(Size, Arc)>, + /// The device size of a rasterization that is currently running. + rendering: Option>, + }, + Failed(SharedString), } +/// Rasters are capped so that a large document cannot ask for a huge texture. +const MAX_RASTER_SIZE: f32 = 2000.; + +/// How long to let an SVG parse before showing a loading state, so that the +/// common case of a small document does not flash one. +const SLOW_PARSE_THRESHOLD: Duration = Duration::from_millis(2); + pub const STANDARD_INDIFFERENT: GeneralPurpose = GeneralPurpose::new( &alphabet::STANDARD, GeneralPurposeConfig::new() @@ -64,27 +98,149 @@ impl ImageView { Ok(ImageView { clipboard_image, - height, - width, - image: Arc::new(gpui_image_data), + source: ImageSource::Raster { + image: Arc::new(gpui_image_data), + size: size(px(width as f32), px(height as f32)), + }, + task: None, }) } + pub fn from_svg(svg: &str, cx: &mut Context) -> Self { + let clipboard_image = + Arc::new(Image::from_bytes(ImageFormat::Svg, svg.as_bytes().to_vec())); + let svg_renderer = cx.svg_renderer(); + + let task = cx.spawn({ + let clipboard_image = clipboard_image.clone(); + async move |this, cx| { + let mut parse = cx + .background_spawn( + async move { svg_renderer.parse_svg(clipboard_image.bytes()) }, + ) + .fuse(); + let mut slow = cx.background_executor().timer(SLOW_PARSE_THRESHOLD).fuse(); + + let parsed = select_biased! { + parsed = parse => parsed, + _ = slow => { + this.update(cx, |this, cx| { + if let ImageSource::Svg(SvgImage::Parsing { slow }) = &mut this.source { + *slow = true; + cx.notify(); + } + }) + .ok(); + parse.await + } + }; + + this.update(cx, |this, cx| { + this.source = ImageSource::Svg(match parsed { + Ok(parsed) => SvgImage::Ready { + parsed: Arc::new(parsed), + raster: None, + rendering: None, + }, + Err(error) => SvgImage::Failed(error.to_string().into()), + }); + cx.notify(); + }) + .ok(); + } + }); + + ImageView { + clipboard_image, + source: ImageSource::Svg(SvgImage::Parsing { slow: false }), + task: Some(task), + } + } + + /// Rasterizes the SVG at the size it is laid out at, so that it stays sharp + /// instead of being scaled from a raster made at some other size. + fn rasterize_svg( + &mut self, + display_size: Size, + scale_factor: f32, + cx: &mut Context, + ) { + let ImageSource::Svg(SvgImage::Ready { + parsed, + raster, + rendering, + }) = &mut self.source + else { + return; + }; + + let target = raster_size(display_size, scale_factor); + if target.width.0 <= 0 + || target.height.0 <= 0 + || raster.as_ref().map(|(size, _)| *size) == Some(target) + || *rendering == Some(target) + { + return; + } + + *rendering = Some(target); + let parsed = parsed.clone(); + let svg_renderer = cx.svg_renderer(); + + self.task = Some(cx.spawn(async move |this, cx| { + let rendered = cx + .background_spawn(async move { + svg_renderer.render_parsed(&parsed, SvgSize::ExactSize(target)) + }) + .await; + + this.update(cx, |this, cx| { + let ImageSource::Svg(svg) = &mut this.source else { + return; + }; + match rendered { + Ok(image) => { + if let SvgImage::Ready { + raster, rendering, .. + } = svg + { + *rendering = None; + if let Some((_, previous)) = raster.replace((target, image)) { + cx.drop_image(previous, None); + } + } + } + Err(error) => *svg = SvgImage::Failed(error.to_string().into()), + } + cx.notify(); + }) + .ok(); + })); + } + + fn render_pending(&self) -> AnyElement { + match &self.source { + ImageSource::Svg(SvgImage::Parsing { slow: false }) => Empty.into_any_element(), + _ => div().child("Rendering SVG...").into_any_element(), + } + } + fn scaled_size( &self, line_height: Pixels, max_width: Option, max_height: Option, - ) -> (Pixels, Pixels) { - let (mut height, mut width) = if self.height as f32 / f32::from(line_height) - == u8::MAX as f32 - { - let height = u8::MAX as f32 * line_height; - let width = Pixels::from(self.width as f32 * f32::from(height) / self.height as f32); - (height, width) - } else { - (self.height.into(), self.width.into()) - }; + ) -> Option> { + let intrinsic_size = self.source.size()?; + + let (mut height, mut width) = + if f32::from(intrinsic_size.height) / f32::from(line_height) == u8::MAX as f32 { + let height = u8::MAX as f32 * line_height; + let width = intrinsic_size.width * (height / intrinsic_size.height); + (height, width) + } else { + (intrinsic_size.height, intrinsic_size.width) + }; let mut scale: f32 = 1.0; if let Some(max_width) = max_width { @@ -104,10 +260,43 @@ impl ImageView { height *= scale; } - (height, width) + Some(size(width, height)) } } +impl ImageSource { + /// The intrinsic size of the image, once it is known. + fn size(&self) -> Option> { + match self { + ImageSource::Raster { size, .. } => Some(*size), + ImageSource::Svg(SvgImage::Ready { parsed, .. }) => Some(parsed.size()), + ImageSource::Svg(_) => None, + } + } + + fn image(&self) -> Option<&Arc> { + match self { + ImageSource::Raster { image, .. } => Some(image), + ImageSource::Svg(SvgImage::Ready { raster, .. }) => { + raster.as_ref().map(|(_, image)| image) + } + ImageSource::Svg(_) => None, + } + } +} + +/// The device size to rasterize at, capped so that a large document cannot ask +/// for a huge texture. +fn raster_size(display_size: Size, scale_factor: f32) -> Size { + let longest_side = f32::from(display_size.width).max(f32::from(display_size.height)); + let mut scale = scale_factor; + if longest_side * scale > MAX_RASTER_SIZE { + scale = MAX_RASTER_SIZE / longest_side; + } + + display_size.map(|side| DevicePixels((f32::from(side) * scale).round() as i32)) +} + impl Render for ImageView { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let settings = ReplSettings::get_global(cx); @@ -121,11 +310,25 @@ impl Render for ImageView { None }; - let (height, width) = self.scaled_size(line_height, max_width, max_height); + if let ImageSource::Svg(SvgImage::Failed(error)) = &self.source { + return div() + .child(format!("Failed to render SVG: {error}")) + .into_any_element(); + } - let image = self.image.clone(); + let Some(display_size) = self.scaled_size(line_height, max_width, max_height) else { + return self.render_pending(); + }; - img(image).w(width).h(height) + self.rasterize_svg(display_size, window.scale_factor(), cx); + + match self.source.image() { + Some(image) => img(image.clone()) + .w(display_size.width) + .h(display_size.height) + .into_any_element(), + None => self.render_pending(), + } } } @@ -157,6 +360,87 @@ mod tests { base64::engine::general_purpose::STANDARD.encode(bytes) } + const TEST_SVG: &str = r#""#; + + #[gpui::test] + async fn test_image_view_parses_svg_in_the_background(cx: &mut gpui::TestAppContext) { + let view = cx.new(|cx| ImageView::from_svg(TEST_SVG, cx)); + + view.read_with(cx, |view, _| { + assert!(matches!( + view.source, + ImageSource::Svg(SvgImage::Parsing { .. }) + )); + }); + + cx.run_until_parked(); + + view.read_with(cx, |view, _| { + assert_eq!(view.source.size(), Some(size(px(120.0), px(80.0)))); + assert!(view.source.image().is_none()); + }); + } + + #[gpui::test] + async fn test_image_view_reports_invalid_svg(cx: &mut gpui::TestAppContext) { + let view = cx.new(|cx| ImageView::from_svg("not an svg", cx)); + cx.run_until_parked(); + + view.read_with(cx, |view, _| { + assert!(matches!(view.source, ImageSource::Svg(SvgImage::Failed(_)))); + }); + } + + #[gpui::test] + async fn test_image_view_rasterizes_svg_at_display_size(cx: &mut gpui::TestAppContext) { + let view = cx.new(|cx| ImageView::from_svg(TEST_SVG, cx)); + cx.run_until_parked(); + + view.update(cx, |view, cx| { + view.rasterize_svg(size(px(60.0), px(40.0)), 2.0, cx) + }); + cx.run_until_parked(); + + let image = view + .read_with(cx, |view, _| view.source.image().cloned()) + .expect("SVG should have been rasterized"); + assert_eq!(image.size(0), size(DevicePixels(120), DevicePixels(80))); + + // Laying out at the same size again reuses the raster. + view.update(cx, |view, cx| { + view.rasterize_svg(size(px(60.0), px(40.0)), 2.0, cx) + }); + cx.run_until_parked(); + view.read_with(cx, |view, _| { + assert!(Arc::ptr_eq( + &image, + view.source.image().expect("raster should be kept") + )); + }); + + view.update(cx, |view, cx| { + view.rasterize_svg(size(px(30.0), px(20.0)), 1.0, cx) + }); + cx.run_until_parked(); + view.read_with(cx, |view, _| { + assert_eq!( + view.source + .image() + .expect("SVG should have been rasterized") + .size(0), + size(DevicePixels(30), DevicePixels(20)) + ); + }); + } + + #[test] + fn test_raster_size_is_capped() { + let capped = raster_size(size(px(4000.0), px(2000.0)), 2.0); + + assert_eq!(capped.width, DevicePixels(MAX_RASTER_SIZE as i32)); + assert_eq!(capped.height, DevicePixels(MAX_RASTER_SIZE as i32 / 2)); + } + #[test] fn test_image_view_scaled_size_respects_limits() { let encoded = encode_test_image(200, 120); @@ -168,11 +452,11 @@ mod tests { let line_height = Pixels::from(10.0); let max_width = Pixels::from(50.0); let max_height = Pixels::from(40.0); - let (height, width) = - image_view.scaled_size(line_height, Some(max_width), Some(max_height)); + let display_size = image_view + .scaled_size(line_height, Some(max_width), Some(max_height)) + .expect("raster images have a known size"); - assert_eq!(f32::from(width), 50.0); - assert_eq!(f32::from(height), 30.0); + assert_eq!(display_size, size(px(50.0), px(30.0))); } #[test] @@ -184,9 +468,10 @@ mod tests { }; let line_height = Pixels::from(10.0); - let (height, width) = image_view.scaled_size(line_height, None, None); + let display_size = image_view + .scaled_size(line_height, None, None) + .expect("raster images have a known size"); - assert_eq!(f32::from(width), 200.0); - assert_eq!(f32::from(height), 120.0); + assert_eq!(display_size, size(px(200.0), px(120.0))); } }