diff --git a/apps/desktop-gpui/src/app_windows.rs b/apps/desktop-gpui/src/app_windows.rs index ae2ecce797..a7acd47563 100644 --- a/apps/desktop-gpui/src/app_windows.rs +++ b/apps/desktop-gpui/src/app_windows.rs @@ -6496,8 +6496,13 @@ pub fn prepare_for_screenshot_capture(cx: &mut App) -> bool { /// the Tauri app -- and open the screenshot editor on the new bundle, the /// `automationShouldOpenScreenshotEditor` default. A failure reveals the main /// window if the picker had hidden it, since nothing else will. -pub fn screenshot_finished(captured: Option, cx: &mut App) { - let Some(png) = captured else { +pub fn screenshot_finished(captured: Option, cx: &mut App) { + let Some(crate::screenshot::CapturedScreenshot { + png, + source, + behaviour, + }) = captured + else { let hidden = std::mem::take(&mut cx.global_mut::().main_hidden_for_picker); if hidden && RecordingSession::global(cx).read(cx).phase == Phase::Idle { show_main_window(cx); @@ -6515,7 +6520,6 @@ pub fn screenshot_finished(captured: Option, cx: &mut App) { main.update(cx, |view, window, cx| view.refresh_open_library(window, cx)) .ok(); - let behaviour = crate::store::GeneralSettings::load().post_screenshot_behaviour; if behaviour.copies() && let Err(error) = platform::copy_image_to_clipboard(&png, cx) { @@ -6527,7 +6531,7 @@ pub fn screenshot_finished(captured: Option, cx: &mut App) { // recording hands off to the video editor; `screenshot_editor_closed` // brings the main window back. cx.global_mut::().main_hidden_for_picker = false; - open_screenshot_editor(png, cx); + open_screenshot_editor_with_source(png, source, cx); return; } @@ -6549,6 +6553,14 @@ pub fn screenshot_finished(captured: Option, cx: &mut App) { /// Must be reached through `cx.defer` from anything inside an entity update: /// opening a window paints it synchronously and would double-lease the caller. pub fn open_screenshot_editor(path: PathBuf, cx: &mut App) { + open_screenshot_editor_with_source(path, None, cx); +} + +fn open_screenshot_editor_with_source( + path: PathBuf, + source: Option, + cx: &mut App, +) { #[cfg(target_os = "linux")] if defer_window_until_capture_safe(cx) { return; @@ -6650,7 +6662,7 @@ pub fn open_screenshot_editor(path: PathBuf, cx: &mut App) { // only showed up once Cap was clicked. cx.defer(move |cx| bring_screenshot_editor_forward(handle, cx)); prewarm_shared_wgpu_device(cx); - screenshot_editor::load_screenshot_project(key, handle, cx); + screenshot_editor::load_screenshot_project(key, handle, source, cx); } fn bring_screenshot_editor_forward(handle: WindowHandle, cx: &mut App) { diff --git a/apps/desktop-gpui/src/screenshot.rs b/apps/desktop-gpui/src/screenshot.rs index b01af48946..4fafc7708e 100644 --- a/apps/desktop-gpui/src/screenshot.rs +++ b/apps/desktop-gpui/src/screenshot.rs @@ -18,7 +18,13 @@ use anyhow::{Context as _, anyhow}; use cap_recording::sources::screen_capture::ScreenCaptureTarget; use gpui::App; -use crate::app_windows; +use crate::{app_windows, screenshot_editor::LoadedSource}; + +pub struct CapturedScreenshot { + pub png: PathBuf, + pub source: Option, + pub behaviour: crate::store::PostScreenshotBehaviour, +} pub fn take_screenshot(target: ScreenCaptureTarget, cx: &mut App) { if matches!(target, ScreenCaptureTarget::CameraOnly) { @@ -28,7 +34,10 @@ pub fn take_screenshot(target: ScreenCaptureTarget, cx: &mut App) { return; } tracing::info!(kind = target.kind_str(), "taking a screenshot"); - + let behaviour = crate::store::GeneralSettings::load().post_screenshot_behaviour; + if behaviour.opens_editor() { + app_windows::prewarm_shared_wgpu_device(cx); + } let settle = app_windows::prepare_for_screenshot_capture(cx); let task = gpui_tokio::Tokio::spawn_result(cx, async move { @@ -38,15 +47,16 @@ pub fn take_screenshot(target: ScreenCaptureTarget, cx: &mut App) { // the compositor needs a beat to actually drop them from screen. tokio::time::sleep(Duration::from_millis(150)).await; } - capture_and_write(target).await + capture_and_write(target, behaviour.opens_editor()).await }); cx.spawn(async move |cx| { let captured = match task.await { - Ok(png) => { - tracing::info!(path = %png.display(), "screenshot captured"); - Some(png) - } + Ok((png, source)) => Some(CapturedScreenshot { + png, + source, + behaviour, + }), Err(error) => { tracing::error!("screenshot capture failed: {error:#}"); None @@ -57,16 +67,12 @@ pub fn take_screenshot(target: ScreenCaptureTarget, cx: &mut App) { .detach(); } -/// Capture, then write the `.cap` bundle. Returns the PNG path, which is what -/// the Tauri command returns (`NewScreenshotAdded.path` is the PNG, not the -/// bundle). -async fn capture_and_write(target: ScreenCaptureTarget) -> anyhow::Result { - let image = cap_recording::screenshot::capture_screenshot(target.clone()) - .await - .context("capturing the screenshot")?; - - crate::app_sounds::AppSound::Notification.play(); - +fn create_capture_project( + target: &ScreenCaptureTarget, +) -> anyhow::Result<( + cap_project::RecordingMeta, + cap_project::ProjectConfiguration, +)> { let base = crate::library::screenshots_dir(); std::fs::create_dir_all(&base) .with_context(|| format!("creating screenshots dir {}", base.display()))?; @@ -93,7 +99,7 @@ async fn capture_and_write(target: ScreenCaptureTarget) -> anyhow::Result anyhow::Result anyhow::Result anyhow::Result<(PathBuf, Option)> { + let image = cap_recording::screenshot::capture_screenshot(target.clone()) + .await + .context("capturing the screenshot")?; + crate::app_sounds::AppSound::Notification.play(); + let (meta, config) = create_capture_project(&target)?; + let png = meta.project_path.join("original.png"); + let width = image.width(); + let height = image.height(); + let rgba = write_png(png.clone(), image, retain_rgba).await?; + tracing::info!(path = %png.display(), "screenshot captured"); + let source = if let Some(rgba) = rgba { + let cap_project::RecordingMetaInner::Studio(studio_meta) = &meta.inner else { + return Err(anyhow!("screenshot metadata is not a studio project")); + }; + Some(LoadedSource { + rgba: std::sync::Arc::new(rgba), + width, + height, + pretty_name: meta.pretty_name.clone(), + config, + studio_meta: (**studio_meta).clone(), + meta, + }) + } else { + None + }; + Ok((png, source)) } /// The Tauri encode settings (`recording.rs:2987-3016`): RGB stays RGB, a /// capture past 8MP trades compression for speed, adaptive filtering either /// way. Blocking work, so off the runtime workers. -async fn write_png(path: PathBuf, image: image::DynamicImage) -> anyhow::Result<()> { +async fn write_png( + path: PathBuf, + image: image::DynamicImage, + retain_rgba: bool, +) -> anyhow::Result>> { tokio::task::spawn_blocking(move || { let width = image.width(); let height = image.height(); @@ -146,6 +187,7 @@ async fn write_png(path: PathBuf, image: image::DynamicImage) -> anyhow::Result< } else { image::codecs::png::CompressionType::Default }; + let rgba = retain_rgba.then(|| image.to_rgba8().into_raw()); let data = image.into_bytes(); let file = @@ -159,8 +201,95 @@ async fn write_png(path: PathBuf, image: image::DynamicImage) -> anyhow::Result< compression, image::codecs::png::FilterType::Adaptive, ) - .context("encoding the screenshot PNG") + .context("encoding the screenshot PNG")?; + Ok(rgba) }) .await .context("the PNG encode task died")? } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn retained_capture_pixels_match_saved_rgb_and_alpha() { + let directory = tempfile::tempdir().unwrap(); + let images = [ + image::DynamicImage::ImageRgb8(image::RgbImage::from_fn(4, 2, |x, y| { + image::Rgb([x as u8 * 40, y as u8 * 80, 173]) + })), + image::DynamicImage::ImageRgba8(image::RgbaImage::from_fn(4, 2, |x, y| { + image::Rgba([x as u8 * 40, y as u8 * 80, 173, x as u8 * 70]) + })), + ]; + for (index, image) in images.into_iter().enumerate() { + let path = directory.path().join(format!("{index}.png")); + let expected = image.to_rgba8().into_raw(); + let retained = write_png(path.clone(), image, true).await.unwrap().unwrap(); + let saved = image::open(path).unwrap().to_rgba8().into_raw(); + assert_eq!(retained, expected); + assert_eq!(saved, retained); + } + } + + #[tokio::test] + async fn oversized_capture_pixels_can_still_be_saved() { + let directory = tempfile::tempdir().unwrap(); + for (width, height) in [(16_385, 2), (4, 16_385)] { + let path = directory.path().join("original.png"); + let image = image::DynamicImage::ImageRgba8(image::RgbaImage::from_pixel( + width, + height, + image::Rgba([13, 71, 129, 255]), + )); + let retained = write_png(path.clone(), image, true).await.unwrap().unwrap(); + let saved = image::open(&path).unwrap().to_rgba8(); + assert_eq!(saved.dimensions(), (width, height)); + assert_eq!(saved.as_raw(), &retained); + assert!(matches!( + crate::screenshot_editor::load_source(directory.path()), + Err(message) if message == format!("Image dimensions exceed maximum: {width}x{height}") + )); + assert!(path.exists()); + } + } + + #[tokio::test] + async fn copy_and_save_only_encode_without_retaining_pixels() { + let directory = tempfile::tempdir().unwrap(); + for (index, image) in [ + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 4, + 2, + image::Rgb([13, 71, 129]), + )), + image::DynamicImage::ImageRgba8(image::RgbaImage::from_pixel( + 4, + 2, + image::Rgba([13, 71, 129, 83]), + )), + ] + .into_iter() + .enumerate() + { + let path = directory.path().join(format!("{index}.png")); + let expected = image.to_rgba8(); + assert!( + write_png(path.clone(), image, false) + .await + .unwrap() + .is_none() + ); + assert_eq!(image::open(path).unwrap().to_rgba8(), expected); + } + } + + #[tokio::test] + async fn failed_capture_save_does_not_deliver_pixels() { + let directory = tempfile::tempdir().unwrap(); + let image = image::DynamicImage::ImageRgba8(image::RgbaImage::new(4, 2)); + let result = write_png(directory.path().to_path_buf(), image, true).await; + assert!(result.is_err()); + } +} diff --git a/apps/desktop-gpui/src/screenshot_editor.rs b/apps/desktop-gpui/src/screenshot_editor.rs index 7e83655f2a..4767780791 100644 --- a/apps/desktop-gpui/src/screenshot_editor.rs +++ b/apps/desktop-gpui/src/screenshot_editor.rs @@ -57,7 +57,7 @@ pub const SCREENSHOT_EDITOR_MIN_WIDTH: f32 = 800.; pub const SCREENSHOT_EDITOR_MIN_HEIGHT: f32 = 600.; /// `MAX_DIMENSION` (`screenshot_editor.rs:38` over there). -const MAX_DIMENSION: u32 = 16_384; +pub(crate) const MAX_DIMENSION: u32 = 16_384; /// `saveConfig`'s debounce (`context.tsx:427-438`) -- 1000ms, not the video /// editor's 250. @@ -106,7 +106,7 @@ pub fn resolve_bundle(path: &Path) -> Option { /// Everything the window and the renderer need off disk. pub struct LoadedSource { - pub rgba: Vec, + pub rgba: Arc>, pub width: u32, pub height: u32, pub pretty_name: String, @@ -153,7 +153,7 @@ pub fn load_source(bundle: &Path) -> Result { let config = ProjectConfiguration::load(bundle).unwrap_or_default(); Ok(LoadedSource { - rgba: image.to_rgba8().into_raw(), + rgba: Arc::new(image.to_rgba8().into_raw()), width, height, pretty_name: meta.pretty_name.clone(), @@ -163,6 +163,20 @@ pub fn load_source(bundle: &Path) -> Result { }) } +fn load_source_for_editor( + bundle: &Path, + captured: Option, +) -> Result { + let source = captured.map_or_else(|| load_source(bundle), Ok)?; + if source.width > MAX_DIMENSION || source.height > MAX_DIMENSION { + return Err(format!( + "Image dimensions exceed maximum: {}x{}", + source.width, source.height + )); + } + Ok(source) +} + // --------------------------------------------------------------------------- // The still renderer // --------------------------------------------------------------------------- @@ -224,7 +238,7 @@ pub async fn run_still_renderer( }; let _ = setup_tx.send(Ok(())); - let decoded = DecodedFrame::new_with_arc(Arc::new(source.rgba), source.width, source.height); + let decoded = DecodedFrame::new_with_arc(source.rgba, source.width, source.height); let mut frame_renderer = FrameRenderer::new(&constants); let mut layers = RendererLayers::new_with_options( @@ -4760,13 +4774,15 @@ fn frame_buffers(frame: &RenderedFrame) -> Option<(Arc>, Arc, + captured: Option, cx: &mut App, ) { cx.spawn(async move |cx| { + let fresh_capture = captured.is_some(); let load_bundle = bundle.clone(); let source = cx .background_executor() - .spawn(async move { load_source(&load_bundle) }) + .spawn(async move { load_source_for_editor(&load_bundle, captured) }) .await; let source = match source { Ok(source) => source, @@ -4789,6 +4805,30 @@ pub fn load_screenshot_project( let (setup_tx, setup_rx) = flume::bounded(1); let image_size = (source.width, source.height); + let (source, preview) = cx + .background_executor() + .spawn(async move { + let preview = if fresh_capture + && source.width.is_multiple_of(4) + && source.height.is_multiple_of(2) + { + image::RgbaImage::from_raw( + source.width, + source.height, + source.rgba.as_ref().clone(), + ) + .map(|image| { + ( + source.rgba.clone(), + crate::library::rgba_to_render_image(image), + ) + }) + } else { + None + }; + (source, preview) + }) + .await; if handle .update(cx, |view, window, cx| { view.set_loaded( @@ -4801,7 +4841,10 @@ pub fn load_screenshot_project( }, window, cx, - ) + ); + if let Some((rgba, image)) = preview { + view.frame_arrived(image, rgba, image_size, window, cx); + } }) .is_err() { @@ -4841,7 +4884,9 @@ pub fn load_screenshot_project( ); if handle .update(cx, |view, window, cx| { - view.frame_arrived(image, rgba, size, window, cx) + if revision >= view.revision { + view.frame_arrived(image, rgba, size, window, cx); + } }) .is_err() { @@ -5134,7 +5179,7 @@ mod tests { let mut config = new_screenshot_config(); config.annotations.push(arrow("a", 4.)); LoadedSource { - rgba, + rgba: Arc::new(rgba), width, height, pretty_name: "bench".into(), @@ -5144,6 +5189,26 @@ mod tests { } } + #[test] + fn retained_capture_pixels_obey_the_editor_dimension_limit() { + for (width, height) in [(MAX_DIMENSION + 1, 2), (4, MAX_DIMENSION + 1)] { + let result = load_source_for_editor( + Path::new("unused.cap"), + Some(synthetic_source(width, height)), + ); + assert!( + matches!(result, Err(message) if message == format!("Image dimensions exceed maximum: {width}x{height}")) + ); + } + assert!( + load_source_for_editor( + Path::new("unused.cap"), + Some(synthetic_source(MAX_DIMENSION, 2)), + ) + .is_ok() + ); + } + struct DragResult { renders: usize, cpu_ms: f64,