From 52f51c182e44f8e4a411af50a04e6e261e1f4d75 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Tue, 4 Jul 2023 12:35:23 -0400 Subject: [PATCH] Basic persistence for blueprints (#2578) ### What When moving over to the new blueprint store, we temporarily lost the ability to persist blueprints. This is one of the major regressions currently preventing us from releasing main (See: #2529) This PR adds new methods to the `store_hub`: - `persist_app_blueprints` - `try_to_load_persisted_blueprint` The blueprints are stored using the standard rrd file format. They go into a sub-directory of our app-state storage: - Linux: /home/UserName/.local/share/rerun/blueprints/ - macOS: /Users/UserName/Library/Application Support/rerun/blueprints/ - Windows: C:\Users\UserName\AppData\Roaming\rerun\blueprints\ We only store a single blueprint in this folder per app-id. Any time we change the app_id via the `store_hub` (such as when loading an rrd) we will search the directory for a persisted blueprint and then load it. We track the `insert_id` on the active blueprint to determine if we need to save back out to disk again. I've tested to confirm that in most cases when we restore from blueprint we don't do anything weird like re-modify the blueprint and save it again. ### Additional Notes: The way that DataBlueprint/AutoValues work required a workaround that I don't particularly love, but does seems to be workable until we refactor to get rid of more of the serde business. - After implementing blueprint-loading, I noticed that every time we start the app fresh, we see a cycle where the AutoValue for `pinhole_image_plane_distance` goes from Auto(100.06751) -> Auto(1) -> Auto(100.03121). Even ignoring the transient jump due to view bounds being a frame delayed to populate, the Auto-value itself is unstable from run to run. This meant that every time we started the app, we would do a new insertion which would cause the blueprint to immediately get saved again. - The work-around is to replace the usage of `!=` (PartialEq) with a conceptually equivalent method `has_edits` for the DataBlueprintTree. We now consider all values of Auto to be considered a match. This seems to work out fine since we (should?) always recompute Auto values at the beginning of each frame. - Longer term, I think the right solution is to move the Auto values out of the blueprint and into the AppState so they persist frame-to-frame and don't even show up in the blueprint comparison logic. However, DataBlueprintTree is a complicated piece of work, and plumbing through that state is proving to be challenging. ### Future work This currently only implements file-backed storage, so doesn't work with web-view. - https://github.com/rerun-io/rerun/issues/2579 ### Testing: * Run colmap fiat: ``` python examples/python/structure_from_motion/main.py --dataset colmap_fiat ``` * Adjust views * Exit application * Check the md5sum of the blueprint: ``` $ md5sum /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint 03fab330d9c23ac3da1163c854aa8518 /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint ``` * Run colmap fiat a *second* time. * Confirm the layout persists: ![image](https://github.com/rerun-io/rerun/assets/3312232/036d6ae9-6d3e-4abf-a981-cc46a7b6fe71) * Close rerun and verify the blueprint hasn't been changed: ``` $ md5sum /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint 03fab330d9c23ac3da1163c854aa8518 /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint ``` ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2578) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/2578) - [Docs preview](https://rerun.io/preview/pr%3Ajleibs%2Fsave_blueprints/docs) - [Examples preview](https://rerun.io/preview/pr%3Ajleibs%2Fsave_blueprints/examples) --- .github/workflows/labels.yml | 2 +- Cargo.lock | 1 + crates/re_arrow_store/src/lib.rs | 2 +- crates/re_arrow_store/src/store.rs | 10 ++ .../re_data_store/src/editable_auto_value.rs | 10 ++ crates/re_data_store/src/entity_properties.rs | 34 +++++ crates/re_data_store/src/store_db.rs | 6 + crates/re_space_view/src/data_blueprint.rs | 62 ++++++-- crates/re_viewer/Cargo.toml | 1 + crates/re_viewer/src/app.rs | 85 +++-------- crates/re_viewer/src/lib.rs | 1 + crates/re_viewer/src/loading.rs | 10 +- crates/re_viewer/src/saving.rs | 135 ++++++++++++++++++ crates/re_viewer/src/store_hub.rs | 103 ++++++++++++- .../src/blueprint_components/space_view.rs | 8 +- crates/re_viewport/src/space_view.rs | 25 +++- crates/re_viewport/src/viewport_blueprint.rs | 2 +- examples/python/blueprint/main.py | 4 +- 18 files changed, 411 insertions(+), 90 deletions(-) create mode 100644 crates/re_viewer/src/saving.rs diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index a8a00e4d98ac..e37000b07d2e 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -29,4 +29,4 @@ jobs: with: mode: minimum count: 1 - labels: "πŸ“Š analytics, πŸͺ³ bug, C/C++ SDK, codegen/idl, πŸ§‘β€πŸ’» dev experience, dependencies, πŸ“– documentation, πŸ’¬ discussion, examples, πŸ“‰ performance, 🐍 python API, ⛃ re_datastore, πŸ“Ί re_viewer, πŸ”Ί re_renderer, 🚜 refactor, β›΄ release, πŸ¦€ rust SDK, πŸ”¨ testing, ui, πŸ•ΈοΈ web" + labels: "πŸ“Š analytics, , 🟦 blueprint, πŸͺ³ bug, C/C++ SDK, codegen/idl, πŸ§‘β€πŸ’» dev experience, dependencies, πŸ“– documentation, πŸ’¬ discussion, examples, πŸ“‰ performance, 🐍 python API, ⛃ re_datastore, πŸ“Ί re_viewer, πŸ”Ί re_renderer, 🚜 refactor, β›΄ release, πŸ¦€ rust SDK, πŸ”¨ testing, ui, πŸ•ΈοΈ web" diff --git a/Cargo.lock b/Cargo.lock index ecbce18a90c3..7ce0582da054 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4572,6 +4572,7 @@ dependencies = [ "bytemuck", "cfg-if", "cocoa", + "directories-next", "eframe", "egui", "egui-wgpu", diff --git a/crates/re_arrow_store/src/lib.rs b/crates/re_arrow_store/src/lib.rs index 0f3c86c464bb..c093f9c9e861 100644 --- a/crates/re_arrow_store/src/lib.rs +++ b/crates/re_arrow_store/src/lib.rs @@ -36,7 +36,7 @@ pub mod polars_util; pub mod test_util; pub use self::arrow_util::ArrayExt; -pub use self::store::{DataStore, DataStoreConfig}; +pub use self::store::{DataStore, DataStoreConfig, StoreGeneration}; pub use self::store_gc::GarbageCollectionTarget; pub use self::store_read::{LatestAtQuery, RangeQuery}; pub use self::store_stats::{DataStoreRowStats, DataStoreStats}; diff --git a/crates/re_arrow_store/src/store.rs b/crates/re_arrow_store/src/store.rs index eb99d22525ae..f87d63ae1350 100644 --- a/crates/re_arrow_store/src/store.rs +++ b/crates/re_arrow_store/src/store.rs @@ -155,6 +155,10 @@ impl std::ops::DerefMut for ClusterCellCache { // --- +/// Incremented on each edit +#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct StoreGeneration(u64); + /// A complete data store: covers all timelines, all entities, everything. /// /// ## Debugging @@ -264,6 +268,12 @@ impl DataStore { "rerun.insert_id".into() } + /// Return the current `StoreGeneration`. This can be used to determine whether the + /// database has been modified since the last time it was queried. + pub fn generation(&self) -> StoreGeneration { + StoreGeneration(self.insert_id) + } + /// See [`Self::cluster_key`] for more information about the cluster key. pub fn cluster_key(&self) -> ComponentName { self.cluster_key diff --git a/crates/re_data_store/src/editable_auto_value.rs b/crates/re_data_store/src/editable_auto_value.rs index 717c373c8eb3..003bd17e9adf 100644 --- a/crates/re_data_store/src/editable_auto_value.rs +++ b/crates/re_data_store/src/editable_auto_value.rs @@ -49,4 +49,14 @@ where self } } + + /// Determine whether this `EditableAutoValue` has user-edits relative to another `EditableAutoValue` + /// If both values are `Auto`, then it is not considered edited. + pub fn has_edits(&self, other: &Self) -> bool { + match (self, other) { + (EditableAutoValue::UserEdited(s), EditableAutoValue::UserEdited(o)) => s != o, + (EditableAutoValue::Auto(_), EditableAutoValue::Auto(_)) => false, + _ => true, + } + } } diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index e15169bb4c6a..be9e039b9cc0 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -34,6 +34,17 @@ impl EntityPropertyMap { pub fn iter(&self) -> impl Iterator { self.props.iter() } + + /// Determine whether this `EntityPropertyMap` has user-edits relative to another `EntityPropertyMap` + pub fn has_edits(&self, other: &Self) -> bool { + self.props.len() != other.props.len() + || self.props.iter().any(|(key, val)| { + other + .props + .get(key) + .map_or(true, |other_val| val.has_edits(other_val)) + }) + } } // ---------------------------------------------------------------------------- @@ -116,6 +127,29 @@ impl EntityProperties { .clone(), } } + + /// Determine whether this `EntityProperty` has user-edits relative to another `EntityProperty` + pub fn has_edits(&self, other: &Self) -> bool { + let Self { + visible, + visible_history, + interactive, + color_mapper, + pinhole_image_plane_distance, + backproject_depth, + depth_from_world_scale, + backproject_radius_scale, + } = self; + + visible != &other.visible + || visible_history != &other.visible_history + || interactive != &other.interactive + || color_mapper.has_edits(&other.color_mapper) + || pinhole_image_plane_distance.has_edits(&other.pinhole_image_plane_distance) + || backproject_depth.has_edits(&other.backproject_depth) + || depth_from_world_scale.has_edits(&other.depth_from_world_scale) + || backproject_radius_scale.has_edits(&other.backproject_radius_scale) + } } // ---------------------------------------------------------------------------- diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index 4ac0091f653b..1490c8b5f830 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -292,6 +292,12 @@ impl StoreDb { + self.entity_db.data_store.num_temporal_rows() as usize } + /// Return the current `StoreGeneration`. This can be used to determine whether the + /// database has been modified since the last time it was queried. + pub fn generation(&self) -> re_arrow_store::StoreGeneration { + self.entity_db.data_store.generation() + } + pub fn is_empty(&self) -> bool { self.recording_msg.is_none() && self.num_rows() == 0 } diff --git a/crates/re_space_view/src/data_blueprint.rs b/crates/re_space_view/src/data_blueprint.rs index 76b518628b5d..3db531ad0eff 100644 --- a/crates/re_space_view/src/data_blueprint.rs +++ b/crates/re_space_view/src/data_blueprint.rs @@ -7,7 +7,7 @@ use slotmap::SlotMap; use smallvec::{smallvec, SmallVec}; /// A grouping of several data-blueprints. -#[derive(Clone, Default, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive(Clone, Default, serde::Deserialize, serde::Serialize)] pub struct DataBlueprintGroup { pub display_name: String, @@ -31,9 +31,29 @@ pub struct DataBlueprintGroup { pub entities: BTreeSet, } +impl DataBlueprintGroup { + /// Determine whether this `DataBlueprints` has user-edits relative to another `DataBlueprints` + fn has_edits(&self, other: &Self) -> bool { + let Self { + display_name, + properties_individual, + properties_projected: _, + parent, + children, + entities, + } = self; + + display_name != &other.display_name + || properties_individual.has_edits(&other.properties_individual) + || parent != &other.parent + || children != &other.children + || entities != &other.entities + } +} + /// Data blueprints for all entity paths in a space view. -#[derive(Clone, Default, PartialEq, serde::Deserialize, serde::Serialize)] -struct DataBlueprints { +#[derive(Clone, Default, serde::Deserialize, serde::Serialize)] +pub struct DataBlueprints { /// Individual settings. Mutate this. individual: EntityPropertyMap, @@ -44,6 +64,19 @@ struct DataBlueprints { projected: EntityPropertyMap, } +// Manually implement `PartialEq` since projected is serde skip +impl DataBlueprints { + /// Determine whether this `DataBlueprints` has user-edits relative to another `DataBlueprints` + fn has_edits(&self, other: &Self) -> bool { + let Self { + individual, + projected: _, + } = self; + + individual.has_edits(&other.individual) + } +} + /// Tree of all data blueprint groups for a single space view. #[derive(Clone, serde::Deserialize, serde::Serialize)] pub struct DataBlueprintTree { @@ -71,9 +104,9 @@ pub struct DataBlueprintTree { data_blueprints: DataBlueprints, } -// Manually implement PartialEq since slotmap doesn't -impl PartialEq for DataBlueprintTree { - fn eq(&self, other: &Self) -> bool { +/// Determine whether this `DataBlueprintTree` has user-edits relative to another `DataBlueprintTree` +impl DataBlueprintTree { + pub fn has_edits(&self, other: &Self) -> bool { let Self { groups, path_to_group, @@ -82,12 +115,17 @@ impl PartialEq for DataBlueprintTree { data_blueprints, } = self; - // Note: this could fail unexpectedly if slotmap iteration order is unstable. - groups.iter().zip(other.groups.iter()).all(|(x, y)| x == y) - && *path_to_group == other.path_to_group - && *entity_paths == other.entity_paths - && *root_group_handle == other.root_group_handle - && *data_blueprints == other.data_blueprints + groups.len() != other.groups.len() + || groups.iter().any(|(key, val)| { + other + .groups + .get(key) + .map_or(true, |other_val| val.has_edits(other_val)) + }) + || *path_to_group != other.path_to_group + || *entity_paths != other.entity_paths + || *root_group_handle != other.root_group_handle + || data_blueprints.has_edits(&other.data_blueprints) } } diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index 526e5b37f1c1..c34be1b87252 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -75,6 +75,7 @@ arrow2.workspace = true arrow2_convert.workspace = true bytemuck.workspace = true cfg-if.workspace = true +directories-next = "2.0.0" eframe = { workspace = true, default-features = false, features = [ "default_fonts", "persistence", diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index eeb2a3cec40d..d8df4ef226fc 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -275,7 +275,8 @@ impl App { } #[cfg(not(target_arch = "wasm32"))] SystemCommand::LoadRrd(path) => { - if let Some(rrd) = crate::loading::load_file_path(&path) { + let with_notification = true; + if let Some(rrd) = crate::loading::load_file_path(&path, with_notification) { store_hub.add_bundle(rrd); } } @@ -763,7 +764,8 @@ impl App { #[cfg(not(target_arch = "wasm32"))] if let Some(path) = &file.path { - if let Some(rrd) = crate::loading::load_file_path(path) { + let with_notification = true; + if let Some(rrd) = crate::loading::load_file_path(path, with_notification) { self.on_rrd_loaded(store_hub, rrd); } } @@ -778,7 +780,20 @@ impl eframe::App for App { fn save(&mut self, storage: &mut dyn eframe::Storage) { if self.startup_options.persist_state { + // Save the app state eframe::set_value(storage, eframe::APP_KEY, &self.state); + + // Save the blueprints + // TODO(2579): implement web-storage for blueprints as well + #[cfg(not(target_arch = "wasm32"))] + if let Some(hub) = &mut self.store_hub { + match hub.persist_app_blueprints() { + Ok(f) => f, + Err(err) => { + re_log::error!("Saving blueprints failed: {err}"); + } + }; + } } } @@ -1032,6 +1047,8 @@ fn save( store_context: Option<&StoreContext<'_>>, loop_selection: Option<(re_data_store::Timeline, re_log_types::TimeRangeF)>, ) { + use crate::saving::save_database_to_file; + 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!"); @@ -1062,67 +1079,3 @@ fn save( } } } - -/// Returns a closure that, when run, will save the contents of the current database -/// to disk, at the specified `path`. -/// -/// If `time_selection` is specified, then only data for that specific timeline over that -/// specific time range will be accounted for. -#[cfg(not(target_arch = "wasm32"))] -fn save_database_to_file( - store_db: &StoreDb, - 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"); - - let begin_rec_msg = store_db - .recording_msg() - .map(|msg| LogMsg::SetStoreInfo(msg.clone())); - - let ent_op_msgs = store_db - .iter_entity_op_msgs() - .map(|msg| LogMsg::EntityPathOpMsg(store_db.store_id().clone(), msg.clone())) - .collect_vec(); - - let time_filter = time_selection.map(|(timeline, range)| { - ( - timeline, - TimeRange::new(range.min.floor(), range.max.ceil()), - ) - }); - let data_msgs: Result, _> = store_db - .entity_db - .data_store - .to_data_tables(time_filter) - .map(|table| { - table - .to_arrow_msg() - .map(|msg| LogMsg::ArrowMsg(store_db.store_id().clone(), msg)) - }) - .collect(); - - use anyhow::Context as _; - let data_msgs = data_msgs.with_context(|| "Failed to export to data tables")?; - - let msgs = std::iter::once(begin_rec_msg) - .flatten() // option - .chain(ent_op_msgs) - .chain(data_msgs); - - Ok(move || { - re_tracing::profile_scope!("save_to_file"); - - use anyhow::Context as _; - let file = std::fs::File::create(path.as_path()) - .with_context(|| format!("Failed to create file at {path:?}"))?; - - let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; - re_log_encoding::encoder::encode_owned(encoding_options, msgs, file) - .map(|_| path) - .context("Message encode") - }) -} diff --git a/crates/re_viewer/src/lib.rs b/crates/re_viewer/src/lib.rs index 845174fab31c..c8af15108593 100644 --- a/crates/re_viewer/src/lib.rs +++ b/crates/re_viewer/src/lib.rs @@ -13,6 +13,7 @@ mod loading; #[cfg(not(target_arch = "wasm32"))] mod profiler; mod remote_viewer_app; +mod saving; mod screenshotter; mod store_hub; mod ui; diff --git a/crates/re_viewer/src/loading.rs b/crates/re_viewer/src/loading.rs index 9cb35af64b85..5c9feadbe56c 100644 --- a/crates/re_viewer/src/loading.rs +++ b/crates/re_viewer/src/loading.rs @@ -2,7 +2,7 @@ use crate::StoreBundle; #[cfg(not(target_arch = "wasm32"))] #[must_use] -pub fn load_file_path(path: &std::path::Path) -> Option { +pub fn load_file_path(path: &std::path::Path, with_notifications: bool) -> Option { fn load_file_path_impl(path: &std::path::Path) -> anyhow::Result { re_tracing::profile_function!(); use anyhow::Context as _; @@ -10,11 +10,15 @@ pub fn load_file_path(path: &std::path::Path) -> Option { StoreBundle::from_rrd(file) } - re_log::info!("Loading {path:?}…"); + if with_notifications { + re_log::info!("Loading {path:?}…"); + } match load_file_path_impl(path) { Ok(mut rrd) => { - re_log::info!("Loaded {path:?}"); + if with_notifications { + re_log::info!("Loaded {path:?}"); + } for store_db in rrd.store_dbs_mut() { store_db.data_source = Some(re_smart_channel::SmartChannelSource::Files { paths: vec![path.into()], diff --git a/crates/re_viewer/src/saving.rs b/crates/re_viewer/src/saving.rs new file mode 100644 index 000000000000..cb307933d596 --- /dev/null +++ b/crates/re_viewer/src/saving.rs @@ -0,0 +1,135 @@ +#[cfg(not(target_arch = "wasm32"))] +use re_data_store::StoreDb; + +#[cfg(not(target_arch = "wasm32"))] +use re_log_types::ApplicationId; + +#[cfg(not(target_arch = "wasm32"))] +/// Convert to lowercase and replace any character that is not a fairly common +/// filename character with '-' +fn sanitize_app_id(app_id: &ApplicationId) -> String { + let output = app_id.0.to_lowercase(); + output.replace( + |c: char| !matches!(c, '0'..='9' | 'a'..='z' | '.' | '_' | '+' | '(' | ')' | '[' | ']'), + "-", + ) +} + +#[cfg(not(target_arch = "wasm32"))] +/// Determine the default path for a blueprint based on its `ApplicationId` +/// This path should be determnistic and unique. +// TODO(#2579): Implement equivalent for web +pub fn default_blueprint_path(app_id: &ApplicationId) -> anyhow::Result { + use std::hash::{BuildHasher, Hash as _, Hasher as _}; + + use anyhow::Context; + + // TODO(jleibs) is there a better way to get this folder from egui? + if let Some(proj_dirs) = directories_next::ProjectDirs::from("", "", "rerun") { + let data_dir = proj_dirs.data_dir().join("blueprints"); + std::fs::create_dir_all(&data_dir).context("Could not create blueprint save directory.")?; + + // We want a unique filename (not a directory) for each app-id. + + // First we sanitize to remove disallowed characters + let mut sanitized_app_id = sanitize_app_id(app_id); + + // Make sure the filename isn't too long + // This is overly conservative in most cases but some versions of Windows 10 + // still have this restriction. + // TODO(jleibs): Determine this value from the environment. + const MAX_PATH: usize = 255; + let directory_part_length = data_dir.as_os_str().len(); + let hash_part_length = 16 + 1; + let extension_part_length = ".blueprint".len(); + let total_reserved_length = + directory_part_length + hash_part_length + extension_part_length; + if total_reserved_length > MAX_PATH { + anyhow::bail!("Could not form blueprint path: total minimum length exceeds {MAX_PATH} characters.") + } + sanitized_app_id.truncate(MAX_PATH - total_reserved_length); + + // If the sanitization actually did something, we no longer have a uniqueness guarantee, + // so insert the hash. + if sanitized_app_id != app_id.0 { + // Hash the original app-id. + + let hash = { + let mut hasher = ahash::RandomState::with_seeds(1, 2, 3, 4).build_hasher(); + app_id.0.hash(&mut hasher); + hasher.finish() + }; + + sanitized_app_id = format!("{sanitized_app_id}-{hash:x}"); + } + + Ok(data_dir.join(format!("{sanitized_app_id}.blueprint"))) + } else { + anyhow::bail!("Error finding project directory for blueprints.") + } +} + +#[cfg(not(target_arch = "wasm32"))] +/// Returns a closure that, when run, will save the contents of the current database +/// to disk, at the specified `path`. +/// +/// If `time_selection` is specified, then only data for that specific timeline over that +/// specific time range will be accounted for. +pub fn save_database_to_file( + store_db: &StoreDb, + 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_function!(); + + let begin_rec_msg = store_db + .recording_msg() + .map(|msg| LogMsg::SetStoreInfo(msg.clone())); + + let ent_op_msgs = store_db + .iter_entity_op_msgs() + .map(|msg| LogMsg::EntityPathOpMsg(store_db.store_id().clone(), msg.clone())) + .collect_vec(); + + let time_filter = time_selection.map(|(timeline, range)| { + ( + timeline, + TimeRange::new(range.min.floor(), range.max.ceil()), + ) + }); + let data_msgs: Result, _> = store_db + .entity_db + .data_store + .to_data_tables(time_filter) + .map(|table| { + table + .to_arrow_msg() + .map(|msg| LogMsg::ArrowMsg(store_db.store_id().clone(), msg)) + }) + .collect(); + + use anyhow::Context as _; + use re_log_types::LogMsg; + let data_msgs = data_msgs.with_context(|| "Failed to export to data tables")?; + + let msgs = std::iter::once(begin_rec_msg) + .flatten() // option + .chain(ent_op_msgs) + .chain(data_msgs); + + Ok(move || { + re_tracing::profile_scope!("save_to_file"); + + use anyhow::Context as _; + let file = std::fs::File::create(path.as_path()) + .with_context(|| format!("Failed to create file at {path:?}"))?; + + let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; + re_log_encoding::encoder::encode_owned(encoding_options, msgs, file) + .map(|_| path) + .context("Message encode") + }) +} diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 671ab90a8a5b..1c8207d62aa2 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -5,6 +5,15 @@ use re_data_store::StoreDb; use re_log_types::{ApplicationId, StoreId, StoreKind}; use re_viewer_context::StoreContext; +#[cfg(not(target_arch = "wasm32"))] +use re_arrow_store::StoreGeneration; + +#[cfg(not(target_arch = "wasm32"))] +use crate::{ + loading::load_file_path, + saving::{default_blueprint_path, save_database_to_file}, +}; + /// Interface for accessing all blueprints and recordings /// /// The [`StoreHub`] provides access to the [`StoreDb`] instances that are used @@ -19,6 +28,10 @@ pub struct StoreHub { application_id: Option, blueprint_by_app_id: HashMap, store_dbs: StoreBundle, + + // The [`StoreGeneration`] from when the [`StoreDb`] was last saved + #[cfg(not(target_arch = "wasm32"))] + blueprint_last_save: HashMap, } /// Convenient information used for `MemoryPanel` @@ -82,7 +95,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 this recording corresponds to an app that we know about, then update the app-id. if let Some(app_id) = self .store_dbs .recording(&recording_id) @@ -97,11 +110,23 @@ impl StoreHub { /// Change the selected [`ApplicationId`] pub fn set_app_id(&mut self, app_id: ApplicationId) { + // If we don't know of a blueprint for this `ApplicationId` yet, + // try to load one from the persisted store + // TODO(2579): implement web-storage for blueprints as well + #[cfg(not(target_arch = "wasm32"))] + if !self.blueprint_by_app_id.contains_key(&app_id) { + if let Err(err) = self.try_to_load_persisted_blueprint(&app_id) { + re_log::warn!("Failed to load persisted blueprint: {err}"); + } + } + self.application_id = Some(app_id); } /// Change which blueprint is active for a given [`ApplicationId`] + #[inline] pub fn set_blueprint_for_app_id(&mut self, blueprint_id: StoreId, app_id: ApplicationId) { + re_log::debug!("Switching blueprint for {app_id:?} to {blueprint_id:?}"); self.blueprint_by_app_id.insert(app_id, blueprint_id); } @@ -141,6 +166,73 @@ impl StoreHub { self.store_dbs.contains_recording(id) } + /// Persist any in-use blueprints to durable storage. + // TODO(#2579): implement persistence for web + #[cfg(not(target_arch = "wasm32"))] + pub fn persist_app_blueprints(&mut self) -> anyhow::Result<()> { + // Because we save blueprints based on their `ApplicationId`, we only + // save the blueprints referenced by `blueprint_by_app_id`, even though + // there may be other Blueprints in the Hub. + for (app_id, blueprint_id) in &self.blueprint_by_app_id { + let blueprint_path = default_blueprint_path(app_id)?; + re_log::debug!("Saving blueprint for {app_id} to {blueprint_path:?}"); + + if let Some(blueprint) = self.store_dbs.blueprint(blueprint_id) { + if self.blueprint_last_save.get(blueprint_id) != Some(&blueprint.generation()) { + // TODO(#1803): We should "flatten" blueprints when we save them + // TODO(jleibs): Should we push this into a background thread? Blueprints should generally + // be small & fast to save, but maybe not once we start adding big pieces of user data? + let file_saver = save_database_to_file(blueprint, blueprint_path, None)?; + file_saver()?; + self.blueprint_last_save + .insert(blueprint_id.clone(), blueprint.generation()); + } + } + } + Ok(()) + } + + /// Try to load the persisted blueprint for the given `ApplicationId`. + /// Note: If no blueprint exists at the expected path, the result is still considered `Ok`. + /// It is only an `Error` if a blueprint exists but fails to load. + // TODO(#2579): implement persistence for web + #[cfg(not(target_arch = "wasm32"))] + pub fn try_to_load_persisted_blueprint( + &mut self, + app_id: &ApplicationId, + ) -> anyhow::Result<()> { + re_tracing::profile_function!(); + let blueprint_path = default_blueprint_path(app_id)?; + if blueprint_path.exists() { + re_log::debug!("Trying to load blueprint for {app_id} from {blueprint_path:?}",); + let with_notification = false; + if let Some(mut bundle) = load_file_path(&blueprint_path, with_notification) { + for store in bundle.drain_store_dbs() { + if store.store_kind() == StoreKind::Blueprint && store.app_id() == Some(app_id) + { + // We found the blueprint we were looking for; make it active. + // borrow-checker won't let us just call `self.set_blueprint_for_app_id` + re_log::debug!( + "Switching blueprint for {app_id:?} to {:?}", + store.store_id(), + ); + self.blueprint_by_app_id + .insert(app_id.clone(), store.store_id().clone()); + self.blueprint_last_save + .insert(store.store_id().clone(), store.generation()); + self.store_dbs.insert_blueprint(store); + } else { + anyhow::bail!( + "Found unexpected store while loading blueprint: {:?}", + store.store_id() + ); + } + } + } + } + Ok(()) + } + /// Populate a [`StoreHubStats`] based on the selected app. // TODO(jleibs): We probably want stats for all recordings, not just // the currently selected recording. @@ -264,6 +356,11 @@ impl StoreBundle { self.store_dbs.insert(store_db.store_id().clone(), store_db); } + pub fn insert_blueprint(&mut self, store_db: StoreDb) { + debug_assert_eq!(store_db.store_kind(), StoreKind::Blueprint); + self.store_dbs.insert(store_db.store_id().clone(), store_db); + } + pub fn recordings(&self) -> impl Iterator { self.store_dbs .values() @@ -333,4 +430,8 @@ impl StoreBundle { store_db.purge_fraction_of_ram(fraction_to_purge); } } + + pub fn drain_store_dbs(&mut self) -> impl Iterator + '_ { + self.store_dbs.drain().map(|(_, store)| store) + } } diff --git a/crates/re_viewport/src/blueprint_components/space_view.rs b/crates/re_viewport/src/blueprint_components/space_view.rs index e7b7da8dbc3e..8851293e14c2 100644 --- a/crates/re_viewport/src/blueprint_components/space_view.rs +++ b/crates/re_viewport/src/blueprint_components/space_view.rs @@ -18,7 +18,7 @@ use crate::space_view::SpaceViewBlueprint; /// ]) /// ); /// ``` -#[derive(Clone, PartialEq, ArrowField, ArrowSerialize, ArrowDeserialize)] +#[derive(Clone, ArrowField, ArrowSerialize, ArrowDeserialize)] pub struct SpaceViewComponent { #[arrow_field(type = "SerdeField")] pub space_view: SpaceViewBlueprint, @@ -57,5 +57,9 @@ fn test_spaceview() { let data = [SpaceViewComponent { space_view }]; let array: Box = data.try_into_arrow().unwrap(); let ret: Vec = array.try_into_collection().unwrap(); - assert_eq!(&data, ret.as_slice()); + assert_eq!(data.len(), ret.len()); + assert!(data + .iter() + .zip(ret) + .all(|(l, r)| !l.space_view.has_edits(&r.space_view))); } diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs index 29fd60e8ad9e..f4f4044369b1 100644 --- a/crates/re_viewport/src/space_view.rs +++ b/crates/re_viewport/src/space_view.rs @@ -16,7 +16,7 @@ use crate::{ // ---------------------------------------------------------------------------- /// A view of a space. -#[derive(Clone, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive(Clone, serde::Deserialize, serde::Serialize)] pub struct SpaceViewBlueprint { pub id: SpaceViewId, pub display_name: String, @@ -40,6 +40,29 @@ pub struct SpaceViewBlueprint { pub entities_determined_by_user: bool, } +/// Determine whether this `SpaceViewBlueprint` has user-edits relative to another `SpaceViewBlueprint` +impl SpaceViewBlueprint { + pub fn has_edits(&self, other: &Self) -> bool { + let Self { + id, + display_name, + class_name, + space_origin, + data_blueprint, + category, + entities_determined_by_user, + } = self; + + id != &other.id + || display_name != &other.display_name + || class_name != &other.class_name + || space_origin != &other.space_origin + || data_blueprint.has_edits(&other.data_blueprint) + || category != &other.category + || entities_determined_by_user != &other.entities_determined_by_user + } +} + impl SpaceViewBlueprint { pub fn new( space_view_class: SpaceViewClassName, diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 5aaeb77e2a56..f00c24d07bd7 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -353,7 +353,7 @@ pub fn sync_space_view( space_view: &SpaceViewBlueprint, snapshot: Option<&SpaceViewBlueprint>, ) { - if Some(space_view) != snapshot { + if snapshot.map_or(true, |snapshot| space_view.has_edits(snapshot)) { let entity_path = EntityPath::from(format!( "{}/{}", SpaceViewComponent::SPACEVIEW_PREFIX, diff --git a/examples/python/blueprint/main.py b/examples/python/blueprint/main.py index 84cbed10cc9a..f619b3a61edc 100644 --- a/examples/python/blueprint/main.py +++ b/examples/python/blueprint/main.py @@ -25,14 +25,14 @@ def main() -> None: rr.init( "Blueprint demo", init_logging=False, - exp_init_blueprint=True, + exp_init_blueprint=not args.skip_blueprint, exp_add_to_app_default_blueprint=args.no_append_default, spawn=True, ) else: rr.init( "Blueprint demo", - exp_init_blueprint=True, + exp_init_blueprint=not args.skip_blueprint, exp_add_to_app_default_blueprint=args.no_append_default, spawn=True, )