Add Undo/Redo support in the viewer (#7546)

### What * Closes #3135 * Proceeded by #7602 * Proceeded by #7603 * Proceeded by #8241 * New issue: #8249 This PR implements Undo and Redo for any edit to the active blueprint. https://github.com/user-attachments/assets/05018729-f01e-42f4-a84f-b48dbf31b060 ### Implementation This implements undo/redo by editing the "time cursor" for the blueprint timeline. Undo moves it backwards, redo forwards. When doing an action, all redo history is erased from the store with a new `ChunkStore::drop_time_range` function. ### Known shortcomings * Undo doesn't work when the blueprint streams panel is open ### 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 the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7546?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7546?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7546) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --------- Co-authored-by: Clement Rey <[email protected]>
rerun-io · Dec 2, 2024 · c9a1e42 · c9a1e42
1 parent 4e17865
commit c9a1e42
Show file tree

Hide file tree

Showing 19 changed files with 412 additions and 109 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -1398,6 +1398,12 @@ dependencies = [
  "unicode-width",
 ]
 
+[[package]]
+name = "color-hex"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ecdffb913a326b6c642290a0d0ec8e8d6597291acdc07cc4c9cb4b3635d44cf9"
+
 [[package]]
 name = "colorchoice"
 version = "1.0.3"
@@ -1920,6 +1926,7 @@ version = "0.29.1"
 source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66"
 dependencies = [
  "bytemuck",
+ "color-hex",
  "emath",
  "serde",
 ]

diff --git a/Cargo.toml b/Cargo.toml
@@ -134,6 +134,7 @@ eframe = { version = "0.29.1", default-features = false, features = [
 ] }
 egui = { version = "0.29.1", features = [
   "callstack",
+  "color-hex",
   "log",
   "puffin",
   "rayon",

diff --git a/crates/store/re_entity_db/src/entity_db.rs b/crates/store/re_entity_db/src/entity_db.rs
@@ -22,7 +22,7 @@ use crate::{Error, TimesPerTimeline};
 // ----------------------------------------------------------------------------
 
 /// See [`GarbageCollectionOptions::time_budget`].
-const DEFAULT_GC_TIME_BUDGET: std::time::Duration = std::time::Duration::from_micros(3500); // empirical
+pub const DEFAULT_GC_TIME_BUDGET: std::time::Duration = std::time::Duration::from_micros(3500); // empirical
 
 // ----------------------------------------------------------------------------
 
@@ -408,19 +408,6 @@ impl EntityDb {
         self.set_store_info = Some(store_info);
     }
 
-    pub fn gc_everything_but_the_latest_row_on_non_default_timelines(
-        &mut self,
-    ) -> Vec<ChunkStoreEvent> {
-        re_tracing::profile_function!();
-
-        self.gc(&GarbageCollectionOptions {
-            target: GarbageCollectionTarget::Everything,
-            protect_latest: 1,
-            time_budget: DEFAULT_GC_TIME_BUDGET,
-            protected_time_ranges: Default::default(), // TODO(#3135): Use this for undo buffer
-        })
-    }
-
     /// Free up some RAM by forgetting the older parts of all timelines.
     pub fn purge_fraction_of_ram(&mut self, fraction_to_purge: f32) -> Vec<ChunkStoreEvent> {
         re_tracing::profile_function!();
@@ -454,7 +441,7 @@ impl EntityDb {
         store_events
     }
 
-    pub(crate) fn gc(&mut self, gc_options: &GarbageCollectionOptions) -> Vec<ChunkStoreEvent> {
+    pub fn gc(&mut self, gc_options: &GarbageCollectionOptions) -> Vec<ChunkStoreEvent> {
         re_tracing::profile_function!();
 
         let mut engine = self.storage_engine.write();

diff --git a/crates/store/re_entity_db/src/lib.rs b/crates/store/re_entity_db/src/lib.rs
@@ -13,7 +13,7 @@ mod times_per_timeline;
 mod versioned_instance_path;
 
 pub use self::{
-    entity_db::EntityDb,
+    entity_db::{EntityDb, DEFAULT_GC_TIME_BUDGET},
     entity_tree::EntityTree,
     instance_path::{InstancePath, InstancePathHash},
     store_bundle::{StoreBundle, StoreLoadError},

diff --git a/crates/viewer/re_data_ui/src/item_ui.rs b/crates/viewer/re_data_ui/src/item_ui.rs
@@ -607,7 +607,7 @@ pub fn instance_hover_card_ui(
     instance_path: &InstancePath,
     include_subtree: bool,
 ) {
-    if !ctx.recording().is_known_entity(&instance_path.entity_path) {
+    if !db.is_known_entity(&instance_path.entity_path) {
         ui.label("Unknown entity.");
         return;
     }
@@ -626,7 +626,7 @@ pub fn instance_hover_card_ui(
     // Then we can move the size view into `data_ui`.
 
     if instance_path.instance.is_all() {
-        if let Some(subtree) = ctx.recording().tree().subtree(&instance_path.entity_path) {
+        if let Some(subtree) = db.tree().subtree(&instance_path.entity_path) {
             entity_tree_stats_ui(ui, &query.timeline(), db, subtree, include_subtree);
         }
     } else {

diff --git a/crates/viewer/re_time_panel/src/lib.rs b/crates/viewer/re_time_panel/src/lib.rs
@@ -157,6 +157,7 @@ impl TimePanel {
         }
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn show_panel(
         &mut self,
         ctx: &ViewerContext<'_>,
@@ -165,6 +166,7 @@ impl TimePanel {
         rec_cfg: &RecordingConfig,
         ui: &mut egui::Ui,
         state: PanelState,
+        mut panel_frame: egui::Frame,
     ) {
         if state.is_hidden() {
             return;
@@ -181,8 +183,6 @@ impl TimePanel {
         // etc.)
         let screen_header_height = ui.cursor().top();
 
-        let mut panel_frame = DesignTokens::bottom_panel_frame();
-
         if state.is_expanded() {
             // Since we use scroll bars we want to fill the whole vertical space downwards:
             panel_frame.inner_margin.bottom = 0.0;

diff --git a/crates/viewer/re_ui/src/command.rs b/crates/viewer/re_ui/src/command.rs
@@ -21,6 +21,9 @@ pub enum UICommand {
     CloseCurrentRecording,
     CloseAllRecordings,
 
+    Undo,
+    Redo,
+
     #[cfg(not(target_arch = "wasm32"))]
     Quit,
 
@@ -120,7 +123,10 @@ impl UICommand {
             ),
 
             Self::CloseAllRecordings => ("Close all recordings",
-                "Close all open current recording (unsaved data will be lost)",),
+                "Close all open current recording (unsaved data will be lost)"),
+
+            Self::Undo => ("Undo", "Undo the last blueprint edit for the open recording"),
+            Self::Redo => ("Redo", "Redo the last undone thing"),
 
             #[cfg(not(target_arch = "wasm32"))]
             Self::Quit => ("Quit", "Close the Rerun Viewer"),
@@ -269,15 +275,15 @@ impl UICommand {
         }
 
         fn cmd_shift(key: Key) -> KeyboardShortcut {
-            KeyboardShortcut::new(Modifiers::COMMAND.plus(Modifiers::SHIFT), key)
+            KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, key)
         }
 
         fn cmd_alt(key: Key) -> KeyboardShortcut {
-            KeyboardShortcut::new(Modifiers::COMMAND.plus(Modifiers::ALT), key)
+            KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::ALT, key)
         }
 
         fn ctrl_shift(key: Key) -> KeyboardShortcut {
-            KeyboardShortcut::new(Modifiers::CTRL.plus(Modifiers::SHIFT), key)
+            KeyboardShortcut::new(Modifiers::CTRL | Modifiers::SHIFT, key)
         }
 
         match self {
@@ -289,6 +295,9 @@ impl UICommand {
             Self::CloseCurrentRecording => None,
             Self::CloseAllRecordings => None,
 
+            Self::Undo => Some(cmd(Key::Z)),
+            Self::Redo => Some(cmd_shift(Key::Z)),
+
             #[cfg(all(not(target_arch = "wasm32"), target_os = "windows"))]
             Self::Quit => Some(KeyboardShortcut::new(Modifiers::ALT, Key::F4)),
 

diff --git a/crates/viewer/re_viewer/src/app.rs b/crates/viewer/re_viewer/src/app.rs
@@ -1,5 +1,6 @@
 use std::sync::Arc;
 
+use itertools::Itertools as _;
 use re_build_info::CrateVersion;
 use re_data_source::{DataSource, FileContents};
 use re_entity_db::entity_db::EntityDb;
@@ -10,9 +11,9 @@ use re_ui::{toasts, DesignTokens, UICommand, UICommandSender};
 use re_viewer_context::{
     command_channel,
     store_hub::{BlueprintPersistence, StoreHub, StoreHubStats},
-    AppOptions, CommandReceiver, CommandSender, ComponentUiRegistry, PlayState, SpaceViewClass,
-    SpaceViewClassRegistry, SpaceViewClassRegistryError, StoreContext, SystemCommand,
-    SystemCommandSender,
+    AppOptions, BlueprintUndoState, CommandReceiver, CommandSender, ComponentUiRegistry, PlayState,
+    SpaceViewClass, SpaceViewClassRegistry, SpaceViewClassRegistryError, StoreContext,
+    SystemCommand, SystemCommandSender,
 };
 
 use crate::app_blueprint::PanelStateOverrides;
@@ -514,36 +515,45 @@ impl App {
                 store_hub.clear_active_blueprint();
                 egui_ctx.request_repaint(); // Many changes take a frame delay to show up.
             }
-            SystemCommand::UpdateBlueprint(blueprint_id, updates) => {
+            SystemCommand::UpdateBlueprint(blueprint_id, chunks) => {
+                re_log::trace!(
+                    "Update blueprint entities: {}",
+                    chunks.iter().map(|c| c.entity_path()).join(", ")
+                );
+
                 let blueprint_db = store_hub.entity_db_mut(&blueprint_id);
 
-                if self.state.app_options.inspect_blueprint_timeline {
-                    // We may we viewing a historical blueprint, and doing an edit based on that.
-                    // We therefor throw away everything after the currently viewed time (like an undo)
-                    let last_kept_event_time = self.state.blueprint_query_for_viewer().at();
-                    let first_dropped_event_time = last_kept_event_time.inc();
-                    blueprint_db.drop_time_range(
-                        &re_viewer_context::blueprint_timeline(),
-                        re_log_types::ResolvedTimeRange::new(
-                            first_dropped_event_time,
-                            re_chunk::TimeInt::MAX,
-                        ),
-                    );
-                }
+                self.state
+                    .blueprint_undo_state
+                    .entry(blueprint_id)
+                    .or_default()
+                    .clear_redo_buffer(blueprint_db);
 
-                for chunk in updates {
+                for chunk in chunks {
                     match blueprint_db.add_chunk(&Arc::new(chunk)) {
                         Ok(_store_events) => {}
                         Err(err) => {
                             re_log::warn_once!("Failed to store blueprint delta: {err}");
                         }
                     }
                 }
-
-                // If we inspect the timeline, make sure we show the latest state:
-                let mut time_ctrl = self.state.blueprint_cfg.time_ctrl.write();
-                time_ctrl.set_play_state(blueprint_db.times_per_timeline(), PlayState::Following);
             }
+            SystemCommand::UndoBlueprint { blueprint_id } => {
+                let blueprint_db = store_hub.entity_db_mut(&blueprint_id);
+                self.state
+                    .blueprint_undo_state
+                    .entry(blueprint_id)
+                    .or_default()
+                    .undo(blueprint_db);
+            }
+            SystemCommand::RedoBlueprint { blueprint_id } => {
+                self.state
+                    .blueprint_undo_state
+                    .entry(blueprint_id)
+                    .or_default()
+                    .redo();
+            }
+
             SystemCommand::DropEntity(blueprint_id, entity_path) => {
                 let blueprint_db = store_hub.entity_db_mut(&blueprint_id);
                 blueprint_db.drop_entity_path_recursive(&entity_path);
@@ -710,6 +720,21 @@ impl App {
                     .send_system(SystemCommand::CloseAllRecordings);
             }
 
+            UICommand::Undo => {
+                if let Some(store_context) = store_context {
+                    let blueprint_id = store_context.blueprint.store_id().clone();
+                    self.command_sender
+                        .send_system(SystemCommand::UndoBlueprint { blueprint_id });
+                }
+            }
+            UICommand::Redo => {
+                if let Some(store_context) = store_context {
+                    let blueprint_id = store_context.blueprint.store_id().clone();
+                    self.command_sender
+                        .send_system(SystemCommand::RedoBlueprint { blueprint_id });
+                }
+            }
+
             #[cfg(not(target_arch = "wasm32"))]
             UICommand::Quit => {
                 egui_ctx.send_viewport_cmd(egui::ViewportCommand::Close);
@@ -1665,7 +1690,7 @@ impl eframe::App for App {
         // TODO(#2579): implement web-storage for blueprints as well
         if let Some(hub) = &mut self.store_hub {
             if self.state.app_options.blueprint_gc {
-                hub.gc_blueprints();
+                hub.gc_blueprints(&self.state.blueprint_undo_state);
             }
 
             if let Err(err) = hub.save_app_blueprints() {
@@ -1793,7 +1818,7 @@ impl eframe::App for App {
         self.receive_messages(&mut store_hub, egui_ctx);
 
         if self.app_options().blueprint_gc {
-            store_hub.gc_blueprints();
+            store_hub.gc_blueprints(&self.state.blueprint_undo_state);
         }
 
         store_hub.purge_empty();
@@ -1820,9 +1845,17 @@ impl eframe::App for App {
         {
             let store_context = store_hub.read_context();
 
+            let blueprint_query = store_context.as_ref().map_or(
+                BlueprintUndoState::default_query(),
+                |store_context| {
+                    self.state
+                        .blueprint_query_for_viewer(store_context.blueprint)
+                },
+            );
+
             let app_blueprint = AppBlueprint::new(
                 store_context.as_ref(),
-                &self.state.blueprint_query_for_viewer(),
+                &blueprint_query,
                 egui_ctx,
                 self.panel_state_overrides_active
                     .then_some(self.panel_state_overrides),