Chunkified text-log view with multi-timeline display (#7027)

Title.

This brings back the multi-timeline display feature, which had regressed
when we introduced the new `ChunkStore`.

- Fixes #6611 
- DNM: requires #7023


![image](https://github.com/user-attachments/assets/15a148b5-92d7-4548-b3e4-fc535f742915)

Author: teh-cmc

Cargo.lock

@@ -5065,12 +5065,13 @@ version = "0.18.0-alpha.1+dev"
 dependencies = [
  "egui",
  "egui_extras",
+ "itertools 0.13.0",
  "re_chunk_store",
  "re_data_ui",
  "re_entity_db",
  "re_log",
  "re_log_types",
- "re_query",
+ "re_query2",
  "re_renderer",
  "re_space_view",
  "re_tracing",

crates/store/re_chunk/src/iter.rs

@@ -10,7 +10,7 @@ use arrow2::{
 };
 use itertools::{izip, Itertools};
 
-use re_log_types::{TimeInt, Timeline};
+use re_log_types::{TimeInt, TimePoint, Timeline};
 use re_types_core::{ArrowBuffer, ArrowString, Component, ComponentName};
 
 use crate::{Chunk, ChunkTimeline, RowId};
@@ -97,6 +97,81 @@ impl Chunk {
         }
     }
 
+    /// Returns an iterator over the [`TimePoint`]s of a [`Chunk`].
+    ///
+    /// See also:
+    /// * [`Self::iter_component_timepoints`].
+    #[inline]
+    pub fn iter_timepoints(&self) -> impl Iterator<Item = TimePoint> + '_ {
+        let mut timelines = self
+            .timelines
+            .values()
+            .map(|time_chunk| (time_chunk.timeline, time_chunk.times()))
+            .collect_vec();
+
+        std::iter::from_fn(move || {
+            let mut timepoint = TimePoint::default();
+            for (timeline, times) in &mut timelines {
+                timepoint.insert(*timeline, times.next()?);
+            }
+            Some(timepoint)
+        })
+    }
+
+    /// Returns an iterator over the [`TimePoint`]s of a [`Chunk`], for a given component.
+    ///
+    /// This is different than [`Self::iter_timepoints`] in that it will only yield timepoints for rows
+    /// at which there is data for the specified `component_name`.
+    ///
+    /// See also [`Self::iter_timepoints`].
+    pub fn iter_component_timepoints(
+        &self,
+        component_name: &ComponentName,
+    ) -> impl Iterator<Item = TimePoint> + '_ {
+        let Some(list_array) = self.components.get(component_name) else {
+            return Either::Left(std::iter::empty());
+        };
+
+        if let Some(validity) = list_array.validity() {
+            let mut timelines = self
+                .timelines
+                .values()
+                .map(|time_chunk| {
+                    (
+                        time_chunk.timeline,
+                        time_chunk
+                            .times()
+                            .enumerate()
+                            .filter(|(i, _)| validity.get_bit(*i))
+                            .map(|(_, time)| time),
+                    )
+                })
+                .collect_vec();
+
+            Either::Right(Either::Left(std::iter::from_fn(move || {
+                let mut timepoint = TimePoint::default();
+                for (timeline, times) in &mut timelines {
+                    timepoint.insert(*timeline, times.next()?);
+                }
+                Some(timepoint)
+            })))
+        } else {
+            let mut timelines = self
+                .timelines
+                .values()
+                .map(|time_chunk| (time_chunk.timeline, time_chunk.times()))
+                .collect_vec();
+
+            Either::Right(Either::Right(std::iter::from_fn(move || {
+                let mut timepoint = TimePoint::default();
+                for (timeline, times) in &mut timelines {
+                    timepoint.insert(*timeline, times.next()?);
+                }
+                Some(timepoint)
+            })))
+        }
+    }
+
     /// Returns an iterator over the offsets (`(offset, len)`) of a [`Chunk`], for a given
     /// component.
     ///

crates/viewer/re_space_view/src/results_ext2.rs

@@ -227,6 +227,25 @@ pub trait RangeResultsExt {
     /// For results that are aware of the blueprint, overrides, store results, and defaults will be
     /// considered.
     fn get_optional_chunks(&self, component_name: &ComponentName) -> Cow<'_, [Chunk]>;
+
+    /// Returns a zero-copy iterator over all the results for the given `(timeline, component)` pair.
+    ///
+    /// Call one of the following methods on the returned [`HybridResultsChunkIter`]:
+    /// * [`HybridResultsChunkIter::primitive`]
+    /// * [`HybridResultsChunkIter::primitive_array`]
+    /// * [`HybridResultsChunkIter::string`]
+    fn iter_as(
+        &self,
+        timeline: Timeline,
+        component_name: ComponentName,
+    ) -> HybridResultsChunkIter<'_> {
+        let chunks = self.get_optional_chunks(&component_name);
+        HybridResultsChunkIter {
+            chunks,
+            timeline,
+            component_name,
+        }
+    }
 }
 
 impl RangeResultsExt for LatestAtResults {
@@ -465,24 +484,3 @@ impl<'a> HybridResultsChunkIter<'a> {
         })
     }
 }
-
-impl<'a> HybridResults<'a> {
-    /// Returns a zero-copy iterator over all the results for the given `(timeline, component)` pair.
-    ///
-    /// Call one of the following methods on the returned [`HybridResultsChunkIter`]:
-    /// * [`HybridResultsChunkIter::primitive`]
-    /// * [`HybridResultsChunkIter::primitive_array`]
-    /// * [`HybridResultsChunkIter::string`]
-    pub fn iter_as(
-        &'a self,
-        timeline: Timeline,
-        component_name: ComponentName,
-    ) -> HybridResultsChunkIter<'a> {
-        let chunks = self.get_optional_chunks(&component_name);
-        HybridResultsChunkIter {
-            chunks,
-            timeline,
-            component_name,
-        }
-    }
-}

crates/viewer/re_space_view_text_log/Cargo.toml

@@ -24,7 +24,7 @@ re_data_ui.workspace = true
 re_entity_db.workspace = true
 re_log_types.workspace = true
 re_log.workspace = true
-re_query.workspace = true
+re_query2.workspace = true
 re_renderer.workspace = true
 re_space_view.workspace = true
 re_tracing.workspace = true
@@ -34,3 +34,4 @@ re_viewer_context.workspace = true
 
 egui_extras.workspace = true
 egui.workspace = true
+itertools.workspace = true

crates/viewer/re_space_view_text_log/src/space_view_class.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use re_data_ui::item_ui;
-use re_log_types::{EntityPath, TimePoint, Timeline};
+use re_log_types::{EntityPath, Timeline};
 use re_types::View;
 use re_types::{components::TextLogLevel, SpaceViewClassIdentifier};
 use re_ui::UiExt as _;
@@ -279,20 +279,12 @@ fn table_ui(
     entries: &[&Entry],
     scroll_to_row: Option<usize>,
 ) {
-    let timelines = vec![*ctx.rec_cfg.time_ctrl.read().timeline()];
-
-    // TODO(#6611): This regressed because adding a metadata registry in the store is an antipattern.
-    //
-    // We'll bring back the multi-timeline display once we get rid of the native cache and start
-    // exposing chunks directly instead.
-    // Since chunks embed the data for all associated timelines, there'll be no extra work needed
-    // to get that information out.
-    // let timelines = state
-    //     .filters
-    //     .col_timelines
-    //     .iter()
-    //     .filter_map(|(timeline, visible)| visible.then_some(timeline))
-    //     .collect::<Vec<_>>();
+    let timelines = state
+        .filters
+        .col_timelines
+        .iter()
+        .filter_map(|(timeline, visible)| visible.then_some(timeline))
+        .collect::<Vec<_>>();
 
     use egui_extras::Column;
 
@@ -366,17 +358,17 @@ fn table_ui(
                 let entry = &entries[row.index()];
 
                 // timeline(s)
-                let timepoint: TimePoint = [(global_timeline, entry.time)].into();
                 for timeline in &timelines {
                     row.col(|ui| {
-                        let row_time = timepoint
+                        let row_time = entry
+                            .timepoint
                             .get(timeline)
                             .copied()
                             .unwrap_or(re_log_types::TimeInt::STATIC);
                         item_ui::time_button(ctx, ui, timeline, row_time);
 
                         if let Some(global_time) = global_time {
-                            if timeline == &global_timeline {
+                            if *timeline == &global_timeline {
                                 #[allow(clippy::comparison_chain)]
                                 if global_time < row_time {
                                     // We've past the global time - it is thus above this row.

crates/viewer/re_space_view_text_log/src/visualizer_system.rs

@@ -1,9 +1,11 @@
+use itertools::izip;
 use re_chunk_store::ResolvedTimeRange;
 use re_chunk_store::RowId;
 use re_entity_db::EntityPath;
 use re_log_types::TimeInt;
-use re_query::{clamped_zip_1x2, range_zip_1x2};
-use re_space_view::{range_with_blueprint_resolved_data, RangeResultsExt};
+use re_log_types::TimePoint;
+use re_query2::{clamped_zip_1x2, range_zip_1x2};
+use re_space_view::{range_with_blueprint_resolved_data2, RangeResultsExt2};
 use re_types::{
     archetypes::TextLog,
     components::{Color, Text, TextLogLevel},
@@ -19,6 +21,7 @@ pub struct Entry {
     pub row_id: RowId,
     pub entity_path: EntityPath,
     pub time: TimeInt,
+    pub timepoint: TimePoint,
     pub color: Option<Color>,
     pub body: Text,
     pub level: Option<TextLogLevel>,
@@ -53,13 +56,7 @@ impl VisualizerSystem for TextLogSystem {
             re_chunk_store::RangeQuery::new(view_query.timeline, ResolvedTimeRange::EVERYTHING);
 
         for data_result in view_query.iter_visible_data_results(ctx, Self::identifier()) {
-            if let Err(err) = self.process_entity(ctx, &query, data_result) {
-                re_log::error_once!(
-                    "Error visualizing text logs for {:?}: {:?}",
-                    data_result.entity_path,
-                    err
-                );
-            }
+            self.process_entity(ctx, &query, data_result);
         }
 
         {
@@ -86,36 +83,49 @@ impl TextLogSystem {
         ctx: &ViewContext<'_>,
         query: &re_chunk_store::RangeQuery,
         data_result: &re_viewer_context::DataResult,
-    ) -> Result<(), SpaceViewSystemExecutionError> {
+    ) {
         re_tracing::profile_function!();
-        let resolver = ctx.recording().resolver();
 
-        let results = range_with_blueprint_resolved_data(
+        let results = range_with_blueprint_resolved_data2(
             ctx,
             None,
             query,
             data_result,
             [Text::name(), TextLogLevel::name(), Color::name()],
         );
 
-        let Some(all_texts) = results
-            .get_required_component_dense::<Text>(resolver)
-            .transpose()?
-        else {
-            return Ok(());
+        let Some(all_text_chunks) = results.get_required_chunks(&Text::name()) else {
+            return;
         };
 
-        let all_levels = results.get_or_empty_dense::<TextLogLevel>(resolver)?;
-        let all_colors = results.get_or_empty_dense::<Color>(resolver)?;
+        // TODO(cmc): It would be more efficient (both space and compute) to do this lazily as
+        // we're rendering the table by indexing back into the original chunk etc.
+        // Let's keep it simple for now, until we have data suggested we need the extra perf.
+        let all_timepoints = all_text_chunks
+            .iter()
+            .flat_map(|chunk| chunk.iter_component_timepoints(&Text::name()));
+
+        let timeline = query.timeline();
+        let all_texts = results.iter_as(timeline, Text::name());
+        let all_levels = results.iter_as(timeline, TextLogLevel::name());
+        let all_colors = results.iter_as(timeline, Color::name());
+
         let all_frames = range_zip_1x2(
-            all_texts.range_indexed(),
-            all_levels.range_indexed(),
-            all_colors.range_indexed(),
+            all_texts.string(),
+            all_levels.component(),
+            all_colors.primitive::<u32>(),
         );
 
-        for (&(data_time, row_id), bodies, levels, colors) in all_frames {
-            let levels = levels.unwrap_or(&[]).iter().cloned().map(Some);
-            let colors = colors.unwrap_or(&[]).iter().copied().map(Some);
+        let all_frames = izip!(all_timepoints, all_frames);
+
+        for (timepoint, ((data_time, row_id), bodies, levels, colors)) in all_frames {
+            let levels = levels.as_deref().unwrap_or(&[]).iter().cloned().map(Some);
+            let colors = colors
+                .unwrap_or(&[])
+                .iter()
+                .copied()
+                .map(Into::into)
+                .map(Some);
 
             let level_default_fn = || None;
             let color_default_fn = || None;
@@ -128,14 +138,13 @@ impl TextLogSystem {
                     row_id,
                     entity_path: data_result.entity_path.clone(),
                     time: data_time,
+                    timepoint: timepoint.clone(),
                     color,
-                    body: text.clone(),
+                    body: text.clone().into(),
                     level,
                 });
             }
         }
-
-        Ok(())
     }
 }