Split SpaceView -> SpaceViewState + SpaceViewBlueprint (#2188)

Explicitly separate SpaceViewState, which is part of the Viewport state and persists frame-to-frame from the SpaceViewBlueprint which otherwise controls its behavior. This is the first phase of: - #2089 ### 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) * [ ] I've included a screenshot or gif (if applicable)  PR Build Summary: https://build.rerun.io/pr/2188 --------- Co-authored-by: Andreas Reich <[email protected]>
rerun-io · May 29, 2023 · 9a4597a · 9a4597a
1 parent ee32067
commit 9a4597a
Show file tree

Hide file tree

Showing 20 changed files with 198 additions and 211 deletions.
diff --git a/crates/re_viewer/src/blueprint_components/space_view.rs b/crates/re_viewer/src/blueprint_components/space_view.rs
@@ -2,9 +2,9 @@ use arrow2_convert::{ArrowDeserialize, ArrowField, ArrowSerialize};
 use re_data_store::ComponentName;
 use re_log_types::{serde_field::SerdeField, Component};
 
-use crate::ui::SpaceView;
+use crate::ui::SpaceViewBlueprint;
 
-/// A [`SpaceView`]
+/// A [`SpaceViewBlueprint`]
 ///
 /// ## Example
 /// ```
@@ -20,8 +20,8 @@ use crate::ui::SpaceView;
 /// ```
 #[derive(Clone, PartialEq, ArrowField, ArrowSerialize, ArrowDeserialize)]
 pub struct SpaceViewComponent {
-    #[arrow_field(type = "SerdeField<SpaceView>")]
-    pub space_view: SpaceView,
+    #[arrow_field(type = "SerdeField<SpaceViewBlueprint>")]
+    pub space_view: SpaceViewBlueprint,
 }
 
 impl SpaceViewComponent {
@@ -46,7 +46,8 @@ impl std::fmt::Debug for SpaceViewComponent {
 fn test_spaceview() {
     use crate::ui::ViewCategory;
     use arrow2_convert::{deserialize::TryIntoCollection, serialize::TryIntoArrow};
-    let space_view = SpaceView::new(ViewCategory::Spatial, &"foo".into(), &["foo/bar".into()]);
+    let space_view =
+        SpaceViewBlueprint::new(ViewCategory::Spatial, &"foo".into(), &["foo/bar".into()]);
 
     let data = [SpaceViewComponent { space_view }];
     let array: Box<dyn arrow2::array::Array> = data.try_into_arrow().unwrap();

diff --git a/crates/re_viewer/src/lib.rs b/crates/re_viewer/src/lib.rs
@@ -17,7 +17,7 @@ use re_log_types::PythonVersion;
 pub(crate) use ui::{memory_panel, selection_panel};
 
 // TODO(jleibs): Do we want to expose this
-pub use ui::{SpaceView, ViewCategory};
+pub use ui::{SpaceViewBlueprint, ViewCategory};
 
 pub use app::{App, StartupOptions};
 pub use remote_viewer_app::RemoteViewerApp;

diff --git a/crates/re_viewer/src/misc/space_view_highlights.rs b/crates/re_viewer/src/misc/space_view_highlights.rs
@@ -9,7 +9,7 @@ use re_viewer_context::{
     HoverHighlight, InteractionHighlight, Item, SelectionHighlight, SelectionState, SpaceViewId,
 };
 
-use crate::ui::SpaceView;
+use crate::ui::SpaceViewBlueprint;
 
 /// Highlights of a specific entity path in a specific space view.
 ///
@@ -90,7 +90,7 @@ impl SpaceViewHighlights {
 pub fn highlights_for_space_view(
     selection_state: &SelectionState,
     space_view_id: SpaceViewId,
-    space_views: &HashMap<SpaceViewId, SpaceView>,
+    space_views: &HashMap<SpaceViewId, SpaceViewBlueprint>,
 ) -> SpaceViewHighlights {
     crate::profile_function!();
 

diff --git a/crates/re_viewer/src/ui/auto_layout.rs b/crates/re_viewer/src/ui/auto_layout.rs
@@ -18,7 +18,10 @@ use itertools::Itertools as _;
 use re_data_store::{EntityPath, EntityPathPart};
 use re_viewer_context::SpaceViewId;
 
-use super::{space_view::SpaceView, view_category::ViewCategory};
+use super::{
+    space_view::{SpaceViewBlueprint, SpaceViewState},
+    view_category::ViewCategory,
+};
 
 #[derive(Clone, Debug)]
 pub struct SpaceMakeInfo {
@@ -47,7 +50,8 @@ enum SplitDirection {
 pub(crate) fn tree_from_space_views(
     viewport_size: egui::Vec2,
     visible: &std::collections::BTreeSet<SpaceViewId>,
-    space_views: &HashMap<SpaceViewId, SpaceView>,
+    space_views: &HashMap<SpaceViewId, SpaceViewBlueprint>,
+    space_view_states: &HashMap<SpaceViewId, SpaceViewState>,
 ) -> egui_tiles::Tree<SpaceViewId> {
     let mut space_make_infos = space_views
         .iter()
@@ -63,15 +67,19 @@ pub(crate) fn tree_from_space_views(
         .map(|(space_view_id, space_view)| {
             let aspect_ratio = match space_view.category {
                 ViewCategory::Spatial => {
-                    let state_spatial = &space_view.view_state.state_spatial;
-                    match *state_spatial.nav_mode.get() {
-                        // This is the only thing where the aspect ratio makes complete sense.
-                        super::view_spatial::SpatialNavigationMode::TwoD => {
-                            let size = state_spatial.scene_bbox_accum.size();
-                            Some(size.x / size.y)
+                    if let Some(space_view_state) = space_view_states.get(space_view_id) {
+                        let state_spatial = &space_view_state.state_spatial;
+                        match *state_spatial.nav_mode.get() {
+                            // This is the only thing where the aspect ratio makes complete sense.
+                            super::view_spatial::SpatialNavigationMode::TwoD => {
+                                let size = state_spatial.scene_bbox_accum.size();
+                                Some(size.x / size.y)
+                            }
+                            // 3D scenes can be pretty flexible
+                            super::view_spatial::SpatialNavigationMode::ThreeD => None,
                         }
-                        // 3D scenes can be pretty flexible
-                        super::view_spatial::SpatialNavigationMode::ThreeD => None,
+                    } else {
+                        None
                     }
                 }
                 ViewCategory::TextBox | ViewCategory::Tensor | ViewCategory::TimeSeries => {

diff --git a/crates/re_viewer/src/ui/blueprint_load.rs b/crates/re_viewer/src/ui/blueprint_load.rs
@@ -11,19 +11,20 @@ use crate::blueprint_components::{
     },
 };
 
-use super::{Blueprint, SpaceView, Viewport};
+use super::{Blueprint, SpaceViewBlueprint, Viewport};
 
 impl Blueprint {
     pub fn from_db(egui_ctx: &egui::Context, blueprint_db: &re_data_store::LogDb) -> Self {
         let mut ret = Self::new(egui_ctx);
 
-        let space_views: HashMap<SpaceViewId, SpaceView> = if let Some(space_views) = blueprint_db
-            .entity_db
-            .tree
-            .children
-            .get(&re_data_store::EntityPathPart::Name(
-                SpaceViewComponent::SPACEVIEW_PREFIX.into(),
-            )) {
+        let space_views: HashMap<SpaceViewId, SpaceViewBlueprint> = if let Some(space_views) =
+            blueprint_db
+                .entity_db
+                .tree
+                .children
+                .get(&re_data_store::EntityPathPart::Name(
+                    SpaceViewComponent::SPACEVIEW_PREFIX.into(),
+                )) {
             space_views
                 .children
                 .values()
@@ -61,14 +62,17 @@ fn load_panel_state(path: &EntityPath, blueprint_db: &re_data_store::LogDb) -> O
         .map(|p| p.expanded)
 }
 
-fn load_space_view(path: &EntityPath, blueprint_db: &re_data_store::LogDb) -> Option<SpaceView> {
+fn load_space_view(
+    path: &EntityPath,
+    blueprint_db: &re_data_store::LogDb,
+) -> Option<SpaceViewBlueprint> {
     query_timeless_single::<SpaceViewComponent>(&blueprint_db.entity_db.data_store, path)
         .map(|c| c.space_view)
 }
 
 fn load_viewport(
     blueprint_db: &re_data_store::LogDb,
-    space_views: HashMap<SpaceViewId, SpaceView>,
+    space_views: HashMap<SpaceViewId, SpaceViewBlueprint>,
 ) -> Viewport {
     let auto_space_views = query_timeless_single::<AutoSpaceViews>(
         &blueprint_db.entity_db.data_store,

diff --git a/crates/re_viewer/src/ui/blueprint_sync.rs b/crates/re_viewer/src/ui/blueprint_sync.rs
@@ -11,7 +11,7 @@ use crate::blueprint_components::{
     },
 };
 
-use super::{Blueprint, SpaceView, Viewport};
+use super::{Blueprint, SpaceViewBlueprint, Viewport};
 
 // Resolving and applying updates
 impl Blueprint {
@@ -73,8 +73,8 @@ pub fn sync_panel_expanded(
 
 pub fn sync_space_view(
     blueprint_db: &mut re_data_store::LogDb,
-    space_view: &SpaceView,
-    snapshot: Option<&SpaceView>,
+    space_view: &SpaceViewBlueprint,
+    snapshot: Option<&SpaceViewBlueprint>,
 ) {
     if Some(space_view) != snapshot {
         let entity_path = EntityPath::from(format!(

diff --git a/crates/re_viewer/src/ui/mod.rs b/crates/re_viewer/src/ui/mod.rs
@@ -25,7 +25,7 @@ pub mod view_spatial;
 
 pub(crate) use self::blueprint::Blueprint;
 // TODO(jleibs) should we avoid leaking this?
-pub use self::space_view::{item_ui, SpaceView};
+pub use self::space_view::{item_ui, SpaceViewBlueprint};
 
 pub use self::view_category::ViewCategory;
 pub use self::viewport::{Viewport, ViewportState, VisibilitySet};
diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs
@@ -11,7 +11,7 @@ use re_viewer_context::{Item, SpaceViewId, UiVerbosity, ViewerContext};
 use crate::ui::Blueprint;
 
 use super::{
-    selection_history_ui::SelectionHistoryUi, space_view::ViewState,
+    selection_history_ui::SelectionHistoryUi, space_view::SpaceViewState,
     view_spatial::SpatialNavigationMode, Viewport, ViewportState,
 };
 
@@ -259,37 +259,48 @@ fn blueprint_ui(
             ui.add_space(ui.spacing().item_spacing.y);
 
             if let Some(space_view) = blueprint.viewport.space_view_mut(space_view_id) {
-                space_view.selection_ui(ctx, ui);
+                // TODO(jleibs): Is this the right place to create default state?
+                let space_view_state = viewport_state
+                    .space_view_states
+                    .entry(*space_view_id)
+                    .or_default();
+
+                space_view.selection_ui(space_view_state, ctx, ui);
             }
         }
 
         Item::InstancePath(space_view_id, instance_path) => {
-            if let Some(space_view) = space_view_id
-                .and_then(|space_view_id| blueprint.viewport.space_view_mut(&space_view_id))
-            {
-                if instance_path.instance_key.is_specific() {
-                    ui.horizontal(|ui| {
-                        ui.label("Part of");
-                        item_ui::entity_path_button(
+            if let Some(space_view_id) = space_view_id {
+                if let Some(space_view) = blueprint.viewport.space_view_mut(space_view_id) {
+                    if instance_path.instance_key.is_specific() {
+                        ui.horizontal(|ui| {
+                            ui.label("Part of");
+                            item_ui::entity_path_button(
+                                ctx,
+                                ui,
+                                Some(*space_view_id),
+                                &instance_path.entity_path,
+                            );
+                        });
+                        // TODO(emilk): show the values of this specific instance (e.g. point in the point cloud)!
+                    } else {
+                        let space_view_state = viewport_state
+                            .space_view_states
+                            .entry(*space_view_id)
+                            .or_default();
+
+                        // splat - the whole entity
+                        let data_blueprint = space_view.data_blueprint.data_blueprints_individual();
+                        let mut props = data_blueprint.get(&instance_path.entity_path);
+                        entity_props_ui(
                             ctx,
                             ui,
-                            *space_view_id,
-                            &instance_path.entity_path,
+                            Some(&instance_path.entity_path),
+                            &mut props,
+                            space_view_state,
                         );
-                    });
-                    // TODO(emilk): show the values of this specific instance (e.g. point in the point cloud)!
-                } else {
-                    // splat - the whole entity
-                    let data_blueprint = space_view.data_blueprint.data_blueprints_individual();
-                    let mut props = data_blueprint.get(&instance_path.entity_path);
-                    entity_props_ui(
-                        ctx,
-                        ui,
-                        Some(&instance_path.entity_path),
-                        &mut props,
-                        &space_view.view_state,
-                    );
-                    data_blueprint.set(instance_path.entity_path.clone(), props);
+                        data_blueprint.set(instance_path.entity_path.clone(), props);
+                    }
                 }
             } else {
                 list_existing_data_blueprints(ui, ctx, &instance_path.entity_path, blueprint);
@@ -302,12 +313,17 @@ fn blueprint_ui(
                     .data_blueprint
                     .group_mut(*data_blueprint_group_handle)
                 {
+                    let space_view_state = viewport_state
+                        .space_view_states
+                        .entry(*space_view_id)
+                        .or_default();
+
                     entity_props_ui(
                         ctx,
                         ui,
                         None,
                         &mut group.properties_individual,
-                        &space_view.view_state,
+                        space_view_state,
                     );
                 } else {
                     ctx.selection_state_mut().clear_current();
@@ -354,7 +370,7 @@ fn entity_props_ui(
     ui: &mut egui::Ui,
     entity_path: Option<&EntityPath>,
     entity_props: &mut EntityProperties,
-    view_state: &ViewState,
+    view_state: &SpaceViewState,
 ) {
     ui.checkbox(&mut entity_props.visible, "Visible");
     ui.checkbox(&mut entity_props.interactive, "Interactive")