Process 2d points always in batches (#1820)

crates/re_log_types/src/component_types/point.rs

@@ -64,6 +64,14 @@ impl From<Point2D> for glam::Vec2 {
     }
 }
 
+#[cfg(feature = "glam")]
+impl From<Point2D> for glam::Vec3 {
+    #[inline]
+    fn from(pt: Point2D) -> Self {
+        Self::new(pt.x, pt.y, 0.0)
+    }
+}
+
 /// A point in 3D space.
 ///
 /// ```

crates/re_renderer/src/point_cloud_builder.rs

@@ -214,34 +214,6 @@ impl<'a> PointCloudBatchBuilder<'a> {
         }
     }
 
-    #[inline]
-    pub fn add_point(&mut self, position: glam::Vec3) -> PointBuilder<'_> {
-        self.extend_defaults();
-
-        debug_assert_eq!(self.0.vertices.len(), self.0.color_buffer.num_written());
-
-        let vertex_index = self.0.vertices.len() as u32;
-        self.0.vertices.push(PointCloudVertex {
-            position,
-            radius: Size::AUTO,
-        });
-        self.batch_mut().point_count += 1;
-
-        PointBuilder {
-            vertex: self.0.vertices.last_mut().unwrap(),
-            color: &mut self.0.color_buffer,
-            picking_instance_id: &mut self.0.picking_instance_ids_buffer,
-            vertex_index,
-            additional_outline_mask_ids: &mut self
-                .0
-                .batches
-                .last_mut()
-                .unwrap()
-                .additional_outline_mask_ids_vertex_ranges,
-            outline_mask_id: OutlineMaskPreference::NONE,
-        }
-    }
-
     /// Adds several 2D points. Uses an autogenerated depth value, the same for all points passed.
     ///
     /// Params:
@@ -257,12 +229,6 @@ impl<'a> PointCloudBatchBuilder<'a> {
         self.add_points(size_hint, positions.map(|p| p.extend(0.0)))
     }
 
-    /// Adds a single 2D point. Uses an autogenerated depth value.
-    #[inline]
-    pub fn add_point_2d(&mut self, position: glam::Vec2) -> PointBuilder<'_> {
-        self.add_point(position.extend(0.0))
-    }
-
     /// Set flags for this batch.
     pub fn flags(mut self, flags: PointCloudBatchFlags) -> Self {
         self.batch_mut().flags = flags;
@@ -275,59 +241,6 @@ impl<'a> PointCloudBatchBuilder<'a> {
     }
 }
 
-// TODO(andreas): Should remove single-point builder, practically this never makes sense as we're almost always dealing with arrays of points.
-pub struct PointBuilder<'a> {
-    vertex: &'a mut PointCloudVertex,
-    color: &'a mut CpuWriteGpuReadBuffer<Color32>,
-    picking_instance_id: &'a mut CpuWriteGpuReadBuffer<PickingLayerInstanceId>,
-    vertex_index: u32,
-
-    additional_outline_mask_ids: &'a mut Vec<(std::ops::Range<u32>, OutlineMaskPreference)>,
-    outline_mask_id: OutlineMaskPreference,
-}
-
-impl<'a> PointBuilder<'a> {
-    #[inline]
-    pub fn radius(self, radius: Size) -> Self {
-        self.vertex.radius = radius;
-        self
-    }
-
-    /// This mustn't call this more than once.
-    #[inline]
-    pub fn color(self, color: Color32) -> Self {
-        self.color.push(color);
-        self
-    }
-
-    /// Pushes additional outline mask ids for this point
-    ///
-    /// Prefer the `overall_outline_mask_ids` setting to set the outline mask ids for the entire batch whenever possible!
-    #[inline]
-    pub fn outline_mask_id(mut self, outline_mask_id: OutlineMaskPreference) -> Self {
-        self.outline_mask_id = outline_mask_id;
-        self
-    }
-
-    /// This mustn't call this more than once.
-    #[inline]
-    pub fn picking_instance_id(self, picking_instance_id: PickingLayerInstanceId) -> Self {
-        self.picking_instance_id.push(picking_instance_id);
-        self
-    }
-}
-
-impl<'a> Drop for PointBuilder<'a> {
-    fn drop(&mut self) {
-        if self.outline_mask_id.is_some() {
-            self.additional_outline_mask_ids.push((
-                self.vertex_index..self.vertex_index + 1,
-                self.outline_mask_id,
-            ));
-        }
-    }
-}
-
 pub struct PointsBuilder<'a> {
     // Vertices is a slice, which radii will update
     vertices: &'a mut [PointCloudVertex],

crates/re_viewer/src/ui/view_spatial/scene/mod.rs

@@ -172,7 +172,7 @@ impl SceneSpatial {
             // --
             // Note: Lines2DPart handles both Segments and LinesPaths since they are unified on the logging-side.
             &scene_part::Lines2DPart,
-            &scene_part::Points2DPart,
+            &scene_part::Points2DPart { max_labels: 10 },
             // ---
             &scene_part::CamerasPart,
         ];

crates/re_viewer/src/ui/view_spatial/scene/scene_part/mod.rs

@@ -11,6 +11,9 @@ mod meshes;
 mod points2d;
 mod points3d;
 
+use std::sync::Arc;
+
+use ahash::HashMap;
 pub(crate) use arrows3d::Arrows3DPart;
 pub(crate) use boxes2d::Boxes2DPart;
 pub(crate) use boxes3d::Boxes3DPart;
@@ -21,11 +24,15 @@ pub(crate) use lines3d::Lines3DPart;
 pub(crate) use meshes::MeshPart;
 pub(crate) use points2d::Points2DPart;
 pub(crate) use points3d::Points3DPart;
+use re_log_types::component_types::{ClassId, ColorRGBA, KeypointId, Radius};
 
 use super::SceneSpatial;
 use crate::{
     misc::{SpaceViewHighlights, TransformCache, ViewerContext},
-    ui::scene::SceneQuery,
+    ui::{
+        annotations::ResolvedAnnotationInfo, scene::SceneQuery, view_spatial::scene::Keypoints,
+        Annotations, DefaultColor,
+    },
 };
 use re_data_store::{EntityPath, EntityProperties, InstancePathHash};
 
@@ -94,3 +101,103 @@ pub fn instance_key_to_picking_id<C: re_log_types::Component>(
         instance_key_for_picking(instance_key, entity_view, any_part_selected).0,
     )
 }
+
+/// Process [`ColorRGBA`] components using annotations and default colors.
+pub fn process_colors<'a, Primary>(
+    entity_view: &'a re_query::EntityView<Primary>,
+    ent_path: &'a EntityPath,
+    annotation_infos: &'a [ResolvedAnnotationInfo],
+) -> Result<impl Iterator<Item = egui::Color32> + 'a, re_query::QueryError>
+where
+    Primary: re_log_types::SerializableComponent + re_log_types::DeserializableComponent,
+    for<'b> &'b Primary::ArrayType: IntoIterator,
+{
+    crate::profile_function!();
+    let default_color = DefaultColor::EntityPath(ent_path);
+
+    Ok(itertools::izip!(
+        annotation_infos.iter(),
+        entity_view.iter_component::<ColorRGBA>()?,
+    )
+    .map(move |(annotation_info, color)| {
+        annotation_info.color(color.map(move |c| c.to_array()).as_ref(), default_color)
+    }))
+}
+
+/// Process [`Radius`] components to [`re_renderer::Size`] using auto size where no radius is specified.
+pub fn process_radii<'a, Primary>(
+    ent_path: &EntityPath,
+    entity_view: &'a re_query::EntityView<Primary>,
+) -> Result<impl Iterator<Item = re_renderer::Size> + 'a, re_query::QueryError>
+where
+    Primary: re_log_types::SerializableComponent + re_log_types::DeserializableComponent,
+    for<'b> &'b Primary::ArrayType: IntoIterator,
+{
+    crate::profile_function!();
+    let ent_path = ent_path.clone();
+    Ok(entity_view.iter_component::<Radius>()?.map(move |radius| {
+        radius.map_or(re_renderer::Size::AUTO, |r| {
+            if 0.0 <= r.0 && r.0.is_finite() {
+                re_renderer::Size::new_scene(r.0)
+            } else {
+                if r.0 < 0.0 {
+                    re_log::warn_once!("Found negative radius in entity {ent_path}");
+                } else if r.0.is_infinite() {
+                    re_log::warn_once!("Found infinite radius in entity {ent_path}");
+                } else {
+                    re_log::warn_once!("Found NaN radius in entity {ent_path}");
+                }
+                re_renderer::Size::AUTO
+            }
+        })
+    }))
+}
+
+/// Resolves all annotations and keypoints for the given entity view.
+fn process_annotations_and_keypoints<Primary>(
+    query: &SceneQuery<'_>,
+    entity_view: &re_query::EntityView<Primary>,
+    annotations: &Arc<Annotations>,
+) -> Result<(Vec<ResolvedAnnotationInfo>, super::Keypoints), re_query::QueryError>
+where
+    Primary: re_log_types::SerializableComponent + re_log_types::DeserializableComponent,
+    for<'b> &'b Primary::ArrayType: IntoIterator,
+    glam::Vec3: std::convert::From<Primary>,
+{
+    crate::profile_function!();
+
+    let mut keypoints: Keypoints = HashMap::default();
+
+    // No need to process annotations if we don't have keypoints or class-ids
+    if !entity_view.has_component::<KeypointId>() && !entity_view.has_component::<ClassId>() {
+        let resolved_annotation = annotations.class_description(None).annotation_info();
+        return Ok((
+            vec![resolved_annotation; entity_view.num_instances()],
+            keypoints,
+        ));
+    }
+
+    let annotation_info = itertools::izip!(
+        entity_view.iter_primary()?,
+        entity_view.iter_component::<KeypointId>()?,
+        entity_view.iter_component::<ClassId>()?,
+    )
+    .map(|(position, keypoint_id, class_id)| {
+        let class_description = annotations.class_description(class_id);
+
+        if let (Some(keypoint_id), Some(class_id), Some(position)) =
+            (keypoint_id, class_id, position)
+        {
+            keypoints
+                .entry((class_id, query.latest_at.as_i64()))
+                .or_insert_with(Default::default)
+                .insert(keypoint_id, position.into());
+            class_description.annotation_info_with_keypoint(keypoint_id)
+        } else {
+            class_description.annotation_info()
+        }
+    })
+    .collect();
+
+    Ok((annotation_info, keypoints))
+}