Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Undo/Redo support in the viewer #7546

Merged
merged 21 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
]
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
17 changes: 2 additions & 15 deletions crates/store/re_entity_db/src/entity_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

// ----------------------------------------------------------------------------

Expand Down Expand Up @@ -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!();
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion crates/store/re_entity_db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
4 changes: 2 additions & 2 deletions crates/viewer/re_data_ui/src/item_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions crates/viewer/re_time_panel/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ impl TimePanel {
}
}

#[allow(clippy::too_many_arguments)]
pub fn show_panel(
&mut self,
ctx: &ViewerContext<'_>,
Expand All @@ -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;
Expand All @@ -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;
Expand Down
17 changes: 13 additions & 4 deletions crates/viewer/re_ui/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub enum UICommand {
CloseCurrentRecording,
CloseAllRecordings,

Undo,
Redo,

#[cfg(not(target_arch = "wasm32"))]
Quit,

Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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 {
Expand All @@ -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)),

Expand Down
83 changes: 58 additions & 25 deletions crates/viewer/re_viewer/src/app.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
Expand All @@ -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),
Expand Down
Loading