From 4b8a487e54d2c1f3c891c23a968814a72388b288 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 8 Jan 2025 16:25:59 +0100 Subject: [PATCH] Implement copy-screenshot-to-clipboard on Web (#8607) ### Related * Closes https://github.com/rerun-io/rerun/issues/8264 --- Cargo.lock | 2 +- Cargo.toml | 1 - .../src/actions/screenshot_action.rs | 24 +++++---- crates/viewer/re_context_menu/src/lib.rs | 1 - crates/viewer/re_data_ui/src/blob.rs | 1 - crates/viewer/re_data_ui/src/image.rs | 12 ++--- crates/viewer/re_data_ui/src/instance_path.rs | 1 - crates/viewer/re_ui/src/ui_ext.rs | 4 -- crates/viewer/re_viewer/src/app.rs | 44 ++++++++------- crates/viewer/re_viewer/src/screenshotter.rs | 6 +-- crates/viewer/re_viewer_context/Cargo.toml | 4 +- .../viewer/re_viewer_context/src/clipboard.rs | 54 ------------------- crates/viewer/re_viewer_context/src/lib.rs | 7 --- .../re_viewer_context/src/viewer_context.rs | 19 +++++++ 14 files changed, 66 insertions(+), 114 deletions(-) delete mode 100644 crates/viewer/re_viewer_context/src/clipboard.rs diff --git a/Cargo.lock b/Cargo.lock index 929018b52039..6335ab9bb5e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6908,7 +6908,6 @@ version = "0.22.0-alpha.1+dev" dependencies = [ "ahash", "anyhow", - "arboard", "arrow", "bit-vec", "bitflags 2.6.0", @@ -6956,6 +6955,7 @@ dependencies = [ "thiserror 1.0.65", "uuid", "wasm-bindgen-futures", + "web-sys", "wgpu", ] diff --git a/Cargo.toml b/Cargo.toml index 6b8eee119203..3de8bf570c44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -150,7 +150,6 @@ emath = "0.30.0" # All of our direct external dependencies should be found here: ahash = "0.8" anyhow = { version = "1.0", default-features = false } -arboard = { version = "3.2", default-features = false } argh = "0.1.12" array-init = "2.1" arrow = { version = "53.1", default-features = false } diff --git a/crates/viewer/re_context_menu/src/actions/screenshot_action.rs b/crates/viewer/re_context_menu/src/actions/screenshot_action.rs index 4886bbc27330..34db07bbbca7 100644 --- a/crates/viewer/re_context_menu/src/actions/screenshot_action.rs +++ b/crates/viewer/re_context_menu/src/actions/screenshot_action.rs @@ -3,9 +3,9 @@ use re_viewer_context::{Item, PublishedViewInfo, ScreenshotTarget, ViewId, ViewR use crate::{ContextMenuAction, ContextMenuContext}; /// View screenshot action. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ScreenshotAction { /// Screenshot the view, and copy the results to clipboard. - #[cfg(not(target_arch = "wasm32"))] // TODO(#8264): copy-to-screenshot on web CopyScreenshot, /// Screenshot the view, and save the results to disk. @@ -13,18 +13,22 @@ pub enum ScreenshotAction { } impl ContextMenuAction for ScreenshotAction { - /// Do we have a context menu for this selection? - fn supports_selection(&self, ctx: &ContextMenuContext<'_>) -> bool { - // Allow if there is a single view selected. - ctx.selection.len() == 1 - && ctx - .selection - .iter() - .all(|(item, _)| self.supports_item(ctx, item)) + fn supports_multi_selection(&self, _ctx: &ContextMenuContext<'_>) -> bool { + match self { + Self::CopyScreenshot => false, + Self::SaveScreenshot => true, + } } /// Do we have a context menu for this item? fn supports_item(&self, ctx: &ContextMenuContext<'_>, item: &Item) -> bool { + if *self == Self::CopyScreenshot && ctx.viewer_context.is_safari_browser() { + // Safari only allows access to clipboard on user action (e.g. on-click). + // However, the screenshot capture results arrives a frame later. + re_log::debug_once!("Copying screenshots not supported on Safari"); + return false; + } + let Item::View(view_id) = item else { return false; }; @@ -39,7 +43,6 @@ impl ContextMenuAction for ScreenshotAction { fn label(&self, _ctx: &ContextMenuContext<'_>) -> String { match self { - #[cfg(not(target_arch = "wasm32"))] // TODO(#8264): copy-to-screenshot on web Self::CopyScreenshot => "Copy screenshot".to_owned(), Self::SaveScreenshot => "Save screenshot…".to_owned(), } @@ -65,7 +68,6 @@ impl ContextMenuAction for ScreenshotAction { } let target = match self { - #[cfg(not(target_arch = "wasm32"))] // TODO(#8264): copy-to-screenshot on web Self::CopyScreenshot => ScreenshotTarget::CopyToClipboard, Self::SaveScreenshot => ScreenshotTarget::SaveToDisk, }; diff --git a/crates/viewer/re_context_menu/src/lib.rs b/crates/viewer/re_context_menu/src/lib.rs index d1817e32042f..e4c03202c1df 100644 --- a/crates/viewer/re_context_menu/src/lib.rs +++ b/crates/viewer/re_context_menu/src/lib.rs @@ -114,7 +114,6 @@ fn action_list( Box::new(RemoveAction), ], vec![ - #[cfg(not(target_arch = "wasm32"))] // TODO(#8264): copy-to-screenshot on web Box::new(actions::ScreenshotAction::CopyScreenshot), Box::new(actions::ScreenshotAction::SaveScreenshot), ], diff --git a/crates/viewer/re_data_ui/src/blob.rs b/crates/viewer/re_data_ui/src/blob.rs index 2508772bd596..5bc17ae23e3e 100644 --- a/crates/viewer/re_data_ui/src/blob.rs +++ b/crates/viewer/re_data_ui/src/blob.rs @@ -163,7 +163,6 @@ pub fn blob_preview_and_save_ui( ); } - #[cfg(not(target_arch = "wasm32"))] if let Some(image) = image { let image_stats = ctx .cache diff --git a/crates/viewer/re_data_ui/src/image.rs b/crates/viewer/re_data_ui/src/image.rs index 7ba2fdc22747..3642c5500a38 100644 --- a/crates/viewer/re_data_ui/src/image.rs +++ b/crates/viewer/re_data_ui/src/image.rs @@ -7,7 +7,6 @@ use re_viewer_context::{ }; /// Show a button letting the user copy the image -#[cfg(not(target_arch = "wasm32"))] pub fn copy_image_button_ui(ui: &mut egui::Ui, image: &ImageInfo, data_range: egui::Rangef) { if ui .button("Copy image") @@ -15,12 +14,11 @@ pub fn copy_image_button_ui(ui: &mut egui::Ui, image: &ImageInfo, data_range: eg .clicked() { if let Some(rgba) = image.to_rgba8_image(data_range.into()) { - re_viewer_context::Clipboard::with(|clipboard| { - clipboard.set_image( - [rgba.width() as _, rgba.height() as _], - bytemuck::cast_slice(rgba.as_raw()), - ); - }); + let egui_image = egui::ColorImage::from_rgba_unmultiplied( + [rgba.width() as _, rgba.height() as _], + bytemuck::cast_slice(rgba.as_raw()), + ); + ui.ctx().copy_image(egui_image); } else { re_log::error!("Invalid image"); } diff --git a/crates/viewer/re_data_ui/src/instance_path.rs b/crates/viewer/re_data_ui/src/instance_path.rs index 7030face69d7..2184700e4c1b 100644 --- a/crates/viewer/re_data_ui/src/instance_path.rs +++ b/crates/viewer/re_data_ui/src/instance_path.rs @@ -362,7 +362,6 @@ fn preview_if_image_ui( ui.horizontal(|ui| { image_download_button_ui(ctx, ui, entity_path, &image, data_range); - #[cfg(not(target_arch = "wasm32"))] crate::image::copy_image_button_ui(ui, &image, data_range); }); diff --git a/crates/viewer/re_ui/src/ui_ext.rs b/crates/viewer/re_ui/src/ui_ext.rs index 26bbdac9d926..152ed6c8d371 100644 --- a/crates/viewer/re_ui/src/ui_ext.rs +++ b/crates/viewer/re_ui/src/ui_ext.rs @@ -1125,10 +1125,6 @@ pub trait UiExt { fn selectable_toggle(&mut self, content: impl FnOnce(&mut egui::Ui) -> R) -> R { let ui = self.ui_mut(); - // ensure cursor is on an integer value, otherwise we get weird optical alignment of the text - //TODO(ab): fix when https://github.com/emilk/egui/issues/4928 is resolved - ui.add_space(-ui.cursor().min.y.fract()); - egui::Frame { inner_margin: egui::Margin::same(3), stroke: design_tokens().bottom_bar_stroke, diff --git a/crates/viewer/re_viewer/src/app.rs b/crates/viewer/re_viewer/src/app.rs index 9f9a7eb67ff3..ffc686916697 100644 --- a/crates/viewer/re_viewer/src/app.rs +++ b/crates/viewer/re_viewer/src/app.rs @@ -1590,7 +1590,7 @@ impl App { ) { use re_viewer_context::ScreenshotInfo; - if let Some(info) = &user_data + if let Some(info) = user_data .data .as_ref() .and_then(|data| data.downcast_ref::()) @@ -1609,14 +1609,8 @@ impl App { }; match target { - #[cfg(not(target_arch = "wasm32"))] // TODO(#8264): copy-to-screenshot on web re_viewer_context::ScreenshotTarget::CopyToClipboard => { - re_viewer_context::Clipboard::with(|clipboard| { - clipboard.set_image( - [rgba.width(), rgba.height()], - bytemuck::cast_slice(rgba.as_raw()), - ); - }); + self.egui_ctx.copy_image((*rgba).clone()); } re_viewer_context::ScreenshotTarget::SaveToDisk => { @@ -1644,7 +1638,7 @@ impl App { } } else { #[cfg(not(target_arch = "wasm32"))] // no full-app screenshotting on web - self.screenshotter.save(image); + self.screenshotter.save(&self.egui_ctx, image); } } } @@ -1929,17 +1923,29 @@ impl eframe::App for App { // Return the `StoreHub` to the Viewer so we have it on the next frame self.store_hub = Some(store_hub); - // Check for returned screenshot: - egui_ctx.input(|i| { - for event in &i.raw.events { - if let egui::Event::Screenshot { - image, user_data, .. - } = event - { - self.process_screenshot_result(image, user_data); - } + { + // Check for returned screenshots: + let screenshots: Vec<_> = egui_ctx.input(|i| { + i.raw + .events + .iter() + .filter_map(|event| { + if let egui::Event::Screenshot { + image, user_data, .. + } = event + { + Some((image.clone(), user_data.clone())) + } else { + None + } + }) + .collect() + }); + + for (image, user_data) in screenshots { + self.process_screenshot_result(&image, &user_data); } - }); + } } #[cfg(target_arch = "wasm32")] diff --git a/crates/viewer/re_viewer/src/screenshotter.rs b/crates/viewer/re_viewer/src/screenshotter.rs index 93d8ac88e4fe..ae0d13c24616 100644 --- a/crates/viewer/re_viewer/src/screenshotter.rs +++ b/crates/viewer/re_viewer/src/screenshotter.rs @@ -82,7 +82,7 @@ impl Screenshotter { self.countdown.is_some() } - pub fn save(&mut self, image: &egui::ColorImage) { + pub fn save(&mut self, egui_ctx: &egui::Context, image: &egui::ColorImage) { self.countdown = None; if let Some(path) = self.target_path.take() { let w = image.width() as _; @@ -100,9 +100,7 @@ impl Screenshotter { } } } else { - re_viewer_context::Clipboard::with(|cb| { - cb.set_image(image.size, bytemuck::cast_slice(&image.pixels)); - }); + egui_ctx.copy_image(image.clone()); } } } diff --git a/crates/viewer/re_viewer_context/Cargo.toml b/crates/viewer/re_viewer_context/Cargo.toml index 9c3d6f233d77..ff18d3d3bcf7 100644 --- a/crates/viewer/re_viewer_context/Cargo.toml +++ b/crates/viewer/re_viewer_context/Cargo.toml @@ -71,11 +71,9 @@ wgpu.workspace = true # Native dependencies: [target.'cfg(not(target_arch = "wasm32"))'.dependencies] -arboard = { workspace = true, default-features = false, features = [ - "image-data", -] } home.workspace = true # Web dependencies: [target.'cfg(target_arch = "wasm32")'.dependencies] wasm-bindgen-futures.workspace = true +web-sys = { workspace = true, features = ["Window"] } diff --git a/crates/viewer/re_viewer_context/src/clipboard.rs b/crates/viewer/re_viewer_context/src/clipboard.rs deleted file mode 100644 index c60b807e865e..000000000000 --- a/crates/viewer/re_viewer_context/src/clipboard.rs +++ /dev/null @@ -1,54 +0,0 @@ -/// Handles interfacing with the OS clipboard. -pub struct Clipboard { - arboard: Option, -} - -impl Clipboard { - fn new() -> Self { - Self { - arboard: init_arboard(), - } - } - - pub fn set_image(&mut self, size: [usize; 2], rgba_unmultiplied: &[u8]) { - let [width, height] = size; - assert_eq!(width * height * 4, rgba_unmultiplied.len()); - - if let Some(clipboard) = &mut self.arboard { - let image_data = arboard::ImageData { - width, - height, - bytes: rgba_unmultiplied.into(), - }; - if let Err(err) = clipboard.set_image(image_data) { - re_log::error!("Failed to copy image to clipboard: {err}"); - } else { - re_log::info!("Image copied to clipboard"); - } - } - } - - /// Get access to the thread-local [`Clipboard`]. - pub fn with(f: impl FnOnce(&mut Self) -> R) -> R { - use std::cell::RefCell; - thread_local! { - static CLIPBOARD: RefCell> = const { RefCell::new(None) }; - } - - CLIPBOARD.with(|clipboard| { - let mut clipboard = clipboard.borrow_mut(); - let clipboard = clipboard.get_or_insert_with(Self::new); - f(clipboard) - }) - } -} - -fn init_arboard() -> Option { - match arboard::Clipboard::new() { - Ok(clipboard) => Some(clipboard), - Err(err) => { - re_log::error!("Failed to initialize clipboard: {err}"); - None - } - } -} diff --git a/crates/viewer/re_viewer_context/src/lib.rs b/crates/viewer/re_viewer_context/src/lib.rs index 6a8c002443dc..82bb73c67971 100644 --- a/crates/viewer/re_viewer_context/src/lib.rs +++ b/crates/viewer/re_viewer_context/src/lib.rs @@ -92,12 +92,6 @@ pub use self::{ pub use re_ui::UiLayout; // Historical reasons -#[cfg(not(target_arch = "wasm32"))] -mod clipboard; - -#[cfg(not(target_arch = "wasm32"))] -pub use clipboard::Clipboard; - pub mod external { pub use nohash_hasher; pub use {re_chunk_store, re_entity_db, re_log_types, re_query, re_ui}; @@ -152,7 +146,6 @@ pub struct ScreenshotInfo { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ScreenshotTarget { /// The screenshot will be copied to the clipboard. - #[cfg(not(target_arch = "wasm32"))] // TODO(#8264): copy-to-screenshot on web CopyToClipboard, /// The screenshot will be saved to disk. diff --git a/crates/viewer/re_viewer_context/src/viewer_context.rs b/crates/viewer/re_viewer_context/src/viewer_context.rs index 52ea2b6988a6..55df464bff6c 100644 --- a/crates/viewer/re_viewer_context/src/viewer_context.rs +++ b/crates/viewer/re_viewer_context/src/viewer_context.rs @@ -250,6 +250,25 @@ impl ViewerContext<'_> { // The nice thing about this would be that we could always give out references (but updating said cache wouldn't be easy in that case). re_types::reflection::generic_placeholder_for_datatype(&datatype) } + + /// Are we running inside the Safari browser? + pub fn is_safari_browser(&self) -> bool { + #![allow(clippy::unused_self)] + + #[cfg(target_arch = "wasm32")] + fn is_safari_browser_inner() -> Option { + use web_sys::wasm_bindgen::JsValue; + let window = web_sys::window()?; + Some(window.has_own_property(&JsValue::from("safari"))) + } + + #[cfg(not(target_arch = "wasm32"))] + fn is_safari_browser_inner() -> Option { + None + } + + is_safari_browser_inner().unwrap_or(false) + } } // ----------------------------------------------------------------------------