Review comments

crates/viewer/re_selection_panel/src/lib.rs

@@ -29,7 +29,7 @@ mod test {
             selection_state.set_selection(Item::SpaceView(SpaceViewId::random()));
         });
 
-        test_ctx.run(|ctx, ui| {
+        test_ctx.run_in_egui_central_panel(|ctx, ui| {
             let (sender, _) = std::sync::mpsc::channel();
             let blueprint = ViewportBlueprint::try_from_db(
                 ctx.store_context.blueprint,

crates/viewer/re_space_view_dataframe/src/view_query/blueprint.rs

@@ -288,12 +288,13 @@ mod test {
 
         let view_id = SpaceViewId::random();
 
-        test_context.run_and_handle_system_commands(|ctx, _| {
+        test_context.run_in_egui_central_panel(|ctx, _| {
             let query = Query::from_blueprint(ctx, view_id);
             query.save_latest_at_enabled(ctx, true);
         });
+        test_context.handle_system_commands();
 
-        test_context.run(|ctx, _| {
+        test_context.run_in_egui_central_panel(|ctx, _| {
             let query = Query::from_blueprint(ctx, view_id);
             assert!(query.latest_at_enabled().unwrap());
         });

crates/viewer/re_time_panel/Cargo.toml

@@ -40,8 +40,8 @@ vec1.workspace = true
 [dev-dependencies]
 anyhow.workspace = true
 criterion.workspace = true
-rand.workspace = true
 egui_kittest.workspace = true
+rand.workspace = true
 
 [lib]
 bench = false

crates/viewer/re_time_panel/src/lib.rs

@@ -348,7 +348,7 @@ impl TimePanel {
         }
     }
 
-    pub fn expanded_ui(
+    fn expanded_ui(
         &mut self,
         ctx: &ViewerContext<'_>,
         viewport_blueprint: &ViewportBlueprint,

crates/viewer/re_time_panel/tests/time_panel_tests.rs

@@ -1,4 +1,7 @@
+use std::sync::Arc;
+
 use egui::{CentralPanel, Vec2};
+
 use re_chunk_store::{Chunk, LatestAtQuery, RowId};
 use re_log_types::example_components::MyPoint;
 use re_log_types::external::re_types_core::Component;
@@ -7,12 +10,10 @@ use re_time_panel::TimePanel;
 use re_viewer_context::blueprint_timeline;
 use re_viewer_context::test_context::TestContext;
 use re_viewport_blueprint::ViewportBlueprint;
-use std::sync::Arc;
 
 #[test]
-pub fn time_panel_should_match_snapshot() {
+pub fn time_panel_two_sections_should_match_snapshot() {
     let mut test_context = TestContext::default();
-    let mut panel = TimePanel::default();
 
     let points1 = MyPoint::from_iter(0..1);
 
@@ -32,10 +33,51 @@ pub fn time_panel_should_match_snapshot() {
             .unwrap();
     }
 
+    run_time_panel_and_save_snapshot(test_context, "time_panel_two_sections");
+}
+
+#[test]
+pub fn time_panel_dense_data_should_match_snapshot() {
+    let mut test_context = TestContext::default();
+
+    let points1 = MyPoint::from_iter(0..1);
+
+    let mut rng_seed = 0b1010_1010_1010_1010_1010_1010_1010_1010u64;
+    let mut rng = || {
+        rng_seed ^= rng_seed >> 12;
+        rng_seed ^= rng_seed << 25;
+        rng_seed ^= rng_seed >> 27;
+        rng_seed.wrapping_mul(0x2545_f491_4f6c_dd1d)
+    };
+
+    let entity_path = EntityPath::from("/entity");
+    let mut builder = Chunk::builder(entity_path.clone());
+    for frame in 0..1_000 {
+        if rng() & 0b1 == 0 {
+            continue;
+        }
+
+        builder = builder.with_sparse_component_batches(
+            RowId::new(),
+            [build_frame_nr(frame)],
+            [(MyPoint::name(), Some(&points1 as _))],
+        );
+    }
+    test_context
+        .recording_store
+        .add_chunk(&Arc::new(builder.build().unwrap()))
+        .unwrap();
+
+    run_time_panel_and_save_snapshot(test_context, "time_panel_dense_data");
+}
+
+//TODO(ab): this contains a lot of boilerplate which should be provided by helpers
+fn run_time_panel_and_save_snapshot(mut test_context: TestContext, snapshot_name: &str) {
+    let mut panel = TimePanel::default();
     let mut harness = egui_kittest::Harness::builder()
         .with_size(Vec2::new(700.0, 300.0))
         .build(move |ctx| {
-            test_context.run_simple(ctx, |viewer_ctx| {
+            test_context.run(ctx, |viewer_ctx| {
                 let (sender, _) = std::sync::mpsc::channel();
                 let blueprint = ViewportBlueprint::try_from_db(
                     viewer_ctx.store_context.blueprint,
@@ -44,23 +86,22 @@ pub fn time_panel_should_match_snapshot() {
                 );
 
                 CentralPanel::default().show(ctx, |ui| {
-                    let time_ctrl_before = viewer_ctx.rec_cfg.time_ctrl.read().clone();
-                    let mut time_ctrl_after = time_ctrl_before.clone();
+                    let mut time_ctrl = viewer_ctx.rec_cfg.time_ctrl.read().clone();
 
                     panel.show_expanded_with_header(
                         viewer_ctx,
                         &blueprint,
                         viewer_ctx.recording(),
-                        &mut time_ctrl_after,
+                        &mut time_ctrl,
                         ui,
                     );
                 });
             });
 
-            test_context.handle_system_command();
+            test_context.handle_system_commands();
         });
 
     harness.run();
 
-    harness.wgpu_snapshot("time_panel");
+    harness.wgpu_snapshot(snapshot_name);
 }

crates/viewer/re_viewer_context/src/test_context.rs

@@ -18,7 +18,7 @@ use crate::{
 /// use re_viewer_context::ViewerContext;
 ///
 /// let mut test_context = TestContext::default();
-/// test_context.run(|ctx: &ViewerContext, _| {
+/// test_context.run_in_egui_central_panel(|ctx: &ViewerContext, _| {
 ///     /* do something with ctx */
 /// });
 /// ```
@@ -73,23 +73,11 @@ impl TestContext {
         self.selection_state.on_frame_start(|_| true, None);
     }
 
-    /// Run the given function with a [`ViewerContext`] produced by the [`Self`].
+    /// Run the provided closure with a [`ViewerContext`] produced by the [`Self`].
     ///
-    /// Note: there is a possibility that the closure will be called more than once, see
-    /// [`egui::Context::run`].
-    pub fn run(&self, mut func: impl FnMut(&ViewerContext<'_>, &mut egui::Ui)) {
-        egui::__run_test_ctx(|ctx| {
-            egui::CentralPanel::default().show(ctx, |ui| {
-                let egui_ctx = ui.ctx().clone();
-
-                self.run_simple(&egui_ctx, |ctx| {
-                    func(ctx, ui);
-                });
-            });
-        });
-    }
-
-    pub fn run_simple(&self, egui_ctx: &egui::Context, func: impl FnOnce(&ViewerContext<'_>)) {
+    /// IMPORTANT: call [`Self::handle_system_commands`] after calling this function if your test
+    /// relies on system commands.
+    pub fn run(&self, egui_ctx: &egui::Context, func: impl FnOnce(&ViewerContext<'_>)) {
         re_ui::apply_style_and_install_loaders(egui_ctx);
 
         let store_context = StoreContext {
@@ -125,18 +113,34 @@ impl TestContext {
         func(&ctx);
     }
 
-    /// Run the given function with a [`ViewerContext`] produced by the [`Self`] and handle any
-    /// system commands issued during execution (see [`Self::handle_system_command`]).
-    pub fn run_and_handle_system_commands(
-        &mut self,
-        func: impl FnMut(&ViewerContext<'_>, &mut egui::Ui),
+    /// Run the given function with a [`ViewerContext`] produced by the [`Self`], in the context of
+    /// an [`egui::CentralPanel`].
+    ///
+    /// IMPORTANT: call [`Self::handle_system_commands`] after calling this function if your test
+    /// relies on system commands.
+    ///
+    /// Notes:
+    /// - Uses [`egui::__run_test_ctx`].
+    /// - There is a possibility that the closure will be called more than once, see
+    ///   [`egui::Context::run`].
+    //TODO(ab): replace this with a kittest-based helper.
+    pub fn run_in_egui_central_panel(
+        &self,
+        mut func: impl FnMut(&ViewerContext<'_>, &mut egui::Ui),
     ) {
-        self.run(func);
-        self.handle_system_command();
+        egui::__run_test_ctx(|ctx| {
+            egui::CentralPanel::default().show(ctx, |ui| {
+                let egui_ctx = ui.ctx().clone();
+
+                self.run(&egui_ctx, |ctx| {
+                    func(ctx, ui);
+                });
+            });
+        });
     }
 
     /// Best-effort attempt to meaningfully handle some of the system commands.
-    pub fn handle_system_command(&mut self) {
+    pub fn handle_system_commands(&mut self) {
         while let Some(command) = self.command_receiver.recv_system() {
             let mut handled = true;
             let command_name = format!("{command:?}");
@@ -216,7 +220,7 @@ mod test {
             selection_state.set_selection(item.clone());
         });
 
-        test_context.run(|ctx, _| {
+        test_context.run_in_egui_central_panel(|ctx, _| {
             assert_eq!(
                 ctx.selection_state.selected_items().single_item(),
                 Some(&item)

crates/viewer/re_viewport_blueprint/src/space_view.rs

@@ -778,7 +778,7 @@ mod tests {
         let mut query_result = contents.execute_query(&store_ctx, visualizable_entities);
         let mut view_states = ViewStates::default();
 
-        test_ctx.run(|ctx, _ui| {
+        test_ctx.run_in_egui_central_panel(|ctx, _ui| {
             resolver.update_overrides(
                 ctx.blueprint_db(),
                 ctx.blueprint_query,