Fix and test all known HybridResults issues from 0.18 (#7297)

This fixes all known bugs/regressions introduced in the visualizer-query
layer (aka `HybridResults`) in 0.18, and adds related checklists for
futureproof-ness.

As always, this is "just" a matter of carefully overriding row indices
when merging data from multiple sources with different priorities.

- Fixes #7227  
- Partially reverts #7199

crates/store/re_chunk/src/chunk.rs

@@ -211,6 +211,25 @@ impl Chunk {
         self
     }
 
+    /// Clones the chunk into a new chunk where all [`RowId`]s are [`RowId::ZERO`].
+    pub fn zeroed(self) -> Self {
+        let row_ids = std::iter::repeat(RowId::ZERO)
+            .take(self.row_ids.len())
+            .collect_vec();
+
+        #[allow(clippy::unwrap_used)]
+        let row_ids = <RowId as Loggable>::to_arrow(&row_ids)
+            // Unwrap: native RowIds cannot fail to serialize.
+            .unwrap()
+            .as_any()
+            .downcast_ref::<ArrowStructArray>()
+            // Unwrap: RowId schema is known in advance to be a struct array -- always.
+            .unwrap()
+            .clone();
+
+        Self { row_ids, ..self }
+    }
+
     /// Computes the time range covered by each individual component column on each timeline.
     ///
     /// This is different from the time range covered by the [`Chunk`] as a whole because component

crates/viewer/re_space_view/src/lib.rs

@@ -8,7 +8,6 @@ mod heuristics;
 mod query;
 mod results_ext;
 mod screenshot;
-mod time_key;
 mod view_property_ui;
 
 pub use heuristics::suggest_space_view_for_each_entity;
@@ -19,7 +18,6 @@ pub use results_ext::{
     HybridLatestAtResults, HybridResults, HybridResultsChunkIter, RangeResultsExt,
 };
 pub use screenshot::ScreenshotMode;
-pub use time_key::TimeKey;
 pub use view_property_ui::view_property_ui;
 
 pub mod external {

crates/viewer/re_space_view/src/results_ext.rs

@@ -1,6 +1,8 @@
 use std::borrow::Cow;
 use std::sync::Arc;
 
+use itertools::Itertools as _;
+
 use re_chunk_store::{Chunk, LatestAtQuery, RangeQuery, UnitChunkShared};
 use re_log_types::external::arrow2::array::Array as ArrowArray;
 use re_log_types::hash::Hash64;
@@ -9,7 +11,6 @@ use re_types_core::ComponentName;
 use re_viewer_context::{DataResult, QueryContext, ViewContext};
 
 use crate::DataResultQuery as _;
-use crate::TimeKey;
 
 // ---
 
@@ -293,7 +294,9 @@ impl RangeResultsExt for HybridRangeResults {
     fn get_required_chunks(&self, component_name: &ComponentName) -> Option<Cow<'_, [Chunk]>> {
         if let Some(unit) = self.overrides.get(component_name) {
             // Because this is an override we always re-index the data as static
-            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk()).into_static();
+            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk())
+                .into_static()
+                .zeroed();
             Some(Cow::Owned(vec![chunk]))
         } else {
             self.results.get_required_chunks(component_name)
@@ -302,27 +305,36 @@ impl RangeResultsExt for HybridRangeResults {
 
     #[inline]
     fn get_optional_chunks(&self, component_name: &ComponentName) -> Cow<'_, [Chunk]> {
+        re_tracing::profile_function!();
+
         if let Some(unit) = self.overrides.get(component_name) {
             // Because this is an override we always re-index the data as static
-            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk()).into_static();
+            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk())
+                .into_static()
+                .zeroed();
             Cow::Owned(vec![chunk])
         } else {
-            let chunks = self.results.get_optional_chunks(component_name);
-
-            // If the data is not empty, return it.
+            re_tracing::profile_scope!("defaults");
 
-            if !chunks.is_empty() {
-                return chunks;
-            }
+            // NOTE: Because this is a range query, we always need the defaults to come first,
+            // since range queries don't have any state to bootstrap from.
+            let defaults = self.defaults.get(component_name).map(|unit| {
+                // Because this is an default from the blueprint we always re-index the data as static
+                Arc::unwrap_or_clone(unit.clone().into_chunk())
+                    .into_static()
+                    .zeroed()
+            });
 
-            // Otherwise try to use the default data.
+            let chunks = self.results.get_optional_chunks(component_name);
 
-            let Some(unit) = self.defaults.get(component_name) else {
-                return Cow::Owned(Vec::new());
-            };
-            // Because this is an default from the blueprint we always re-index the data as static
-            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk()).into_static();
-            Cow::Owned(vec![chunk])
+            // TODO(cmc): this `collect_vec()` sucks, let's keep an eye on it and see if it ever
+            // becomes an issue.
+            Cow::Owned(
+                defaults
+                    .into_iter()
+                    .chain(chunks.iter().cloned())
+                    .collect_vec(),
+            )
         }
     }
 }
@@ -332,7 +344,9 @@ impl<'a> RangeResultsExt for HybridLatestAtResults<'a> {
     fn get_required_chunks(&self, component_name: &ComponentName) -> Option<Cow<'_, [Chunk]>> {
         if let Some(unit) = self.overrides.get(component_name) {
             // Because this is an override we always re-index the data as static
-            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk()).into_static();
+            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk())
+                .into_static()
+                .zeroed();
             Some(Cow::Owned(vec![chunk]))
         } else {
             self.results.get_required_chunks(component_name)
@@ -343,24 +357,35 @@ impl<'a> RangeResultsExt for HybridLatestAtResults<'a> {
     fn get_optional_chunks(&self, component_name: &ComponentName) -> Cow<'_, [Chunk]> {
         if let Some(unit) = self.overrides.get(component_name) {
             // Because this is an override we always re-index the data as static
-            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk()).into_static();
+            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk())
+                .into_static()
+                .zeroed();
             Cow::Owned(vec![chunk])
         } else {
-            let chunks = self.results.get_optional_chunks(component_name);
+            let chunks = self
+                .results
+                .get_optional_chunks(component_name)
+                .iter()
+                // NOTE: Since this is a latest-at query that is being coerced into a range query, we
+                // need to make sure that every secondary column has an index smaller then the primary column
+                // (we use `(TimeInt::STATIC, RowId::ZERO)`), otherwise range zipping would yield unexpected
+                // results.
+                .map(|chunk| chunk.clone().into_static().zeroed())
+                .collect_vec();
 
             // If the data is not empty, return it.
-
             if !chunks.is_empty() {
-                return chunks;
+                return Cow::Owned(chunks);
             }
 
             // Otherwise try to use the default data.
-
             let Some(unit) = self.defaults.get(component_name) else {
                 return Cow::Owned(Vec::new());
             };
             // Because this is an default from the blueprint we always re-index the data as static
-            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk()).into_static();
+            let chunk = Arc::unwrap_or_clone(unit.clone().into_chunk())
+                .into_static()
+                .zeroed();
             Cow::Owned(vec![chunk])
         }
     }
@@ -386,11 +411,10 @@ impl<'a> RangeResultsExt for HybridResults<'a> {
 
 // ---
 
-use re_chunk::{ChunkComponentIterItem, Timeline};
+use re_chunk::{ChunkComponentIterItem, RowId, TimeInt, Timeline};
 use re_chunk_store::external::{re_chunk, re_chunk::external::arrow2};
 
 /// The iterator type backing [`HybridResults::iter_as`].
-#[derive(Debug)]
 pub struct HybridResultsChunkIter<'a> {
     chunks: Cow<'a, [Chunk]>,
     timeline: Timeline,
@@ -403,12 +427,10 @@ impl<'a> HybridResultsChunkIter<'a> {
     /// See [`Chunk::iter_component`] for more information.
     pub fn component<C: re_types_core::Component>(
         &'a self,
-    ) -> impl Iterator<Item = (TimeKey, ChunkComponentIterItem<C>)> + 'a {
+    ) -> impl Iterator<Item = ((TimeInt, RowId), ChunkComponentIterItem<C>)> + 'a {
         self.chunks.iter().flat_map(move |chunk| {
             itertools::izip!(
-                chunk
-                    .iter_component_indices(&self.timeline, &self.component_name)
-                    .map(TimeKey::from),
+                chunk.iter_component_indices(&self.timeline, &self.component_name),
                 chunk.iter_component::<C>(),
             )
         })
@@ -419,12 +441,10 @@ impl<'a> HybridResultsChunkIter<'a> {
     /// See [`Chunk::iter_primitive`] for more information.
     pub fn primitive<T: arrow2::types::NativeType>(
         &'a self,
-    ) -> impl Iterator<Item = (TimeKey, &'a [T])> + 'a {
+    ) -> impl Iterator<Item = ((TimeInt, RowId), &'a [T])> + 'a {
         self.chunks.iter().flat_map(move |chunk| {
             itertools::izip!(
-                chunk
-                    .iter_component_indices(&self.timeline, &self.component_name)
-                    .map(TimeKey::from),
+                chunk.iter_component_indices(&self.timeline, &self.component_name),
                 chunk.iter_primitive::<T>(&self.component_name)
             )
         })
@@ -435,15 +455,13 @@ impl<'a> HybridResultsChunkIter<'a> {
     /// See [`Chunk::iter_primitive_array`] for more information.
     pub fn primitive_array<const N: usize, T: arrow2::types::NativeType>(
         &'a self,
-    ) -> impl Iterator<Item = (TimeKey, &'a [[T; N]])> + 'a
+    ) -> impl Iterator<Item = ((TimeInt, RowId), &'a [[T; N]])> + 'a
     where
         [T; N]: bytemuck::Pod,
     {
         self.chunks.iter().flat_map(move |chunk| {
             itertools::izip!(
-                chunk
-                    .iter_component_indices(&self.timeline, &self.component_name)
-                    .map(TimeKey::from),
+                chunk.iter_component_indices(&self.timeline, &self.component_name),
                 chunk.iter_primitive_array::<N, T>(&self.component_name)
             )
         })
@@ -454,15 +472,13 @@ impl<'a> HybridResultsChunkIter<'a> {
     /// See [`Chunk::iter_primitive_array_list`] for more information.
     pub fn primitive_array_list<const N: usize, T: arrow2::types::NativeType>(
         &'a self,
-    ) -> impl Iterator<Item = (TimeKey, Vec<&'a [[T; N]]>)> + 'a
+    ) -> impl Iterator<Item = ((TimeInt, RowId), Vec<&'a [[T; N]]>)> + 'a
     where
         [T; N]: bytemuck::Pod,
     {
         self.chunks.iter().flat_map(move |chunk| {
             itertools::izip!(
-                chunk
-                    .iter_component_indices(&self.timeline, &self.component_name)
-                    .map(TimeKey::from),
+                chunk.iter_component_indices(&self.timeline, &self.component_name),
                 chunk.iter_primitive_array_list::<N, T>(&self.component_name)
             )
         })
@@ -473,12 +489,10 @@ impl<'a> HybridResultsChunkIter<'a> {
     /// See [`Chunk::iter_string`] for more information.
     pub fn string(
         &'a self,
-    ) -> impl Iterator<Item = (TimeKey, Vec<re_types_core::ArrowString>)> + 'a {
+    ) -> impl Iterator<Item = ((TimeInt, RowId), Vec<re_types_core::ArrowString>)> + 'a {
         self.chunks.iter().flat_map(|chunk| {
             itertools::izip!(
-                chunk
-                    .iter_component_indices(&self.timeline, &self.component_name)
-                    .map(TimeKey::from),
+                chunk.iter_component_indices(&self.timeline, &self.component_name),
                 chunk.iter_string(&self.component_name)
             )
         })
@@ -489,12 +503,10 @@ impl<'a> HybridResultsChunkIter<'a> {
     /// See [`Chunk::iter_buffer`] for more information.
     pub fn buffer<T: arrow2::types::NativeType>(
         &'a self,
-    ) -> impl Iterator<Item = (TimeKey, Vec<re_types_core::ArrowBuffer<T>>)> + 'a {
+    ) -> impl Iterator<Item = ((TimeInt, RowId), Vec<re_types_core::ArrowBuffer<T>>)> + 'a {
         self.chunks.iter().flat_map(|chunk| {
             itertools::izip!(
-                chunk
-                    .iter_component_indices(&self.timeline, &self.component_name)
-                    .map(TimeKey::from),
+                chunk.iter_component_indices(&self.timeline, &self.component_name),
                 chunk.iter_buffer(&self.component_name)
             )
         })

crates/viewer/re_space_view_spatial/src/visualizers/assets3d.rs

@@ -1,7 +1,7 @@
-use re_log_types::{hash::Hash64, Instance};
+use re_chunk_store::RowId;
+use re_log_types::{hash::Hash64, Instance, TimeInt};
 use re_renderer::renderer::MeshInstance;
 use re_renderer::RenderContext;
-use re_space_view::TimeKey;
 use re_types::{
     archetypes::Asset3D,
     components::{Blob, MediaType},
@@ -33,7 +33,7 @@ impl Default for Asset3DVisualizer {
 }
 
 struct Asset3DComponentData {
-    index: TimeKey,
+    index: (TimeInt, RowId),
 
     blob: ArrowBuffer<u8>,
     media_type: Option<ArrowString>,
@@ -58,7 +58,7 @@ impl Asset3DVisualizer {
                 media_type: data.media_type.clone().map(Into::into),
             };
 
-            let primary_row_id = data.index.row_id;
+            let primary_row_id = data.index.1;
             let picking_instance_hash = re_entity_db::InstancePathHash::entity_all(entity_path);
             let outline_mask_ids = ent_context.highlight.index_outline_mask(Instance::ALL);
 

crates/viewer/re_space_view_spatial/src/visualizers/depth_images.rs

@@ -282,7 +282,7 @@ impl VisualizerSystem for DepthImageVisualizer {
 
                         Some(DepthImageComponentData {
                             image: ImageInfo {
-                                buffer_row_id: index.row_id,
+                                buffer_row_id: index.1,
                                 buffer: buffer.clone().into(),
                                 format: first_copied(format.as_deref())?.0,
                                 kind: ImageKind::Depth,

crates/viewer/re_space_view_spatial/src/visualizers/encoded_image.rs

@@ -144,7 +144,7 @@ impl EncodedImageVisualizer {
         let all_media_types = results.iter_as(timeline, MediaType::name());
         let all_opacities = results.iter_as(timeline, Opacity::name());
 
-        for (index, blobs, media_types, opacities) in re_query::range_zip_1x2(
+        for ((_time, tensor_data_row_id), blobs, media_types, opacities) in re_query::range_zip_1x2(
             all_blobs_indexed,
             all_media_types.string(),
             all_opacities.primitive::<f32>(),
@@ -156,7 +156,7 @@ impl EncodedImageVisualizer {
 
             let image = ctx.viewer_ctx.cache.entry(|c: &mut ImageDecodeCache| {
                 c.entry(
-                    index.row_id,
+                    tensor_data_row_id,
                     blob,
                     media_type.as_ref().map(|mt| mt.as_str()),
                 )

crates/viewer/re_space_view_spatial/src/visualizers/images.rs

@@ -165,7 +165,7 @@ impl ImageVisualizer {
 
             Some(ImageComponentData {
                 image: ImageInfo {
-                    buffer_row_id: index.row_id,
+                    buffer_row_id: index.1,
                     buffer: buffer.clone().into(),
                     format: first_copied(formats.as_deref())?.0,
                     kind: ImageKind::Color,