From 0bb756d878f6972b6a3ce3b2722c066c03f262b9 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Wed, 7 Jun 2023 18:56:37 +0200 Subject: [PATCH 01/21] Move StoreHub out of viewer --- Cargo.lock | 2 + crates/re_viewer/Cargo.toml | 2 + crates/re_viewer/src/app.rs | 340 +++++++++------------ crates/re_viewer/src/app_state.rs | 65 ++-- crates/re_viewer/src/lib.rs | 2 +- crates/re_viewer/src/loading.rs | 12 +- crates/re_viewer/src/store_hub.rs | 165 +++++++++- crates/re_viewer/src/ui/blueprint.rs | 9 +- crates/re_viewer/src/ui/blueprint_load.rs | 80 ++--- crates/re_viewer/src/ui/memory_panel.rs | 9 +- crates/re_viewer/src/ui/rerun_menu.rs | 81 +++-- crates/re_viewer/src/ui/top_panel.rs | 8 +- examples/rust/extend_viewer_ui/src/main.rs | 12 +- 13 files changed, 434 insertions(+), 353 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d38ad6025a71..560dc5a12d42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4470,7 +4470,9 @@ dependencies = [ "egui-wgpu", "image", "itertools", + "lazy_static", "objc", + "parking_lot 0.12.1", "poll-promise", "puffin", "puffin_http", diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index 2b8743366d27..97c91a4d6c3b 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -87,6 +87,8 @@ image = { workspace = true, default-features = false, features = [ "png", ] } itertools = { workspace = true } +lazy_static.workspace = true +parking_lot.workspace = true poll-promise = "0.2" rfd.workspace = true serde = { version = "1", features = ["derive"] } diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index fb2de5fbfba2..ad7d3af9f8cf 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -1,9 +1,8 @@ use itertools::Itertools as _; use web_time::Instant; -use re_arrow_store::{DataStoreConfig, DataStoreStats}; use re_data_store::store_db::StoreDb; -use re_log_types::{ApplicationId, LogMsg, StoreId, StoreKind}; +use re_log_types::{LogMsg, StoreId, StoreKind}; use re_renderer::WgpuResourcePoolStatistics; use re_smart_channel::Receiver; use re_ui::{toasts, Command}; @@ -13,8 +12,11 @@ use re_viewer_context::{ }; use crate::{ - background_tasks::BackgroundTasks, ui::Blueprint, viewer_analytics::ViewerAnalytics, AppState, - StoreHub, + background_tasks::BackgroundTasks, + store_hub::{StoreHub, StoreHubStats, StoreView}, + ui::Blueprint, + viewer_analytics::ViewerAnalytics, + AppState, StoreBundle, }; // ---------------------------------------------------------------------------- @@ -87,15 +89,15 @@ pub struct App { rx: Receiver, - /// Where the recordings and blueprints are stored. - pub(crate) store_hub: crate::StoreHub, - /// What is serialized pub(crate) state: AppState, /// Pending background tasks, e.g. files being saved. pub(crate) background_tasks: BackgroundTasks, + /// Recording Id to switch to + pub(crate) requested_recording_id: Option, + /// Toast notifications. toasts: toasts::Toasts, @@ -177,9 +179,9 @@ impl App { text_log_rx, component_ui_registry: re_data_ui::create_component_ui_registry(), rx, - store_hub: Default::default(), state, background_tasks: Default::default(), + requested_recording_id: Default::default(), toasts: toasts::Toasts::new(), memory_panel: Default::default(), memory_panel_open: false, @@ -226,10 +228,6 @@ impl App { &self.rx } - pub fn store_hub(&self) -> &crate::StoreHub { - &self.store_hub - } - /// Adds a new space view class to the viewer. pub fn add_space_view_class( &mut self, @@ -246,12 +244,13 @@ impl App { fn run_pending_commands( &mut self, blueprint: &mut Blueprint, + store_view: &StoreView<'_>, egui_ctx: &egui::Context, frame: &mut eframe::Frame, ) { let commands = self.pending_commands.drain(..).collect_vec(); for cmd in commands { - self.run_command(cmd, blueprint, frame, egui_ctx); + self.run_command(cmd, blueprint, store_view, frame, egui_ctx); } } @@ -259,6 +258,7 @@ impl App { &mut self, cmd: Command, blueprint: &mut Blueprint, + store_view: &StoreView<'_>, _frame: &mut eframe::Frame, egui_ctx: &egui::Context, ) { @@ -267,11 +267,11 @@ impl App { match cmd { #[cfg(not(target_arch = "wasm32"))] Command::Save => { - save(self, None); + save(self, store_view, None); } #[cfg(not(target_arch = "wasm32"))] Command::SaveSelection => { - save(self, self.state.loop_selection()); + save(self, store_view, self.state.loop_selection(store_view)); } #[cfg(not(target_arch = "wasm32"))] Command::Open => { @@ -335,18 +335,20 @@ impl App { Command::SelectionPrevious => { let state = &mut self.state; - if let Some(rec_cfg) = state - .recording_id() - .and_then(|rec_id| state.recording_config_mut(&rec_id)) + if let Some(rec_cfg) = store_view + .recording + .map(|rec| rec.store_id()) + .and_then(|rec_id| state.recording_config_mut(rec_id)) { rec_cfg.selection_state.select_previous(); } } Command::SelectionNext => { let state = &mut self.state; - if let Some(rec_cfg) = state - .recording_id() - .and_then(|rec_id| state.recording_config_mut(&rec_id)) + if let Some(rec_cfg) = store_view + .recording + .map(|rec| rec.store_id()) + .and_then(|rec_id| state.recording_config_mut(rec_id)) { rec_cfg.selection_state.select_next(); } @@ -356,19 +358,19 @@ impl App { } Command::PlaybackTogglePlayPause => { - self.run_time_control_command(TimeControlCommand::TogglePlayPause); + self.run_time_control_command(store_view, TimeControlCommand::TogglePlayPause); } Command::PlaybackFollow => { - self.run_time_control_command(TimeControlCommand::Follow); + self.run_time_control_command(store_view, TimeControlCommand::Follow); } Command::PlaybackStepBack => { - self.run_time_control_command(TimeControlCommand::StepBack); + self.run_time_control_command(store_view, TimeControlCommand::StepBack); } Command::PlaybackStepForward => { - self.run_time_control_command(TimeControlCommand::StepForward); + self.run_time_control_command(store_view, TimeControlCommand::StepForward); } Command::PlaybackRestart => { - self.run_time_control_command(TimeControlCommand::Restart); + self.run_time_control_command(store_view, TimeControlCommand::Restart); } #[cfg(not(target_arch = "wasm32"))] @@ -378,12 +380,16 @@ impl App { } } - fn run_time_control_command(&mut self, command: TimeControlCommand) { - let Some(rec_id) = self.state.recording_id() else { return; }; - let Some(rec_cfg) = self.state.recording_config_mut(&rec_id) else { return; }; + fn run_time_control_command( + &mut self, + store_view: &StoreView<'_>, + command: TimeControlCommand, + ) { + let Some(store_db) = store_view.recording else { return; }; + let rec_id = store_db.store_id(); + let Some(rec_cfg) = self.state.recording_config_mut(rec_id) else { return; }; let time_ctrl = &mut rec_cfg.time_ctrl; - let Some(store_db) = self.store_hub.recording(&rec_id) else { return; }; let times_per_timeline = store_db.times_per_timeline(); match command { @@ -405,25 +411,11 @@ impl App { } } - /// The app ID of active blueprint. - pub fn selected_app_id(&self) -> ApplicationId { - self.recording_db() - .and_then(|store_db| { - store_db - .store_info() - .map(|store_info| store_info.application_id.clone()) - }) - .unwrap_or(ApplicationId::unknown()) - } - fn memory_panel_ui( &mut self, ui: &mut egui::Ui, gpu_resource_stats: &WgpuResourcePoolStatistics, - store_config: &DataStoreConfig, - blueprint_config: &DataStoreConfig, - store_stats: &DataStoreStats, - blueprint_stats: &DataStoreStats, + store_stats: &StoreHubStats, ) { let frame = egui::Frame { fill: ui.visuals().panel_fill, @@ -439,24 +431,14 @@ impl App { ui, &self.startup_options.memory_limit, gpu_resource_stats, - store_config, - blueprint_config, - store_stats, - blueprint_stats, + &store_stats.recording_config, + &store_stats.blueprint_config, + &store_stats.recording_stats, + &store_stats.blueprint_stats, ); }); } - // Materialize the blueprint from the DB if the selected blueprint id isn't the default one. - fn load_or_create_blueprint( - &mut self, - this_frame_blueprint_id: &StoreId, - egui_ctx: &egui::Context, - ) -> Blueprint { - let blueprint_db = self.store_hub.blueprint_entry(this_frame_blueprint_id); - Blueprint::from_db(egui_ctx, blueprint_db) - } - /// Top-level ui function. /// /// Shows the viewer ui. @@ -467,15 +449,9 @@ impl App { frame: &mut eframe::Frame, blueprint: &mut Blueprint, gpu_resource_stats: &WgpuResourcePoolStatistics, - blueprint_config: &DataStoreConfig, - store_stats: &DataStoreStats, - blueprint_stats: &DataStoreStats, + store_view: &StoreView<'_>, + store_stats: &StoreHubStats, ) { - let store_config = self - .recording_db() - .map(|store_db| store_db.entity_db.data_store.config().clone()) - .unwrap_or_default(); - let mut main_panel_frame = egui::Frame::default(); if re_ui::CUSTOM_WINDOW_DECORATIONS { // Add some margin so that we can later paint an outline around it all. @@ -489,23 +465,11 @@ impl App { crate::ui::mobile_warning_ui(&self.re_ui, ui); - crate::ui::top_panel(&*blueprint, ui, frame, self, gpu_resource_stats); + crate::ui::top_panel(blueprint, store_view, ui, frame, self, gpu_resource_stats); - self.memory_panel_ui( - ui, - gpu_resource_stats, - &store_config, - blueprint_config, - store_stats, - blueprint_stats, - ); + self.memory_panel_ui(ui, gpu_resource_stats, store_stats); - // NOTE: cannot call `.store_db()` due to borrowck shenanigans - if let Some(store_db) = self - .state - .recording_id() - .and_then(|rec_id| self.store_hub.recording(&rec_id)) - { + if let Some(store_db) = store_view.recording { self.state .recording_config_entry( store_db.store_id().clone(), @@ -600,14 +564,16 @@ impl App { match msg.info.store_id.kind { StoreKind::Recording => { re_log::debug!("Opening a new recording: {:?}", msg.info); - self.state.set_recording_id(store_id.clone()); + StoreHub::set_recording_id(store_id.clone()); + StoreHub::set_app_id(msg.info.application_id.clone()); } StoreKind::Blueprint => { re_log::debug!("Opening a new blueprint: {:?}", msg.info); - self.state - .selected_blueprint_by_app - .insert(msg.info.application_id.clone(), msg.info.store_id.clone()); + StoreHub::set_blueprint_for_app_id( + store_id.clone(), + msg.info.application_id.clone(), + ); } } true @@ -615,22 +581,22 @@ impl App { false }; - let store_db = self.store_hub.store_db_entry(store_id); - - if store_db.data_source.is_none() { - store_db.data_source = Some(self.rx.source().clone()); - } + StoreHub::access_store_db_mut(store_id, |store_db| { + if store_db.data_source.is_none() { + store_db.data_source = Some(self.rx.source().clone()); + } - if let Err(err) = store_db.add(&msg) { - re_log::error!("Failed to add incoming msg: {err}"); - }; + if let Err(err) = store_db.add(&msg) { + re_log::error!("Failed to add incoming msg: {err}"); + }; - if is_new_store && store_db.store_kind() == StoreKind::Recording { - // Do analytics after ingesting the new message, - // because thats when the `store_db.store_info` is set, - // which we use in the analytics call. - self.analytics.on_open_recording(store_db); - } + if is_new_store && store_db.store_kind() == StoreKind::Recording { + // Do analytics after ingesting the new message, + // because thats when the `store_db.store_info` is set, + // which we use in the analytics call. + self.analytics.on_open_recording(store_db); + } + }); if start.elapsed() > web_time::Duration::from_millis(10) { egui_ctx.request_repaint(); // make sure we keep receiving messages asap @@ -667,18 +633,16 @@ impl App { re_log::debug!("Counted: {}", format_bytes(counted as _)); } - { - re_tracing::profile_scope!("pruning"); - if let Some(counted) = mem_use_before.counted { - re_log::debug!( - "Attempting to purge {:.1}% of used RAM ({})…", - 100.0 * fraction_to_purge, - format_bytes(counted as f64 * fraction_to_purge as f64) - ); - } - self.store_hub.purge_fraction_of_ram(fraction_to_purge); - self.state.cache.purge_memory(); + re_tracing::profile_scope!("pruning"); + if let Some(counted) = mem_use_before.counted { + re_log::debug!( + "Attempting to purge {:.1}% of used RAM ({})…", + 100.0 * fraction_to_purge, + format_bytes(counted as f64 * fraction_to_purge as f64) + ); } + StoreHub::purge_fraction_of_ram(fraction_to_purge); + self.state.cache.purge_memory(); let mem_use_after = MemoryUse::capture(); @@ -700,11 +664,8 @@ impl App { /// Reset the viewer to how it looked the first time you ran it. fn reset(&mut self, egui_ctx: &egui::Context) { - let selected_rec_id = self.state.recording_id(); self.state = Default::default(); - if let Some(selected_rec_id) = selected_rec_id { - self.state.set_recording_id(selected_rec_id); - } + // TODO(jleibs): This presumably means throwing away the Blueprint? // Keep the style: let style = egui_ctx.style(); @@ -712,28 +673,23 @@ impl App { egui_ctx.set_style((*style).clone()); } - /// Get access to the [`StoreDb`] of the currently shown recording, if any. - pub fn recording_db(&self) -> Option<&StoreDb> { - self.state - .recording_id() - .and_then(|rec_id| self.store_hub.recording(&rec_id)) + pub fn recording_db(&self, f: impl FnOnce(Option<&StoreDb>) -> R) -> R { + StoreHub::with_view(|store_view| f(store_view.recording)) } - fn on_rrd_loaded(&mut self, store_hub: StoreHub) { - if let Some(store_db) = store_hub.recordings().next() { - self.state.set_recording_id(store_db.store_id().clone()); + fn on_rrd_loaded(&mut self, loaded_store_bundle: StoreBundle) { + if let Some(store_db) = loaded_store_bundle.recordings().next() { + StoreHub::set_recording_id(store_db.store_id().clone()); self.analytics.on_open_recording(store_db); } - for blueprint_db in store_hub.blueprints() { + for blueprint_db in loaded_store_bundle.blueprints() { if let Some(app_id) = blueprint_db.app_id() { - self.state - .selected_blueprint_by_app - .insert(app_id.clone(), blueprint_db.store_id().clone()); + StoreHub::set_blueprint_for_app_id(blueprint_db.store_id().clone(), app_id.clone()); } } - self.store_hub.append(store_hub); + StoreHub::add_bundle(loaded_store_bundle); } fn handle_dropping_files(&mut self, egui_ctx: &egui::Context) { @@ -831,33 +787,10 @@ impl eframe::App for App { render_ctx.gpu_resources.statistics() }; - // Look up the blueprint in use for this frame. - // Note that it's important that we save this because it's possible that it will be changed - // by the end up the frame (e.g. if the user selects a different recording), but we need it - // to save changes back to the correct blueprint at the end. - let active_blueprint_id = self - .state - .selected_blueprint_by_app - .get(&self.selected_app_id()) - .cloned() - .unwrap_or_else(|| { - StoreId::from_string(StoreKind::Blueprint, self.selected_app_id().0) - }); - - let store_stats = self - .recording_db() - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) - .unwrap_or_default(); - - let blueprint_stats = self - .store_hub - .blueprint_mut(&active_blueprint_id) - .map(|bp_db| DataStoreStats::from_store(&bp_db.entity_db.data_store)) - .unwrap_or_default(); + let store_stats = StoreHub::store_stats(); // do early, before doing too many allocations - self.memory_panel - .update(&gpu_resource_stats, &store_stats, &blueprint_stats); + self.memory_panel.update(&gpu_resource_stats, &store_stats); self.check_keyboard_shortcuts(egui_ctx); @@ -868,57 +801,61 @@ impl eframe::App for App { self.show_text_logs_as_notifications(); self.receive_messages(egui_ctx); - self.store_hub.purge_empty(); - self.state.cleanup(&self.store_hub); + StoreHub::purge_empty(); + StoreHub::access_bundle(|bundle| { + self.state.cleanup(bundle); + }); file_saver_progress_ui(egui_ctx, &mut self.background_tasks); // toasts for background file saver - let blueprint_snapshot = self.load_or_create_blueprint(&active_blueprint_id, egui_ctx); - - // Make a mutable copy we can edit. - let mut blueprint = blueprint_snapshot.clone(); - - let blueprint_config = self - .store_hub - .blueprint_mut(&active_blueprint_id) - .map(|bp_db| bp_db.entity_db.data_store.config().clone()) - .unwrap_or_default(); - - self.ui( - egui_ctx, - frame, - &mut blueprint, - &gpu_resource_stats, - &blueprint_config, - &store_stats, - &blueprint_stats, - ); + // This implicitly holds a lock on the `StoreHub` + // TODO(jleibs): ideally this whole thing would just be a member of App, but the borrow-checker + // means holding a reference to a StoreDb precludes us from calling any mut functions on + // App. A future refactor to move those mut functions out of app into a separate helper would + // streamline this. + let (blueprint_snapshot, blueprint) = StoreHub::with_view(|store_view| { + let blueprint_snapshot = Blueprint::from_db(egui_ctx, store_view.blueprint); + + // Make a mutable copy we can edit. + let mut blueprint = blueprint_snapshot.clone(); + + self.ui( + egui_ctx, + frame, + &mut blueprint, + &gpu_resource_stats, + &store_view, + &store_stats, + ); - if re_ui::CUSTOM_WINDOW_DECORATIONS { - // Paint the main window frame on top of everything else - paint_native_window_frame(egui_ctx); - } + if re_ui::CUSTOM_WINDOW_DECORATIONS { + // Paint the main window frame on top of everything else + paint_native_window_frame(egui_ctx); + } - self.handle_dropping_files(egui_ctx); + self.handle_dropping_files(egui_ctx); - if !self.screenshotter.is_screenshotting() { - 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); - } + if let Some(cmd) = self.cmd_palette.show(egui_ctx) { + self.pending_commands.push(cmd); + } - self.run_pending_commands(&mut blueprint, egui_ctx, frame); + self.run_pending_commands(&mut blueprint, &store_view, egui_ctx, frame); - // If there was a real active blueprint that came from the store, save the changes back. - if let Some(blueprint_db) = self.store_hub.blueprint_mut(&active_blueprint_id) { - blueprint.sync_changes_to_store(&blueprint_snapshot, blueprint_db); - } else { - // This shouldn't happen because we should have used `active_blueprint_id` to - // create this same blueprint in `load_or_create_blueprint`, but we couldn't - // keep it around for borrow-checker reasons. - re_log::warn_once!("Blueprint unexpectedly missing from store."); + (blueprint_snapshot, blueprint) + }); + + if let Some(blueprint_id) = &blueprint.blueprint_id { + StoreHub::access_store_db_mut(blueprint_id, |blueprint_db| { + blueprint.sync_changes_to_store(&blueprint_snapshot, blueprint_db); + }); + } + + if let Some(recording_id) = self.requested_recording_id.take() { + StoreHub::set_recording_id(recording_id); } // Frame time measurer - must be last @@ -1046,7 +983,7 @@ fn file_saver_progress_ui(egui_ctx: &egui::Context, background_tasks: &mut Backg } #[cfg(not(target_arch = "wasm32"))] -fn open_rrd_dialog() -> Option { +fn open_rrd_dialog() -> Option { if let Some(path) = rfd::FileDialog::new() .add_filter("rerun data file", &["rrd"]) .pick_file() @@ -1060,13 +997,14 @@ fn open_rrd_dialog() -> Option { #[cfg(not(target_arch = "wasm32"))] fn save( app: &mut App, + store_view: &StoreView<'_>, loop_selection: Option<(re_data_store::Timeline, re_log_types::TimeRangeF)>, ) { - let Some(store_db) = app.recording_db() else { - // NOTE: Can only happen if saving through the command palette. - re_log::error!("No data to save!"); - return; - }; + let Some(store_db) = store_view.recording else { + // NOTE: Can only happen if saving through the command palette. + re_log::error!("No data to save!"); + return; + }; let title = if loop_selection.is_some() { "Save loop selection" diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index e6c1a2d80ed3..de5a33ec16f6 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -1,7 +1,7 @@ use ahash::HashMap; use re_data_store::StoreDb; -use re_log_types::{ApplicationId, LogMsg, StoreId, TimeRangeF}; +use re_log_types::{LogMsg, StoreId, TimeRangeF}; use re_smart_channel::Receiver; use re_viewer_context::{ AppOptions, Caches, ComponentUiRegistry, PlayState, RecordingConfig, SpaceViewClassRegistry, @@ -9,7 +9,7 @@ use re_viewer_context::{ }; use re_viewport::ViewportState; -use crate::ui::Blueprint; +use crate::{store_hub::StoreView, ui::Blueprint}; const WATERMARK: bool = false; // Nice for recording media material @@ -23,11 +23,6 @@ pub struct AppState { #[serde(skip)] pub(crate) cache: Caches, - #[serde(skip)] - selected_rec_id: Option, - #[serde(skip)] - pub(crate) selected_blueprint_by_app: HashMap, - /// Configuration for the current recording (found in [`StoreDb`]). recording_configs: HashMap, @@ -41,16 +36,6 @@ pub struct AppState { } impl AppState { - /// The selected/visible recording id, if any. - pub fn recording_id(&self) -> Option { - self.selected_rec_id.clone() - } - - /// The selected/visible recording id, if any. - pub fn set_recording_id(&mut self, recording_id: StoreId) { - self.selected_rec_id = Some(recording_id); - } - pub fn app_options(&self) -> &AppOptions { &self.app_options } @@ -61,18 +46,24 @@ impl AppState { /// Currently selected section of time, if any. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] - pub fn loop_selection(&self) -> Option<(re_data_store::Timeline, TimeRangeF)> { - self.selected_rec_id.as_ref().and_then(|rec_id| { - self.recording_configs - .get(rec_id) - // is there an active loop selection? - .and_then(|rec_cfg| { - rec_cfg - .time_ctrl - .loop_selection() - .map(|q| (*rec_cfg.time_ctrl.timeline(), q)) - }) - }) + pub fn loop_selection( + &self, + store_view: &StoreView<'_>, + ) -> Option<(re_data_store::Timeline, TimeRangeF)> { + store_view + .recording + .map(|rec| rec.store_id()) + .and_then(|rec_id| { + self.recording_configs + .get(rec_id) + // is there an active loop selection? + .and_then(|rec_cfg| { + rec_cfg + .time_ctrl + .loop_selection() + .map(|q| (*rec_cfg.time_ctrl.timeline(), q)) + }) + }) } #[allow(clippy::too_many_arguments)] @@ -92,8 +83,6 @@ impl AppState { let Self { app_options, cache, - selected_rec_id: _, - selected_blueprint_by_app: _, recording_configs, selection_panel, time_panel, @@ -166,21 +155,9 @@ impl AppState { recording_config_entry(&mut self.recording_configs, id, data_source, store_db) } - pub fn cleanup(&mut self, store_hub: &crate::StoreHub) { + pub fn cleanup(&mut self, store_hub: &crate::StoreBundle) { re_tracing::profile_function!(); - if !self - .selected_rec_id - .as_ref() - .map_or(false, |rec_id| store_hub.contains_recording(rec_id)) - { - // Pick any: - self.selected_rec_id = store_hub - .recordings() - .next() - .map(|log| log.store_id().clone()); - } - self.recording_configs .retain(|store_id, _| store_hub.contains_recording(store_id)); } diff --git a/crates/re_viewer/src/lib.rs b/crates/re_viewer/src/lib.rs index 633ad7bf6476..01aa95268926 100644 --- a/crates/re_viewer/src/lib.rs +++ b/crates/re_viewer/src/lib.rs @@ -24,7 +24,7 @@ pub(crate) use { pub use app::{App, StartupOptions}; pub use remote_viewer_app::RemoteViewerApp; -pub use store_hub::StoreHub; +pub use store_hub::StoreBundle; pub mod external { pub use {eframe, egui}; diff --git a/crates/re_viewer/src/loading.rs b/crates/re_viewer/src/loading.rs index d213456c7cd7..9cb35af64b85 100644 --- a/crates/re_viewer/src/loading.rs +++ b/crates/re_viewer/src/loading.rs @@ -1,13 +1,13 @@ -use crate::StoreHub; +use crate::StoreBundle; #[cfg(not(target_arch = "wasm32"))] #[must_use] -pub fn load_file_path(path: &std::path::Path) -> Option { - fn load_file_path_impl(path: &std::path::Path) -> anyhow::Result { +pub fn load_file_path(path: &std::path::Path) -> Option { + fn load_file_path_impl(path: &std::path::Path) -> anyhow::Result { re_tracing::profile_function!(); use anyhow::Context as _; let file = std::fs::File::open(path).context("Failed to open file")?; - StoreHub::from_rrd(file) + StoreBundle::from_rrd(file) } re_log::info!("Loading {path:?}…"); @@ -35,8 +35,8 @@ pub fn load_file_path(path: &std::path::Path) -> Option { } #[must_use] -pub fn load_file_contents(name: &str, read: impl std::io::Read) -> Option { - match StoreHub::from_rrd(read) { +pub fn load_file_contents(name: &str, read: impl std::io::Read) -> Option { + match StoreBundle::from_rrd(read) { Ok(mut rrd) => { re_log::info!("Loaded {name:?}"); for store_db in rrd.store_dbs_mut() { diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 5e5c21142223..bf91ffe9b603 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -1,14 +1,173 @@ +use ahash::HashMap; +use re_arrow_store::{DataStoreConfig, DataStoreStats}; use re_data_store::StoreDb; -use re_log_types::{StoreId, StoreKind}; +use re_log_types::{ApplicationId, StoreId, StoreKind}; + +use lazy_static::lazy_static; +use parking_lot::Mutex; + +lazy_static! { + static ref GLOBAL_HUB: Mutex = Mutex::new(StoreHubImpl::default()); +} + +#[derive(Default)] +pub struct StoreHubStats { + pub blueprint_stats: DataStoreStats, + pub blueprint_config: DataStoreConfig, + pub recording_stats: DataStoreStats, + pub recording_config: DataStoreConfig, +} + +pub struct StoreView<'a> { + pub blueprint: Option<&'a StoreDb>, + pub recording: Option<&'a StoreDb>, + pub bundle: &'a StoreBundle, +} + +pub struct StoreHub; + +impl StoreHub { + pub fn add_bundle(bundle: StoreBundle) { + GLOBAL_HUB.lock().add_bundle(bundle); + } + + pub fn with_view(f: impl FnOnce(StoreView<'_>) -> R) -> R { + GLOBAL_HUB.lock().with_view(f) + } + + /// The selected/visible recording id, if any. + pub fn set_recording_id(recording_id: StoreId) { + GLOBAL_HUB.lock().set_recording_id(recording_id); + } + + pub fn set_app_id(app_id: ApplicationId) { + GLOBAL_HUB.lock().set_app_id(app_id); + } + + pub fn set_blueprint_for_app_id(blueprint_id: StoreId, app_id: ApplicationId) { + GLOBAL_HUB + .lock() + .set_blueprint_for_app_id(blueprint_id, app_id); + } + + pub fn store_stats() -> StoreHubStats { + // TODO + StoreHubStats::default() + /* + self.recording_db(|store_db| { + store_db + .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .unwrap_or_default() + }); + */ + } + + pub fn purge_empty() { + GLOBAL_HUB.lock().purge_empty(); + } + + pub fn purge_fraction_of_ram(fraction_to_purge: f32) { + GLOBAL_HUB.lock().purge_fraction_of_ram(fraction_to_purge); + } + + pub fn access_bundle(f: impl FnOnce(&StoreBundle) -> R) -> R { + GLOBAL_HUB.lock().access_bundle(f) + } + + pub fn access_store_db_mut(store_id: &StoreId, f: impl FnOnce(&mut StoreDb) -> R) -> R { + GLOBAL_HUB.lock().access_store_db_mut(store_id, f) + } +} + +#[derive(Default)] +struct StoreHubImpl { + selected_rec_id: Option, + application_id: Option, + blueprint_by_app_id: HashMap, + store_dbs: StoreBundle, +} + +impl StoreHubImpl { + fn add_bundle(&mut self, bundle: StoreBundle) { + self.store_dbs.append(bundle); + + // TODO: mutate app_id / selected_rec_id + } + + fn with_view(&mut self, f: impl FnOnce(StoreView<'_>) -> R) -> R { + // First maybe create blueprint if it's necessary. + // TODO(jleibs): Can we hold onto this version here instead of + // requerying below? + if let Some(id) = self.application_id.as_ref().map(|app_id| { + self.blueprint_by_app_id + .entry(app_id.clone()) + .or_insert_with(|| StoreId::from_string(StoreKind::Blueprint, app_id.clone().0)) + }) { + self.store_dbs.blueprint_entry(id); + } + + let recording = self + .selected_rec_id + .as_ref() + .and_then(|id| self.store_dbs.recording(id)); + + let blueprint: Option<&StoreDb> = self + .application_id + .as_ref() + .and_then(|app_id| self.blueprint_by_app_id.get(app_id)) + .and_then(|id| self.store_dbs.blueprint(id)); + + let view = StoreView { + blueprint, + recording, + bundle: &self.store_dbs, + }; + + f(view) + } + + /// The selected/visible recording id, if any. + fn set_recording_id(&mut self, recording_id: StoreId) { + self.selected_rec_id = Some(recording_id); + } + + fn set_app_id(&mut self, app_id: ApplicationId) { + self.application_id = Some(app_id); + } + + pub fn set_blueprint_for_app_id(&mut self, blueprint_id: StoreId, app_id: ApplicationId) { + self.blueprint_by_app_id.insert(app_id, blueprint_id); + } + + pub fn access_bundle(&self, f: impl FnOnce(&StoreBundle) -> R) -> R { + f(&self.store_dbs) + } + + pub fn access_store_db_mut( + &mut self, + store_id: &StoreId, + f: impl FnOnce(&mut StoreDb) -> R, + ) -> R { + f(self.store_dbs.store_db_entry(store_id)) + } + + pub fn purge_empty(&mut self) { + self.store_dbs.purge_empty(); + } + + pub fn purge_fraction_of_ram(&mut self, fraction_to_purge: f32) { + self.store_dbs.purge_fraction_of_ram(fraction_to_purge); + } +} /// Stores many [`StoreDb`]s of recordings and blueprints. #[derive(Default)] -pub struct StoreHub { +pub struct StoreBundle { // TODO(emilk): two separate maps per [`StoreKind`]. store_dbs: ahash::HashMap, } -impl StoreHub { +impl StoreBundle { /// Decode an rrd stream. /// It can theoretically contain multiple recordings, and blueprints. pub fn from_rrd(read: impl std::io::Read) -> anyhow::Result { diff --git a/crates/re_viewer/src/ui/blueprint.rs b/crates/re_viewer/src/ui/blueprint.rs index fea78efb7392..6bc9efc698c3 100644 --- a/crates/re_viewer/src/ui/blueprint.rs +++ b/crates/re_viewer/src/ui/blueprint.rs @@ -1,10 +1,12 @@ +use re_log_types::StoreId; use re_viewer_context::{Item, ViewerContext}; use re_viewport::{SpaceInfoCollection, Viewport, ViewportState}; /// Defines the layout of the whole Viewer (or will, eventually). -#[derive(Clone, Default, serde::Deserialize, serde::Serialize)] -#[serde(default)] +#[derive(Clone)] pub struct Blueprint { + pub blueprint_id: Option, + pub blueprint_panel_expanded: bool, pub selection_panel_expanded: bool, pub time_panel_expanded: bool, @@ -14,9 +16,10 @@ pub struct Blueprint { impl Blueprint { /// Prefer this to [`Blueprint::default`] to get better defaults based on screen size. - pub fn new(egui_ctx: &egui::Context) -> Self { + pub fn new(blueprint_id: Option, egui_ctx: &egui::Context) -> Self { let screen_size = egui_ctx.screen_rect().size(); Self { + blueprint_id, blueprint_panel_expanded: screen_size.x > 750.0, selection_panel_expanded: screen_size.x > 1000.0, time_panel_expanded: screen_size.y > 600.0, diff --git a/crates/re_viewer/src/ui/blueprint_load.rs b/crates/re_viewer/src/ui/blueprint_load.rs index a5cde4dbad99..63f0e7c3afe8 100644 --- a/crates/re_viewer/src/ui/blueprint_load.rs +++ b/crates/re_viewer/src/ui/blueprint_load.rs @@ -16,45 +16,49 @@ use super::Blueprint; use crate::blueprint_components::panel::PanelState; impl Blueprint { - pub fn from_db(egui_ctx: &egui::Context, blueprint_db: &re_data_store::StoreDb) -> Self { - let mut ret = Self::new(egui_ctx); - - let space_views: HashMap = if let Some(space_views) = - blueprint_db - .entity_db - .tree - .children - .get(&re_data_store::EntityPathPart::Name( - SpaceViewComponent::SPACEVIEW_PREFIX.into(), - )) { - space_views - .children - .values() - .filter_map(|view_tree| load_space_view(&view_tree.path, blueprint_db)) - .map(|sv| (sv.id, sv)) - .collect() - } else { - Default::default() - }; - - ret.viewport = load_viewport(blueprint_db, space_views); - - if let Some(expanded) = - load_panel_state(&PanelState::BLUEPRINT_VIEW_PATH.into(), blueprint_db) - { - ret.blueprint_panel_expanded = expanded; - } - if let Some(expanded) = - load_panel_state(&PanelState::SELECTION_VIEW_PATH.into(), blueprint_db) - { - ret.selection_panel_expanded = expanded; + pub fn from_db( + egui_ctx: &egui::Context, + blueprint_db: Option<&re_data_store::StoreDb>, + ) -> Self { + let mut ret = Self::new(blueprint_db.map(|bp| bp.store_id()).cloned(), egui_ctx); + + if let Some(blueprint_db) = blueprint_db { + let space_views: HashMap = if let Some(space_views) = + blueprint_db + .entity_db + .tree + .children + .get(&re_data_store::EntityPathPart::Name( + SpaceViewComponent::SPACEVIEW_PREFIX.into(), + )) { + space_views + .children + .values() + .filter_map(|view_tree| load_space_view(&view_tree.path, blueprint_db)) + .map(|sv| (sv.id, sv)) + .collect() + } else { + Default::default() + }; + + ret.viewport = load_viewport(blueprint_db, space_views); + + if let Some(expanded) = + load_panel_state(&PanelState::BLUEPRINT_VIEW_PATH.into(), blueprint_db) + { + ret.blueprint_panel_expanded = expanded; + } + if let Some(expanded) = + load_panel_state(&PanelState::SELECTION_VIEW_PATH.into(), blueprint_db) + { + ret.selection_panel_expanded = expanded; + } + if let Some(expanded) = + load_panel_state(&PanelState::TIMELINE_VIEW_PATH.into(), blueprint_db) + { + ret.time_panel_expanded = expanded; + } } - if let Some(expanded) = - load_panel_state(&PanelState::TIMELINE_VIEW_PATH.into(), blueprint_db) - { - ret.time_panel_expanded = expanded; - } - ret } } diff --git a/crates/re_viewer/src/ui/memory_panel.rs b/crates/re_viewer/src/ui/memory_panel.rs index 2ed7307aeef0..4d866aeeea1a 100644 --- a/crates/re_viewer/src/ui/memory_panel.rs +++ b/crates/re_viewer/src/ui/memory_panel.rs @@ -3,7 +3,7 @@ use re_format::{format_bytes, format_number}; use re_memory::{util::sec_since_start, MemoryHistory, MemoryLimit, MemoryUse}; use re_renderer::WgpuResourcePoolStatistics; -use crate::env_vars::RERUN_TRACK_ALLOCATIONS; +use crate::{env_vars::RERUN_TRACK_ALLOCATIONS, store_hub::StoreHubStats}; // ---------------------------------------------------------------------------- @@ -18,8 +18,7 @@ impl MemoryPanel { pub fn update( &mut self, gpu_resource_stats: &WgpuResourcePoolStatistics, - store_stats: &DataStoreStats, - blueprint_stats: &DataStoreStats, + store_stats: &StoreHubStats, ) { re_tracing::profile_function!(); self.history.capture( @@ -27,8 +26,8 @@ impl MemoryPanel { (gpu_resource_stats.total_buffer_size_in_bytes + gpu_resource_stats.total_texture_size_in_bytes) as _, ), - Some(store_stats.total.num_bytes as _), - Some(blueprint_stats.total.num_bytes as _), + Some(store_stats.recording_stats.total.num_bytes as _), + Some(store_stats.blueprint_stats.total.num_bytes as _), ); } diff --git a/crates/re_viewer/src/ui/rerun_menu.rs b/crates/re_viewer/src/ui/rerun_menu.rs index 16ca0b50a439..3407b2a84df3 100644 --- a/crates/re_viewer/src/ui/rerun_menu.rs +++ b/crates/re_viewer/src/ui/rerun_menu.rs @@ -3,13 +3,17 @@ use egui::NumExt as _; use itertools::Itertools as _; -use re_log_types::{ApplicationId, StoreKind}; use re_ui::Command; use re_viewer_context::AppOptions; -use crate::{app_state::AppState, App, StoreHub}; +use crate::{store_hub::StoreView, App}; -pub fn rerun_menu_button_ui(ui: &mut egui::Ui, frame: &mut eframe::Frame, app: &mut App) { +pub fn rerun_menu_button_ui( + store_view: &StoreView<'_>, + ui: &mut egui::Ui, + frame: &mut eframe::Frame, + app: &mut App, +) { // let desired_icon_height = ui.max_rect().height() - 2.0 * ui.spacing_mut().button_padding.y; let desired_icon_height = ui.max_rect().height() - 4.0; // TODO(emilk): figure out this fudge let desired_icon_height = desired_icon_height.at_most(28.0); // figma size 2023-02-03 @@ -34,7 +38,7 @@ pub fn rerun_menu_button_ui(ui: &mut egui::Ui, frame: &mut eframe::Frame, app: & { Command::Open.menu_button_ui(ui, &mut app.pending_commands); - save_buttons_ui(ui, app); + save_buttons_ui(ui, store_view, app); ui.add_space(spacing); @@ -64,13 +68,14 @@ pub fn rerun_menu_button_ui(ui: &mut egui::Ui, frame: &mut eframe::Frame, app: & ui.add_space(spacing); - ui.menu_button("Recordings", |ui| { - recordings_menu(ui, &app.store_hub, &mut app.state); - }); - - ui.menu_button("Blueprints", |ui| { - blueprints_menu(ui, &app.selected_app_id(), &app.store_hub, &mut app.state); - }); + { + ui.menu_button("Recordings", |ui| { + recordings_menu(ui, store_view, app); + }); + ui.menu_button("Blueprints", |ui| { + blueprints_menu(ui, store_view, app); + }); + } ui.menu_button("Options", |ui| { options_menu_ui(ui, frame, app.app_options_mut()); @@ -131,8 +136,9 @@ fn about_rerun_ui(ui: &mut egui::Ui, build_info: &re_build_info::BuildInfo) { ui.hyperlink_to("www.rerun.io", "https://www.rerun.io/"); } -fn recordings_menu(ui: &mut egui::Ui, store_hub: &StoreHub, app_state: &mut AppState) { - let store_dbs = store_hub +fn recordings_menu(ui: &mut egui::Ui, store_view: &StoreView<'_>, app: &mut App) { + let store_dbs = store_view + .bundle .recordings() .sorted_by_key(|store_db| store_db.store_info().map(|ri| ri.started)) .collect_vec(); @@ -142,6 +148,8 @@ fn recordings_menu(ui: &mut egui::Ui, store_hub: &StoreHub, app_state: &mut AppS return; } + let active_recording = store_view.recording.map(|rec| rec.store_id()); + ui.style_mut().wrap = Some(false); for store_db in &store_dbs { let info = if let Some(store_info) = store_db.store_info() { @@ -154,30 +162,20 @@ fn recordings_menu(ui: &mut egui::Ui, store_hub: &StoreHub, app_state: &mut AppS "".to_owned() }; if ui - .radio( - app_state.recording_id().as_ref() == Some(store_db.store_id()), - info, - ) + .radio(active_recording == Some(store_db.store_id()), info) .clicked() { - app_state.set_recording_id(store_db.store_id().clone()); + // TODO(jleibs): This is gross but necessary to avoid a deadlock + app.requested_recording_id = Some(store_db.store_id().clone()); } } } -fn blueprints_menu( - ui: &mut egui::Ui, - app_id: &ApplicationId, - store_hub: &StoreHub, - app_state: &mut AppState, -) { - let blueprint_dbs = store_hub +fn blueprints_menu(ui: &mut egui::Ui, store_view: &StoreView<'_>, _app: &mut App) { + let blueprint_dbs = store_view + .bundle .blueprints() .sorted_by_key(|store_db| store_db.store_info().map(|ri| ri.started)) - .filter(|log| { - log.store_info() - .map_or(false, |ri| &ri.application_id == app_id) - }) .collect_vec(); if blueprint_dbs.is_empty() { @@ -185,11 +183,10 @@ fn blueprints_menu( return; } + let active_blueprint = store_view.blueprint.map(|rec| rec.store_id()); + ui.style_mut().wrap = Some(false); - for store_db in blueprint_dbs - .iter() - .filter(|log| log.store_kind() == StoreKind::Blueprint) - { + for store_db in &blueprint_dbs { let info = if let Some(store_info) = store_db.store_info() { if store_info.is_app_default_blueprint() { format!("{} - Default Blueprint", store_info.application_id,) @@ -203,16 +200,12 @@ fn blueprints_menu( } else { "".to_owned() }; + if ui - .radio( - app_state.selected_blueprint_by_app.get(app_id) == Some(store_db.store_id()), - info, - ) + .radio(active_blueprint == Some(store_db.store_id()), info) .clicked() { - app_state - .selected_blueprint_by_app - .insert(app_id.clone(), store_db.store_id().clone()); + re_log::info!("Clicked! {}", store_db.store_id()); } } } @@ -252,7 +245,7 @@ fn options_menu_ui(ui: &mut egui::Ui, _frame: &mut eframe::Frame, options: &mut // TODO(emilk): support saving data on web #[cfg(not(target_arch = "wasm32"))] -fn save_buttons_ui(ui: &mut egui::Ui, app: &mut App) { +fn save_buttons_ui(ui: &mut egui::Ui, store_view: &StoreView<'_>, app: &mut App) { let file_save_in_progress = app.background_tasks.is_file_save_in_progress(); let save_button = Command::Save.menu_button(ui.ctx()); @@ -270,8 +263,8 @@ fn save_buttons_ui(ui: &mut egui::Ui, app: &mut App) { }); }); } else { - let store_db_is_nonempty = app - .recording_db() + let store_db_is_nonempty = store_view + .recording .map_or(false, |store_db| !store_db.is_empty()); ui.add_enabled_ui(store_db_is_nonempty, |ui| { if ui @@ -287,7 +280,7 @@ fn save_buttons_ui(ui: &mut egui::Ui, app: &mut App) { // button, as this will determine whether its grayed out or not! // TODO(cmc): In practice the loop (green) selection is always there // at the moment so... - let loop_selection = app.state.loop_selection(); + let loop_selection = app.state.loop_selection(store_view); if ui .add_enabled(loop_selection.is_some(), save_selection_button) diff --git a/crates/re_viewer/src/ui/top_panel.rs b/crates/re_viewer/src/ui/top_panel.rs index eb15793763e9..22952329753a 100644 --- a/crates/re_viewer/src/ui/top_panel.rs +++ b/crates/re_viewer/src/ui/top_panel.rs @@ -2,10 +2,11 @@ use re_format::format_number; use re_renderer::WgpuResourcePoolStatistics; use re_ui::Command; -use crate::{ui::Blueprint, App}; +use crate::{store_hub::StoreView, ui::Blueprint, App}; pub fn top_panel( blueprint: &Blueprint, + store_view: &StoreView<'_>, ui: &mut egui::Ui, frame: &mut eframe::Frame, app: &mut App, @@ -37,7 +38,7 @@ pub fn top_panel( ui.set_height(top_bar_style.height); ui.add_space(top_bar_style.indent); - top_bar_ui(blueprint, ui, frame, app, gpu_resource_stats); + top_bar_ui(blueprint, store_view, ui, frame, app, gpu_resource_stats); }) .response; @@ -55,12 +56,13 @@ pub fn top_panel( fn top_bar_ui( blueprint: &Blueprint, + store_view: &StoreView<'_>, ui: &mut egui::Ui, frame: &mut eframe::Frame, app: &mut App, gpu_resource_stats: &WgpuResourcePoolStatistics, ) { - crate::ui::rerun_menu_button_ui(ui, frame, app); + crate::ui::rerun_menu_button_ui(store_view, ui, frame, app); if app.app_options().show_metrics { ui.separator(); diff --git a/examples/rust/extend_viewer_ui/src/main.rs b/examples/rust/extend_viewer_ui/src/main.rs index 52bb2973c29e..11b41ca4471e 100644 --- a/examples/rust/extend_viewer_ui/src/main.rs +++ b/examples/rust/extend_viewer_ui/src/main.rs @@ -102,11 +102,13 @@ impl MyApp { }); ui.separator(); - if let Some(store_db) = self.rerun_app.recording_db() { - store_db_ui(ui, store_db); - } else { - ui.label("No log database loaded yet."); - } + self.rerun_app.recording_db(|store_db| { + if let Some(store_db) = store_db { + store_db_ui(ui, store_db); + } else { + ui.label("No log database loaded yet."); + } + }); } } From 9e048eda945491bcbfc3f4311afbf5819a7409a4 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 01:47:51 +0200 Subject: [PATCH 02/21] Add the blueprint-db to the ViewerContext --- crates/re_viewer/src/app.rs | 5 ++++- crates/re_viewer/src/app_state.rs | 2 ++ crates/re_viewer_context/src/viewer_context.rs | 3 +++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index ad7d3af9f8cf..794f951b4a88 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -469,7 +469,9 @@ impl App { self.memory_panel_ui(ui, gpu_resource_stats, store_stats); - if let Some(store_db) = store_view.recording { + if let (Some(store_db), Some(blueprint_db)) = + (store_view.recording, store_view.blueprint) + { self.state .recording_config_entry( store_db.store_id().clone(), @@ -498,6 +500,7 @@ impl App { ui, render_ctx, store_db, + blueprint_db, &self.re_ui, &self.component_ui_registry, &self.space_view_class_registry, diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index de5a33ec16f6..033aa5a3fb6a 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -73,6 +73,7 @@ impl AppState { ui: &mut egui::Ui, render_ctx: &mut re_renderer::RenderContext, store_db: &StoreDb, + blueprint_db: &StoreDb, re_ui: &re_ui::ReUi, component_ui_registry: &ComponentUiRegistry, space_view_class_registry: &SpaceViewClassRegistry, @@ -102,6 +103,7 @@ impl AppState { space_view_class_registry, component_ui_registry, store_db, + blueprint_db, rec_cfg, re_ui, render_ctx, diff --git a/crates/re_viewer_context/src/viewer_context.rs b/crates/re_viewer_context/src/viewer_context.rs index 2515285e1a0f..1f59066c711f 100644 --- a/crates/re_viewer_context/src/viewer_context.rs +++ b/crates/re_viewer_context/src/viewer_context.rs @@ -24,6 +24,9 @@ pub struct ViewerContext<'a> { /// The current recording. pub store_db: &'a StoreDb, + /// The current blueprint. + pub blueprint_db: &'a StoreDb, + /// UI config for the current recording (found in [`StoreDb`]). pub rec_cfg: &'a mut RecordingConfig, From fc955f63a37e34a1ff408846665b38b4c5990748 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 03:04:57 +0200 Subject: [PATCH 03/21] Simplify ownership of StoreHub --- Cargo.lock | 2 - crates/re_viewer/Cargo.toml | 2 - crates/re_viewer/src/app.rs | 161 +++++++++++---------- crates/re_viewer/src/store_hub.rs | 94 +++--------- examples/rust/extend_viewer_ui/src/main.rs | 12 +- 5 files changed, 115 insertions(+), 156 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 560dc5a12d42..d38ad6025a71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4470,9 +4470,7 @@ dependencies = [ "egui-wgpu", "image", "itertools", - "lazy_static", "objc", - "parking_lot 0.12.1", "poll-promise", "puffin", "puffin_http", diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index 97c91a4d6c3b..2b8743366d27 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -87,8 +87,6 @@ image = { workspace = true, default-features = false, features = [ "png", ] } itertools = { workspace = true } -lazy_static.workspace = true -parking_lot.workspace = true poll-promise = "0.2" rfd.workspace = true serde = { version = "1", features = ["derive"] } diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 794f951b4a88..8a349f96c1a6 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -13,7 +13,7 @@ use re_viewer_context::{ use crate::{ background_tasks::BackgroundTasks, - store_hub::{StoreHub, StoreHubStats, StoreView}, + store_hub::{StoreHubImpl, StoreHubStats, StoreView}, ui::Blueprint, viewer_analytics::ViewerAnalytics, AppState, StoreBundle, @@ -96,6 +96,7 @@ pub struct App { pub(crate) background_tasks: BackgroundTasks, /// Recording Id to switch to + pub(crate) store_hub: Option, pub(crate) requested_recording_id: Option, /// Toast notifications. @@ -181,6 +182,7 @@ impl App { rx, state, background_tasks: Default::default(), + store_hub: Some(StoreHubImpl::default()), requested_recording_id: Default::default(), toasts: toasts::Toasts::new(), memory_panel: Default::default(), @@ -244,13 +246,13 @@ impl App { fn run_pending_commands( &mut self, blueprint: &mut Blueprint, - store_view: &StoreView<'_>, + store_hub: &mut StoreHubImpl, egui_ctx: &egui::Context, frame: &mut eframe::Frame, ) { let commands = self.pending_commands.drain(..).collect_vec(); for cmd in commands { - self.run_command(cmd, blueprint, store_view, frame, egui_ctx); + self.run_command(cmd, blueprint, store_hub, frame, egui_ctx); } } @@ -258,7 +260,7 @@ impl App { &mut self, cmd: Command, blueprint: &mut Blueprint, - store_view: &StoreView<'_>, + store_hub: &mut StoreHubImpl, _frame: &mut eframe::Frame, egui_ctx: &egui::Context, ) { @@ -267,16 +269,18 @@ impl App { match cmd { #[cfg(not(target_arch = "wasm32"))] Command::Save => { - save(self, store_view, None); + let store_view = store_hub.view(); + save(self, &store_view, None); } #[cfg(not(target_arch = "wasm32"))] Command::SaveSelection => { - save(self, store_view, self.state.loop_selection(store_view)); + let store_view = store_hub.view(); + save(self, &store_view, self.state.loop_selection(&store_view)); } #[cfg(not(target_arch = "wasm32"))] Command::Open => { if let Some(rrd) = open_rrd_dialog() { - self.on_rrd_loaded(rrd); + self.on_rrd_loaded(store_hub, rrd); } } #[cfg(not(target_arch = "wasm32"))] @@ -334,6 +338,7 @@ impl App { } Command::SelectionPrevious => { + let store_view = store_hub.view(); let state = &mut self.state; if let Some(rec_cfg) = store_view .recording @@ -344,6 +349,7 @@ impl App { } } Command::SelectionNext => { + let store_view = store_hub.view(); let state = &mut self.state; if let Some(rec_cfg) = store_view .recording @@ -358,19 +364,24 @@ impl App { } Command::PlaybackTogglePlayPause => { - self.run_time_control_command(store_view, TimeControlCommand::TogglePlayPause); + let store_view = store_hub.view(); + self.run_time_control_command(&store_view, TimeControlCommand::TogglePlayPause); } Command::PlaybackFollow => { - self.run_time_control_command(store_view, TimeControlCommand::Follow); + let store_view = store_hub.view(); + self.run_time_control_command(&store_view, TimeControlCommand::Follow); } Command::PlaybackStepBack => { - self.run_time_control_command(store_view, TimeControlCommand::StepBack); + let store_view = store_hub.view(); + self.run_time_control_command(&store_view, TimeControlCommand::StepBack); } Command::PlaybackStepForward => { - self.run_time_control_command(store_view, TimeControlCommand::StepForward); + let store_view = store_hub.view(); + self.run_time_control_command(&store_view, TimeControlCommand::StepForward); } Command::PlaybackRestart => { - self.run_time_control_command(store_view, TimeControlCommand::Restart); + let store_view = store_hub.view(); + self.run_time_control_command(&store_view, TimeControlCommand::Restart); } #[cfg(not(target_arch = "wasm32"))] @@ -543,7 +554,7 @@ impl App { } } - fn receive_messages(&mut self, egui_ctx: &egui::Context) { + fn receive_messages(&mut self, store_hub: &mut StoreHubImpl, egui_ctx: &egui::Context) { re_tracing::profile_function!(); let start = web_time::Instant::now(); @@ -567,13 +578,13 @@ impl App { match msg.info.store_id.kind { StoreKind::Recording => { re_log::debug!("Opening a new recording: {:?}", msg.info); - StoreHub::set_recording_id(store_id.clone()); - StoreHub::set_app_id(msg.info.application_id.clone()); + store_hub.set_recording_id(store_id.clone()); + store_hub.set_app_id(msg.info.application_id.clone()); } StoreKind::Blueprint => { re_log::debug!("Opening a new blueprint: {:?}", msg.info); - StoreHub::set_blueprint_for_app_id( + store_hub.set_blueprint_for_app_id( store_id.clone(), msg.info.application_id.clone(), ); @@ -584,22 +595,22 @@ impl App { false }; - StoreHub::access_store_db_mut(store_id, |store_db| { - if store_db.data_source.is_none() { - store_db.data_source = Some(self.rx.source().clone()); - } + let store_db = store_hub.store_db_mut(store_id); - if let Err(err) = store_db.add(&msg) { - re_log::error!("Failed to add incoming msg: {err}"); - }; + if store_db.data_source.is_none() { + store_db.data_source = Some(self.rx.source().clone()); + } - if is_new_store && store_db.store_kind() == StoreKind::Recording { - // Do analytics after ingesting the new message, - // because thats when the `store_db.store_info` is set, - // which we use in the analytics call. - self.analytics.on_open_recording(store_db); - } - }); + if let Err(err) = store_db.add(&msg) { + re_log::error!("Failed to add incoming msg: {err}"); + }; + + if is_new_store && store_db.store_kind() == StoreKind::Recording { + // Do analytics after ingesting the new message, + // because thats when the `store_db.store_info` is set, + // which we use in the analytics call. + self.analytics.on_open_recording(store_db); + } if start.elapsed() > web_time::Duration::from_millis(10) { egui_ctx.request_repaint(); // make sure we keep receiving messages asap @@ -608,7 +619,7 @@ impl App { } } - fn purge_memory_if_needed(&mut self) { + fn purge_memory_if_needed(&mut self, store_hub: &mut StoreHubImpl) { re_tracing::profile_function!(); fn format_limit(limit: Option) -> String { @@ -644,7 +655,7 @@ impl App { format_bytes(counted as f64 * fraction_to_purge as f64) ); } - StoreHub::purge_fraction_of_ram(fraction_to_purge); + store_hub.purge_fraction_of_ram(fraction_to_purge); self.state.cache.purge_memory(); let mem_use_after = MemoryUse::capture(); @@ -676,26 +687,28 @@ impl App { egui_ctx.set_style((*style).clone()); } - pub fn recording_db(&self, f: impl FnOnce(Option<&StoreDb>) -> R) -> R { - StoreHub::with_view(|store_view| f(store_view.recording)) + pub fn recording_db(&self) -> Option<&StoreDb> { + self.store_hub + .as_ref() + .and_then(|store_hub| store_hub.recording_id()) } - fn on_rrd_loaded(&mut self, loaded_store_bundle: StoreBundle) { + fn on_rrd_loaded(&mut self, store_hub: &mut StoreHubImpl, loaded_store_bundle: StoreBundle) { if let Some(store_db) = loaded_store_bundle.recordings().next() { - StoreHub::set_recording_id(store_db.store_id().clone()); + store_hub.set_recording_id(store_db.store_id().clone()); self.analytics.on_open_recording(store_db); } for blueprint_db in loaded_store_bundle.blueprints() { if let Some(app_id) = blueprint_db.app_id() { - StoreHub::set_blueprint_for_app_id(blueprint_db.store_id().clone(), app_id.clone()); + store_hub.set_blueprint_for_app_id(blueprint_db.store_id().clone(), app_id.clone()); } } - StoreHub::add_bundle(loaded_store_bundle); + store_hub.add_bundle(loaded_store_bundle); } - fn handle_dropping_files(&mut self, egui_ctx: &egui::Context) { + fn handle_dropping_files(&mut self, store_hub: &mut StoreHubImpl, egui_ctx: &egui::Context) { preview_files_being_dropped(egui_ctx); // Collect dropped files: @@ -709,9 +722,8 @@ impl App { if let Some(bytes) = &file.bytes { let mut bytes: &[u8] = &(*bytes)[..]; if let Some(rrd) = crate::loading::load_file_contents(&file.name, &mut bytes) { - self.on_rrd_loaded(rrd); + self.on_rrd_loaded(store_hub, rrd); - #[allow(clippy::needless_return)] // false positive on wasm32 return; } } @@ -719,7 +731,7 @@ impl App { #[cfg(not(target_arch = "wasm32"))] if let Some(path) = &file.path { if let Some(rrd) = crate::loading::load_file_path(path) { - self.on_rrd_loaded(rrd); + self.on_rrd_loaded(store_hub, rrd); } } } @@ -740,6 +752,8 @@ impl eframe::App for App { fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { let frame_start = Instant::now(); + let mut store_hub = self.store_hub.take().unwrap(); + #[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()); @@ -766,7 +780,7 @@ impl eframe::App for App { let mut zoom_factor = self.app_options().zoom_factor; zoom_factor = zoom_factor.clamp(MIN_ZOOM_FACTOR, MAX_ZOOM_FACTOR); zoom_factor = (zoom_factor * 10.).round() / 10.; - self.app_options_mut().zoom_factor = zoom_factor; + self.state.app_options_mut().zoom_factor = zoom_factor; } // Apply zoom factor on top of natively reported pixel per point. @@ -790,22 +804,22 @@ impl eframe::App for App { render_ctx.gpu_resources.statistics() }; - let store_stats = StoreHub::store_stats(); + let store_stats = store_hub.store_stats(); // do early, before doing too many allocations self.memory_panel.update(&gpu_resource_stats, &store_stats); self.check_keyboard_shortcuts(egui_ctx); - self.purge_memory_if_needed(); + self.purge_memory_if_needed(&mut store_hub); self.state.cache.begin_frame(); self.show_text_logs_as_notifications(); - self.receive_messages(egui_ctx); + self.receive_messages(&mut store_hub, egui_ctx); - StoreHub::purge_empty(); - StoreHub::access_bundle(|bundle| { + store_hub.purge_empty(); + store_hub.access_bundle(|bundle| { self.state.cleanup(bundle); }); @@ -816,27 +830,28 @@ impl eframe::App for App { // means holding a reference to a StoreDb precludes us from calling any mut functions on // App. A future refactor to move those mut functions out of app into a separate helper would // streamline this. - let (blueprint_snapshot, blueprint) = StoreHub::with_view(|store_view| { - let blueprint_snapshot = Blueprint::from_db(egui_ctx, store_view.blueprint); - - // Make a mutable copy we can edit. - let mut blueprint = blueprint_snapshot.clone(); - - self.ui( - egui_ctx, - frame, - &mut blueprint, - &gpu_resource_stats, - &store_view, - &store_stats, - ); + let store_view = store_hub.view(); + let blueprint_snapshot = Blueprint::from_db(egui_ctx, store_view.blueprint); + + // Make a mutable copy we can edit. + let mut blueprint = blueprint_snapshot.clone(); + + self.ui( + egui_ctx, + frame, + &mut blueprint, + &gpu_resource_stats, + &store_view, + &store_stats, + ); - if re_ui::CUSTOM_WINDOW_DECORATIONS { - // Paint the main window frame on top of everything else - paint_native_window_frame(egui_ctx); - } + if re_ui::CUSTOM_WINDOW_DECORATIONS { + // Paint the main window frame on top of everything else + paint_native_window_frame(egui_ctx); + } - self.handle_dropping_files(egui_ctx); + { + self.handle_dropping_files(&mut store_hub, egui_ctx); if !self.screenshotter.is_screenshotting() { self.toasts.show(egui_ctx); @@ -846,21 +861,21 @@ impl eframe::App for App { self.pending_commands.push(cmd); } - self.run_pending_commands(&mut blueprint, &store_view, egui_ctx, frame); - - (blueprint_snapshot, blueprint) - }); + self.run_pending_commands(&mut blueprint, &mut store_hub, egui_ctx, frame); + } if let Some(blueprint_id) = &blueprint.blueprint_id { - StoreHub::access_store_db_mut(blueprint_id, |blueprint_db| { + store_hub.access_store_db_mut(blueprint_id, |blueprint_db| { blueprint.sync_changes_to_store(&blueprint_snapshot, blueprint_db); }); } if let Some(recording_id) = self.requested_recording_id.take() { - StoreHub::set_recording_id(recording_id); + store_hub.set_recording_id(recording_id); } + self.store_hub = Some(store_hub); + // Frame time measurer - must be last self.frame_time_history.add( egui_ctx.input(|i| i.time), diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index bf91ffe9b603..b22ebbd3ff32 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -3,13 +3,6 @@ use re_arrow_store::{DataStoreConfig, DataStoreStats}; use re_data_store::StoreDb; use re_log_types::{ApplicationId, StoreId, StoreKind}; -use lazy_static::lazy_static; -use parking_lot::Mutex; - -lazy_static! { - static ref GLOBAL_HUB: Mutex = Mutex::new(StoreHubImpl::default()); -} - #[derive(Default)] pub struct StoreHubStats { pub blueprint_stats: DataStoreStats, @@ -24,63 +17,8 @@ pub struct StoreView<'a> { pub bundle: &'a StoreBundle, } -pub struct StoreHub; - -impl StoreHub { - pub fn add_bundle(bundle: StoreBundle) { - GLOBAL_HUB.lock().add_bundle(bundle); - } - - pub fn with_view(f: impl FnOnce(StoreView<'_>) -> R) -> R { - GLOBAL_HUB.lock().with_view(f) - } - - /// The selected/visible recording id, if any. - pub fn set_recording_id(recording_id: StoreId) { - GLOBAL_HUB.lock().set_recording_id(recording_id); - } - - pub fn set_app_id(app_id: ApplicationId) { - GLOBAL_HUB.lock().set_app_id(app_id); - } - - pub fn set_blueprint_for_app_id(blueprint_id: StoreId, app_id: ApplicationId) { - GLOBAL_HUB - .lock() - .set_blueprint_for_app_id(blueprint_id, app_id); - } - - pub fn store_stats() -> StoreHubStats { - // TODO - StoreHubStats::default() - /* - self.recording_db(|store_db| { - store_db - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) - .unwrap_or_default() - }); - */ - } - - pub fn purge_empty() { - GLOBAL_HUB.lock().purge_empty(); - } - - pub fn purge_fraction_of_ram(fraction_to_purge: f32) { - GLOBAL_HUB.lock().purge_fraction_of_ram(fraction_to_purge); - } - - pub fn access_bundle(f: impl FnOnce(&StoreBundle) -> R) -> R { - GLOBAL_HUB.lock().access_bundle(f) - } - - pub fn access_store_db_mut(store_id: &StoreId, f: impl FnOnce(&mut StoreDb) -> R) -> R { - GLOBAL_HUB.lock().access_store_db_mut(store_id, f) - } -} - #[derive(Default)] -struct StoreHubImpl { +pub struct StoreHubImpl { selected_rec_id: Option, application_id: Option, blueprint_by_app_id: HashMap, @@ -88,13 +26,13 @@ struct StoreHubImpl { } impl StoreHubImpl { - fn add_bundle(&mut self, bundle: StoreBundle) { + pub fn add_bundle(&mut self, bundle: StoreBundle) { self.store_dbs.append(bundle); // TODO: mutate app_id / selected_rec_id } - fn with_view(&mut self, f: impl FnOnce(StoreView<'_>) -> R) -> R { + pub fn view(&mut self) -> StoreView<'_> { // First maybe create blueprint if it's necessary. // TODO(jleibs): Can we hold onto this version here instead of // requerying below? @@ -117,21 +55,19 @@ impl StoreHubImpl { .and_then(|app_id| self.blueprint_by_app_id.get(app_id)) .and_then(|id| self.store_dbs.blueprint(id)); - let view = StoreView { + StoreView { blueprint, recording, bundle: &self.store_dbs, - }; - - f(view) + } } /// The selected/visible recording id, if any. - fn set_recording_id(&mut self, recording_id: StoreId) { + pub fn set_recording_id(&mut self, recording_id: StoreId) { self.selected_rec_id = Some(recording_id); } - fn set_app_id(&mut self, app_id: ApplicationId) { + pub fn set_app_id(&mut self, app_id: ApplicationId) { self.application_id = Some(app_id); } @@ -143,12 +79,16 @@ impl StoreHubImpl { f(&self.store_dbs) } + pub fn store_db_mut(&mut self, store_id: &StoreId) -> &mut StoreDb { + self.store_dbs.store_db_entry(store_id) + } + pub fn access_store_db_mut( &mut self, store_id: &StoreId, f: impl FnOnce(&mut StoreDb) -> R, ) -> R { - f(self.store_dbs.store_db_entry(store_id)) + f(self.store_db_mut(store_id)) } pub fn purge_empty(&mut self) { @@ -158,6 +98,16 @@ impl StoreHubImpl { pub fn purge_fraction_of_ram(&mut self, fraction_to_purge: f32) { self.store_dbs.purge_fraction_of_ram(fraction_to_purge); } + + pub fn recording_id(&self) -> Option<&StoreDb> { + self.selected_rec_id + .as_ref() + .and_then(|id| self.store_dbs.recording(id)) + } + + pub fn store_stats(&self) -> StoreHubStats { + StoreHubStats::default() + } } /// Stores many [`StoreDb`]s of recordings and blueprints. diff --git a/examples/rust/extend_viewer_ui/src/main.rs b/examples/rust/extend_viewer_ui/src/main.rs index 11b41ca4471e..52bb2973c29e 100644 --- a/examples/rust/extend_viewer_ui/src/main.rs +++ b/examples/rust/extend_viewer_ui/src/main.rs @@ -102,13 +102,11 @@ impl MyApp { }); ui.separator(); - self.rerun_app.recording_db(|store_db| { - if let Some(store_db) = store_db { - store_db_ui(ui, store_db); - } else { - ui.label("No log database loaded yet."); - } - }); + if let Some(store_db) = self.rerun_app.recording_db() { + store_db_ui(ui, store_db); + } else { + ui.label("No log database loaded yet."); + } } } From d2189e44dd900105305684a062a69c532193c480 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 14:19:49 +0200 Subject: [PATCH 04/21] StoreHubImpl -> StoreHub --- crates/re_viewer/src/app.rs | 18 +++++++++--------- crates/re_viewer/src/store_hub.rs | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 8a349f96c1a6..f15e9b0f1177 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -13,7 +13,7 @@ use re_viewer_context::{ use crate::{ background_tasks::BackgroundTasks, - store_hub::{StoreHubImpl, StoreHubStats, StoreView}, + store_hub::{StoreHub, StoreHubStats, StoreView}, ui::Blueprint, viewer_analytics::ViewerAnalytics, AppState, StoreBundle, @@ -96,7 +96,7 @@ pub struct App { pub(crate) background_tasks: BackgroundTasks, /// Recording Id to switch to - pub(crate) store_hub: Option, + pub(crate) store_hub: Option, pub(crate) requested_recording_id: Option, /// Toast notifications. @@ -182,7 +182,7 @@ impl App { rx, state, background_tasks: Default::default(), - store_hub: Some(StoreHubImpl::default()), + store_hub: Some(StoreHub::default()), requested_recording_id: Default::default(), toasts: toasts::Toasts::new(), memory_panel: Default::default(), @@ -246,7 +246,7 @@ impl App { fn run_pending_commands( &mut self, blueprint: &mut Blueprint, - store_hub: &mut StoreHubImpl, + store_hub: &mut StoreHub, egui_ctx: &egui::Context, frame: &mut eframe::Frame, ) { @@ -260,7 +260,7 @@ impl App { &mut self, cmd: Command, blueprint: &mut Blueprint, - store_hub: &mut StoreHubImpl, + store_hub: &mut StoreHub, _frame: &mut eframe::Frame, egui_ctx: &egui::Context, ) { @@ -554,7 +554,7 @@ impl App { } } - fn receive_messages(&mut self, store_hub: &mut StoreHubImpl, egui_ctx: &egui::Context) { + fn receive_messages(&mut self, store_hub: &mut StoreHub, egui_ctx: &egui::Context) { re_tracing::profile_function!(); let start = web_time::Instant::now(); @@ -619,7 +619,7 @@ impl App { } } - fn purge_memory_if_needed(&mut self, store_hub: &mut StoreHubImpl) { + fn purge_memory_if_needed(&mut self, store_hub: &mut StoreHub) { re_tracing::profile_function!(); fn format_limit(limit: Option) -> String { @@ -693,7 +693,7 @@ impl App { .and_then(|store_hub| store_hub.recording_id()) } - fn on_rrd_loaded(&mut self, store_hub: &mut StoreHubImpl, loaded_store_bundle: StoreBundle) { + fn on_rrd_loaded(&mut self, store_hub: &mut StoreHub, loaded_store_bundle: StoreBundle) { if let Some(store_db) = loaded_store_bundle.recordings().next() { store_hub.set_recording_id(store_db.store_id().clone()); self.analytics.on_open_recording(store_db); @@ -708,7 +708,7 @@ impl App { store_hub.add_bundle(loaded_store_bundle); } - fn handle_dropping_files(&mut self, store_hub: &mut StoreHubImpl, egui_ctx: &egui::Context) { + fn handle_dropping_files(&mut self, store_hub: &mut StoreHub, egui_ctx: &egui::Context) { preview_files_being_dropped(egui_ctx); // Collect dropped files: diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index b22ebbd3ff32..0ab46e0089d7 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -18,14 +18,14 @@ pub struct StoreView<'a> { } #[derive(Default)] -pub struct StoreHubImpl { +pub struct StoreHub { selected_rec_id: Option, application_id: Option, blueprint_by_app_id: HashMap, store_dbs: StoreBundle, } -impl StoreHubImpl { +impl StoreHub { pub fn add_bundle(&mut self, bundle: StoreBundle) { self.store_dbs.append(bundle); From 1d739ea227b30957add38b2ae3b626b81a0362ac Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 15:19:32 +0200 Subject: [PATCH 05/21] Actually implement the store stats --- crates/re_viewer/src/app.rs | 20 +++++-------- crates/re_viewer/src/store_hub.rs | 49 +++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index f15e9b0f1177..f1538bc0f928 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -804,7 +804,10 @@ impl eframe::App for App { render_ctx.gpu_resources.statistics() }; - let store_stats = store_hub.store_stats(); + // TODO(jleibs): Work through this ordering. Would be great to move the + // memory panel update *after* the calls to purge/receive so we don't + // need to generate the view twice. + let store_stats = store_hub.view().stats(); // do early, before doing too many allocations self.memory_panel.update(&gpu_resource_stats, &store_stats); @@ -819,18 +822,12 @@ impl eframe::App for App { self.receive_messages(&mut store_hub, egui_ctx); store_hub.purge_empty(); - store_hub.access_bundle(|bundle| { - self.state.cleanup(bundle); - }); + self.state.cleanup(store_hub.bundle()); file_saver_progress_ui(egui_ctx, &mut self.background_tasks); // toasts for background file saver - // This implicitly holds a lock on the `StoreHub` - // TODO(jleibs): ideally this whole thing would just be a member of App, but the borrow-checker - // means holding a reference to a StoreDb precludes us from calling any mut functions on - // App. A future refactor to move those mut functions out of app into a separate helper would - // streamline this. let store_view = store_hub.view(); + let blueprint_snapshot = Blueprint::from_db(egui_ctx, store_view.blueprint); // Make a mutable copy we can edit. @@ -865,9 +862,8 @@ impl eframe::App for App { } if let Some(blueprint_id) = &blueprint.blueprint_id { - store_hub.access_store_db_mut(blueprint_id, |blueprint_db| { - blueprint.sync_changes_to_store(&blueprint_snapshot, blueprint_db); - }); + let blueprint_db = store_hub.store_db_mut(blueprint_id); + blueprint.sync_changes_to_store(&blueprint_snapshot, blueprint_db); } if let Some(recording_id) = self.requested_recording_id.take() { diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 0ab46e0089d7..d77e28d62eb9 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -17,6 +17,37 @@ pub struct StoreView<'a> { pub bundle: &'a StoreBundle, } +impl<'a> StoreView<'a> { + pub fn stats(&self) -> StoreHubStats { + let blueprint_stats = self + .blueprint + .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .unwrap_or_default(); + + let blueprint_config = self + .blueprint + .map(|store_db| store_db.entity_db.data_store.config().clone()) + .unwrap_or_default(); + + let recording_stats = self + .recording + .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .unwrap_or_default(); + + let recording_config = self + .recording + .map(|store_db| store_db.entity_db.data_store.config().clone()) + .unwrap_or_default(); + + StoreHubStats { + blueprint_stats, + blueprint_config, + recording_stats, + recording_config, + } + } +} + #[derive(Default)] pub struct StoreHub { selected_rec_id: Option, @@ -28,8 +59,6 @@ pub struct StoreHub { impl StoreHub { pub fn add_bundle(&mut self, bundle: StoreBundle) { self.store_dbs.append(bundle); - - // TODO: mutate app_id / selected_rec_id } pub fn view(&mut self) -> StoreView<'_> { @@ -75,22 +104,14 @@ impl StoreHub { self.blueprint_by_app_id.insert(app_id, blueprint_id); } - pub fn access_bundle(&self, f: impl FnOnce(&StoreBundle) -> R) -> R { - f(&self.store_dbs) + pub fn bundle(&self) -> &StoreBundle { + &self.store_dbs } pub fn store_db_mut(&mut self, store_id: &StoreId) -> &mut StoreDb { self.store_dbs.store_db_entry(store_id) } - pub fn access_store_db_mut( - &mut self, - store_id: &StoreId, - f: impl FnOnce(&mut StoreDb) -> R, - ) -> R { - f(self.store_db_mut(store_id)) - } - pub fn purge_empty(&mut self) { self.store_dbs.purge_empty(); } @@ -104,10 +125,6 @@ impl StoreHub { .as_ref() .and_then(|id| self.store_dbs.recording(id)) } - - pub fn store_stats(&self) -> StoreHubStats { - StoreHubStats::default() - } } /// Stores many [`StoreDb`]s of recordings and blueprints. From 59fcdc8ddeabcf248db6c7e91320cea62f493a8a Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 15:24:11 +0200 Subject: [PATCH 06/21] Grab a single view for all of run_command --- crates/re_viewer/src/app.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index f1538bc0f928..4302b988203c 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -264,17 +264,16 @@ impl App { _frame: &mut eframe::Frame, egui_ctx: &egui::Context, ) { + let store_view = store_hub.view(); let is_narrow_screen = egui_ctx.screen_rect().width() < 600.0; // responsive ui for mobiles etc match cmd { #[cfg(not(target_arch = "wasm32"))] Command::Save => { - let store_view = store_hub.view(); save(self, &store_view, None); } #[cfg(not(target_arch = "wasm32"))] Command::SaveSelection => { - let store_view = store_hub.view(); save(self, &store_view, self.state.loop_selection(&store_view)); } #[cfg(not(target_arch = "wasm32"))] @@ -338,7 +337,6 @@ impl App { } Command::SelectionPrevious => { - let store_view = store_hub.view(); let state = &mut self.state; if let Some(rec_cfg) = store_view .recording @@ -349,7 +347,6 @@ impl App { } } Command::SelectionNext => { - let store_view = store_hub.view(); let state = &mut self.state; if let Some(rec_cfg) = store_view .recording @@ -364,23 +361,18 @@ impl App { } Command::PlaybackTogglePlayPause => { - let store_view = store_hub.view(); self.run_time_control_command(&store_view, TimeControlCommand::TogglePlayPause); } Command::PlaybackFollow => { - let store_view = store_hub.view(); self.run_time_control_command(&store_view, TimeControlCommand::Follow); } Command::PlaybackStepBack => { - let store_view = store_hub.view(); self.run_time_control_command(&store_view, TimeControlCommand::StepBack); } Command::PlaybackStepForward => { - let store_view = store_hub.view(); self.run_time_control_command(&store_view, TimeControlCommand::StepForward); } Command::PlaybackRestart => { - let store_view = store_hub.view(); self.run_time_control_command(&store_view, TimeControlCommand::Restart); } From d9367b9124cce2a0dbc0e7590a4b1073a66b4635 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 19:25:56 +0200 Subject: [PATCH 07/21] Rename to StoreContext and assorted cleanup --- crates/re_viewer/src/app.rs | 166 +++++++++-------- crates/re_viewer/src/app_state.rs | 17 +- crates/re_viewer/src/store_hub.rs | 169 +++++++++++------- crates/re_viewer/src/ui/rerun_menu.rs | 71 ++------ crates/re_viewer/src/ui/top_panel.rs | 11 +- crates/re_viewer_context/src/lib.rs | 2 + crates/re_viewer_context/src/store.rs | 8 + .../re_viewer_context/src/viewer_context.rs | 7 +- 8 files changed, 236 insertions(+), 215 deletions(-) create mode 100644 crates/re_viewer_context/src/store.rs diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 4302b988203c..9f14412c4e87 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -8,12 +8,12 @@ use re_smart_channel::Receiver; use re_ui::{toasts, Command}; use re_viewer_context::{ AppOptions, ComponentUiRegistry, DynSpaceViewClass, PlayState, SpaceViewClassRegistry, - SpaceViewClassRegistryError, + SpaceViewClassRegistryError, StoreContext, }; use crate::{ background_tasks::BackgroundTasks, - store_hub::{StoreHub, StoreHubStats, StoreView}, + store_hub::{StoreHub, StoreHubStats}, ui::Blueprint, viewer_analytics::ViewerAnalytics, AppState, StoreBundle, @@ -264,17 +264,21 @@ impl App { _frame: &mut eframe::Frame, egui_ctx: &egui::Context, ) { - let store_view = store_hub.view(); + let store_context = store_hub.read_context(); let is_narrow_screen = egui_ctx.screen_rect().width() < 600.0; // responsive ui for mobiles etc match cmd { #[cfg(not(target_arch = "wasm32"))] Command::Save => { - save(self, &store_view, None); + save(self, store_context.as_ref(), None); } #[cfg(not(target_arch = "wasm32"))] Command::SaveSelection => { - save(self, &store_view, self.state.loop_selection(&store_view)); + save( + self, + store_context.as_ref(), + self.state.loop_selection(store_context.as_ref()), + ); } #[cfg(not(target_arch = "wasm32"))] Command::Open => { @@ -338,8 +342,8 @@ impl App { Command::SelectionPrevious => { let state = &mut self.state; - if let Some(rec_cfg) = store_view - .recording + if let Some(rec_cfg) = store_context + .and_then(|ctx| ctx.recording) .map(|rec| rec.store_id()) .and_then(|rec_id| state.recording_config_mut(rec_id)) { @@ -348,8 +352,8 @@ impl App { } Command::SelectionNext => { let state = &mut self.state; - if let Some(rec_cfg) = store_view - .recording + if let Some(rec_cfg) = store_context + .and_then(|ctx| ctx.recording) .map(|rec| rec.store_id()) .and_then(|rec_id| state.recording_config_mut(rec_id)) { @@ -361,19 +365,25 @@ impl App { } Command::PlaybackTogglePlayPause => { - self.run_time_control_command(&store_view, TimeControlCommand::TogglePlayPause); + self.run_time_control_command( + store_context.as_ref(), + TimeControlCommand::TogglePlayPause, + ); } Command::PlaybackFollow => { - self.run_time_control_command(&store_view, TimeControlCommand::Follow); + self.run_time_control_command(store_context.as_ref(), TimeControlCommand::Follow); } Command::PlaybackStepBack => { - self.run_time_control_command(&store_view, TimeControlCommand::StepBack); + self.run_time_control_command(store_context.as_ref(), TimeControlCommand::StepBack); } Command::PlaybackStepForward => { - self.run_time_control_command(&store_view, TimeControlCommand::StepForward); + self.run_time_control_command( + store_context.as_ref(), + TimeControlCommand::StepForward, + ); } Command::PlaybackRestart => { - self.run_time_control_command(&store_view, TimeControlCommand::Restart); + self.run_time_control_command(store_context.as_ref(), TimeControlCommand::Restart); } #[cfg(not(target_arch = "wasm32"))] @@ -385,10 +395,10 @@ impl App { fn run_time_control_command( &mut self, - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, command: TimeControlCommand, ) { - let Some(store_db) = store_view.recording else { return; }; + let Some(store_db) = store_context.as_ref().and_then(|ctx| ctx.recording) else { return; }; let rec_id = store_db.store_id(); let Some(rec_cfg) = self.state.recording_config_mut(rec_id) else { return; }; let time_ctrl = &mut rec_cfg.time_ctrl; @@ -452,7 +462,7 @@ impl App { frame: &mut eframe::Frame, blueprint: &mut Blueprint, gpu_resource_stats: &WgpuResourcePoolStatistics, - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, store_stats: &StoreHubStats, ) { let mut main_panel_frame = egui::Frame::default(); @@ -468,42 +478,50 @@ impl App { crate::ui::mobile_warning_ui(&self.re_ui, ui); - crate::ui::top_panel(blueprint, store_view, ui, frame, self, gpu_resource_stats); + crate::ui::top_panel( + blueprint, + store_context, + ui, + frame, + self, + gpu_resource_stats, + ); self.memory_panel_ui(ui, gpu_resource_stats, store_stats); - if let (Some(store_db), Some(blueprint_db)) = - (store_view.recording, store_view.blueprint) - { - self.state - .recording_config_entry( - store_db.store_id().clone(), - self.rx.source(), - store_db, - ) - .selection_state - .on_frame_start(|item| blueprint.is_item_valid(item)); - - // TODO(andreas): store the re_renderer somewhere else. - let egui_renderer = { - let render_state = frame.wgpu_render_state().unwrap(); - &mut render_state.renderer.write() - }; - if let Some(render_ctx) = egui_renderer - .paint_callback_resources - .get_mut::() - { - render_ctx.begin_frame(); - - if store_db.is_empty() { - crate::ui::wait_screen_ui(ui, &self.rx); - } else { + if let Some(store_view) = store_context { + // TODO(jleibs): We don't necessarily want to show the wait + // screen just because we're missing a recording. If we've + // loaded a blueprint, we can still show the empty layouts or + // static data, but we need to jump through some hoops to + // handle a missing `RecordingConfig` in this case. + if let Some(store_db) = store_view.recording { + self.state + .recording_config_entry( + store_db.store_id().clone(), + self.rx.source(), + store_db, + ) + .selection_state + .on_frame_start(|item| blueprint.is_item_valid(item)); + + // TODO(andreas): store the re_renderer somewhere else. + let egui_renderer = { + let render_state = frame.wgpu_render_state().unwrap(); + &mut render_state.renderer.write() + }; + if let Some(render_ctx) = egui_renderer + .paint_callback_resources + .get_mut::() + { + render_ctx.begin_frame(); + self.state.show( blueprint, ui, render_ctx, store_db, - blueprint_db, + store_view, &self.re_ui, &self.component_ui_registry, &self.space_view_class_registry, @@ -512,6 +530,8 @@ impl App { render_ctx.before_submit(); } + } else { + crate::ui::wait_screen_ui(ui, &self.rx); } } else { crate::ui::wait_screen_ui(ui, &self.rx); @@ -566,26 +586,7 @@ impl App { let store_id = msg.store_id(); - let is_new_store = if let LogMsg::SetStoreInfo(msg) = &msg { - match msg.info.store_id.kind { - StoreKind::Recording => { - re_log::debug!("Opening a new recording: {:?}", msg.info); - store_hub.set_recording_id(store_id.clone()); - store_hub.set_app_id(msg.info.application_id.clone()); - } - - StoreKind::Blueprint => { - re_log::debug!("Opening a new blueprint: {:?}", msg.info); - store_hub.set_blueprint_for_app_id( - store_id.clone(), - msg.info.application_id.clone(), - ); - } - } - true - } else { - false - }; + let is_new_store = matches!(&msg, LogMsg::SetStoreInfo(_msg)); let store_db = store_hub.store_db_mut(store_id); @@ -604,6 +605,24 @@ impl App { self.analytics.on_open_recording(store_db); } + // Set the recording-id *after* creating the store + if let LogMsg::SetStoreInfo(msg) = &msg { + match msg.info.store_id.kind { + StoreKind::Recording => { + re_log::debug!("Opening a new recording: {:?}", msg.info); + store_hub.set_recording_id(store_id.clone()); + } + + StoreKind::Blueprint => { + re_log::debug!("Opening a new blueprint: {:?}", msg.info); + store_hub.set_blueprint_for_app_id( + store_id.clone(), + msg.info.application_id.clone(), + ); + } + } + } + if start.elapsed() > web_time::Duration::from_millis(10) { egui_ctx.request_repaint(); // make sure we keep receiving messages asap break; // don't block the main thread for too long @@ -682,7 +701,7 @@ impl App { pub fn recording_db(&self) -> Option<&StoreDb> { self.store_hub .as_ref() - .and_then(|store_hub| store_hub.recording_id()) + .and_then(|store_hub| store_hub.current_recording()) } fn on_rrd_loaded(&mut self, store_hub: &mut StoreHub, loaded_store_bundle: StoreBundle) { @@ -799,7 +818,7 @@ impl eframe::App for App { // TODO(jleibs): Work through this ordering. Would be great to move the // memory panel update *after* the calls to purge/receive so we don't // need to generate the view twice. - let store_stats = store_hub.view().stats(); + let store_stats = store_hub.stats(); // do early, before doing too many allocations self.memory_panel.update(&gpu_resource_stats, &store_stats); @@ -814,13 +833,14 @@ impl eframe::App for App { self.receive_messages(&mut store_hub, egui_ctx); store_hub.purge_empty(); - self.state.cleanup(store_hub.bundle()); + self.state.cleanup(&store_hub); file_saver_progress_ui(egui_ctx, &mut self.background_tasks); // toasts for background file saver - let store_view = store_hub.view(); + let store_context = store_hub.read_context(); - let blueprint_snapshot = Blueprint::from_db(egui_ctx, store_view.blueprint); + let blueprint_snapshot = + Blueprint::from_db(egui_ctx, store_context.as_ref().map(|ctx| ctx.blueprint)); // Make a mutable copy we can edit. let mut blueprint = blueprint_snapshot.clone(); @@ -830,7 +850,7 @@ impl eframe::App for App { frame, &mut blueprint, &gpu_resource_stats, - &store_view, + store_context.as_ref(), &store_stats, ); @@ -1003,10 +1023,10 @@ fn open_rrd_dialog() -> Option { #[cfg(not(target_arch = "wasm32"))] fn save( app: &mut App, - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, loop_selection: Option<(re_data_store::Timeline, re_log_types::TimeRangeF)>, ) { - let Some(store_db) = store_view.recording else { + let Some(store_db) = store_context.as_ref().and_then(|view| view.recording) else { // NOTE: Can only happen if saving through the command palette. re_log::error!("No data to save!"); return; diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 033aa5a3fb6a..3ff828065280 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -5,11 +5,11 @@ use re_log_types::{LogMsg, StoreId, TimeRangeF}; use re_smart_channel::Receiver; use re_viewer_context::{ AppOptions, Caches, ComponentUiRegistry, PlayState, RecordingConfig, SpaceViewClassRegistry, - ViewerContext, + StoreContext, ViewerContext, }; use re_viewport::ViewportState; -use crate::{store_hub::StoreView, ui::Blueprint}; +use crate::{store_hub::StoreHub, ui::Blueprint}; const WATERMARK: bool = false; // Nice for recording media material @@ -48,10 +48,11 @@ impl AppState { #[cfg_attr(target_arch = "wasm32", allow(dead_code))] pub fn loop_selection( &self, - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, ) -> Option<(re_data_store::Timeline, TimeRangeF)> { - store_view - .recording + store_context + .as_ref() + .and_then(|ctx| ctx.recording) .map(|rec| rec.store_id()) .and_then(|rec_id| { self.recording_configs @@ -73,7 +74,7 @@ impl AppState { ui: &mut egui::Ui, render_ctx: &mut re_renderer::RenderContext, store_db: &StoreDb, - blueprint_db: &StoreDb, + store_context: &StoreContext<'_>, re_ui: &re_ui::ReUi, component_ui_registry: &ComponentUiRegistry, space_view_class_registry: &SpaceViewClassRegistry, @@ -103,7 +104,7 @@ impl AppState { space_view_class_registry, component_ui_registry, store_db, - blueprint_db, + store_context, rec_cfg, re_ui, render_ctx, @@ -157,7 +158,7 @@ impl AppState { recording_config_entry(&mut self.recording_configs, id, data_source, store_db) } - pub fn cleanup(&mut self, store_hub: &crate::StoreBundle) { + pub fn cleanup(&mut self, store_hub: &StoreHub) { re_tracing::profile_function!(); self.recording_configs diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index d77e28d62eb9..772bdd257419 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -1,52 +1,9 @@ use ahash::HashMap; +use itertools::Itertools; use re_arrow_store::{DataStoreConfig, DataStoreStats}; use re_data_store::StoreDb; use re_log_types::{ApplicationId, StoreId, StoreKind}; - -#[derive(Default)] -pub struct StoreHubStats { - pub blueprint_stats: DataStoreStats, - pub blueprint_config: DataStoreConfig, - pub recording_stats: DataStoreStats, - pub recording_config: DataStoreConfig, -} - -pub struct StoreView<'a> { - pub blueprint: Option<&'a StoreDb>, - pub recording: Option<&'a StoreDb>, - pub bundle: &'a StoreBundle, -} - -impl<'a> StoreView<'a> { - pub fn stats(&self) -> StoreHubStats { - let blueprint_stats = self - .blueprint - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) - .unwrap_or_default(); - - let blueprint_config = self - .blueprint - .map(|store_db| store_db.entity_db.data_store.config().clone()) - .unwrap_or_default(); - - let recording_stats = self - .recording - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) - .unwrap_or_default(); - - let recording_config = self - .recording - .map(|store_db| store_db.entity_db.data_store.config().clone()) - .unwrap_or_default(); - - StoreHubStats { - blueprint_stats, - blueprint_config, - recording_stats, - recording_config, - } - } -} +use re_viewer_context::StoreContext; #[derive(Default)] pub struct StoreHub { @@ -56,75 +13,151 @@ pub struct StoreHub { store_dbs: StoreBundle, } +#[derive(Default)] +/// Convenient information used for `MemoryPanel` +pub struct StoreHubStats { + pub blueprint_stats: DataStoreStats, + pub blueprint_config: DataStoreConfig, + pub recording_stats: DataStoreStats, + pub recording_config: DataStoreConfig, +} + +/// Interface for accessing all blueprints and recordings +/// +/// The [`StoreHub`] provides access to the [`StoreDb`] instances that are used +/// to store both blueprints and recordings. +/// +/// Internally, the [`StoreHub`] tracks which [`ApplicationId`] and `recording +/// id` ([`StoreId`]) are currently selected in the viewer. These can be configured +/// using [`set_recording_id`] and [`set_app_id`] respectively. impl StoreHub { + /// Add a [`StoreBundle`] to the [`StoreHub`] pub fn add_bundle(&mut self, bundle: StoreBundle) { self.store_dbs.append(bundle); } - pub fn view(&mut self) -> StoreView<'_> { - // First maybe create blueprint if it's necessary. - // TODO(jleibs): Can we hold onto this version here instead of - // requerying below? - if let Some(id) = self.application_id.as_ref().map(|app_id| { + /// Get a read-only [`StoreContext`] from the [`StoreHub`] if one is available. + /// + /// All of the returned references to blueprints and recordings will have a + /// matching [`ApplicationId`]. + pub fn read_context(&mut self) -> Option> { + // If we have an app-id, then use it to look up the blueprint or else + // create the default blueprint. + let blueprint_id = self.application_id.as_ref().map(|app_id| { self.blueprint_by_app_id .entry(app_id.clone()) .or_insert_with(|| StoreId::from_string(StoreKind::Blueprint, app_id.clone().0)) - }) { - self.store_dbs.blueprint_entry(id); - } + }); + + // TODO(jleibs) entry is what I want, but holds a mutable reference. We know that + // unwrap won't fail since we just created the entry, but + blueprint_id + .as_ref() + .map(|id| self.store_dbs.blueprint_entry(id)); + + let blueprint = blueprint_id.map(|id| self.store_dbs.blueprint(id).unwrap()); let recording = self .selected_rec_id .as_ref() .and_then(|id| self.store_dbs.recording(id)); - let blueprint: Option<&StoreDb> = self - .application_id - .as_ref() - .and_then(|app_id| self.blueprint_by_app_id.get(app_id)) - .and_then(|id| self.store_dbs.blueprint(id)); - - StoreView { + // TODO(antoine): The below filter will limit our recording view to the current + // `ApplicationId`. Leaving this commented out for now since that is a bigger + // behavioral change we might want to plan/communicate around as it breaks things + // like --split-recordings in the api_demo. + blueprint.map(|blueprint| StoreContext { blueprint, recording, - bundle: &self.store_dbs, - } + alternate_recordings: self + .store_dbs + .recordings() + //.filter(|rec| rec.app_id() == self.application_id.as_ref()) + .collect_vec(), + }) } - /// The selected/visible recording id, if any. + /// Change the selected/visible recording id. pub fn set_recording_id(&mut self, recording_id: StoreId) { + // If this recording corresponds to an app that we know about, then apdate the app-id. + if let Some(app_id) = self + .store_dbs + .recording(&recording_id) + .as_ref() + .and_then(|recording| recording.app_id()) + { + self.set_app_id(app_id.clone()); + } + self.selected_rec_id = Some(recording_id); } + /// Change the selected [`ApplicationId`] pub fn set_app_id(&mut self, app_id: ApplicationId) { self.application_id = Some(app_id); } + /// Change which blueprint is active for a given [`ApplicationId`] pub fn set_blueprint_for_app_id(&mut self, blueprint_id: StoreId, app_id: ApplicationId) { self.blueprint_by_app_id.insert(app_id, blueprint_id); } - pub fn bundle(&self) -> &StoreBundle { - &self.store_dbs - } - + /// Mutable access to a [`StoreDb`] by id pub fn store_db_mut(&mut self, store_id: &StoreId) -> &mut StoreDb { self.store_dbs.store_db_entry(store_id) } + /// Remove any empty [`StoreDb`]s from the hub pub fn purge_empty(&mut self) { self.store_dbs.purge_empty(); } + /// Call [`StoreDb::purge_fraction_of_ram`] on every recording pub fn purge_fraction_of_ram(&mut self, fraction_to_purge: f32) { self.store_dbs.purge_fraction_of_ram(fraction_to_purge); } - pub fn recording_id(&self) -> Option<&StoreDb> { + /// Directly access the [`StoreDb`] for the selected recording + pub fn current_recording(&self) -> Option<&StoreDb> { self.selected_rec_id .as_ref() .and_then(|id| self.store_dbs.recording(id)) } + + /// Check whether the [`StoreHub`] contains the referenced recording + pub fn contains_recording(&self, id: &StoreId) -> bool { + self.store_dbs.contains_recording(id) + } + + /// Populate a [`StoreHubStats`] based on the selected app. + // TODO(jleibs): We probably want stats for all recordings, not just + // the currently selected recording. + pub fn stats(&mut self) -> StoreHubStats { + if let Some(ctx) = self.read_context() { + let blueprint_stats = DataStoreStats::from_store(&ctx.blueprint.entity_db.data_store); + + let blueprint_config = ctx.blueprint.entity_db.data_store.config().clone(); + + let recording_stats = ctx + .recording + .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .unwrap_or_default(); + + let recording_config = ctx + .recording + .map(|store_db| store_db.entity_db.data_store.config().clone()) + .unwrap_or_default(); + + StoreHubStats { + blueprint_stats, + blueprint_config, + recording_stats, + recording_config, + } + } else { + StoreHubStats::default() + } + } } /// Stores many [`StoreDb`]s of recordings and blueprints. diff --git a/crates/re_viewer/src/ui/rerun_menu.rs b/crates/re_viewer/src/ui/rerun_menu.rs index 3407b2a84df3..1ddbbae752e6 100644 --- a/crates/re_viewer/src/ui/rerun_menu.rs +++ b/crates/re_viewer/src/ui/rerun_menu.rs @@ -1,15 +1,14 @@ //! The main Rerun drop-down menu found in the top panel. use egui::NumExt as _; -use itertools::Itertools as _; use re_ui::Command; -use re_viewer_context::AppOptions; +use re_viewer_context::{AppOptions, StoreContext}; -use crate::{store_hub::StoreView, App}; +use crate::App; pub fn rerun_menu_button_ui( - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, ui: &mut egui::Ui, frame: &mut eframe::Frame, app: &mut App, @@ -38,7 +37,7 @@ pub fn rerun_menu_button_ui( { Command::Open.menu_button_ui(ui, &mut app.pending_commands); - save_buttons_ui(ui, store_view, app); + save_buttons_ui(ui, store_context, app); ui.add_space(spacing); @@ -70,10 +69,7 @@ pub fn rerun_menu_button_ui( { ui.menu_button("Recordings", |ui| { - recordings_menu(ui, store_view, app); - }); - ui.menu_button("Blueprints", |ui| { - blueprints_menu(ui, store_view, app); + recordings_menu(ui, store_context, app); }); } @@ -136,19 +132,17 @@ fn about_rerun_ui(ui: &mut egui::Ui, build_info: &re_build_info::BuildInfo) { ui.hyperlink_to("www.rerun.io", "https://www.rerun.io/"); } -fn recordings_menu(ui: &mut egui::Ui, store_view: &StoreView<'_>, app: &mut App) { - let store_dbs = store_view - .bundle - .recordings() - .sorted_by_key(|store_db| store_db.store_info().map(|ri| ri.started)) - .collect_vec(); +fn recordings_menu(ui: &mut egui::Ui, store_context: Option<&StoreContext<'_>>, app: &mut App) { + let store_dbs = store_context.map_or(vec![], |ctx| ctx.alternate_recordings.clone()); if store_dbs.is_empty() { ui.weak("(empty)"); return; } - let active_recording = store_view.recording.map(|rec| rec.store_id()); + let active_recording = store_context + .and_then(|ctx| ctx.recording) + .map(|rec| rec.store_id()); ui.style_mut().wrap = Some(false); for store_db in &store_dbs { @@ -171,45 +165,6 @@ fn recordings_menu(ui: &mut egui::Ui, store_view: &StoreView<'_>, app: &mut App) } } -fn blueprints_menu(ui: &mut egui::Ui, store_view: &StoreView<'_>, _app: &mut App) { - let blueprint_dbs = store_view - .bundle - .blueprints() - .sorted_by_key(|store_db| store_db.store_info().map(|ri| ri.started)) - .collect_vec(); - - if blueprint_dbs.is_empty() { - ui.weak("(empty)"); - return; - } - - let active_blueprint = store_view.blueprint.map(|rec| rec.store_id()); - - ui.style_mut().wrap = Some(false); - for store_db in &blueprint_dbs { - let info = if let Some(store_info) = store_db.store_info() { - if store_info.is_app_default_blueprint() { - format!("{} - Default Blueprint", store_info.application_id,) - } else { - format!( - "{} - {}", - store_info.application_id, - store_info.started.format() - ) - } - } else { - "".to_owned() - }; - - if ui - .radio(active_blueprint == Some(store_db.store_id()), info) - .clicked() - { - re_log::info!("Clicked! {}", store_db.store_id()); - } - } -} - fn options_menu_ui(ui: &mut egui::Ui, _frame: &mut eframe::Frame, options: &mut AppOptions) { ui.style_mut().wrap = Some(false); @@ -245,7 +200,7 @@ fn options_menu_ui(ui: &mut egui::Ui, _frame: &mut eframe::Frame, options: &mut // TODO(emilk): support saving data on web #[cfg(not(target_arch = "wasm32"))] -fn save_buttons_ui(ui: &mut egui::Ui, store_view: &StoreView<'_>, app: &mut App) { +fn save_buttons_ui(ui: &mut egui::Ui, store_view: Option<&StoreContext<'_>>, app: &mut App) { let file_save_in_progress = app.background_tasks.is_file_save_in_progress(); let save_button = Command::Save.menu_button(ui.ctx()); @@ -264,8 +219,8 @@ fn save_buttons_ui(ui: &mut egui::Ui, store_view: &StoreView<'_>, app: &mut App) }); } else { let store_db_is_nonempty = store_view - .recording - .map_or(false, |store_db| !store_db.is_empty()); + .and_then(|view| view.recording) + .map_or(false, |recording| !recording.is_empty()); ui.add_enabled_ui(store_db_is_nonempty, |ui| { if ui .add(save_button) diff --git a/crates/re_viewer/src/ui/top_panel.rs b/crates/re_viewer/src/ui/top_panel.rs index 22952329753a..44db63274fe6 100644 --- a/crates/re_viewer/src/ui/top_panel.rs +++ b/crates/re_viewer/src/ui/top_panel.rs @@ -1,12 +1,13 @@ use re_format::format_number; use re_renderer::WgpuResourcePoolStatistics; use re_ui::Command; +use re_viewer_context::StoreContext; -use crate::{store_hub::StoreView, ui::Blueprint, App}; +use crate::{ui::Blueprint, App}; pub fn top_panel( blueprint: &Blueprint, - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, ui: &mut egui::Ui, frame: &mut eframe::Frame, app: &mut App, @@ -38,7 +39,7 @@ pub fn top_panel( ui.set_height(top_bar_style.height); ui.add_space(top_bar_style.indent); - top_bar_ui(blueprint, store_view, ui, frame, app, gpu_resource_stats); + top_bar_ui(blueprint, store_context, ui, frame, app, gpu_resource_stats); }) .response; @@ -56,13 +57,13 @@ pub fn top_panel( fn top_bar_ui( blueprint: &Blueprint, - store_view: &StoreView<'_>, + store_context: Option<&StoreContext<'_>>, ui: &mut egui::Ui, frame: &mut eframe::Frame, app: &mut App, gpu_resource_stats: &WgpuResourcePoolStatistics, ) { - crate::ui::rerun_menu_button_ui(store_view, ui, frame, app); + crate::ui::rerun_menu_button_ui(store_context, ui, frame, app); if app.app_options().show_metrics { ui.separator(); diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index fad410958005..da4d90e0cbb9 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -10,6 +10,7 @@ mod item; mod selection_history; mod selection_state; mod space_view; +mod store; mod tensor; mod time_control; mod utils; @@ -35,6 +36,7 @@ pub use space_view::{ SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewState, TypedScene, }; +pub use store::StoreContext; pub use tensor::{TensorDecodeCache, TensorStats, TensorStatsCache}; pub use time_control::{Looping, PlayState, TimeControl, TimeView}; pub use utils::{auto_color, level_to_rich_text, DefaultColor}; diff --git a/crates/re_viewer_context/src/store.rs b/crates/re_viewer_context/src/store.rs new file mode 100644 index 000000000000..00898d973fd0 --- /dev/null +++ b/crates/re_viewer_context/src/store.rs @@ -0,0 +1,8 @@ +use re_data_store::StoreDb; + +/// The current Blueprint and Recording being displayed by the viewer +pub struct StoreContext<'a> { + pub blueprint: &'a StoreDb, + pub recording: Option<&'a StoreDb>, + pub alternate_recordings: Vec<&'a StoreDb>, +} diff --git a/crates/re_viewer_context/src/viewer_context.rs b/crates/re_viewer_context/src/viewer_context.rs index 1f59066c711f..feeb2e3a8c45 100644 --- a/crates/re_viewer_context/src/viewer_context.rs +++ b/crates/re_viewer_context/src/viewer_context.rs @@ -2,7 +2,7 @@ use re_data_store::store_db::StoreDb; use crate::{ AppOptions, Caches, ComponentUiRegistry, Item, ItemCollection, SelectionState, - SpaceViewClassRegistry, TimeControl, + SpaceViewClassRegistry, StoreContext, TimeControl, }; /// Common things needed by many parts of the viewer. @@ -22,10 +22,11 @@ pub struct ViewerContext<'a> { pub space_view_class_registry: &'a SpaceViewClassRegistry, /// The current recording. + /// TODO(jleibs): This can go away pub store_db: &'a StoreDb, - /// The current blueprint. - pub blueprint_db: &'a StoreDb, + /// The current view of the store + pub store_context: &'a StoreContext<'a>, /// UI config for the current recording (found in [`StoreDb`]). pub rec_cfg: &'a mut RecordingConfig, From 877c14bba584241e9bdf554e11fe0af7f6c7d9c6 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 19:31:37 +0200 Subject: [PATCH 08/21] Comments and scope fixes --- crates/re_viewer/src/app.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 9f14412c4e87..69158f0dce03 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -763,6 +763,7 @@ impl eframe::App for App { fn update(&mut self, egui_ctx: &egui::Context, frame: &mut eframe::Frame) { let frame_start = Instant::now(); + // Temporarily take the `StoreHub` out of the Viewer so it doesn't interfere with mutability let mut store_hub = self.store_hub.take().unwrap(); #[cfg(not(target_arch = "wasm32"))] @@ -859,20 +860,20 @@ impl eframe::App for App { paint_native_window_frame(egui_ctx); } - { - self.handle_dropping_files(&mut store_hub, egui_ctx); - - if !self.screenshotter.is_screenshotting() { - self.toasts.show(egui_ctx); - } + self.handle_dropping_files(&mut store_hub, egui_ctx); - if let Some(cmd) = self.cmd_palette.show(egui_ctx) { - self.pending_commands.push(cmd); - } + if !self.screenshotter.is_screenshotting() { + self.toasts.show(egui_ctx); + } - self.run_pending_commands(&mut blueprint, &mut store_hub, egui_ctx, frame); + if let Some(cmd) = self.cmd_palette.show(egui_ctx) { + self.pending_commands.push(cmd); } + self.run_pending_commands(&mut blueprint, &mut store_hub, egui_ctx, frame); + + // The only we we don't have a `blueprint_id` is if we don't have a blueprint + // and the only way we don't have a blueprint is if we don't have an app. if let Some(blueprint_id) = &blueprint.blueprint_id { let blueprint_db = store_hub.store_db_mut(blueprint_id); blueprint.sync_changes_to_store(&blueprint_snapshot, blueprint_db); @@ -882,6 +883,7 @@ impl eframe::App for App { store_hub.set_recording_id(recording_id); } + // Return the `StoreHub` to the Viewer so we have it on the next frame self.store_hub = Some(store_hub); // Frame time measurer - must be last From b28a7f3d962eeb9a2a39c82a71f53e4c79f491c5 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 19:33:02 +0200 Subject: [PATCH 09/21] comment --- 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 69158f0dce03..421380324066 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -95,8 +95,11 @@ pub struct App { /// Pending background tasks, e.g. files being saved. pub(crate) background_tasks: BackgroundTasks, - /// Recording Id to switch to + /// Interface for all recordings and blueprints pub(crate) store_hub: Option, + + /// Recording Id to switch to + // TODO(jleibs): Switch this to use `pending_commands` pub(crate) requested_recording_id: Option, /// Toast notifications. From 5b8595cacc0a1993e00b882ec7997937fcb1fca0 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 19:35:01 +0200 Subject: [PATCH 10/21] comment --- crates/re_viewer/src/store_hub.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 772bdd257419..2f47ff694a29 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -78,6 +78,7 @@ impl StoreHub { } /// Change the selected/visible recording id. + /// This will also change the application-id to match the newly selected recording. pub fn set_recording_id(&mut self, recording_id: StoreId) { // If this recording corresponds to an app that we know about, then apdate the app-id. if let Some(app_id) = self From 999eeba6280be4a1eabcd20047921bf902dad984 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 19:46:06 +0200 Subject: [PATCH 11/21] Set recording-id after adding things to store --- crates/re_viewer/src/app.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 421380324066..1b75b67faf48 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -708,8 +708,9 @@ impl App { } fn on_rrd_loaded(&mut self, store_hub: &mut StoreHub, loaded_store_bundle: StoreBundle) { + let mut new_rec_id = None; if let Some(store_db) = loaded_store_bundle.recordings().next() { - store_hub.set_recording_id(store_db.store_id().clone()); + new_rec_id = Some(store_db.store_id().clone()); self.analytics.on_open_recording(store_db); } @@ -720,6 +721,12 @@ impl App { } store_hub.add_bundle(loaded_store_bundle); + + // Set recording-id after adding to the store so that app-id, etc. + // is available internally. + if let Some(rec_id) = new_rec_id { + store_hub.set_recording_id(rec_id); + } } fn handle_dropping_files(&mut self, store_hub: &mut StoreHub, egui_ctx: &egui::Context) { From 33828ab45af84faa8f27a918118da65fd2d43836 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 21:52:15 +0200 Subject: [PATCH 12/21] doc links --- crates/re_viewer/src/store_hub.rs | 2 +- crates/re_viewer/src/ui/blueprint.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 2f47ff694a29..b3e4caceeefd 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -29,7 +29,7 @@ pub struct StoreHubStats { /// /// Internally, the [`StoreHub`] tracks which [`ApplicationId`] and `recording /// id` ([`StoreId`]) are currently selected in the viewer. These can be configured -/// using [`set_recording_id`] and [`set_app_id`] respectively. +/// using [`StoreHub::set_recording_id`] and [`StoreHub::set_app_id`] respectively. impl StoreHub { /// Add a [`StoreBundle`] to the [`StoreHub`] pub fn add_bundle(&mut self, bundle: StoreBundle) { diff --git a/crates/re_viewer/src/ui/blueprint.rs b/crates/re_viewer/src/ui/blueprint.rs index 6bc9efc698c3..d0f4b6d09918 100644 --- a/crates/re_viewer/src/ui/blueprint.rs +++ b/crates/re_viewer/src/ui/blueprint.rs @@ -15,7 +15,7 @@ pub struct Blueprint { } impl Blueprint { - /// Prefer this to [`Blueprint::default`] to get better defaults based on screen size. + /// Create a [`Blueprint`] with appropriate defaults. pub fn new(blueprint_id: Option, egui_ctx: &egui::Context) -> Self { let screen_size = egui_ctx.screen_rect().size(); Self { From faedb945b623e3acfc34338b26a1c2d86fd10565 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 21:55:14 +0200 Subject: [PATCH 13/21] wasm build --- crates/re_viewer/src/app.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index afd569aa0b3e..0fe5325b1c68 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -1,4 +1,3 @@ -use itertools::Itertools as _; use web_time::Instant; use re_data_store::store_db::StoreDb; @@ -748,6 +747,7 @@ impl App { if let Some(rrd) = crate::loading::load_file_contents(&file.name, &mut bytes) { self.on_rrd_loaded(store_hub, rrd); + #[cfg(not(target_arch = "wasm32"))] return; } } @@ -1083,6 +1083,7 @@ fn save_database_to_file( path: std::path::PathBuf, time_selection: Option<(re_data_store::Timeline, re_log_types::TimeRangeF)>, ) -> anyhow::Result anyhow::Result> { + use itertools::Itertools as _; use re_arrow_store::TimeRange; re_tracing::profile_scope!("dump_messages"); From 920ea604b8a5efddedc0f111843fdd901fdb77ac Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 21:58:27 +0200 Subject: [PATCH 14/21] typos --- crates/re_components/src/coordinates.rs | 2 +- crates/re_renderer/shader/outlines/jumpflooding_init.wgsl | 2 +- crates/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl | 2 +- crates/re_tuid/Cargo.toml | 2 +- crates/re_tuid/README.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/re_components/src/coordinates.rs b/crates/re_components/src/coordinates.rs index d7e57901d254..6961189c94c7 100644 --- a/crates/re_components/src/coordinates.rs +++ b/crates/re_components/src/coordinates.rs @@ -448,7 +448,7 @@ impl Handedness { #[cfg(feature = "glam")] #[test] -fn view_coordinatess() { +fn view_coordinates() { use glam::{vec3, Mat3}; { diff --git a/crates/re_renderer/shader/outlines/jumpflooding_init.wgsl b/crates/re_renderer/shader/outlines/jumpflooding_init.wgsl index 4ef9567e6f9c..d728b97f9294 100644 --- a/crates/re_renderer/shader/outlines/jumpflooding_init.wgsl +++ b/crates/re_renderer/shader/outlines/jumpflooding_init.wgsl @@ -21,7 +21,7 @@ fn main(in: FragmentInput) -> @location(0) Vec4 { var edge_pos_a_and_b = Vec4(0.0); var num_edges_a_and_b = Vec2(0.0); - // A lot of this code is repetetive, but wgsl/naga doesn't know yet how to do static indexing from unrolled loops. + // A lot of this code is repetitive, but wgsl/naga doesn't know yet how to do static indexing from unrolled loops. // Sample closest neighbors top/bottom/left/right { // right diff --git a/crates/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl b/crates/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl index af440fd6a09a..410b543f075c 100644 --- a/crates/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl +++ b/crates/re_renderer/shader/outlines/jumpflooding_init_msaa.wgsl @@ -69,7 +69,7 @@ fn main(in: FragmentInput) -> @location(0) Vec4 { edge_pos_a_and_b += edge.xxyy * 0.5; } - // A lot of this code is repetetive, but wgsl/naga doesn't know yet how to do static indexing from unrolled loops. + // A lot of this code is repetitive, but wgsl/naga doesn't know yet how to do static indexing from unrolled loops. // Sample closest neighbors top/bottom/left/right { // right diff --git a/crates/re_tuid/Cargo.toml b/crates/re_tuid/Cargo.toml index 53207a9ff8b7..0cd1e3b025f8 100644 --- a/crates/re_tuid/Cargo.toml +++ b/crates/re_tuid/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "re_tuid" authors.workspace = true -description = "128-bit Time-based Unique IDentifier" +description = "128-bit Time-based Unique Identifier" edition.workspace = true homepage.workspace = true include.workspace = true diff --git a/crates/re_tuid/README.md b/crates/re_tuid/README.md index d70b8d22f233..d75c7961c2fc 100644 --- a/crates/re_tuid/README.md +++ b/crates/re_tuid/README.md @@ -1,4 +1,4 @@ -# TUID: Time-based Unique IDentifier +# TUID: Time-based Unique Identifier Part of the [`rerun`](https://github.com/rerun-io/rerun) family of crates. From fffc0a8f812902c7dfaa4a14bbbf338adbc9baea Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 8 Jun 2023 23:14:04 +0200 Subject: [PATCH 15/21] Restore needless return --- crates/re_viewer/src/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 0fe5325b1c68..7896e2b544ef 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -747,7 +747,7 @@ impl App { if let Some(rrd) = crate::loading::load_file_contents(&file.name, &mut bytes) { self.on_rrd_loaded(store_hub, rrd); - #[cfg(not(target_arch = "wasm32"))] + #[allow(clippy::needless_return)] // false positive on wasm32 return; } } From 8c2c15da4b6831bd116fcd355a63e6ef8f8df173 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 9 Jun 2023 16:41:21 +0200 Subject: [PATCH 16/21] PR tweaks --- crates/re_viewer/src/store_hub.rs | 68 ++++++++++--------- .../src/{store.rs => store_context.rs} | 0 2 files changed, 35 insertions(+), 33 deletions(-) rename crates/re_viewer_context/src/{store.rs => store_context.rs} (100%) diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index b3e4caceeefd..b32338cae9ff 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -5,6 +5,14 @@ use re_data_store::StoreDb; use re_log_types::{ApplicationId, StoreId, StoreKind}; use re_viewer_context::StoreContext; +/// Interface for accessing all blueprints and recordings +/// +/// The [`StoreHub`] provides access to the [`StoreDb`] instances that are used +/// to store both blueprints and recordings. +/// +/// Internally, the [`StoreHub`] tracks which [`ApplicationId`] and `recording +/// id` ([`StoreId`]) are currently selected in the viewer. These can be configured +/// using [`StoreHub::set_recording_id`] and [`StoreHub::set_app_id`] respectively. #[derive(Default)] pub struct StoreHub { selected_rec_id: Option, @@ -13,8 +21,8 @@ pub struct StoreHub { store_dbs: StoreBundle, } -#[derive(Default)] /// Convenient information used for `MemoryPanel` +#[derive(Default)] pub struct StoreHubStats { pub blueprint_stats: DataStoreStats, pub blueprint_config: DataStoreConfig, @@ -22,14 +30,6 @@ pub struct StoreHubStats { pub recording_config: DataStoreConfig, } -/// Interface for accessing all blueprints and recordings -/// -/// The [`StoreHub`] provides access to the [`StoreDb`] instances that are used -/// to store both blueprints and recordings. -/// -/// Internally, the [`StoreHub`] tracks which [`ApplicationId`] and `recording -/// id` ([`StoreId`]) are currently selected in the viewer. These can be configured -/// using [`StoreHub::set_recording_id`] and [`StoreHub::set_app_id`] respectively. impl StoreHub { /// Add a [`StoreBundle`] to the [`StoreHub`] pub fn add_bundle(&mut self, bundle: StoreBundle) { @@ -41,40 +41,42 @@ impl StoreHub { /// All of the returned references to blueprints and recordings will have a /// matching [`ApplicationId`]. pub fn read_context(&mut self) -> Option> { - // If we have an app-id, then use it to look up the blueprint or else - // create the default blueprint. + // If we have an app-id, then use it to look up the blueprint. let blueprint_id = self.application_id.as_ref().map(|app_id| { self.blueprint_by_app_id .entry(app_id.clone()) .or_insert_with(|| StoreId::from_string(StoreKind::Blueprint, app_id.clone().0)) }); - // TODO(jleibs) entry is what I want, but holds a mutable reference. We know that - // unwrap won't fail since we just created the entry, but + // As long as we have a blueprint-id, create the blueprint. blueprint_id .as_ref() .map(|id| self.store_dbs.blueprint_entry(id)); - let blueprint = blueprint_id.map(|id| self.store_dbs.blueprint(id).unwrap()); - - let recording = self - .selected_rec_id - .as_ref() - .and_then(|id| self.store_dbs.recording(id)); - - // TODO(antoine): The below filter will limit our recording view to the current - // `ApplicationId`. Leaving this commented out for now since that is a bigger - // behavioral change we might want to plan/communicate around as it breaks things - // like --split-recordings in the api_demo. - blueprint.map(|blueprint| StoreContext { - blueprint, - recording, - alternate_recordings: self - .store_dbs - .recordings() - //.filter(|rec| rec.app_id() == self.application_id.as_ref()) - .collect_vec(), - }) + // If we have a blueprint, we can return the `StoreContext`. In most + // cases it should have already existed or been created above. + blueprint_id + .and_then(|id| self.store_dbs.blueprint(id)) + .map(|blueprint| { + let recording = self + .selected_rec_id + .as_ref() + .and_then(|id| self.store_dbs.recording(id)); + + // TODO(antoine): The below filter will limit our recording view to the current + // `ApplicationId`. Leaving this commented out for now since that is a bigger + // behavioral change we might want to plan/communicate around as it breaks things + // like --split-recordings in the api_demo. + StoreContext { + blueprint, + recording, + alternate_recordings: self + .store_dbs + .recordings() + //.filter(|rec| rec.app_id() == self.application_id.as_ref()) + .collect_vec(), + } + }) } /// Change the selected/visible recording id. diff --git a/crates/re_viewer_context/src/store.rs b/crates/re_viewer_context/src/store_context.rs similarity index 100% rename from crates/re_viewer_context/src/store.rs rename to crates/re_viewer_context/src/store_context.rs From 14c09dc2eb45b52f5bc2ba2217912fb66cab5392 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 9 Jun 2023 16:42:01 +0200 Subject: [PATCH 17/21] Actually rename store_context --- crates/re_viewer_context/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index da4d90e0cbb9..8a7807ff7d43 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -10,7 +10,7 @@ mod item; mod selection_history; mod selection_state; mod space_view; -mod store; +mod store_context; mod tensor; mod time_control; mod utils; @@ -36,7 +36,7 @@ pub use space_view::{ SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewState, TypedScene, }; -pub use store::StoreContext; +pub use store_context::StoreContext; pub use tensor::{TensorDecodeCache, TensorStats, TensorStatsCache}; pub use time_control::{Looping, PlayState, TimeControl, TimeView}; pub use utils::{auto_color, level_to_rich_text, DefaultColor}; From 8ea69708327412b2fae6df7e015845d944336541 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 9 Jun 2023 16:45:38 +0200 Subject: [PATCH 18/21] Plumb StoreHubStats through further --- crates/re_viewer/src/app.rs | 5 +--- crates/re_viewer/src/ui/memory_panel.rs | 32 ++++++++++--------------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 7896e2b544ef..a48222d89279 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -449,10 +449,7 @@ impl App { ui, &self.startup_options.memory_limit, gpu_resource_stats, - &store_stats.recording_config, - &store_stats.blueprint_config, - &store_stats.recording_stats, - &store_stats.blueprint_stats, + store_stats, ); }); } diff --git a/crates/re_viewer/src/ui/memory_panel.rs b/crates/re_viewer/src/ui/memory_panel.rs index 4d866aeeea1a..4559d796f193 100644 --- a/crates/re_viewer/src/ui/memory_panel.rs +++ b/crates/re_viewer/src/ui/memory_panel.rs @@ -42,10 +42,7 @@ impl MemoryPanel { ui: &mut egui::Ui, limit: &MemoryLimit, gpu_resource_stats: &WgpuResourcePoolStatistics, - store_config: &DataStoreConfig, - blueprint_config: &DataStoreConfig, - store_stats: &DataStoreStats, - blueprint_stats: &DataStoreStats, + store_stats: &StoreHubStats, ) { re_tracing::profile_function!(); @@ -57,15 +54,7 @@ impl MemoryPanel { .min_width(250.0) .default_width(300.0) .show_inside(ui, |ui| { - Self::left_side( - ui, - limit, - gpu_resource_stats, - store_config, - blueprint_config, - store_stats, - blueprint_stats, - ); + Self::left_side(ui, limit, gpu_resource_stats, store_stats); }); egui::CentralPanel::default().show_inside(ui, |ui| { @@ -78,10 +67,7 @@ impl MemoryPanel { ui: &mut egui::Ui, limit: &MemoryLimit, gpu_resource_stats: &WgpuResourcePoolStatistics, - store_config: &DataStoreConfig, - blueprint_config: &DataStoreConfig, - store_stats: &DataStoreStats, - blueprint_stats: &DataStoreStats, + store_stats: &StoreHubStats, ) { ui.strong("Rerun Viewer resource usage"); @@ -97,12 +83,20 @@ impl MemoryPanel { ui.separator(); ui.collapsing("Datastore Resources", |ui| { - Self::store_stats(ui, store_config, store_stats); + Self::store_stats( + ui, + &store_stats.recording_config, + &store_stats.recording_stats, + ); }); ui.separator(); ui.collapsing("Blueprint Resources", |ui| { - Self::store_stats(ui, blueprint_config, blueprint_stats); + Self::store_stats( + ui, + &store_stats.blueprint_config, + &store_stats.blueprint_stats, + ); }); } From fcef540f6df68febd64119e27872ec15d4d1ad4c Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 9 Jun 2023 17:15:13 +0200 Subject: [PATCH 19/21] Add some comments and clear the blueprint on reset --- crates/re_viewer/src/app.rs | 10 ++++++---- crates/re_viewer/src/store_hub.rs | 13 +++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index a48222d89279..63459e95c960 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -297,7 +297,7 @@ impl App { } Command::ResetViewer => { - self.reset(egui_ctx); + self.reset(store_hub, egui_ctx); } #[cfg(not(target_arch = "wasm32"))] @@ -607,7 +607,9 @@ impl App { self.analytics.on_open_recording(store_db); } - // Set the recording-id *after* creating the store + // Set the recording-id after potentially creating the store in the + // hub. This ordering is important because the `StoreHub` internally + // updates the app-id when changing the recording. if let LogMsg::SetStoreInfo(msg) = &msg { match msg.info.store_id.kind { StoreKind::Recording => { @@ -690,9 +692,9 @@ impl App { } /// Reset the viewer to how it looked the first time you ran it. - fn reset(&mut self, egui_ctx: &egui::Context) { + fn reset(&mut self, store_hub: &mut StoreHub, egui_ctx: &egui::Context) { self.state = Default::default(); - // TODO(jleibs): This presumably means throwing away the Blueprint? + store_hub.clear_blueprint(); // Keep the style: let style = egui_ctx.style(); diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index b32338cae9ff..4bcb161b481c 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -105,6 +105,15 @@ impl StoreHub { self.blueprint_by_app_id.insert(app_id, blueprint_id); } + /// Clear the current blueprint + pub fn clear_blueprint(&mut self) { + if let Some(app_id) = &self.application_id { + if let Some(blueprint_id) = self.blueprint_by_app_id.remove(app_id) { + self.store_dbs.remove(&blueprint_id); + } + } + } + /// Mutable access to a [`StoreDb`] by id pub fn store_db_mut(&mut self, store_id: &StoreId) -> &mut StoreDb { self.store_dbs.store_db_entry(store_id) @@ -211,6 +220,10 @@ impl StoreBundle { } } + pub fn remove(&mut self, id: &StoreId) { + self.store_dbs.remove(id); + } + // -- pub fn contains_recording(&self, id: &StoreId) -> bool { From 7c8617babb3055aab0f5243875b2ce265a285276 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 9 Jun 2023 17:24:52 +0200 Subject: [PATCH 20/21] Cleanup store_hub.stats() a bit more --- crates/re_viewer/src/app.rs | 3 -- crates/re_viewer/src/store_hub.rs | 58 ++++++++++++++++++------------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 63459e95c960..10fec9a6648d 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -828,9 +828,6 @@ impl eframe::App for App { render_ctx.gpu_resources.statistics() }; - // TODO(jleibs): Work through this ordering. Would be great to move the - // memory panel update *after* the calls to purge/receive so we don't - // need to generate the view twice. let store_stats = store_hub.stats(); // do early, before doing too many allocations diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 4bcb161b481c..671ab90a8a5b 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -144,30 +144,40 @@ impl StoreHub { /// Populate a [`StoreHubStats`] based on the selected app. // TODO(jleibs): We probably want stats for all recordings, not just // the currently selected recording. - pub fn stats(&mut self) -> StoreHubStats { - if let Some(ctx) = self.read_context() { - let blueprint_stats = DataStoreStats::from_store(&ctx.blueprint.entity_db.data_store); - - let blueprint_config = ctx.blueprint.entity_db.data_store.config().clone(); - - let recording_stats = ctx - .recording - .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) - .unwrap_or_default(); - - let recording_config = ctx - .recording - .map(|store_db| store_db.entity_db.data_store.config().clone()) - .unwrap_or_default(); - - StoreHubStats { - blueprint_stats, - blueprint_config, - recording_stats, - recording_config, - } - } else { - StoreHubStats::default() + pub fn stats(&self) -> StoreHubStats { + // If we have an app-id, then use it to look up the blueprint. + let blueprint = self + .application_id + .as_ref() + .and_then(|app_id| self.blueprint_by_app_id.get(app_id)) + .and_then(|blueprint_id| self.store_dbs.blueprint(blueprint_id)); + + let blueprint_stats = blueprint + .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .unwrap_or_default(); + + let blueprint_config = blueprint + .map(|store_db| store_db.entity_db.data_store.config().clone()) + .unwrap_or_default(); + + let recording = self + .selected_rec_id + .as_ref() + .and_then(|rec_id| self.store_dbs.recording(rec_id)); + + let recording_stats = recording + .map(|store_db| DataStoreStats::from_store(&store_db.entity_db.data_store)) + .unwrap_or_default(); + + let recording_config = recording + .map(|store_db| store_db.entity_db.data_store.config().clone()) + .unwrap_or_default(); + + StoreHubStats { + blueprint_stats, + blueprint_config, + recording_stats, + recording_config, } } } From ee1b14e7b0832e1c53372b5513d6ab4546c30281 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 9 Jun 2023 17:25:23 +0200 Subject: [PATCH 21/21] Typo --- crates/re_viewer/src/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 10fec9a6648d..e34872ddf4e9 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -881,7 +881,7 @@ impl eframe::App for App { self.run_pending_commands(&mut blueprint, &mut store_hub, egui_ctx, frame); - // The only we we don't have a `blueprint_id` is if we don't have a blueprint + // The only way we don't have a `blueprint_id` is if we don't have a blueprint // and the only way we don't have a blueprint is if we don't have an app. if let Some(blueprint_id) = &blueprint.blueprint_id { let blueprint_db = store_hub.store_db_mut(blueprint_id);