Replace complex uses of query_entity_with_primary with `query_lates…

…t_single` (#2137)

* Add `Result::warn_on_err_once` extension method

* Improve Debug formatting of EntityPath

* Recommend `query_latest_single` and alternative to
`query_entity_with_primary`

* Use query_latest_single in more places

* Use query_latest_single in another place

* Stop requiring EntityView in so many places

* Document that Tensor is a "mono-component"

* Explain how and why Transform is a mono-component

* Use query_latest_single for Transform

* Use query_latest_single in one more place

* Document when to use query_latest_single

* query_latest_single only need data_store, not full EntityDb

* Fix bad docstring

* Clarify "splat" comment

* Simplify bar-chart code

crates/re_data_store/src/entity_properties.rs

@@ -1,8 +1,6 @@
 use re_arrow_store::LatestAtQuery;
 use re_log_types::{DeserializableComponent, EntityPath};
 
-use crate::log_db::EntityDb;
-
 #[cfg(feature = "serde")]
 use crate::EditableAutoValue;
 
@@ -195,8 +193,10 @@ impl Default for ColorMapper {
 ///
 /// This assumes that the row we get from the store only contains a single instance for this
 /// component; it will log a warning otherwise.
+///
+/// This should only be used for "mono-components" such as `Transform` and `Tensor`.
 pub fn query_latest_single<C: DeserializableComponent>(
-    entity_db: &EntityDb,
+    data_store: &re_arrow_store::DataStore,
     entity_path: &EntityPath,
     query: &LatestAtQuery,
 ) -> Option<C>
@@ -208,7 +208,6 @@ where
     // Although it would be nice to use the `re_query` helpers for this, we would need to move
     // this out of re_data_store to avoid a circular dep. Since we don't need to do a join for
     // single components this is easy enough.
-    let data_store = &entity_db.data_store;
 
     let (_, cells) = data_store.latest_at(query, entity_path, C::name(), &[C::name()])?;
     let cell = cells.get(0)?.as_ref()?;

crates/re_data_ui/src/annotation_context.rs

@@ -77,7 +77,8 @@ fn annotation_info(
     query: &re_arrow_store::LatestAtQuery,
     keypoint_id: &re_log_types::component_types::KeypointId,
 ) -> Option<re_log_types::context::AnnotationInfo> {
-    let class_id = re_data_store::query_latest_single(&ctx.log_db.entity_db, entity_path, query)?;
+    let class_id =
+        re_data_store::query_latest_single(&ctx.log_db.entity_db.data_store, entity_path, query)?;
     let annotations = crate::annotations(ctx, query, entity_path);
     let class = annotations
         .class_description(Some(class_id))

crates/re_error/Cargo.toml

@@ -16,5 +16,8 @@ version.workspace = true
 all-features = true
 
 
+[dependencies]
+re_log.workspace = true
+
 [dev-dependencies]
 anyhow.workspace = true

crates/re_error/src/lib.rs

@@ -27,3 +27,20 @@ fn test_format() {
     // Now we do:
     assert_eq!(format(&err), "outer_context -> inner_context -> root_cause");
 }
+
+pub trait ResultExt<T> {
+    fn warn_on_err_once(self, msg: impl std::fmt::Display) -> Option<T>;
+}
+
+impl<T, E: std::fmt::Display> ResultExt<T> for Result<T, E> {
+    /// Log a warning if there is an `Err`, but only log the exact same message once.
+    fn warn_on_err_once(self, msg: impl std::fmt::Display) -> Option<T> {
+        match self {
+            Ok(value) => Some(value),
+            Err(err) => {
+                re_log::warn_once!("{msg}: {err}");
+                None
+            }
+        }
+    }
+}

crates/re_log_types/src/component_types/tensor.rs

@@ -340,6 +340,9 @@ pub enum TensorDataMeaning {
 ///
 /// All clones are shallow.
 ///
+/// The `Tensor` component is special, as you can only have one instance of it per entity.
+/// This is because each element in a tensor is considered to be a separate instance.
+///
 /// ## Examples
 ///
 /// ```

crates/re_log_types/src/component_types/transform.rs

@@ -193,6 +193,14 @@ impl Pinhole {
 
 /// A transform between two spaces.
 ///
+/// The [`Transform`] component is special, as all instances of
+/// an Entity always share the same transform.
+/// In other words, a [`Transform`] component acts like a splat
+/// (all instances of an entity share the same transform).
+/// This is because each entity must have a unique transform chain,
+/// e.g. the entity `foo/bar/baz` is has the transform that is the product of
+/// `foo.transform * foo/bar.transform * foo/bar/baz.transform`.
+///
 /// ```
 /// use re_log_types::component_types::{Transform, Rigid3, Pinhole};
 /// use arrow2_convert::field::ArrowField;

crates/re_log_types/src/path/entity_path_impl.rs

@@ -78,7 +78,9 @@ where
 
 impl std::fmt::Debug for EntityPathImpl {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "EntityPath({self})")
+        // Show the nicely formatted entity path, but surround with quotes and escape
+        // troublesome characters such as back-slashes and newlines.
+        write!(f, "{:?}", self.to_string())
     }
 }
 

crates/re_query/src/query.rs

@@ -78,6 +78,9 @@ pub fn get_component_with_instances(
 /// available, they are implicitly treated as an integer sequence of the correct
 /// length.
 ///
+/// If you expect only one instance (e.g. for mono-components like `Transform` `Tensor`]
+/// and have no additional components you can use [`re_data_store::query_latest_single`] instead.
+///
 /// ```
 /// # use re_arrow_store::LatestAtQuery;
 /// # use re_log_types::{Timeline, component_types::{Point2D, ColorRGBA}, Component};

crates/re_viewer/src/misc/queries.rs

@@ -11,11 +11,13 @@ pub fn closest_pinhole_transform(
 ) -> Option<EntityPath> {
     crate::profile_function!();
 
+    let data_store = &ctx.log_db.entity_db.data_store;
+
     let mut pinhole_ent_path = None;
     let mut cur_path = Some(entity_path.clone());
     while let Some(path) = cur_path {
         if let Some(Transform::Pinhole(_)) =
-            query_latest_single::<Transform>(&ctx.log_db.entity_db, &path, query)
+            query_latest_single::<Transform>(data_store, &path, query)
         {
             pinhole_ent_path = Some(path);
             break;

crates/re_viewer/src/misc/space_info.rs

@@ -4,8 +4,7 @@ use nohash_hasher::IntSet;
 
 use re_arrow_store::{LatestAtQuery, TimeInt, Timeline};
 use re_data_store::{log_db::EntityDb, query_latest_single, EntityPath, EntityTree};
-use re_log_types::{Transform, ViewCoordinates};
-use re_query::query_entity_with_primary;
+use re_log_types::Transform;
 
 use super::UnreachableTransform;
 
@@ -112,7 +111,8 @@ impl SpaceInfoCollection {
             tree: &EntityTree,
             query: &LatestAtQuery,
         ) {
-            if let Some(transform) = query_latest_single::<Transform>(entity_db, &tree.path, query)
+            if let Some(transform) =
+                query_latest_single::<Transform>(&entity_db.data_store, &tree.path, query)
             {
                 // A set transform (likely non-identity) - create a new space.
                 parent_space
@@ -157,7 +157,9 @@ impl SpaceInfoCollection {
         let mut spaces_info = Self::default();
 
         // Start at the root. The root is always part of the collection!
-        if query_latest_single::<Transform>(entity_db, &EntityPath::root(), &query).is_some() {
+        if query_latest_single::<Transform>(&entity_db.data_store, &EntityPath::root(), &query)
+            .is_some()
+        {
             re_log::warn_once!("The root entity has a 'transform' component! This will have no effect. Did you mean to apply the transform elsewhere?");
         }
         let mut root_space_info = SpaceInfo::new(EntityPath::root());
@@ -268,24 +270,3 @@ impl SpaceInfoCollection {
 }
 
 // ----------------------------------------------------------------------------
-
-pub fn query_view_coordinates(
-    entity_db: &EntityDb,
-    ent_path: &EntityPath,
-    query: &LatestAtQuery,
-) -> Option<re_log_types::ViewCoordinates> {
-    let data_store = &entity_db.data_store;
-
-    let entity_view =
-        query_entity_with_primary::<ViewCoordinates>(data_store, query, ent_path, &[]).ok()?;
-
-    let mut iter = entity_view.iter_primary().ok()?;
-
-    let view_coords = iter.next()?;
-
-    if iter.next().is_some() {
-        re_log::warn_once!("Unexpected batch for ViewCoordinates at: {}", ent_path);
-    }
-
-    view_coords
-}

crates/re_viewer/src/misc/transform_cache.rs

@@ -93,7 +93,7 @@ impl TransformCache {
         // Child transforms of this space
         transforms.gather_descendants_transforms(
             current_tree,
-            entity_db,
+            &entity_db.data_store,
             &query,
             entity_prop_map,
             glam::Affine3A::IDENTITY,
@@ -117,7 +117,7 @@ impl TransformCache {
             // Generally, the transform _at_ a node isn't relevant to it's children, but only to get to its parent in turn!
             match transform_at(
                 &current_tree.path,
-                entity_db,
+                &entity_db.data_store,
                 &query,
                 // TODO(#1988): See comment in transform_at. This is a workaround for precision issues
                 // and the fact that there is no meaningful image plane distance for 3D->2D views.
@@ -138,7 +138,7 @@ impl TransformCache {
             // (skip over everything at and under `current_tree` automatically)
             transforms.gather_descendants_transforms(
                 parent_tree,
-                entity_db,
+                &entity_db.data_store,
                 &query,
                 entity_prop_map,
                 reference_from_ancestor,
@@ -154,7 +154,7 @@ impl TransformCache {
     fn gather_descendants_transforms(
         &mut self,
         tree: &EntityTree,
-        entity_db: &EntityDb,
+        data_store: &re_arrow_store::DataStore,
         query: &LatestAtQuery,
         entity_properties: &EntityPropertyMap,
         reference_from_entity: glam::Affine3A,
@@ -176,7 +176,7 @@ impl TransformCache {
             let mut encountered_pinhole = encountered_pinhole;
             let reference_from_child = match transform_at(
                 &child_tree.path,
-                entity_db,
+                data_store,
                 query,
                 |p| *entity_properties.get(p).pinhole_image_plane_distance.get(),
                 &mut encountered_pinhole,
@@ -191,7 +191,7 @@ impl TransformCache {
             };
             self.gather_descendants_transforms(
                 child_tree,
-                entity_db,
+                data_store,
                 query,
                 entity_properties,
                 reference_from_child,
@@ -224,12 +224,12 @@ impl TransformCache {
 
 fn transform_at(
     entity_path: &EntityPath,
-    entity_db: &EntityDb,
+    data_store: &re_arrow_store::DataStore,
     query: &LatestAtQuery,
     pinhole_image_plane_distance: impl Fn(&EntityPath) -> f32,
     encountered_pinhole: &mut bool,
 ) -> Result<Option<glam::Affine3A>, UnreachableTransform> {
-    if let Some(transform) = query_latest_single(entity_db, entity_path, query) {
+    if let Some(transform) = query_latest_single(data_store, entity_path, query) {
         match transform {
             re_log_types::Transform::Rigid3(rigid) => Ok(Some(rigid.parent_from_child().into())),
             // If we're connected via 'unknown' it's not reachable

crates/re_viewer/src/ui/selection_panel.rs

@@ -438,7 +438,7 @@ fn pinhole_props_ui(
 ) {
     let query = ctx.current_query();
     if let Some(re_log_types::Transform::Pinhole(_)) =
-        query_latest_single::<Transform>(&ctx.log_db.entity_db, entity_path, &query)
+        query_latest_single::<Transform>(&ctx.log_db.entity_db.data_store, entity_path, &query)
     {
         ui.label("Image plane distance");
         let mut distance = *entity_props.pinhole_image_plane_distance.get();
@@ -467,7 +467,8 @@ fn depth_props_ui(
     crate::profile_function!();
 
     let query = ctx.current_query();
-    let tensor = query_latest_single::<Tensor>(&ctx.log_db.entity_db, entity_path, &query)?;
+    let tensor =
+        query_latest_single::<Tensor>(&ctx.log_db.entity_db.data_store, entity_path, &query)?;
     if tensor.meaning != TensorDataMeaning::Depth {
         return Some(());
     }