From 4773e0d8e227c129c8872866fd528bc904623ef4 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 14 Dec 2023 02:27:07 +0100 Subject: [PATCH] First pass tearing out the old viewport sync logic --- Cargo.lock | 1 + crates/re_viewer/src/app_state.rs | 23 +- crates/re_viewer/src/ui/blueprint_panel.rs | 2 +- .../re_viewer/src/ui/selection_history_ui.rs | 12 +- crates/re_viewer/src/ui/selection_panel.rs | 80 +++-- crates/re_viewport/Cargo.toml | 1 + crates/re_viewport/src/space_view.rs | 93 ++++-- .../src/space_view_entity_picker.rs | 12 +- crates/re_viewport/src/viewport.rs | 146 +++++---- crates/re_viewport/src/viewport_blueprint.rs | 299 ++++++------------ .../re_viewport/src/viewport_blueprint_ui.rs | 162 ++++++---- 11 files changed, 433 insertions(+), 398 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b4265b3835668..b53a4ff93cb3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5223,6 +5223,7 @@ dependencies = [ "itertools 0.11.0", "nohash-hasher", "once_cell", + "parking_lot 0.12.1", "rayon", "re_arrow_store", "re_data_store", diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index e3fcba41d46a2..647d31ce7ac2b 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -141,8 +141,9 @@ impl AppState { .values() .flat_map(|space_view| { space_view.queries.iter().map(|query| { - let resolver = - query.build_resolver(space_view.id, &space_view.auto_properties); + let state = viewport.state.read(); + let props = state.space_view_props(space_view.id); + let resolver = query.build_resolver(space_view.id, props); ( query.id, query.execute_query( @@ -186,11 +187,12 @@ impl AppState { viewport .blueprint .space_views - .values_mut() + .values() .flat_map(|space_view| { space_view.queries.iter().map(|query| { - let resolver = - query.build_resolver(space_view.id, &space_view.auto_properties); + let state = viewport.state.read(); + let props = state.space_view_props(space_view.id); + let resolver = query.build_resolver(space_view.id, props); ( query.id, query.execute_query( @@ -206,12 +208,7 @@ impl AppState { ctx.query_results = &updated_query_results; time_panel.show_panel(&ctx, ui, app_blueprint.time_panel_expanded); - selection_panel.show_panel( - &ctx, - ui, - &mut viewport, - app_blueprint.selection_panel_expanded, - ); + selection_panel.show_panel(&ctx, ui, &viewport, app_blueprint.selection_panel_expanded); let central_panel_frame = egui::Frame { fill: ui.style().visuals.panel_fill, @@ -250,7 +247,7 @@ impl AppState { ui.add_space(4.0); } - blueprint_panel_ui(&mut viewport.blueprint, &ctx, ui, &spaces_info); + blueprint_panel_ui(&viewport.blueprint, &ctx, ui, &spaces_info); }, ); @@ -273,8 +270,6 @@ impl AppState { }); }); - viewport.sync_blueprint_changes(command_sender); - { // We move the time at the very end of the frame, // so we have one frame to see the first data before we move the time. diff --git a/crates/re_viewer/src/ui/blueprint_panel.rs b/crates/re_viewer/src/ui/blueprint_panel.rs index b7fae9404540e..f946d3a0e32c7 100644 --- a/crates/re_viewer/src/ui/blueprint_panel.rs +++ b/crates/re_viewer/src/ui/blueprint_panel.rs @@ -3,7 +3,7 @@ use re_viewport::{SpaceInfoCollection, ViewportBlueprint}; /// Show the Blueprint section of the left panel based on the current [`ViewportBlueprint`] pub fn blueprint_panel_ui( - blueprint: &mut ViewportBlueprint<'_>, + blueprint: &ViewportBlueprint, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, spaces_info: &SpaceInfoCollection, diff --git a/crates/re_viewer/src/ui/selection_history_ui.rs b/crates/re_viewer/src/ui/selection_history_ui.rs index b7e00769bc189..76845c17fe195 100644 --- a/crates/re_viewer/src/ui/selection_history_ui.rs +++ b/crates/re_viewer/src/ui/selection_history_ui.rs @@ -15,7 +15,7 @@ impl SelectionHistoryUi { &mut self, re_ui: &re_ui::ReUi, ui: &mut egui::Ui, - blueprint: &ViewportBlueprint<'_>, + blueprint: &ViewportBlueprint, history: &mut SelectionHistory, ) -> Option { let next = self.next_button_ui(re_ui, ui, blueprint, history); @@ -27,7 +27,7 @@ impl SelectionHistoryUi { &mut self, re_ui: &re_ui::ReUi, ui: &mut egui::Ui, - blueprint: &ViewportBlueprint<'_>, + blueprint: &ViewportBlueprint, history: &mut SelectionHistory, ) -> Option { // undo selection @@ -77,7 +77,7 @@ impl SelectionHistoryUi { &mut self, re_ui: &re_ui::ReUi, ui: &mut egui::Ui, - blueprint: &ViewportBlueprint<'_>, + blueprint: &ViewportBlueprint, history: &mut SelectionHistory, ) -> Option { // redo selection @@ -126,7 +126,7 @@ impl SelectionHistoryUi { #[allow(clippy::unused_self)] fn history_item_ui( &mut self, - blueprint: &ViewportBlueprint<'_>, + blueprint: &ViewportBlueprint, ui: &mut egui::Ui, index: usize, history: &mut SelectionHistory, @@ -157,7 +157,7 @@ fn item_kind_ui(ui: &mut egui::Ui, sel: &Item) { ui.weak(RichText::new(format!("({})", sel.kind()))); } -fn item_collection_to_string(blueprint: &ViewportBlueprint<'_>, items: &ItemCollection) -> String { +fn item_collection_to_string(blueprint: &ViewportBlueprint, items: &ItemCollection) -> String { assert!(!items.is_empty()); // history never contains empty selections. if items.len() == 1 { item_to_string(blueprint, items.iter().next().unwrap()) @@ -168,7 +168,7 @@ fn item_collection_to_string(blueprint: &ViewportBlueprint<'_>, items: &ItemColl } } -fn item_to_string(blueprint: &ViewportBlueprint<'_>, item: &Item) -> String { +fn item_to_string(blueprint: &ViewportBlueprint, item: &Item) -> String { match item { Item::SpaceView(sid) => { if let Some(space_view) = blueprint.space_view(sid) { diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 8c03ec96ecac7..1a4bd77b997d8 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -39,7 +39,7 @@ impl SelectionPanel { &mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, - viewport: &mut Viewport<'_, '_>, + viewport: &Viewport<'_>, expanded: bool, ) { let screen_width = ui.ctx().screen_rect().width(); @@ -97,12 +97,7 @@ impl SelectionPanel { } #[allow(clippy::unused_self)] - fn contents( - &mut self, - ctx: &ViewerContext<'_>, - ui: &mut egui::Ui, - viewport: &mut Viewport<'_, '_>, - ) { + fn contents(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, viewport: &Viewport<'_>) { re_tracing::profile_function!(); let query = ctx.current_query(); @@ -122,18 +117,18 @@ impl SelectionPanel { }; for (i, item) in selection.iter().enumerate() { ui.push_id(i, |ui| { - what_is_selected_ui(ui, ctx, &mut viewport.blueprint, item); + what_is_selected_ui(ui, ctx, &viewport.blueprint, item); match item { Item::Container(tile_id) => { - container_top_level_properties(ui, ctx, &mut viewport.blueprint, tile_id); + container_top_level_properties(ui, ctx, &viewport.blueprint, tile_id); } Item::SpaceView(space_view_id) => { space_view_top_level_properties( ui, ctx, - &mut viewport.blueprint, + &viewport.blueprint, space_view_id, ); } @@ -197,7 +192,7 @@ fn space_view_button( fn what_is_selected_ui( ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &mut ViewportBlueprint<'_>, + viewport: &ViewportBlueprint, item: &Item, ) { match item { @@ -236,7 +231,7 @@ fn what_is_selected_ui( list_existing_data_blueprints(ui, ctx, entity_path, viewport); } Item::SpaceView(space_view_id) => { - if let Some(space_view) = viewport.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.space_view(space_view_id) { let space_view_class = space_view.class(ctx.space_view_class_registry); item_title_ui( ctx.re_ui, @@ -259,7 +254,7 @@ fn what_is_selected_ui( }; if let Some(space_view_id) = space_view_id { - if let Some(space_view) = viewport.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.space_view(space_view_id) { item_title_ui( ctx.re_ui, ui, @@ -334,7 +329,7 @@ fn list_existing_data_blueprints( ui: &mut egui::Ui, ctx: &ViewerContext<'_>, entity_path: &EntityPath, - blueprint: &ViewportBlueprint<'_>, + blueprint: &ViewportBlueprint, ) { let space_views_with_path = blueprint.space_views_containing_entity_path(ctx, entity_path); @@ -367,20 +362,23 @@ fn list_existing_data_blueprints( fn space_view_top_level_properties( ui: &mut egui::Ui, ctx: &ViewerContext<'_>, - viewport: &mut ViewportBlueprint<'_>, + viewport: &ViewportBlueprint, space_view_id: &SpaceViewId, ) { - if let Some(space_view) = viewport.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.space_view(space_view_id) { egui::Grid::new("space_view_top_level_properties") .num_columns(2) .show(ui, |ui| { + let mut name = space_view.display_name.clone(); ui.label("Name").on_hover_text( "The name of the Space View used for display purposes. This can be any text \ string.", ); - ui.text_edit_singleline(&mut space_view.display_name); + ui.text_edit_singleline(&mut name); ui.end_row(); + space_view.set_display_name(name, ctx); + ui.label("Space origin").on_hover_text( "The origin Entity for this Space View. For spatial Space Views, the Space \ View's origin is the same as this Entity's origin and all transforms are \ @@ -409,10 +407,11 @@ fn space_view_top_level_properties( fn container_top_level_properties( ui: &mut egui::Ui, _ctx: &ViewerContext<'_>, - viewport: &mut ViewportBlueprint<'_>, + viewport: &ViewportBlueprint, tile_id: &egui_tiles::TileId, ) { - if let Some(Tile::Container(container)) = viewport.tree.tiles.get_mut(*tile_id) { + // TODO(jleibs): fix container-editing + if let Some(Tile::Container(container)) = viewport.tree.tiles.get(*tile_id) { egui::Grid::new("container_top_level_properties") .num_columns(2) .show(ui, |ui| { @@ -447,7 +446,9 @@ fn container_top_level_properties( ); }); - container.set_kind(container_kind); + // TODO(jleibs): fix container-editing + re_log::warn!("TODO(jleibs): Fix container editing"); + //container.set_kind(container_kind); ui.end_row(); @@ -467,19 +468,30 @@ fn container_top_level_properties( ui.style_mut().wrap = Some(false); ui.set_min_width(64.0); + // TODO(jleibs): Fix container-editing + ui.label(format!("Layout: {}", grid_layout_to_string(&grid.layout))); + /* ui.selectable_value( &mut grid.layout, GridLayout::Auto, grid_layout_to_string(&GridLayout::Auto), ); + */ ui.separator(); for columns in 1..=grid.num_children() { + // TODO(jleibs): fix container-editing + ui.label(format!( + "Layout: {}", + grid_layout_to_string(&GridLayout::Columns(columns)) + )); + /* ui.selectable_value( &mut grid.layout, GridLayout::Columns(columns), grid_layout_to_string(&GridLayout::Columns(columns)), ); + */ } }); @@ -498,12 +510,7 @@ fn has_blueprint_section(item: &Item) -> bool { } /// What is the blueprint stuff for this item? -fn blueprint_ui( - ui: &mut egui::Ui, - ctx: &ViewerContext<'_>, - viewport: &mut Viewport<'_, '_>, - item: &Item, -) { +fn blueprint_ui(ui: &mut egui::Ui, ctx: &ViewerContext<'_>, viewport: &Viewport<'_>, item: &Item) { match item { Item::SpaceView(space_view_id) => { ui.horizontal(|ui| { @@ -524,13 +531,13 @@ fn blueprint_ui( if let Some(space_view) = viewport.blueprint.space_view(space_view_id) { let mut new_space_view = space_view.clone(); new_space_view.id = SpaceViewId::random(); - viewport.blueprint.add_space_view(new_space_view); - viewport.blueprint.mark_user_interaction(); + viewport.blueprint.add_space_view(new_space_view, ctx); + viewport.blueprint.mark_user_interaction(ctx); } } }); - if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.blueprint.space_view(space_view_id) { if let Some(query) = space_view.queries.first() { let inclusions = query.expressions.inclusions.join("\n"); let mut edited_inclusions = inclusions.clone(); @@ -565,16 +572,19 @@ fn blueprint_ui( vec![row], )); - space_view.entities_determined_by_user = true; + space_view.set_entity_determined_by_user(ctx); } } } ui.add_space(ui.spacing().item_spacing.y); - if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.blueprint.space_view(space_view_id) { let space_view_class = *space_view.class_identifier(); - let space_view_state = viewport.state.space_view_state_mut( + + let mut state = viewport.state.write(); + + let space_view_state = state.space_view_state_mut( ctx.space_view_class_registry, space_view.id, space_view.class_identifier(), @@ -602,7 +612,7 @@ fn blueprint_ui( .selection_ui( ctx, ui, - space_view_state, + space_view_state.space_view_state.as_mut(), &space_view.space_origin, space_view.id, &mut props, @@ -630,7 +640,7 @@ fn blueprint_ui( Item::InstancePath(space_view_id, instance_path) => { if let Some(space_view_id) = space_view_id { - if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.blueprint.space_view(space_view_id) { if instance_path.instance_key.is_specific() { ui.horizontal(|ui| { ui.label("Part of"); @@ -674,7 +684,7 @@ fn blueprint_ui( } Item::DataBlueprintGroup(space_view_id, query_id, group_path) => { - if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) { + if let Some(space_view) = viewport.blueprint.space_view(space_view_id) { let as_group = true; let query_result = ctx.lookup_query_result(*query_id); diff --git a/crates/re_viewport/Cargo.toml b/crates/re_viewport/Cargo.toml index 53c5385a7e98b..d873246e22167 100644 --- a/crates/re_viewport/Cargo.toml +++ b/crates/re_viewport/Cargo.toml @@ -45,6 +45,7 @@ image = { workspace = true, default-features = false, features = ["png"] } itertools.workspace = true nohash-hasher.workspace = true once_cell.workspace = true +parking_lot.workspace = true rayon.workspace = true rmp-serde.workspace = true tinyvec.workspace = true diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs index 40bfe88d078ef..9eb2e2e591432 100644 --- a/crates/re_viewport/src/space_view.rs +++ b/crates/re_viewport/src/space_view.rs @@ -3,18 +3,21 @@ use re_arrow_store::LatestAtQuery; use re_data_store::{EntityPath, EntityProperties, StoreDb, TimeInt, VisibleHistory}; use re_data_store::{EntityPropertiesComponent, EntityPropertyMap}; -use re_log_types::{EntityPathExpr, Timeline}; +use re_log_types::{DataRow, EntityPathExpr, RowId, TimePoint, Timeline}; use re_query::query_archetype; use re_renderer::ScreenshotProcessor; use re_space_view::{DataQueryBlueprint, ScreenshotMode}; use re_space_view_time_series::TimeSeriesSpaceView; +use re_types::blueprint::components::{EntitiesDeterminedByUser, Name, SpaceViewOrigin}; use re_viewer_context::{ DataQueryId, DataResult, DynSpaceViewClass, PerSystemDataResults, PerSystemEntities, SpaceViewClass, SpaceViewClassIdentifier, SpaceViewHighlights, SpaceViewId, SpaceViewState, - SpaceViewSystemRegistry, StoreContext, SystemExecutionOutput, ViewQuery, ViewerContext, + SpaceViewSystemRegistry, StoreContext, SystemCommand, SystemCommandSender as _, + SystemExecutionOutput, ViewQuery, ViewerContext, }; use crate::system_execution::create_and_run_space_view_systems; +use crate::viewport_blueprint::{add_delta_from_single_component, save_single_component}; // ---------------------------------------------------------------------------- @@ -36,10 +39,6 @@ pub struct SpaceViewBlueprint { /// True if the user is expected to add entities themselves. False otherwise. pub entities_determined_by_user: bool, - - /// Auto Properties - // TODO(jleibs): This needs to be per-query - pub auto_properties: EntityPropertyMap, } /// Determine whether this `SpaceViewBlueprint` has user-edits relative to another `SpaceViewBlueprint` @@ -52,7 +51,6 @@ impl SpaceViewBlueprint { space_origin, queries, entities_determined_by_user, - auto_properties: _, } = self; id != &other.id @@ -92,7 +90,6 @@ impl SpaceViewBlueprint { space_origin: space_path.clone(), queries: vec![query], entities_determined_by_user: false, - auto_properties: Default::default(), } } @@ -160,10 +157,63 @@ impl SpaceViewBlueprint { space_origin, queries, entities_determined_by_user, - auto_properties: Default::default(), }) } + pub fn save_full(&self, ctx: &ViewerContext<'_>) { + let timepoint = TimePoint::timeless(); + + let arch = re_types::blueprint::archetypes::SpaceViewBlueprint::new( + self.class_identifier().as_str(), + ) + .with_display_name(self.display_name.clone()) + .with_space_origin(&self.space_origin) + .with_entities_determined_by_user(self.entities_determined_by_user) + .with_contents(self.queries.iter().map(|q| q.id)); + + let mut deltas = vec![]; + + if let Ok(row) = + DataRow::from_archetype(RowId::new(), timepoint.clone(), self.entity_path(), &arch) + { + deltas.push(row); + } + + for query in &self.queries { + add_delta_from_single_component( + &mut deltas, + &query.id.as_entity_path(), + &timepoint, + query.expressions.clone(), + ); + } + + ctx.command_sender + .send_system(SystemCommand::UpdateBlueprint( + ctx.store_context.blueprint.store_id().clone(), + deltas, + )); + } + + pub fn set_entity_determined_by_user(&self, ctx: &ViewerContext<'_>) { + let component = EntitiesDeterminedByUser(true); + save_single_component(&self.entity_path(), component, ctx); + } + + pub fn set_display_name(&self, name: String, ctx: &ViewerContext<'_>) { + if name != self.display_name { + let component = Name(name.into()); + save_single_component(&self.entity_path(), component, ctx); + } + } + + pub fn set_origin(&self, origin: &EntityPath, ctx: &ViewerContext<'_>) { + if origin != &self.space_origin { + let component = SpaceViewOrigin(origin.into()); + save_single_component(&self.entity_path(), component, ctx); + } + } + pub fn class_identifier(&self) -> &SpaceViewClassIdentifier { &self.class_identifier } @@ -182,7 +232,12 @@ impl SpaceViewBlueprint { space_view_class_registry.get_system_registry_or_log_error(&self.class_identifier) } - pub fn on_frame_start(&mut self, ctx: &ViewerContext<'_>, view_state: &mut dyn SpaceViewState) { + pub fn on_frame_start( + &self, + ctx: &ViewerContext<'_>, + view_state: &mut dyn SpaceViewState, + view_props: &mut EntityPropertyMap, + ) { while ScreenshotProcessor::next_readback_result( ctx.render_ctx, self.id.gpu_readback_id(), @@ -214,7 +269,7 @@ impl SpaceViewBlueprint { ctx, view_state, &per_system_entities, - &mut self.auto_properties, + view_props, ); } @@ -371,26 +426,26 @@ impl SpaceViewBlueprint { } // TODO(jleibs): Get rid of mut by sending blueprint update - pub fn add_entity_exclusion(&mut self, ctx: &ViewerContext<'_>, expr: EntityPathExpr) { + pub fn add_entity_exclusion(&self, ctx: &ViewerContext<'_>, expr: EntityPathExpr) { if let Some(query) = self.queries.first() { query.add_entity_exclusion(ctx, expr); } - self.entities_determined_by_user = true; + self.set_entity_determined_by_user(ctx); } // TODO(jleibs): Get rid of mut by sending blueprint update - pub fn add_entity_inclusion(&mut self, ctx: &ViewerContext<'_>, expr: EntityPathExpr) { + pub fn add_entity_inclusion(&self, ctx: &ViewerContext<'_>, expr: EntityPathExpr) { if let Some(query) = self.queries.first() { query.add_entity_inclusion(ctx, expr); } - self.entities_determined_by_user = true; + self.set_entity_determined_by_user(ctx); } - pub fn clear_entity_expression(&mut self, ctx: &ViewerContext<'_>, expr: &EntityPathExpr) { + pub fn clear_entity_expression(&self, ctx: &ViewerContext<'_>, expr: &EntityPathExpr) { if let Some(query) = self.queries.first() { query.clear_entity_expression(ctx, expr); } - self.entities_determined_by_user = true; + self.set_entity_determined_by_user(ctx); } pub fn exclusions(&self) -> impl Iterator + '_ { @@ -458,6 +513,8 @@ mod tests { ), ); + let auto_properties = Default::default(); + let mut entities_per_system_per_class = EntitiesPerSystemPerClass::default(); entities_per_system_per_class .entry("3D".into()) @@ -474,7 +531,7 @@ mod tests { let query = space_view.queries.first().unwrap(); - let resolver = query.build_resolver(space_view.id, &space_view.auto_properties); + let resolver = query.build_resolver(space_view.id, &auto_properties); // No overrides set. Everybody has default values. { diff --git a/crates/re_viewport/src/space_view_entity_picker.rs b/crates/re_viewport/src/space_view_entity_picker.rs index 2c6adb715c344..3376dab23c1d5 100644 --- a/crates/re_viewport/src/space_view_entity_picker.rs +++ b/crates/re_viewport/src/space_view_entity_picker.rs @@ -27,7 +27,7 @@ impl SpaceViewEntityPicker { &mut self, ctx: &ViewerContext<'_>, ui: &egui::Ui, - space_view: &mut SpaceViewBlueprint, + space_view: &SpaceViewBlueprint, ) -> bool { // This function fakes a modal window, since egui doesn't have them yet: https://github.com/emilk/egui/issues/686 @@ -83,11 +83,7 @@ impl SpaceViewEntityPicker { } } -fn add_entities_ui( - ctx: &ViewerContext<'_>, - ui: &mut egui::Ui, - space_view: &mut SpaceViewBlueprint, -) { +fn add_entities_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, space_view: &SpaceViewBlueprint) { let spaces_info = SpaceInfoCollection::new(ctx.store_db); let tree = &ctx.store_db.tree(); let heuristic_context_per_entity = compute_heuristic_context_for_entities(ctx.store_db); @@ -123,7 +119,7 @@ fn add_entities_tree_ui( ui: &mut egui::Ui, name: &str, tree: &EntityTree, - space_view: &mut SpaceViewBlueprint, + space_view: &SpaceViewBlueprint, query_result: &DataQueryResult, inclusions: &HashSet, exclusions: &HashSet, @@ -193,7 +189,7 @@ fn add_entities_line_ui( ui: &mut egui::Ui, name: &str, entity_tree: &EntityTree, - space_view: &mut SpaceViewBlueprint, + space_view: &SpaceViewBlueprint, query_result: &DataQueryResult, inclusions: &HashSet, exclusions: &HashSet, diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index fa95c6ce8ce03..fb5daec05b1ef 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -7,12 +7,15 @@ use std::collections::BTreeMap; use ahash::HashMap; use egui_tiles::Behavior as _; +use once_cell::sync::Lazy; +use parking_lot::RwLock; +use re_data_store::EntityPropertyMap; use re_data_ui::item_ui; use re_ui::{Icon, ReUi}; use re_viewer_context::{ - CommandSender, Item, SpaceViewClassIdentifier, SpaceViewClassRegistry, SpaceViewId, - SpaceViewState, SystemExecutionOutput, ViewQuery, ViewerContext, + Item, SpaceViewClassIdentifier, SpaceViewClassRegistry, SpaceViewId, SpaceViewState, + SystemExecutionOutput, ViewQuery, ViewerContext, }; use crate::{ @@ -23,13 +26,20 @@ use crate::{ SpaceInfoCollection, SpaceViewBlueprint, ViewportBlueprint, }; +// State for each `SpaceView` including both the auto properties and +// the internal state of the space view itself. +pub struct PerSpaceViewState { + pub auto_properties: EntityPropertyMap, + pub space_view_state: Box, +} + // ---------------------------------------------------------------------------- /// State for the [`Viewport`] that persists across frames but otherwise /// is not saved. #[derive(Default)] pub struct ViewportState { pub(crate) space_view_entity_window: Option, - space_view_states: HashMap>, + space_view_states: HashMap, /// List of all space views that were visible *on screen* (excluding e.g. unselected tabs) the last frame. /// @@ -37,75 +47,65 @@ pub struct ViewportState { space_views_displayed_last_frame: Vec, } +static DEFAULT_PROPS: Lazy = Lazy::::new(Default::default); + impl ViewportState { pub fn space_view_state_mut( &mut self, space_view_class_registry: &SpaceViewClassRegistry, space_view_id: SpaceViewId, space_view_class: &SpaceViewClassIdentifier, - ) -> &mut dyn SpaceViewState { + ) -> &mut PerSpaceViewState { self.space_view_states .entry(space_view_id) - .or_insert_with(|| { - space_view_class_registry + .or_insert_with(|| PerSpaceViewState { + auto_properties: Default::default(), + space_view_state: space_view_class_registry .get_class_or_log_error(space_view_class) - .new_state() + .new_state(), }) - .as_mut() + } + + pub fn space_view_props(&self, space_view_id: SpaceViewId) -> &EntityPropertyMap { + self.space_view_states + .get(&space_view_id) + .map_or(&DEFAULT_PROPS, |state| &state.auto_properties) } } // ---------------------------------------------------------------------------- /// Defines the layout of the Viewport -pub struct Viewport<'a, 'b> { - /// The initial state of the Viewport read from the blueprint store on this frame. - /// - /// This is used to compare to the possibly mutated blueprint to - /// determine whether or not we need to save changes back - /// to the store as part of `sync_blueprint_changes`. - start_of_frame_snapshot: ViewportBlueprint<'a>, - +pub struct Viewport<'a> { // This is what me mutate during the frame. - pub blueprint: ViewportBlueprint<'a>, + pub blueprint: ViewportBlueprint, - pub state: &'b mut ViewportState, + pub state: RwLock<&'a mut ViewportState>, } -impl<'a, 'b> Viewport<'a, 'b> { - pub fn from_db(blueprint_db: &'a re_data_store::StoreDb, state: &'b mut ViewportState) -> Self { +impl<'a> Viewport<'a> { + pub fn from_db(blueprint_db: &re_data_store::StoreDb, state: &'a mut ViewportState) -> Self { re_tracing::profile_function!(); let blueprint = load_viewport_blueprint(blueprint_db); - let start_of_frame_snapshot = blueprint.clone(); - Self { - start_of_frame_snapshot, blueprint, - state, + state: RwLock::new(state), } } - pub fn sync_blueprint_changes(&self, command_sender: &CommandSender) { - ViewportBlueprint::sync_viewport_blueprint( - &self.start_of_frame_snapshot, - &self.blueprint, - command_sender, - ); + pub fn show_add_remove_entities_window(&self, space_view_id: SpaceViewId) { + self.state.write().space_view_entity_window = Some(SpaceViewEntityPicker { space_view_id }); } - pub fn show_add_remove_entities_window(&mut self, space_view_id: SpaceViewId) { - self.state.space_view_entity_window = Some(SpaceViewEntityPicker { space_view_id }); - } + pub fn viewport_ui(&self, ui: &mut egui::Ui, ctx: &'a ViewerContext<'_>) { + let Viewport { blueprint, state } = self; - pub fn viewport_ui(&mut self, ui: &mut egui::Ui, ctx: &'a ViewerContext<'_>) { - let Viewport { - blueprint, state, .. - } = self; + let mut state = state.write(); if let Some(window) = &mut state.space_view_entity_window { - if let Some(space_view) = blueprint.space_views.get_mut(&window.space_view_id) { + if let Some(space_view) = blueprint.space_views.get(&window.space_view_id) { if !window.ui(ctx, ui, space_view) { state.space_view_entity_window = None; } @@ -120,33 +120,31 @@ impl<'a, 'b> Viewport<'a, 'b> { return; } + let mut maximized = blueprint.maximized; + if let Some(space_view_id) = blueprint.maximized { if !blueprint.space_views.contains_key(&space_view_id) { - blueprint.maximized = None; // protect against bad deserialized data + maximized = None; } else if let Some(tile_id) = blueprint.tree.tiles.find_pane(&space_view_id) { if !blueprint.tree.tiles.is_visible(tile_id) { - blueprint.maximized = None; // Automatically de-maximize views that aren't visible anymore. + maximized = None; } } } - let mut maximized_tree; - - let tree = if let Some(space_view_id) = blueprint.maximized { + // TODO(jleibs): This tree won't have the edits from `viewport_blueprint_ui`. + // Maybe we should route that all the way through to here and only save it once. + let mut tree = if let Some(space_view_id) = blueprint.maximized { let mut tiles = egui_tiles::Tiles::default(); let root = tiles.insert_pane(space_view_id); - maximized_tree = egui_tiles::Tree::new("viewport_tree", root, tiles); - &mut maximized_tree + egui_tiles::Tree::new("viewport_tree", root, tiles) } else { - if blueprint.tree.is_empty() { - blueprint.tree = super::auto_layout::tree_from_space_views( - ctx.space_view_class_registry, - &blueprint.space_views, - ); - } - &mut blueprint.tree + blueprint.tree.clone() }; + // Snapshot the tree so we can save it if it changes + let tree_snapshot = tree.clone(); + let executed_systems_per_space_view = execute_systems_for_space_views( ctx, std::mem::take(&mut state.space_views_displayed_last_frame), @@ -159,10 +157,10 @@ impl<'a, 'b> Viewport<'a, 'b> { re_tracing::profile_scope!("tree.ui"); let mut tab_viewer = TabViewer { - viewport_state: state, + viewport_state: &mut state, ctx, space_views: &blueprint.space_views, - maximized: &mut blueprint.maximized, + maximized: &mut maximized, edited: false, space_views_displayed_current_frame: Vec::new(), executed_systems_per_space_view, @@ -180,24 +178,39 @@ impl<'a, 'b> Viewport<'a, 'b> { ); } - blueprint.auto_layout = false; + ViewportBlueprint::set_auto_layout(false, ctx); } state.space_views_displayed_last_frame = tab_viewer.space_views_displayed_current_frame; }); + + // TODO(jleibs): These edits happen independently of those made by `viewport_blueprint_ui`. + // If both edit there will be a conflict and some will get missed. + if tree != tree_snapshot && blueprint.maximized.is_none() { + ViewportBlueprint::set_tree(tree, ctx); + } + + if maximized != blueprint.maximized { + ViewportBlueprint::set_maximized(maximized, ctx); + } } - pub fn on_frame_start(&mut self, ctx: &ViewerContext<'_>, spaces_info: &SpaceInfoCollection) { + pub fn on_frame_start(&self, ctx: &ViewerContext<'_>, spaces_info: &SpaceInfoCollection) { re_tracing::profile_function!(); - for space_view in self.blueprint.space_views.values_mut() { - let space_view_state = self.state.space_view_state_mut( + let mut state = self.state.write(); + + for space_view in self.blueprint.space_views.values() { + let PerSpaceViewState { + auto_properties, + space_view_state, + } = state.space_view_state_mut( ctx.space_view_class_registry, space_view.id, space_view.class_identifier(), ); - space_view.on_frame_start(ctx, space_view_state); + space_view.on_frame_start(ctx, space_view_state.as_mut(), auto_properties); } if self.blueprint.auto_space_views { @@ -205,7 +218,7 @@ impl<'a, 'b> Viewport<'a, 'b> { default_created_space_views(ctx, spaces_info, ctx.entities_per_system_per_class) { if self.should_auto_add_space_view(&space_view_candidate) { - self.blueprint.add_space_view(space_view_candidate); + self.blueprint.add_space_view(space_view_candidate, ctx); } } } @@ -306,7 +319,10 @@ impl<'a, 'b> egui_tiles::Behavior for TabViewer<'a, 'b> { space_view_blueprint.execute_systems(self.ctx, latest_at, highlights) }; - let space_view_state = self.viewport_state.space_view_state_mut( + let PerSpaceViewState { + auto_properties: _, + space_view_state, + } = self.viewport_state.space_view_state_mut( self.ctx.space_view_class_registry, space_view_blueprint.id, space_view_blueprint.class_identifier(), @@ -315,7 +331,13 @@ impl<'a, 'b> egui_tiles::Behavior for TabViewer<'a, 'b> { self.space_views_displayed_current_frame .push(space_view_blueprint.id); - space_view_blueprint.scene_ui(space_view_state, self.ctx, ui, &query, system_output); + space_view_blueprint.scene_ui( + space_view_state.as_mut(), + self.ctx, + ui, + &query, + system_output, + ); Default::default() } diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs index 16847cdcff890..59f80f24c5665 100644 --- a/crates/re_viewport/src/viewport_blueprint.rs +++ b/crates/re_viewport/src/viewport_blueprint.rs @@ -1,22 +1,20 @@ use std::collections::BTreeMap; +use parking_lot::Mutex; use re_arrow_store::LatestAtQuery; -use re_data_store::{EntityPath, StoreDb}; +use re_data_store::EntityPath; use re_log_types::{DataRow, RowId, TimePoint, Timeline}; use re_query::query_archetype; use re_types_core::{archetypes::Clear, AsComponents as _}; use re_viewer_context::{ - CommandSender, Item, SpaceViewClassIdentifier, SpaceViewId, SystemCommand, SystemCommandSender, - ViewerContext, + Item, SpaceViewClassIdentifier, SpaceViewId, SystemCommand, SystemCommandSender, ViewerContext, }; use crate::{ blueprint::components::{ AutoLayout, AutoSpaceViews, IncludedSpaceViews, SpaceViewMaximized, ViewportLayout, }, - space_info::SpaceInfoCollection, space_view::SpaceViewBlueprint, - space_view_heuristics::default_created_space_views, VIEWPORT_PATH, }; @@ -26,16 +24,14 @@ use crate::{ // so that we don't iterate over something while modifying it. #[derive(Clone, Default)] pub(crate) struct TreeActions { + pub reset: bool, + pub create: Option, pub focus_tab: Option, pub remove: Vec, } /// Describes the layout and contents of the Viewport Panel. -#[derive(Clone)] -pub struct ViewportBlueprint<'a> { - /// The StoreDb used to instantiate this blueprint - blueprint_db: &'a StoreDb, - +pub struct ViewportBlueprint { /// Where the space views are stored. /// /// Not a hashmap in order to preserve the order of the space views. @@ -59,10 +55,11 @@ pub struct ViewportBlueprint<'a> { /// /// We delay any modifications to the tree until the end of the frame, /// so that we don't mutate something while inspecitng it. - pub(crate) deferred_tree_actions: TreeActions, + //TODO(jleibs): Can we use the SystemCommandSender for this, too? + pub(crate) deferred_tree_actions: Mutex, } -impl<'a> ViewportBlueprint<'a> { +impl ViewportBlueprint { /// Determine whether all views in a blueprint are invalid. /// /// This most commonly happens due to a change in struct definition that @@ -80,41 +77,6 @@ impl<'a> ViewportBlueprint<'a> { .all(|sv| sv.class_identifier() == &SpaceViewClassIdentifier::invalid()) } - /// Reset the blueprint to a default state using some heuristics. - pub fn reset(&mut self, ctx: &ViewerContext<'_>, spaces_info: &SpaceInfoCollection) { - // TODO(jleibs): When using blueprint API, "reset" should go back to the initially transmitted - // blueprint, not the default blueprint. - re_tracing::profile_function!(); - - let ViewportBlueprint { - blueprint_db: _, - space_views, - tree, - maximized, - auto_layout, - auto_space_views, - deferred_tree_actions: tree_actions, - } = self; - - // Note, it's important that these values match the behavior in `load_viewport_blueprint` below. - *space_views = Default::default(); - *tree = egui_tiles::Tree::empty("viewport_tree"); - *maximized = None; - *auto_layout = true; - // Only enable auto-space-views if this is the app-default blueprint - *auto_space_views = self - .blueprint_db - .store_info() - .map_or(false, |ri| ri.is_app_default_blueprint()); - *tree_actions = Default::default(); - - for space_view in - default_created_space_views(ctx, spaces_info, ctx.entities_per_system_per_class) - { - self.add_space_view(space_view); - } - } - pub fn space_view_ids(&self) -> impl Iterator + '_ { self.space_views.keys() } @@ -130,28 +92,38 @@ impl<'a> ViewportBlueprint<'a> { self.space_views.get_mut(space_view_id) } - pub(crate) fn remove(&mut self, space_view_id: &SpaceViewId) -> Option { - self.mark_user_interaction(); - - let Self { - blueprint_db: _, - space_views, - tree, - maximized, - auto_layout: _, - auto_space_views: _, - deferred_tree_actions: _, - } = self; - - if *maximized == Some(*space_view_id) { - *maximized = None; - } + pub(crate) fn remove_space_view(&self, space_view_id: &SpaceViewId, ctx: &ViewerContext<'_>) { + self.mark_user_interaction(ctx); - if let Some(tile_id) = tree.tiles.find_pane(space_view_id) { - tree.tiles.remove(tile_id); + let timepoint = TimePoint::timeless(); + let mut deltas = vec![]; + + if self.maximized == Some(*space_view_id) { + let component = SpaceViewMaximized(None); + add_delta_from_single_component( + &mut deltas, + &VIEWPORT_PATH.into(), + &timepoint, + component, + ); } - space_views.remove(space_view_id) + let component = IncludedSpaceViews( + self.space_views + .keys() + .filter(|id| id != &space_view_id) + .map(|id| (*id).into()) + .collect(), + ); + add_delta_from_single_component(&mut deltas, &VIEWPORT_PATH.into(), &timepoint, component); + + clear_space_view(&mut deltas, space_view_id); + + ctx.command_sender + .send_system(SystemCommand::UpdateBlueprint( + ctx.store_context.blueprint.store_id().clone(), + deltas, + )); } /// If `false`, the item is referring to data that is not present in this blueprint. @@ -184,16 +156,23 @@ impl<'a> ViewportBlueprint<'a> { } } - pub fn mark_user_interaction(&mut self) { + pub fn mark_user_interaction(&self, ctx: &ViewerContext<'_>) { if self.auto_layout { re_log::trace!("User edits - will no longer auto-layout"); } - self.auto_layout = false; - self.auto_space_views = false; + let component = AutoLayout(false); + save_single_component(&VIEWPORT_PATH.into(), component, ctx); + + let component = AutoSpaceViews(false); + save_single_component(&VIEWPORT_PATH.into(), component, ctx); } - pub fn add_space_view(&mut self, mut space_view: SpaceViewBlueprint) -> SpaceViewId { + pub fn add_space_view( + &self, + mut space_view: SpaceViewBlueprint, + ctx: &ViewerContext<'_>, + ) -> SpaceViewId { let space_view_id = space_view.id; // Find a unique name for the space view @@ -213,31 +192,20 @@ impl<'a> ViewportBlueprint<'a> { space_view.display_name = unique_name; - self.space_views.insert(space_view_id, space_view); + // Save the space view to the store + space_view.save_full(ctx); - if self.auto_layout { - // Re-run the auto-layout next frame: - re_log::trace!("Added a space view with no user edits yet - will re-run auto-layout"); - self.tree = egui_tiles::Tree::empty("viewport_tree"); - } else { - // Try to insert it in the tree, in the top level: - if let Some(root_id) = self.tree.root { - let tile_id = self.tree.tiles.insert_pane(space_view_id); - if let Some(egui_tiles::Tile::Container(container)) = - self.tree.tiles.get_mut(root_id) - { - re_log::trace!("Inserting new space view into root container"); - container.add_child(tile_id); - } else { - re_log::trace!("Root was not a container - will re-run auto-layout"); - self.tree = egui_tiles::Tree::empty("viewport_tree"); - } - } else { - re_log::trace!("No root found - will re-run auto-layout"); - } - } + // Update the space-view ids: + let component = IncludedSpaceViews( + self.space_views + .keys() + .map(|id| (*id).into()) + .chain(std::iter::once(space_view_id.into())) + .collect(), + ); + save_single_component(&VIEWPORT_PATH.into(), component, ctx); - self.deferred_tree_actions.focus_tab = Some(space_view_id); + self.deferred_tree_actions.lock().create = Some(space_view_id); space_view_id } @@ -265,79 +233,26 @@ impl<'a> ViewportBlueprint<'a> { .collect() } - /// Compares the before and after snapshots and sends any necessary deltas to the store. - pub fn sync_viewport_blueprint( - before: &ViewportBlueprint<'_>, - after: &ViewportBlueprint<'_>, - command_sender: &CommandSender, - ) { - let mut deltas = vec![]; - - let entity_path = EntityPath::from(VIEWPORT_PATH); - - // TODO(jleibs): Seq instead of timeless? - let timepoint = TimePoint::timeless(); - - if after.space_views.len() != before.space_views.len() - || after - .space_views - .keys() - .zip(before.space_views.keys()) - .any(|(a, b)| a != b) - { - let component = - IncludedSpaceViews(after.space_views.keys().map(|id| (*id).into()).collect()); - add_delta_from_single_component(&mut deltas, &entity_path, &timepoint, component); - } - - if after.auto_layout != before.auto_layout { - let component = AutoLayout(after.auto_layout); - add_delta_from_single_component(&mut deltas, &entity_path, &timepoint, component); - } - - if after.auto_space_views != before.auto_space_views { - let component = AutoSpaceViews(after.auto_space_views); - add_delta_from_single_component(&mut deltas, &entity_path, &timepoint, component); - } - - if after.maximized != before.maximized { - let component = SpaceViewMaximized(after.maximized.map(|id| id.into())); - add_delta_from_single_component(&mut deltas, &entity_path, &timepoint, component); - } - - if after.tree != before.tree { - re_log::trace!("Syncing tree"); - - let component = ViewportLayout(after.tree.clone()); - - add_delta_from_single_component(&mut deltas, &entity_path, &timepoint, component); - } - - // Add any new or modified space views - for id in after.space_view_ids() { - if let Some(space_view) = after.space_view(id) { - sync_space_view(&mut deltas, space_view, before.space_view(id)); - } - } + pub fn set_auto_layout(value: bool, ctx: &ViewerContext<'_>) { + let component = AutoLayout(value); + save_single_component(&VIEWPORT_PATH.into(), component, ctx); + } - // Remove any deleted space views - for space_view_id in before.space_view_ids() { - if after.space_view(space_view_id).is_none() { - clear_space_view(&mut deltas, space_view_id); - } - } + pub fn set_maximized(space_view_id: Option, ctx: &ViewerContext<'_>) { + let component = SpaceViewMaximized(space_view_id.map(|id| id.into())); + save_single_component(&VIEWPORT_PATH.into(), component, ctx); + } - command_sender.send_system(SystemCommand::UpdateBlueprint( - after.blueprint_db.store_id().clone(), - deltas, - )); + pub fn set_tree(tree: egui_tiles::Tree, ctx: &ViewerContext<'_>) { + let component = ViewportLayout(tree); + save_single_component(&VIEWPORT_PATH.into(), component, ctx); } } // ---------------------------------------------------------------------------- // TODO(jleibs): Move this helper to a better location -fn add_delta_from_single_component<'a, C>( +pub fn add_delta_from_single_component<'a, C>( deltas: &mut Vec, entity_path: &EntityPath, timepoint: &TimePoint, @@ -358,9 +273,33 @@ fn add_delta_from_single_component<'a, C>( deltas.push(row); } +// TODO(jleibs): Move this helper to a better location +pub fn save_single_component<'a, C>(entity_path: &EntityPath, component: C, ctx: &ViewerContext<'_>) +where + C: re_types::Component + Clone + 'a, + std::borrow::Cow<'a, C>: std::convert::From, +{ + let timepoint = TimePoint::timeless(); + + let row = DataRow::from_cells1_sized( + RowId::new(), + entity_path.clone(), + timepoint.clone(), + 1, + [component], + ) + .unwrap(); // TODO(emilk): statically check that the component is a mono-component - then this cannot fail! + + ctx.command_sender + .send_system(SystemCommand::UpdateBlueprint( + ctx.store_context.blueprint.store_id().clone(), + vec![row], + )); +} + // ---------------------------------------------------------------------------- -pub fn load_viewport_blueprint(blueprint_db: &re_data_store::StoreDb) -> ViewportBlueprint<'_> { +pub fn load_viewport_blueprint(blueprint_db: &re_data_store::StoreDb) -> ViewportBlueprint { re_tracing::profile_function!(); let query = LatestAtQuery::latest(Timeline::default()); @@ -420,7 +359,6 @@ pub fn load_viewport_blueprint(blueprint_db: &re_data_store::StoreDb) -> Viewpor .0; ViewportBlueprint { - blueprint_db, space_views, tree, maximized, @@ -453,47 +391,6 @@ pub fn load_viewport_blueprint(blueprint_db: &re_data_store::StoreDb) -> Viewpor // ---------------------------------------------------------------------------- -pub fn sync_space_view( - deltas: &mut Vec, - space_view: &SpaceViewBlueprint, - snapshot: Option<&SpaceViewBlueprint>, -) { - if snapshot.map_or(true, |snapshot| space_view.has_edits(snapshot)) { - // TODO(jleibs): Seq instead of timeless? - let timepoint = TimePoint::timeless(); - - let arch = re_types::blueprint::archetypes::SpaceViewBlueprint::new( - space_view.class_identifier().as_str(), - ) - .with_display_name(space_view.display_name.clone()) - .with_space_origin(&space_view.space_origin) - .with_entities_determined_by_user(space_view.entities_determined_by_user) - .with_contents(space_view.queries.iter().map(|q| q.id)); - - if let Ok(row) = DataRow::from_archetype( - RowId::new(), - timepoint.clone(), - space_view.entity_path(), - &arch, - ) { - deltas.push(row); - } - - // The only time we need to create a query is if this is a new space-view. All other edits - // happen directly via `UpdateBlueprint` commands. - if snapshot.is_none() { - for query in &space_view.queries { - add_delta_from_single_component( - deltas, - &query.id.as_entity_path(), - &timepoint, - query.expressions.clone(), - ); - } - } - } -} - pub fn clear_space_view(deltas: &mut Vec, space_view_id: &SpaceViewId) { // TODO(jleibs): Seq instead of timeless? let timepoint = TimePoint::timeless(); diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 98e8a46ddbcf4..dad9aadfff05d 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -13,30 +13,63 @@ use re_viewer_context::{ }; use crate::{ - space_view_heuristics::all_possible_space_views, viewport_blueprint::TreeActions, - SpaceInfoCollection, SpaceViewBlueprint, ViewportBlueprint, + space_view_heuristics::all_possible_space_views, SpaceInfoCollection, SpaceViewBlueprint, + ViewportBlueprint, }; -impl ViewportBlueprint<'_> { +impl ViewportBlueprint { /// Show the blueprint panel tree view. - pub fn tree_ui(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { + pub fn tree_ui(&self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { re_tracing::profile_function!(); + let mut tree = self.tree.clone(); + egui::ScrollArea::both() .id_source("blueprint_tree_scroll_area") .auto_shrink([true, false]) .show(ui, |ui| { ctx.re_ui.panel_content(ui, |_, ui| { if let Some(root) = self.tree.root() { - self.tile_ui(ctx, ui, root); + self.tile_ui(&mut tree, ctx, ui, root); } }); }); - let TreeActions { focus_tab, remove } = std::mem::take(&mut self.deferred_tree_actions); + // At the end of the Tree-UI, we can safely apply deferred actions. + + let mut deferred = self.deferred_tree_actions.lock(); + + let mut reset = std::mem::take(&mut deferred.reset); + let create = std::mem::take(&mut deferred.create); + let mut focus_tab = std::mem::take(&mut deferred.focus_tab); + let remove = std::mem::take(&mut deferred.remove); + + if let Some(create) = &create { + if self.auto_layout { + // Re-run the auto-layout next frame: + re_log::trace!( + "Added a space view with no user edits yet - will re-run auto-layout" + ); + + reset = true; + } else if let Some(root_id) = tree.root { + let tile_id = tree.tiles.insert_pane(*create); + if let Some(egui_tiles::Tile::Container(container)) = tree.tiles.get_mut(root_id) { + re_log::trace!("Inserting new space view into root container"); + container.add_child(tile_id); + } else { + re_log::trace!("Root was not a container - will re-run auto-layout"); + reset = true; + } + } else { + re_log::trace!("No root found - will re-run auto-layout"); + } + + focus_tab = Some(*create); + } if let Some(focus_tab) = &focus_tab { - let found = self.tree.make_active(|tile| match tile { + let found = tree.make_active(|tile| match tile { egui_tiles::Tile::Pane(space_view_id) => space_view_id == focus_tab, egui_tiles::Tile::Container(_) => false, }); @@ -44,16 +77,28 @@ impl ViewportBlueprint<'_> { } for tile_id in remove { - for tile in self.tree.tiles.remove_recursively(tile_id) { + for tile in tree.tiles.remove_recursively(tile_id) { if let egui_tiles::Tile::Pane(space_view_id) = tile { - self.remove(&space_view_id); + tree.tiles.remove(tile_id); + self.remove_space_view(&space_view_id, ctx); } } if Some(tile_id) == self.tree.root { - self.tree.root = None; + tree.root = None; } } + + if reset { + re_log::trace!("Resetting viewport tree"); + tree = super::auto_layout::tree_from_space_views( + ctx.space_view_class_registry, + &self.space_views, + ); + Self::set_tree(tree, ctx); + } else if tree != self.tree { + Self::set_tree(tree, ctx); + } } /// If a group or spaceview has a total of this number of elements, show its subtree by default? @@ -62,27 +107,34 @@ impl ViewportBlueprint<'_> { 2 <= num_children && num_children <= 3 } - fn tile_ui(&mut self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, tile_id: egui_tiles::TileId) { + fn tile_ui( + &self, + tree: &mut egui_tiles::Tree, + ctx: &ViewerContext<'_>, + ui: &mut egui::Ui, + tile_id: egui_tiles::TileId, + ) { // Temporarily remove the tile so we don't get borrow-checker fights: - let Some(mut tile) = self.tree.tiles.remove(tile_id) else { + let Some(mut tile) = tree.tiles.remove(tile_id) else { return; }; match &mut tile { egui_tiles::Tile::Container(container) => { - self.container_tree_ui(ctx, ui, tile_id, container); + self.container_tree_ui(tree, ctx, ui, tile_id, container); } egui_tiles::Tile::Pane(space_view_id) => { // A space view - self.space_view_entry_ui(ctx, ui, tile_id, space_view_id); + self.space_view_entry_ui(tree, ctx, ui, tile_id, space_view_id); } }; - self.tree.tiles.insert(tile_id, tile); + tree.tiles.insert(tile_id, tile); } fn container_tree_ui( - &mut self, + &self, + tree: &mut egui_tiles::Tree, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, tile_id: egui_tiles::TileId, @@ -93,8 +145,8 @@ impl ViewportBlueprint<'_> { // This means we won't be showing the visibility button of the parent container, // so if the child is made invisible, we should do the same for the parent. let child_is_visible = self.tree.is_visible(child_id); - self.tree.set_visible(tile_id, child_is_visible); - return self.tile_ui(ctx, ui, child_id); + tree.set_visible(tile_id, child_is_visible); + return self.tile_ui(tree, ctx, ui, child_id); } let item = Item::Container(tile_id); @@ -119,7 +171,7 @@ impl ViewportBlueprint<'_> { }) .show_collapsing(ui, ui.id().with(tile_id), default_open, |_, ui| { for &child in container.children() { - self.tile_ui(ctx, ui, child); + self.tile_ui(tree, ctx, ui, child); } }) .item_response; @@ -127,8 +179,8 @@ impl ViewportBlueprint<'_> { item_ui::select_hovered_on_click(ctx, &response, &[item]); if remove { - self.mark_user_interaction(); - self.deferred_tree_actions.remove.push(tile_id); + self.mark_user_interaction(ctx); + self.deferred_tree_actions.lock().remove.push(tile_id); } if visibility_changed { @@ -136,21 +188,24 @@ impl ViewportBlueprint<'_> { re_log::trace!("Container visibility changed - will no longer auto-layout"); } - self.auto_layout = false; // Keep `auto_space_views` enabled. - self.tree.set_visible(tile_id, visible); + // Keep `auto_space_views` enabled. + Self::set_auto_layout(false, ctx); + + tree.set_visible(tile_id, visible); } } fn space_view_entry_ui( - &mut self, + &self, + tree: &mut egui_tiles::Tree, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, tile_id: egui_tiles::TileId, space_view_id: &SpaceViewId, ) { - let Some(space_view) = self.space_views.get_mut(space_view_id) else { + let Some(space_view) = self.space_views.get(space_view_id) else { re_log::warn_once!("Bug: asked to show a ui for a Space View that doesn't exist"); - self.deferred_tree_actions.remove.push(tile_id); + self.deferred_tree_actions.lock().remove.push(tile_id); return; }; debug_assert_eq!(space_view.id, *space_view_id); @@ -185,7 +240,7 @@ impl ViewportBlueprint<'_> { let response = remove_button_ui(re_ui, ui, "Remove Space View from the Viewport"); if response.clicked() { - self.deferred_tree_actions.remove.push(tile_id); + self.deferred_tree_actions.lock().remove.push(tile_id); } response | vis_response @@ -211,7 +266,7 @@ impl ViewportBlueprint<'_> { .on_hover_text("Space View"); if response.clicked() { - self.deferred_tree_actions.focus_tab = Some(space_view.id); + self.deferred_tree_actions.lock().focus_tab = Some(space_view.id); } item_ui::select_hovered_on_click(ctx, &response, &[item]); @@ -221,8 +276,10 @@ impl ViewportBlueprint<'_> { re_log::trace!("Space view visibility changed - will no longer auto-layout"); } - self.auto_layout = false; // Keep `auto_space_views` enabled. - self.tree.set_visible(tile_id, visible); + // Keep `auto_space_views` enabled. + Self::set_auto_layout(false, ctx); + + tree.set_visible(tile_id, visible); } } @@ -231,7 +288,7 @@ impl ViewportBlueprint<'_> { ui: &mut egui::Ui, query_result: &DataQueryResult, result_handle: DataResultHandle, - space_view: &mut SpaceViewBlueprint, + space_view: &SpaceViewBlueprint, space_view_visible: bool, ) { let Some(top_node) = query_result.tree.lookup_node(result_handle) else { @@ -391,7 +448,7 @@ impl ViewportBlueprint<'_> { } pub fn add_new_spaceview_button_ui( - &mut self, + &self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui, spaces_info: &SpaceInfoCollection, @@ -403,27 +460,26 @@ impl ViewportBlueprint<'_> { |ui| { ui.style_mut().wrap = Some(false); - let mut add_space_view_item = - |ui: &mut egui::Ui, space_view: SpaceViewBlueprint| { - if ctx - .re_ui - .selectable_label_with_icon( - ui, - space_view.class(ctx.space_view_class_registry).icon(), - if space_view.space_origin.is_root() { - space_view.display_name.clone() - } else { - space_view.space_origin.to_string() - }, - false, - ) - .clicked() - { - ui.close_menu(); - let new_space_view_id = self.add_space_view(space_view); - ctx.set_single_selection(&Item::SpaceView(new_space_view_id)); - } - }; + let add_space_view_item = |ui: &mut egui::Ui, space_view: SpaceViewBlueprint| { + if ctx + .re_ui + .selectable_label_with_icon( + ui, + space_view.class(ctx.space_view_class_registry).icon(), + if space_view.space_origin.is_root() { + space_view.display_name.clone() + } else { + space_view.space_origin.to_string() + }, + false, + ) + .clicked() + { + ui.close_menu(); + let new_space_view_id = self.add_space_view(space_view, ctx); + ctx.set_single_selection(&Item::SpaceView(new_space_view_id)); + } + }; // Space view options proposed by heuristics let mut possible_space_views =