From d942f377d59bdd100ff5687ddd0048c73c72bd9c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 17:17:26 +0200 Subject: [PATCH 01/14] Add command take screenshot and copy result to clipboard --- Cargo.lock | 1 + crates/re_ui/src/command.rs | 13 +++++++++++++ crates/re_viewer/Cargo.toml | 1 + crates/re_viewer/src/app.rs | 14 ++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index a1e48932b63c..be879bbb880d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4358,6 +4358,7 @@ dependencies = [ "anyhow", "arrow2", "arrow2_convert", + "bytemuck", "cfg-if", "cocoa", "eframe", diff --git a/crates/re_ui/src/command.rs b/crates/re_ui/src/command.rs index d2f3efba9d79..c9bdeff85718 100644 --- a/crates/re_ui/src/command.rs +++ b/crates/re_ui/src/command.rs @@ -47,6 +47,10 @@ pub enum Command { PlaybackStepBack, PlaybackStepForward, PlaybackRestart, + + // Dev-tools: + #[cfg(not(target_arch = "wasm32"))] + ScreenshotWholeApp, } impl Command { @@ -128,6 +132,12 @@ impl Command { "Move the time marker to the next point in time with any data", ), Command::PlaybackRestart => ("Restart", "Restart from beginning of timeline"), + + #[cfg(not(target_arch = "wasm32"))] + Command::ScreenshotWholeApp => ( + "Screenshot", + "Copy screenshot of the whole app to clipboard", + ), } } @@ -189,6 +199,9 @@ impl Command { Command::PlaybackStepBack => Some(key(Key::ArrowLeft)), Command::PlaybackStepForward => Some(key(Key::ArrowRight)), Command::PlaybackRestart => Some(cmd(Key::ArrowLeft)), + + #[cfg(not(target_arch = "wasm32"))] + Command::ScreenshotWholeApp => None, } } diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index 367ca7aed457..44199428d2ba 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -69,6 +69,7 @@ ahash.workspace = true anyhow.workspace = true arrow2.workspace = true arrow2_convert.workspace = true +bytemuck.workspace = true cfg-if.workspace = true eframe = { workspace = true, default-features = false, features = [ "default_fonts", diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 1dc59804914b..e21d69061327 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -389,6 +389,11 @@ impl App { Command::PlaybackRestart => { self.run_time_control_command(TimeControlCommand::Restart); } + + #[cfg(not(target_arch = "wasm32"))] + Command::ScreenshotWholeApp => { + _frame.request_screenshot(); + } } } @@ -702,6 +707,15 @@ impl eframe::App for App { re_log::warn_once!("Blueprint unexpectedly missing from store."); } } + + #[cfg(not(target_arch = "wasm32"))] + fn post_rendering(&mut self, _window_size: [u32; 2], frame: &eframe::Frame) { + if let Some(screenshot) = frame.screenshot() { + re_viewer_context::Clipboard::with(|cb| { + cb.set_image(screenshot.size, bytemuck::cast_slice(&screenshot.pixels)); + }); + } + } } fn paint_background_fill(ui: &mut egui::Ui) { From c026d0633e178f3c0d7a6ae104c9f0670c635d3e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 17:42:36 +0200 Subject: [PATCH 02/14] Style as web app when taking screenshot --- crates/re_ui/examples/re_ui_example.rs | 2 +- crates/re_ui/src/lib.rs | 3 +- crates/re_viewer/src/app.rs | 16 +++++--- crates/re_viewer/src/lib.rs | 1 + crates/re_viewer/src/screenshotter.rs | 52 ++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 crates/re_viewer/src/screenshotter.rs diff --git a/crates/re_ui/examples/re_ui_example.rs b/crates/re_ui/examples/re_ui_example.rs index cfa777aefe04..07736446326e 100644 --- a/crates/re_ui/examples/re_ui_example.rs +++ b/crates/re_ui/examples/re_ui_example.rs @@ -238,7 +238,7 @@ impl ExampleApp { }; let top_bar_style = self .re_ui - .top_bar_style(native_pixels_per_point, fullscreen); + .top_bar_style(native_pixels_per_point, fullscreen, false); egui::TopBottomPanel::top("top_bar") .frame(self.re_ui.top_panel_frame()) diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index a3375341d0b1..ea0970d5b0d6 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -236,6 +236,7 @@ impl ReUi { &self, native_pixels_per_point: Option, fullscreen: bool, + style_like_web: bool, ) -> TopBarStyle { let gui_zoom = if let Some(native_pixels_per_point) = native_pixels_per_point { native_pixels_per_point / self.egui_ctx.pixels_per_point() @@ -245,7 +246,7 @@ impl ReUi { // On Mac, we share the same space as the native red/yellow/green close/minimize/maximize buttons. // This means we need to make room for them. - let make_room_for_window_buttons = { + let make_room_for_window_buttons = !style_like_web && { #[cfg(target_os = "macos")] { crate::FULLSIZE_CONTENT && !fullscreen diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index e21d69061327..619b705d8666 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -60,6 +60,7 @@ pub struct App { startup_options: StartupOptions, ram_limit_warner: re_memory::RamLimitWarner, re_ui: re_ui::ReUi, + screenshotter: crate::screenshotter::Screenshotter, /// Listens to the local text log stream text_log_rx: std::sync::mpsc::Receiver, @@ -151,6 +152,8 @@ impl App { startup_options, ram_limit_warner: re_memory::RamLimitWarner::warn_at_fraction_of_max(0.75), re_ui, + screenshotter: Default::default(), + text_log_rx, component_ui_registry: re_data_ui::create_component_ui_registry(), rx, @@ -392,7 +395,7 @@ impl App { #[cfg(not(target_arch = "wasm32"))] Command::ScreenshotWholeApp => { - _frame.request_screenshot(); + self.screenshotter.request_screenshot(); } } } @@ -512,6 +515,8 @@ impl eframe::App for App { fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { let frame_start = Instant::now(); + self.screenshotter.update(egui_ctx, frame); + if self.startup_options.memory_limit.limit.is_none() { // we only warn about high memory usage if the user hasn't specified a limit self.ram_limit_warner.update(); @@ -711,9 +716,7 @@ impl eframe::App for App { #[cfg(not(target_arch = "wasm32"))] fn post_rendering(&mut self, _window_size: [u32; 2], frame: &eframe::Frame) { if let Some(screenshot) = frame.screenshot() { - re_viewer_context::Clipboard::with(|cb| { - cb.set_image(screenshot.size, bytemuck::cast_slice(&screenshot.pixels)); - }); + self.screenshotter.save(&screenshot); } } } @@ -1251,7 +1254,10 @@ fn top_panel( frame.info().window_info.fullscreen } }; - let top_bar_style = app.re_ui.top_bar_style(native_pixels_per_point, fullscreen); + let style_like_web = app.screenshotter.is_screenshotting(); + let top_bar_style = + app.re_ui + .top_bar_style(native_pixels_per_point, fullscreen, style_like_web); egui::TopBottomPanel::top("top_bar") .frame(app.re_ui.top_panel_frame()) diff --git a/crates/re_viewer/src/lib.rs b/crates/re_viewer/src/lib.rs index d723c9eccfb0..23bc4937ccdc 100644 --- a/crates/re_viewer/src/lib.rs +++ b/crates/re_viewer/src/lib.rs @@ -9,6 +9,7 @@ pub mod env_vars; #[cfg(not(target_arch = "wasm32"))] mod profiler; mod remote_viewer_app; +mod screenshotter; mod ui; mod viewer_analytics; diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs new file mode 100644 index 000000000000..f57e143df675 --- /dev/null +++ b/crates/re_viewer/src/screenshotter.rs @@ -0,0 +1,52 @@ +//! Screenshotting not implemented on web yet because +//! we haven't implemented "copy image to clipbaord". + +/// Helper for screenshotting the entire app +#[derive(Default)] +pub struct Screenshotter { + #[cfg(not(target_arch = "wasm32"))] + countdown: Option, +} + +#[cfg(not(target_arch = "wasm32"))] +impl Screenshotter { + /// Call once per frame + pub fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { + if let Some(countdown) = &mut self.countdown { + if *countdown == 0 { + frame.request_screenshot(); + } else { + *countdown -= 1; + } + + egui_ctx.request_repaint(); // Make sure we keep counting down + } + } + + /// If true, temporarily re-style the UI to make it suitable for capture! + pub fn is_screenshotting(&self) -> bool { + self.countdown.is_some() + } + + pub fn request_screenshot(&mut self) { + self.countdown = Some(1); + } + + pub fn save(&mut self, image: &egui::ColorImage) { + self.countdown = None; + re_viewer_context::Clipboard::with(|cb| { + cb.set_image(image.size, bytemuck::cast_slice(&image.pixels)); + }); + } +} + +#[cfg(target_arch = "wasm32")] +impl Screenshotter { + #[allow(clippy::unused_self)] + pub fn update(&mut self, _egui_ctx: &egui::Context, _frame: &mut eframe::Frame) {} + + #[allow(clippy::unused_self)] + pub fn is_screenshotting(&self) -> bool { + false + } +} From 793c74d9fa437cb90e05bdc6214408bfb3b964f9 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 17:47:55 +0200 Subject: [PATCH 03/14] Hide toast notifications from screenshots --- crates/re_viewer/src/app.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 619b705d8666..c7c316b14b7d 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -689,7 +689,10 @@ impl eframe::App for App { } self.handle_dropping_files(egui_ctx); - self.toasts.show(egui_ctx); + + if !self.screenshotter.is_screenshotting() { + self.toasts.show(egui_ctx); + } if let Some(cmd) = self.cmd_palette.show(egui_ctx) { self.pending_commands.push(cmd); From de0f78f328846ae7c45ca568660e9fae2d33204e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 17:48:50 +0200 Subject: [PATCH 04/14] improve docstring --- crates/re_viewer/src/screenshotter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index f57e143df675..c80a4211884c 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -1,5 +1,5 @@ -//! Screenshotting not implemented on web yet because -//! we haven't implemented "copy image to clipbaord". +//! Screenshotting not implemented on web yet because we +//! haven't implemented "copy image to clipboard" there. /// Helper for screenshotting the entire app #[derive(Default)] From e526e69b72d0ebd8502f7fb740bc813898214426 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 17:52:35 +0200 Subject: [PATCH 05/14] Explain why we re-style the UI --- crates/re_viewer/src/screenshotter.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index c80a4211884c..3f2519834ef6 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -24,6 +24,9 @@ impl Screenshotter { } /// If true, temporarily re-style the UI to make it suitable for capture! + /// + /// We do the re-styling to create consistent screenshots across platforms. + /// In particular, we style the UI to look like the web viewer. pub fn is_screenshotting(&self) -> bool { self.countdown.is_some() } From 1d6c4d94fd1c886bcae7e07a3ea1eaad6b0e3720 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 18:13:49 +0200 Subject: [PATCH 06/14] Add command line option to take a screenshot and then quit --- crates/re_data_ui/src/image.rs | 1 - crates/re_viewer/src/app.rs | 26 ++++++++++++++++-- crates/re_viewer/src/remote_viewer_app.rs | 2 +- crates/re_viewer/src/screenshotter.rs | 32 ++++++++++++++++++++-- crates/rerun/src/run.rs | 14 +++++++--- examples/rust/extend_viewer_ui/src/main.rs | 2 +- 6 files changed, 65 insertions(+), 12 deletions(-) diff --git a/crates/re_data_ui/src/image.rs b/crates/re_data_ui/src/image.rs index a086fe44b28e..75809168b913 100644 --- a/crates/re_data_ui/src/image.rs +++ b/crates/re_data_ui/src/image.rs @@ -713,7 +713,6 @@ fn save_image(tensor: &re_components::Tensor, dynamic_image: &image::DynamicImag .save_file() { match dynamic_image.save(&path) { - // TODO(emilk): show a popup instead of logging result Ok(()) => { re_log::info!("Image saved to {path:?}"); } diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index c7c316b14b7d..cd95c10893f0 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -41,10 +41,25 @@ enum TimeControlCommand { // ---------------------------------------------------------------------------- /// Settings set once at startup (e.g. via command-line options) and not serialized. -#[derive(Clone, Copy, Default)] +#[derive(Clone)] pub struct StartupOptions { pub memory_limit: re_memory::MemoryLimit, + pub persist_state: bool, + + /// Take a screenshot of the app and quit. + /// We use this to generate screenshots of our exmples. + pub screenshot_to_path_then_quite: Option, +} + +impl Default for StartupOptions { + fn default() -> Self { + Self { + memory_limit: re_memory::MemoryLimit::default(), + persist_state: true, + screenshot_to_path_then_quite: None, + } + } } // ---------------------------------------------------------------------------- @@ -147,12 +162,19 @@ impl App { ); } + let mut screenshotter = crate::screenshotter::Screenshotter::default(); + + #[cfg(not(target_arch = "wasm32"))] + if let Some(screenshot_path) = startup_options.screenshot_to_path_then_quite.clone() { + screenshotter.screenshot_to_path_then_quit(screenshot_path); + } + Self { build_info, startup_options, ram_limit_warner: re_memory::RamLimitWarner::warn_at_fraction_of_max(0.75), re_ui, - screenshotter: Default::default(), + screenshotter, text_log_rx, component_ui_registry: re_data_ui::create_component_ui_registry(), diff --git a/crates/re_viewer/src/remote_viewer_app.rs b/crates/re_viewer/src/remote_viewer_app.rs index fb3c8995de17..6f0302c97167 100644 --- a/crates/re_viewer/src/remote_viewer_app.rs +++ b/crates/re_viewer/src/remote_viewer_app.rs @@ -75,7 +75,7 @@ impl RemoteViewerApp { let app = crate::App::from_receiver( self.build_info, &self.app_env, - self.startup_options, + self.startup_options.clone(), self.re_ui.clone(), storage, rx, diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index 3f2519834ef6..8260850abf8e 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -6,10 +6,19 @@ pub struct Screenshotter { #[cfg(not(target_arch = "wasm32"))] countdown: Option, + + #[cfg(not(target_arch = "wasm32"))] + target_path: Option, } #[cfg(not(target_arch = "wasm32"))] impl Screenshotter { + /// Used for generating screenshots in dev builds. + pub fn screenshot_to_path_then_quit(&mut self, path: std::path::PathBuf) { + self.request_screenshot(); + self.target_path = Some(path); + } + /// Call once per frame pub fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { if let Some(countdown) = &mut self.countdown { @@ -37,9 +46,26 @@ impl Screenshotter { pub fn save(&mut self, image: &egui::ColorImage) { self.countdown = None; - re_viewer_context::Clipboard::with(|cb| { - cb.set_image(image.size, bytemuck::cast_slice(&image.pixels)); - }); + if let Some(path) = self.target_path.take() { + let w = image.width() as _; + let h = image.height() as _; + let image = + image::RgbaImage::from_raw(w, h, bytemuck::pod_collect_to_vec(&image.pixels)) + .expect("Failed to create image"); + match image.save(&path) { + Ok(()) => { + re_log::info!("Screenshot saved to {path:?}"); + std::process::exit(0); // Close nicely + } + Err(err) => { + panic!("Failed saving screenshot to {path:?}: {err}"); + } + } + } else { + re_viewer_context::Clipboard::with(|cb| { + cb.set_image(image.size, bytemuck::cast_slice(&image.pixels)); + }); + } } } diff --git a/crates/rerun/src/run.rs b/crates/rerun/src/run.rs index e6c1c9b56790..c9b5e28fbcc0 100644 --- a/crates/rerun/src/run.rs +++ b/crates/rerun/src/run.rs @@ -43,6 +43,10 @@ struct Args { #[command(subcommand)] commands: Option, + /// What bind address IP to use. + #[clap(long, default_value = "0.0.0.0")] + bind: String, + /// Set a maximum input latency, e.g. "200ms" or "10s". /// /// If we go over this, we start dropping packets. @@ -86,6 +90,11 @@ struct Args { #[clap(long)] save: Option, + /// Take a screenshot of the app and quit. + /// We use this to generate screenshots of our exmples. + #[clap(long)] + screenshot_to: Option, + /// Exit with a non-zero exit code if any warning or error is logged. Useful for tests. #[clap(long)] strict: bool, @@ -115,10 +124,6 @@ struct Args { #[clap(long)] web_viewer: bool, - /// What bind address IP to use. - #[clap(long, default_value = "0.0.0.0")] - bind: String, - /// What port do we listen to for hosting the web viewer over HTTP. /// A port of 0 will pick a random port. #[cfg(feature = "web_viewer")] @@ -308,6 +313,7 @@ async fn run_impl( .unwrap_or_else(|err| panic!("Bad --memory-limit: {err}")) }), persist_state: args.persist_state, + screenshot_to_path_then_quite: args.screenshot_to.clone(), }; // Where do we get the data from? diff --git a/examples/rust/extend_viewer_ui/src/main.rs b/examples/rust/extend_viewer_ui/src/main.rs index 317062d7a7fd..356ad5d09e77 100644 --- a/examples/rust/extend_viewer_ui/src/main.rs +++ b/examples/rust/extend_viewer_ui/src/main.rs @@ -39,7 +39,7 @@ async fn main() -> Result<(), Box> { // Start pruning the data once we reach this much memory allocated limit: Some(12_000_000_000), }, - persist_state: true, + ..Default::default() }; // This is used for analytics, if the `analytics` feature is on in `Cargo.toml` From 1b7962fb30f369f151def6a21f98d66dfde02a47 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 18:24:29 +0200 Subject: [PATCH 07/14] Fix web build --- crates/re_viewer/src/app.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index cd95c10893f0..da001fbae61c 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -49,6 +49,7 @@ pub struct StartupOptions { /// Take a screenshot of the app and quit. /// We use this to generate screenshots of our exmples. + #[cfg(not(target_arch = "wasm32"))] pub screenshot_to_path_then_quite: Option, } @@ -57,6 +58,7 @@ impl Default for StartupOptions { Self { memory_limit: re_memory::MemoryLimit::default(), persist_state: true, + #[cfg(not(target_arch = "wasm32"))] screenshot_to_path_then_quite: None, } } @@ -162,6 +164,7 @@ impl App { ); } + #[allow(unused_mut, clippy::needless_update)] // false positive on web let mut screenshotter = crate::screenshotter::Screenshotter::default(); #[cfg(not(target_arch = "wasm32"))] From 8884d494b3cea12c149d21cb6bef9170fef85b5c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 18:37:29 +0200 Subject: [PATCH 08/14] Take the screenshots at 800x600 with 2x pixels-per-point --- crates/re_viewer/src/app.rs | 6 +++++- crates/re_viewer/src/screenshotter.rs | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index da001fbae61c..4dd265d732a0 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -548,7 +548,11 @@ impl eframe::App for App { } #[cfg(not(target_arch = "wasm32"))] - { + if self.screenshotter.is_screenshotting() { + // Set a standard screenshot resolution (for our docs and examples). + frame.set_window_size(egui::Vec2::new(800.0, 600.0)); + egui_ctx.set_pixels_per_point(2.0); + } else { // Ensure zoom factor is sane and in 10% steps at all times before applying it. { let mut zoom_factor = self.state.app_options.zoom_factor; diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index 8260850abf8e..6939f5290bf5 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -41,7 +41,8 @@ impl Screenshotter { } pub fn request_screenshot(&mut self) { - self.countdown = Some(1); + // give app time to set window size, and then wait for animations to finish etc: + self.countdown = Some(10); } pub fn save(&mut self, image: &egui::ColorImage) { From 4e184db5cdeeaeb7d804e009d4bd6f3a7ca67035 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 19:27:50 +0200 Subject: [PATCH 09/14] Fix typo --- crates/re_viewer/src/app.rs | 6 +++--- crates/rerun/src/run.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 4dd265d732a0..a610131d6e18 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -50,7 +50,7 @@ pub struct StartupOptions { /// Take a screenshot of the app and quit. /// We use this to generate screenshots of our exmples. #[cfg(not(target_arch = "wasm32"))] - pub screenshot_to_path_then_quite: Option, + pub screenshot_to_path_then_quit: Option, } impl Default for StartupOptions { @@ -59,7 +59,7 @@ impl Default for StartupOptions { memory_limit: re_memory::MemoryLimit::default(), persist_state: true, #[cfg(not(target_arch = "wasm32"))] - screenshot_to_path_then_quite: None, + screenshot_to_path_then_quit: None, } } } @@ -168,7 +168,7 @@ impl App { let mut screenshotter = crate::screenshotter::Screenshotter::default(); #[cfg(not(target_arch = "wasm32"))] - if let Some(screenshot_path) = startup_options.screenshot_to_path_then_quite.clone() { + if let Some(screenshot_path) = startup_options.screenshot_to_path_then_quit.clone() { screenshotter.screenshot_to_path_then_quit(screenshot_path); } diff --git a/crates/rerun/src/run.rs b/crates/rerun/src/run.rs index c9b5e28fbcc0..64f9954f1bb9 100644 --- a/crates/rerun/src/run.rs +++ b/crates/rerun/src/run.rs @@ -313,7 +313,7 @@ async fn run_impl( .unwrap_or_else(|err| panic!("Bad --memory-limit: {err}")) }), persist_state: args.persist_state, - screenshot_to_path_then_quite: args.screenshot_to.clone(), + screenshot_to_path_then_quit: args.screenshot_to.clone(), }; // Where do we get the data from? From 84e36147a0577b47d25bea4a87d76ede65a21571 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 19:42:27 +0200 Subject: [PATCH 10/14] Add --window-size argument --- crates/re_viewer/src/app.rs | 15 +++++++++++++-- crates/re_viewer/src/screenshotter.rs | 4 ++-- crates/rerun/src/run.rs | 25 +++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index a610131d6e18..20becf04b9db 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -51,6 +51,10 @@ pub struct StartupOptions { /// We use this to generate screenshots of our exmples. #[cfg(not(target_arch = "wasm32"))] pub screenshot_to_path_then_quit: Option, + + /// Set the screen resolution in logical points. + #[cfg(not(target_arch = "wasm32"))] + pub resolution_in_points: Option<[f32; 2]>, } impl Default for StartupOptions { @@ -58,8 +62,12 @@ impl Default for StartupOptions { Self { memory_limit: re_memory::MemoryLimit::default(), persist_state: true, + #[cfg(not(target_arch = "wasm32"))] screenshot_to_path_then_quit: None, + + #[cfg(not(target_arch = "wasm32"))] + resolution_in_points: None, } } } @@ -540,6 +548,10 @@ impl eframe::App for App { fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { let frame_start = Instant::now(); + if let Some(resolution_in_points) = self.startup_options.resolution_in_points.take() { + frame.set_window_size(resolution_in_points.into()); + } + self.screenshotter.update(egui_ctx, frame); if self.startup_options.memory_limit.limit.is_none() { @@ -549,8 +561,7 @@ impl eframe::App for App { #[cfg(not(target_arch = "wasm32"))] if self.screenshotter.is_screenshotting() { - // Set a standard screenshot resolution (for our docs and examples). - frame.set_window_size(egui::Vec2::new(800.0, 600.0)); + // Make screenshots high-quality by pretending we have a high-dpi display, whether we do or not: egui_ctx.set_pixels_per_point(2.0); } else { // Ensure zoom factor is sane and in 10% steps at all times before applying it. diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index 6939f5290bf5..ce0772b8ecff 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -41,8 +41,8 @@ impl Screenshotter { } pub fn request_screenshot(&mut self) { - // give app time to set window size, and then wait for animations to finish etc: - self.countdown = Some(10); + // Give app time to change the style, and then wait for animations to finish: + self.countdown = Some(3); } pub fn save(&mut self, image: &egui::ColorImage) { diff --git a/crates/rerun/src/run.rs b/crates/rerun/src/run.rs index 64f9954f1bb9..df663808de2f 100644 --- a/crates/rerun/src/run.rs +++ b/crates/rerun/src/run.rs @@ -92,6 +92,7 @@ struct Args { /// Take a screenshot of the app and quit. /// We use this to generate screenshots of our exmples. + /// Useful together with `--window-size`. #[clap(long)] screenshot_to: Option, @@ -130,6 +131,11 @@ struct Args { #[clap(long, default_value_t = Default::default())] web_viewer_port: WebViewerServerPort, + /// Set the screen resolution (in logical points), e.g. "1920x1080". + /// Useful together with `--screenshot-to`. + #[clap(long)] + window_size: Option, + /// What port do we listen to for incoming websocket connections from the viewer /// A port of 0 will pick a random port. #[cfg(feature = "web_viewer")] @@ -314,6 +320,13 @@ async fn run_impl( }), persist_state: args.persist_state, screenshot_to_path_then_quit: args.screenshot_to.clone(), + + // TODO(emilk): make it easy to set this on eframe instead + resolution_in_points: if let Some(size) = &args.window_size { + Some(parse_size(size)?) + } else { + None + }, }; // Where do we get the data from? @@ -507,6 +520,18 @@ async fn run_impl( } } +fn parse_size(size: &str) -> anyhow::Result<[f32; 2]> { + fn parse_size_inner(size: &str) -> Option<[f32; 2]> { + let (w, h) = size.split_once('x')?; + let w = w.parse().ok()?; + let h = h.parse().ok()?; + Some([w, h]) + } + + parse_size_inner(size) + .ok_or_else(|| anyhow::anyhow!("Invalid size {:?}, expected e.g. 800x600", size)) +} + // NOTE: This is only used as part of end-to-end tests. fn assert_receive_into_log_db(rx: &Receiver) -> anyhow::Result { use re_smart_channel::RecvTimeoutError; From 3df8b6d0975dab067d575eb60b414b0754bd8b7f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 19:46:11 +0200 Subject: [PATCH 11/14] Leave the quitting to app.rs --- crates/re_viewer/src/app.rs | 5 ++++- crates/re_viewer/src/screenshotter.rs | 27 ++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 20becf04b9db..b86be5d81aee 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -552,7 +552,10 @@ impl eframe::App for App { frame.set_window_size(resolution_in_points.into()); } - self.screenshotter.update(egui_ctx, frame); + if self.screenshotter.update(egui_ctx, frame).quit { + frame.close(); + return; + } if self.startup_options.memory_limit.limit.is_none() { // we only warn about high memory usage if the user hasn't specified a limit diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index ce0772b8ecff..3ecbcf0d254e 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -2,13 +2,18 @@ //! haven't implemented "copy image to clipboard" there. /// Helper for screenshotting the entire app +#[cfg(not(target_arch = "wasm32"))] #[derive(Default)] pub struct Screenshotter { - #[cfg(not(target_arch = "wasm32"))] countdown: Option, - - #[cfg(not(target_arch = "wasm32"))] target_path: Option, + quit: bool, +} + +#[must_use] +pub struct ScreenshotterOutput { + /// If true, the screenshotter was told at startup to quit after its donw. + pub quit: bool, } #[cfg(not(target_arch = "wasm32"))] @@ -20,7 +25,11 @@ impl Screenshotter { } /// Call once per frame - pub fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { + pub fn update( + &mut self, + egui_ctx: &egui::Context, + frame: &mut eframe::Frame, + ) -> ScreenshotterOutput { if let Some(countdown) = &mut self.countdown { if *countdown == 0 { frame.request_screenshot(); @@ -30,6 +39,8 @@ impl Screenshotter { egui_ctx.request_repaint(); // Make sure we keep counting down } + + ScreenshotterOutput { quit: self.quit } } /// If true, temporarily re-style the UI to make it suitable for capture! @@ -56,7 +67,7 @@ impl Screenshotter { match image.save(&path) { Ok(()) => { re_log::info!("Screenshot saved to {path:?}"); - std::process::exit(0); // Close nicely + self.quit = true; } Err(err) => { panic!("Failed saving screenshot to {path:?}: {err}"); @@ -70,6 +81,12 @@ impl Screenshotter { } } +// ---------------------------------------------------------------------------- + +#[cfg(target_arch = "wasm32")] +#[derive(Default)] +pub struct Screenshotter {} + #[cfg(target_arch = "wasm32")] impl Screenshotter { #[allow(clippy::unused_self)] From 174c389148068902daed7f35e6371a90790e3f89 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 19:50:22 +0200 Subject: [PATCH 12/14] Add some clarifying comments and asserts --- crates/re_viewer/src/screenshotter.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index 3ecbcf0d254e..497eb6c11584 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -19,11 +19,19 @@ pub struct ScreenshotterOutput { #[cfg(not(target_arch = "wasm32"))] impl Screenshotter { /// Used for generating screenshots in dev builds. + /// + /// Should only be called at startup. pub fn screenshot_to_path_then_quit(&mut self, path: std::path::PathBuf) { + assert!(self.countdown.is_none(), "screenshotter misused"); self.request_screenshot(); self.target_path = Some(path); } + pub fn request_screenshot(&mut self) { + // Give app time to change the style, and then wait for animations to finish: + self.countdown = Some(10); + } + /// Call once per frame pub fn update( &mut self, @@ -51,11 +59,6 @@ impl Screenshotter { self.countdown.is_some() } - pub fn request_screenshot(&mut self) { - // Give app time to change the style, and then wait for animations to finish: - self.countdown = Some(3); - } - pub fn save(&mut self, image: &egui::ColorImage) { self.countdown = None; if let Some(path) = self.target_path.take() { From fa9164de985d61ca85105f05ae72637ffa6d6c1b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 19:59:41 +0200 Subject: [PATCH 13/14] Fix wasm build --- crates/re_viewer/src/app.rs | 2 ++ crates/re_viewer/src/screenshotter.rs | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index b86be5d81aee..9c4aa0860b1a 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -548,10 +548,12 @@ impl eframe::App for App { fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { let frame_start = Instant::now(); + #[cfg(not(target_arch = "wasm32"))] if let Some(resolution_in_points) = self.startup_options.resolution_in_points.take() { frame.set_window_size(resolution_in_points.into()); } + #[cfg(not(target_arch = "wasm32"))] if self.screenshotter.update(egui_ctx, frame).quit { frame.close(); return; diff --git a/crates/re_viewer/src/screenshotter.rs b/crates/re_viewer/src/screenshotter.rs index 497eb6c11584..1d2728b8d070 100644 --- a/crates/re_viewer/src/screenshotter.rs +++ b/crates/re_viewer/src/screenshotter.rs @@ -10,6 +10,7 @@ pub struct Screenshotter { quit: bool, } +#[cfg(not(target_arch = "wasm32"))] #[must_use] pub struct ScreenshotterOutput { /// If true, the screenshotter was told at startup to quit after its donw. @@ -92,9 +93,6 @@ pub struct Screenshotter {} #[cfg(target_arch = "wasm32")] impl Screenshotter { - #[allow(clippy::unused_self)] - pub fn update(&mut self, _egui_ctx: &egui::Context, _frame: &mut eframe::Frame) {} - #[allow(clippy::unused_self)] pub fn is_screenshotting(&self) -> bool { false From 11b16c5e427b64b9dec7c895e9daec58d590b205 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 31 May 2023 20:02:05 +0200 Subject: [PATCH 14/14] Fix warning --- crates/rerun/src/run.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/rerun/src/run.rs b/crates/rerun/src/run.rs index df663808de2f..1f410cc1a9c7 100644 --- a/crates/rerun/src/run.rs +++ b/crates/rerun/src/run.rs @@ -520,6 +520,7 @@ async fn run_impl( } } +#[cfg(feature = "native_viewer")] fn parse_size(size: &str) -> anyhow::Result<[f32; 2]> { fn parse_size_inner(size: &str) -> Option<[f32; 2]> { let (w, h) = size.split_once('x')?;