Make the new container blueprints the default behavior (#4642)

### What - Instead of opting in, users can now opt out using the app option: `legacy_container_blueprint` - Fix some egui-tiles warnings (these were present even with the legacy behavior). ### 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 newly built examples: [app.rerun.io](https://app.rerun.io/pr/4642/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/4642/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [app.rerun.io](https://app.rerun.io/pr/4642/index.html?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 - [PR Build Summary](https://build.rerun.io/pr/4642) - [Docs preview](https://rerun.io/preview/5a4de6505dfef6a8567d99c4b03de6e54e798735/docs)  - [Examples preview](https://rerun.io/preview/5a4de6505dfef6a8567d99c4b03de6e54e798735/examples)  - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
rerun-io · Jan 3, 2024 · da88fdd · da88fdd
1 parent 0888f20
commit da88fdd
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 87 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -281,4 +281,4 @@ debug = true
 # egui-wgpu = { path = "../../egui/crates/egui-wgpu" }
 # emath = { path = "../../egui/crates/emath" }
 
-egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "dcdcde10b493683f3e97db47d128683bec0e65b9" } # https://github.com/rerun-io/egui_tiles/pull/40
+egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "b6e4fd457b2eee2c671747ead12f4a20feb380e8" } # Merge of: https://github.com/rerun-io/egui_tiles/pull/41
diff --git a/crates/re_viewer/src/ui/rerun_menu.rs b/crates/re_viewer/src/ui/rerun_menu.rs
@@ -238,6 +238,14 @@ impl App {
                 .on_hover_text("Show an entity filter DSL when selecting a space-view.");
         }
 
+        self.re_ui
+            .checkbox(
+                ui,
+                &mut self.state.app_options.legacy_container_blueprint,
+                "Use the legacy container blueprint storage for the viewport",
+            )
+            .on_hover_text("The legacy container blueprint storage is deprecated, but may be helpful if unexpected regressions are found in the new container blueprints.");
+
         #[cfg(debug_assertions)]
         {
             ui.separator();
@@ -375,14 +383,6 @@ impl App {
             "Show Blueprint in the Time Panel",
         ).on_hover_text("Show the Blueprint data in the Time Panel tree view. This is useful for debugging the internal blueprint state.");
 
-        self.re_ui
-            .checkbox(
-                ui,
-                &mut self.state.app_options.experimental_container_blueprints,
-                "Use experimental container blueprints",
-            )
-            .on_hover_text("Load and save the container state using new container archetypes");
-
         ui.menu_button("Crash", |ui| {
             #[allow(clippy::manual_assert)]
             if ui.button("panic!").clicked() {

diff --git a/crates/re_viewer_context/src/app_options.rs b/crates/re_viewer_context/src/app_options.rs
@@ -17,8 +17,8 @@ pub struct AppOptions {
     /// Enable experimental dataframe space views.
     pub experimental_dataframe_space_view: bool,
 
-    /// Enable experimental support for new container blueprints
-    pub experimental_container_blueprints: bool,
+    /// Use the legacy container blueprint storage for the space view.
+    pub legacy_container_blueprint: bool,
 
     pub experimental_entity_filter_editor: bool,
 
@@ -45,7 +45,7 @@ impl Default for AppOptions {
 
             experimental_dataframe_space_view: false,
 
-            experimental_container_blueprints: cfg!(debug_assertions),
+            legacy_container_blueprint: false,
 
             experimental_entity_filter_editor: false,
 

diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs
@@ -91,8 +91,8 @@ pub enum TreeAction {
     /// Remove a tile and all its children.
     Remove(egui_tiles::TileId),
 
-    /// Simplify the specified subtree with the provided options
-    SimplifyTree(egui_tiles::TileId, egui_tiles::SimplificationOptions),
+    /// Simplify the container with the provided options
+    SimplifyContainer(egui_tiles::TileId, egui_tiles::SimplificationOptions),
 }
 
 // ----------------------------------------------------------------------------
@@ -380,7 +380,7 @@ impl<'a, 'b> Viewport<'a, 'b> {
                     self.edited = true;
                 }
                 TreeAction::Remove(tile_id) => {
-                    for tile in self.tree.tiles.remove_recursively(tile_id) {
+                    for tile in self.tree.remove_recursively(tile_id) {
                         re_log::trace!("Removing tile {tile_id:?}");
                         if let egui_tiles::Tile::Pane(space_view_id) = tile {
                             re_log::trace!("Removing space view {space_view_id}");
@@ -394,9 +394,9 @@ impl<'a, 'b> Viewport<'a, 'b> {
                     }
                     self.edited = true;
                 }
-                TreeAction::SimplifyTree(tile_id, options) => {
+                TreeAction::SimplifyContainer(tile_id, options) => {
                     re_log::trace!("Simplifying tree with options: {options:?}");
-                    self.tree.simplify_tile(tile_id, &options);
+                    self.tree.simplify_children_of_tile(tile_id, &options);
                     self.edited = true;
                 }
             }
@@ -412,24 +412,22 @@ impl<'a, 'b> Viewport<'a, 'b> {
 
         // Finally, save any edits to the blueprint tree
         // This is a no-op if the tree hasn't changed.
-        if ctx.app_options.experimental_container_blueprints {
-            if self.edited {
-                // TODO(abey79): Decide what simplification to do here. Some of this
-                // might need to get rolled into the save logic instead.
-
-                // Simplify before we save the tree. Normally additional simplification will
-                // happen on the next render loop, but that's too late -- unsimplified
-                // changes will be baked into the tree.
-                let options = egui_tiles::SimplificationOptions {
-                    all_panes_must_have_tabs: true,
-                    ..Default::default()
-                };
-                self.tree.simplify(&options);
-
-                self.blueprint.save_tree_as_containers(&self.tree, ctx);
-            }
-        } else {
+        if ctx.app_options.legacy_container_blueprint {
             self.blueprint.set_tree(&self.tree, ctx);
+        } else if self.edited {
+            // TODO(abey79): Decide what simplification to do here. Some of this
+            // might need to get rolled into the save logic instead.
+
+            // Simplify before we save the tree. Normally additional simplification will
+            // happen on the next render loop, but that's too late -- unsimplified
+            // changes will be baked into the tree.
+            let options = egui_tiles::SimplificationOptions {
+                all_panes_must_have_tabs: true,
+                ..Default::default()
+            };
+            self.tree.simplify(&options);
+
+            self.blueprint.save_tree_as_containers(&self.tree, ctx);
         }
     }
 

diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs
@@ -132,14 +132,22 @@ impl ViewportBlueprint {
 
         let maximized = arch.maximized.and_then(|id| id.0.map(|id| id.into()));
 
-        let tree = blueprint_db
-            .store()
-            .query_timeless_component_quiet::<ViewportLayout>(&VIEWPORT_PATH.into())
-            .map(|space_view| space_view.value)
-            .unwrap_or_default()
-            .0;
-
-        let mut blueprint = ViewportBlueprint {
+        let tree = if app_options.legacy_container_blueprint {
+            blueprint_db
+                .store()
+                .query_timeless_component_quiet::<ViewportLayout>(&VIEWPORT_PATH.into())
+                .map(|space_view| space_view.value)
+                .unwrap_or_default()
+                .0
+        } else {
+            build_tree_from_space_views_and_containers(
+                space_views.keys(),
+                containers.values(),
+                root_container,
+            )
+        };
+
+        ViewportBlueprint {
             space_views,
             containers,
             root_container,
@@ -148,28 +156,8 @@ impl ViewportBlueprint {
             auto_layout,
             auto_space_views,
             tree_action_sender,
-        };
-
-        if app_options.experimental_container_blueprints {
-            let mut tree = blueprint.build_tree_from_containers();
-
-            // TODO(abey79): Figure out if we want to simplify here or not.
-            if false {
-                let options = egui_tiles::SimplificationOptions {
-                    all_panes_must_have_tabs: true,
-                    ..Default::default()
-                };
-
-                tree.simplify(&options);
-            }
-
-            re_log::trace!("Loaded tree from blueprint: {tree:#?}");
-
-            blueprint.tree = tree;
         }
 
-        blueprint
-
         // TODO(jleibs): Need to figure out if we have to re-enable support for
         // auto-discovery of SpaceViews logged via the experimental blueprint APIs.
         /*
@@ -402,13 +390,16 @@ impl ViewportBlueprint {
         self.send_tree_action(TreeAction::SetContainerKind(container_id, kind));
     }
 
-    /// Simplify the container subtree with the provided options.
-    pub fn simplify_tree(
+    /// Simplify the container tree with the provided options.
+    pub fn simplify_container(
         &self,
         tile_id: egui_tiles::TileId,
         simplification_options: SimplificationOptions,
     ) {
-        self.send_tree_action(TreeAction::SimplifyTree(tile_id, simplification_options));
+        self.send_tree_action(TreeAction::SimplifyContainer(
+            tile_id,
+            simplification_options,
+        ));
     }
 
     #[allow(clippy::unused_self)]
@@ -565,30 +556,34 @@ impl ViewportBlueprint {
             ctx.save_empty_blueprint_component::<RootContainer>(&VIEWPORT_PATH.into());
         }
     }
+}
 
-    pub fn build_tree_from_containers(&self) -> egui_tiles::Tree<SpaceViewId> {
-        re_tracing::profile_function!();
-        let mut tree = egui_tiles::Tree::empty("viewport_tree");
-
-        // First add all the space_views
-        for space_view in self.space_views.keys() {
-            let tile_id = blueprint_id_to_tile_id(space_view);
-            let pane = egui_tiles::Tile::Pane(*space_view);
-            tree.tiles.insert(tile_id, pane);
-        }
-
-        // Now add all the containers
-        for container in self.containers.values() {
-            let tile_id = blueprint_id_to_tile_id(&container.id);
+fn build_tree_from_space_views_and_containers<'a>(
+    space_views: impl Iterator<Item = &'a SpaceViewId>,
+    containers: impl Iterator<Item = &'a ContainerBlueprint>,
+    root_container: Option<ContainerId>,
+) -> egui_tiles::Tree<SpaceViewId> {
+    re_tracing::profile_function!();
+    let mut tree = egui_tiles::Tree::empty("viewport_tree");
+
+    // First add all the space_views
+    for space_view in space_views {
+        let tile_id = blueprint_id_to_tile_id(space_view);
+        let pane = egui_tiles::Tile::Pane(*space_view);
+        tree.tiles.insert(tile_id, pane);
+    }
 
-            tree.tiles.insert(tile_id, container.to_tile());
-        }
+    // Now add all the containers
+    for container in containers {
+        let tile_id = blueprint_id_to_tile_id(&container.id);
 
-        // And finally, set the root
-        if let Some(root_container) = self.root_container.map(|id| blueprint_id_to_tile_id(&id)) {
-            tree.root = Some(root_container);
-        }
+        tree.tiles.insert(tile_id, container.to_tile());
+    }
 
-        tree
+    // And finally, set the root
+    if let Some(root_container) = root_container.map(|id| blueprint_id_to_tile_id(&id)) {
+        tree.root = Some(root_container);
     }
+
+    tree
 }