Add hierarchical display in recordings panel (#2971)

### What This PR updates the recording UI to display recording hierarchically when multiple recordings for the same app ID are loaded. If a single recording for a given app ID is loaded, a single line is still used for compactness. This PR also improves the `egui::Id` handling for `ListItem::show_collapsing()`. TODO: - [x] sort out vertical spacing <img width="1422" alt="image" src="https://github.com/rerun-io/rerun/assets/49431240/9beef915-d953-476d-9f40-7fb9d028221d"> ### 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 [demo.rerun.io](https://demo.rerun.io/pr/2971) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/2971) - [Docs preview](https://rerun.io/preview/pr%3Aantoine%2Frecordings-hierarchy/docs) - [Examples preview](https://rerun.io/preview/pr%3Aantoine%2Frecordings-hierarchy/examples) --------- Co-authored-by: Emil Ernerfeldt <[email protected]>
rerun-io · Aug 14, 2023 · 4f23c15 · 4f23c15
1 parent 4c20e4b
commit 4f23c15
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 50 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/re_ui/src/list_item.rs b/crates/re_ui/src/list_item.rs
@@ -125,7 +125,7 @@ impl<'a> ListItem<'a> {
     ) {
         let mut state = egui::collapsing_header::CollapsingState::load_with_default_open(
             ui.ctx(),
-            ui.make_persistent_id(self.text.text()),
+            ui.make_persistent_id(ui.id().with(self.text.text())),
             default_open,
         );
 
@@ -142,9 +142,7 @@ impl<'a> ListItem<'a> {
         }
 
         state.show_body_indented(&response.response, ui, |ui| {
-            ui.add_space(4.0); // Add space only if there is a body to make minimized headers stick together.
             add_body(re_ui, ui);
-            ui.add_space(4.0); // Same here
         });
     }
 

diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml
@@ -87,6 +87,7 @@ egui.workspace = true
 egui-wgpu.workspace = true
 image = { workspace = true, default-features = false, features = ["png"] }
 itertools = { workspace = true }
+once_cell = { workspace = true }
 poll-promise = "0.2"
 rfd.workspace = true
 ron = "0.8.0"

diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs
@@ -165,19 +165,21 @@ impl AppState {
                         // doesn't have inner margins.
                         ui.set_clip_rect(ui.max_rect());
 
-                        // TODO(ab): this might be promoted higher in the hierarchy once list item are
-                        // used in the blueprint panel section.
-                        ui.scope(|ui| {
-                            ui.spacing_mut().item_spacing.y = 0.0;
-                            recordings_panel_ui(&mut ctx, ui);
-                        });
+                        // ListItem don't need vertical spacing so we disable it, but restore it
+                        // before drawing the blueprint panel.
+                        // TODO(ab): remove this once the blueprint tree uses list items
+                        let v_space = ui.spacing().item_spacing.y;
+                        ui.spacing_mut().item_spacing.y = 0.0;
+
+                        recordings_panel_ui(&mut ctx, ui);
 
                         // TODO(ab): remove this frame once the blueprint tree uses list items
                         egui::Frame {
                             inner_margin: re_ui::ReUi::panel_margin(),
                             ..Default::default()
                         }
                         .show(ui, |ui| {
+                            ui.spacing_mut().item_spacing.y = v_space;
                             blueprint_panel_ui(&mut viewport.blueprint, &mut ctx, ui, &spaces_info);
                         });
                     },

diff --git a/crates/re_viewer/src/ui/recordings_panel.rs b/crates/re_viewer/src/ui/recordings_panel.rs
@@ -1,7 +1,11 @@
-use re_data_store::StoreDb;
 use re_viewer_context::{SystemCommand, SystemCommandSender, ViewerContext};
+use std::collections::BTreeMap;
 use time::macros::format_description;
 
+static TIME_FORMAT_DESCRIPTION: once_cell::sync::Lazy<
+    &'static [time::format_description::FormatItem<'static>],
+> = once_cell::sync::Lazy::new(|| format_description!(version = 2, "[hour]:[minute]:[second]Z"));
+
 /// Show the currently open Recordings in a selectable list.
 pub fn recordings_panel_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) {
     ctx.re_ui.panel_content(ui, |re_ui, ui| {
@@ -34,58 +38,88 @@ fn recording_list_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) {
         ..
     } = ctx;
 
-    let mut store_dbs = store_context.alternate_recordings.clone();
-    if store_dbs.is_empty() {
-        return;
+    let mut store_dbs_map: BTreeMap<_, Vec<_>> = BTreeMap::new();
+    for store_db in &store_context.alternate_recordings {
+        let key = store_db
+            .store_info()
+            .map_or("<unknown>", |info| info.application_id.as_str());
+        store_dbs_map.entry(key).or_default().push(*store_db);
     }
 
-    fn store_db_key(store_db: &StoreDb) -> impl Ord + '_ {
-        store_db
-            .store_info()
-            .map(|info| (info.application_id.0.as_str(), info.started))
+    if store_dbs_map.is_empty() {
+        return;
     }
 
-    store_dbs.sort_by_key(|store_db| store_db_key(store_db));
+    for store_dbs in store_dbs_map.values_mut() {
+        store_dbs.sort_by_key(|store_db| store_db.store_info().map(|info| info.started));
+    }
 
     let active_recording = store_context.recording.map(|rec| rec.store_id());
 
-    let desc = format_description!(version = 2, "[hour]:[minute]:[second]Z");
-    for store_db in &store_dbs {
-        let info = if let Some(store_info) = store_db.store_info() {
-            format!(
-                "{} - {}",
-                store_info.application_id,
-                store_info
-                    .started
-                    .to_datetime()
-                    .and_then(|dt| dt.format(&desc).ok())
-                    .unwrap_or("<unknown>".to_owned())
-            )
+    for (app_id, store_dbs) in store_dbs_map {
+        if store_dbs.len() == 1 {
+            let store_db = store_dbs[0];
+            if recording_ui(ctx.re_ui, ui, store_db, Some(app_id), active_recording).clicked() {
+                command_sender
+                    .send_system(SystemCommand::SetRecordingId(store_db.store_id().clone()));
+            }
         } else {
-            "<UNKNOWN>".to_owned()
-        };
-
-        if ctx
-            .re_ui
-            .list_item(info)
-            .with_icon_fn(|_re_ui, ui, rect, visuals| {
-                let color = if active_recording == Some(store_db.store_id()) {
-                    visuals.fg_stroke.color
-                } else {
-                    ui.visuals().widgets.noninteractive.fg_stroke.color
-                };
-
-                ui.painter()
-                    .circle(rect.center(), 4.0, color, egui::Stroke::NONE);
-            })
-            .show(ui)
-            .clicked()
-        {
-            command_sender.send_system(SystemCommand::SetRecordingId(store_db.store_id().clone()));
+            ctx.re_ui
+                .list_item(app_id)
+                .active(false)
+                .show_collapsing(ui, true, |_, ui| {
+                    for store_db in store_dbs {
+                        if recording_ui(ctx.re_ui, ui, store_db, None, active_recording).clicked() {
+                            command_sender.send_system(SystemCommand::SetRecordingId(
+                                store_db.store_id().clone(),
+                            ));
+                        }
+                    }
+                });
         }
     }
 }
 
+/// Show the UI for a single recording.
+///
+/// If an `app_id_label` is provided, it will be shown in front of the recording time.
+fn recording_ui(
+    re_ui: &re_ui::ReUi,
+    ui: &mut egui::Ui,
+    store_db: &re_data_store::StoreDb,
+    app_id_label: Option<&str>,
+    active_recording: Option<&re_log_types::StoreId>,
+) -> egui::Response {
+    let prefix = if let Some(app_id_label) = app_id_label {
+        format!("{app_id_label} - ")
+    } else {
+        String::new()
+    };
+
+    let name = store_db
+        .store_info()
+        .and_then(|info| {
+            info.started
+                .to_datetime()
+                .and_then(|dt| dt.format(&TIME_FORMAT_DESCRIPTION).ok())
+        })
+        .unwrap_or("<unknown time>".to_owned());
+
+    re_ui
+        .list_item(format!("{prefix}{name}"))
+        .with_icon_fn(|_re_ui, ui, rect, visuals| {
+            let color = if active_recording == Some(store_db.store_id()) {
+                visuals.fg_stroke.color
+            } else {
+                ui.visuals().widgets.noninteractive.fg_stroke.color
+            };
+
+            ui.painter()
+                .circle(rect.center(), 4.0, color, egui::Stroke::NONE);
+        })
+        .show(ui)
+}
+
 #[cfg(not(target_arch = "wasm32"))]
 fn add_button_ui(ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui) {
     use re_ui::UICommandSender;