Use gpu picking for points, streamline/share picking code some more (#1814)

* use gpu picking for picking points
* gpu based picking no longer works like a fallback but integrates with other picking sources
* fix incorrect cursor rounding for picking
* refactor picking context to be a pub struct with exposed state
* unify ui picking method for 2d & 3d space views
* less indentation for picking method
* picking rect size is dynamically chosen
* fix accidental z scaling in projection correction for picking & make cropped_projection_from_projection easier to read

Author: Wumpf

crates/re_renderer/examples/2d.rs

@@ -149,7 +149,7 @@ impl framework::Example for Render2D {
         // Moving the windows to a high dpi screen makes the second one bigger.
         // Also, it looks different under perspective projection.
         // The third point is automatic thickness which is determined by the point renderer implementation.
-        let mut point_cloud_builder = PointCloudBuilder::<()>::new(re_ctx);
+        let mut point_cloud_builder = PointCloudBuilder::new(re_ctx);
         point_cloud_builder
             .batch("points")
             .add_points_2d(

crates/re_renderer/examples/depth_cloud.rs

@@ -98,7 +98,7 @@ impl RenderDepthClouds {
                 })
                 .multiunzip();
 
-            let mut builder = PointCloudBuilder::<()>::new(re_ctx);
+            let mut builder = PointCloudBuilder::new(re_ctx);
             builder
                 .batch("backprojected point cloud")
                 .add_points(num_points as _, points.into_iter())

crates/re_renderer/examples/framework.rs

@@ -210,10 +210,10 @@ impl<E: Example + 'static> Application<E> {
                 Event::WindowEvent {
                     event: WindowEvent::CursorMoved { position, .. },
                     ..
-                } => self.example.on_cursor_moved(glam::uvec2(
-                    position.x.round() as u32,
-                    position.y.round() as u32,
-                )),
+                } => self
+                    .example
+                    // Don't round the position: The entire range from 0 to excluding 1 should fall into pixel coordinate 0!
+                    .on_cursor_moved(glam::uvec2(position.x as u32, position.y as u32)),
                 Event::WindowEvent {
                     event:
                         WindowEvent::ScaleFactorChanged {

crates/re_renderer/examples/multiview.rs

@@ -316,7 +316,7 @@ impl Example for Multiview {
         let skybox = GenericSkyboxDrawData::new(re_ctx);
         let lines = build_lines(re_ctx, seconds_since_startup);
 
-        let mut builder = PointCloudBuilder::<()>::new(re_ctx);
+        let mut builder = PointCloudBuilder::new(re_ctx);
         builder
             .batch("Random Points")
             .world_from_obj(glam::Mat4::from_rotation_x(seconds_since_startup))

crates/re_renderer/examples/picking.rs

@@ -157,7 +157,7 @@ impl framework::Example for Picking {
             .schedule_picking_rect(re_ctx, picking_rect, READBACK_IDENTIFIER, (), false)
             .unwrap();
 
-        let mut point_builder = PointCloudBuilder::<()>::new(re_ctx);
+        let mut point_builder = PointCloudBuilder::new(re_ctx);
         for (i, point_set) in self.point_sets.iter().enumerate() {
             point_builder
                 .batch(format!("Random Points {i}"))

crates/re_renderer/src/draw_phases/picking_layer.rs

@@ -224,18 +224,18 @@ impl PickingLayerProcessor {
             DepthReadbackWorkaround::new(ctx, picking_rect.extent, picking_depth_target.handle)
         });
 
-        let rect_min = picking_rect.top_left_corner.as_vec2();
+        let rect_min = picking_rect.left_top.as_vec2();
         let rect_max = rect_min + picking_rect.extent.as_vec2();
         let screen_resolution = screen_resolution.as_vec2();
         // y axis is flipped in NDC, therefore we need to flip the y axis of the rect.
         let rect_min_ndc =
             pixel_coord_to_ndc(glam::vec2(rect_min.x, rect_max.y), screen_resolution);
         let rect_max_ndc =
             pixel_coord_to_ndc(glam::vec2(rect_max.x, rect_min.y), screen_resolution);
-        let rect_center_ndc = (rect_min_ndc + rect_max_ndc) * 0.5;
-        let cropped_projection_from_projection =
-            glam::Mat4::from_scale(2.0 / (rect_max_ndc - rect_min_ndc).extend(1.0))
-                * glam::Mat4::from_translation(-rect_center_ndc.extend(0.0));
+        let scale = 2.0 / (rect_max_ndc - rect_min_ndc);
+        let translation = -0.5 * (rect_min_ndc + rect_max_ndc);
+        let cropped_projection_from_projection = glam::Mat4::from_scale(scale.extend(1.0))
+            * glam::Mat4::from_translation(translation.extend(0.0));
 
         // Setup frame uniform buffer
         let previous_projection_from_world: glam::Mat4 =

crates/re_renderer/src/point_cloud_builder.rs

@@ -9,23 +9,19 @@ use crate::{
 };
 
 /// Builder for point clouds, making it easy to create [`crate::renderer::PointCloudDrawData`].
-pub struct PointCloudBuilder<PerPointUserData> {
-    // Size of `point`/color`/`per_point_user_data` must be equal.
+pub struct PointCloudBuilder {
+    // Size of `point`/color` must be equal.
     pub vertices: Vec<PointCloudVertex>,
 
     pub(crate) color_buffer: CpuWriteGpuReadBuffer<Color32>,
     pub(crate) picking_instance_ids_buffer: CpuWriteGpuReadBuffer<PickingLayerInstanceId>,
-    pub user_data: Vec<PerPointUserData>,
 
     pub(crate) batches: Vec<PointCloudBatchInfo>,
 
     pub(crate) radius_boost_in_ui_points_for_outlines: f32,
 }
 
-impl<PerPointUserData> PointCloudBuilder<PerPointUserData>
-where
-    PerPointUserData: Default + Copy,
-{
+impl PointCloudBuilder {
     pub fn new(ctx: &RenderContext) -> Self {
         const RESERVE_SIZE: usize = 512;
 
@@ -48,7 +44,6 @@ where
             vertices: Vec::with_capacity(RESERVE_SIZE),
             color_buffer,
             picking_instance_ids_buffer,
-            user_data: Vec::with_capacity(RESERVE_SIZE),
             batches: Vec::with_capacity(16),
             radius_boost_in_ui_points_for_outlines: 0.0,
         }
@@ -65,10 +60,7 @@ where
 
     /// Start of a new batch.
     #[inline]
-    pub fn batch(
-        &mut self,
-        label: impl Into<DebugLabel>,
-    ) -> PointCloudBatchBuilder<'_, PerPointUserData> {
+    pub fn batch(&mut self, label: impl Into<DebugLabel>) -> PointCloudBatchBuilder<'_> {
         self.batches.push(PointCloudBatchInfo {
             label: label.into(),
             world_from_obj: glam::Mat4::IDENTITY,
@@ -105,30 +97,6 @@ where
         })
     }
 
-    // Iterate over all batches, yielding the batch info and a point vertex iterator zipped with its user data.
-    pub fn iter_vertices_and_userdata_by_batch(
-        &self,
-    ) -> impl Iterator<
-        Item = (
-            &PointCloudBatchInfo,
-            impl Iterator<Item = (&PointCloudVertex, &PerPointUserData)>,
-        ),
-    > {
-        let mut vertex_offset = 0;
-        self.batches.iter().map(move |batch| {
-            let out = (
-                batch,
-                self.vertices
-                    .iter()
-                    .zip(self.user_data.iter())
-                    .skip(vertex_offset)
-                    .take(batch.point_count as usize),
-            );
-            vertex_offset += batch.point_count as usize;
-            out
-        })
-    }
-
     /// Finalizes the builder and returns a point cloud draw data with all the points added so far.
     pub fn to_draw_data(
         self,
@@ -138,16 +106,9 @@ where
     }
 }
 
-pub struct PointCloudBatchBuilder<'a, PerPointUserData>(
-    &'a mut PointCloudBuilder<PerPointUserData>,
-)
-where
-    PerPointUserData: Default + Copy;
+pub struct PointCloudBatchBuilder<'a>(&'a mut PointCloudBuilder);
 
-impl<'a, PerPointUserData> Drop for PointCloudBatchBuilder<'a, PerPointUserData>
-where
-    PerPointUserData: Default + Copy,
-{
+impl<'a> Drop for PointCloudBatchBuilder<'a> {
     fn drop(&mut self) {
         // Remove batch again if it wasn't actually used.
         if self.0.batches.last().unwrap().point_count == 0 {
@@ -157,10 +118,7 @@ where
     }
 }
 
-impl<'a, PerPointUserData> PointCloudBatchBuilder<'a, PerPointUserData>
-where
-    PerPointUserData: Default + Copy,
-{
+impl<'a> PointCloudBatchBuilder<'a> {
     #[inline]
     fn batch_mut(&mut self) -> &mut PointCloudBatchInfo {
         self.0
@@ -200,13 +158,6 @@ where
                     self.0.vertices.len() - self.0.picking_instance_ids_buffer.num_written(),
                 ));
         }
-
-        if self.0.user_data.len() < self.0.vertices.len() {
-            self.0.user_data.extend(
-                std::iter::repeat(PerPointUserData::default())
-                    .take(self.0.vertices.len() - self.0.user_data.len()),
-            );
-        }
     }
 
     #[inline]
@@ -222,7 +173,7 @@ where
         &mut self,
         size_hint: usize,
         positions: impl Iterator<Item = glam::Vec3>,
-    ) -> PointsBuilder<'_, PerPointUserData> {
+    ) -> PointsBuilder<'_> {
         // TODO(jleibs): Figure out if we can plumb-through proper support for `Iterator::size_hints()`
         // or potentially make `FixedSizedIterator` work correctly. This should be possible size the
         // underlying arrow structures are of known-size, but carries some complexity with the amount of
@@ -232,7 +183,6 @@ where
         self.extend_defaults();
 
         debug_assert_eq!(self.0.vertices.len(), self.0.color_buffer.num_written());
-        debug_assert_eq!(self.0.vertices.len(), self.0.user_data.len());
 
         let old_size = self.0.vertices.len();
 
@@ -245,8 +195,6 @@ where
         let num_points = self.0.vertices.len() - old_size;
         self.batch_mut().point_count += num_points as u32;
 
-        self.0.user_data.reserve(num_points);
-
         let new_range = old_size..self.0.vertices.len();
 
         let max_points = self.0.vertices.len();
@@ -256,7 +204,6 @@ where
             max_points,
             colors: &mut self.0.color_buffer,
             picking_instance_ids: &mut self.0.picking_instance_ids_buffer,
-            user_data: &mut self.0.user_data,
             additional_outline_mask_ids: &mut self
                 .0
                 .batches
@@ -268,24 +215,22 @@ where
     }
 
     #[inline]
-    pub fn add_point(&mut self, position: glam::Vec3) -> PointBuilder<'_, PerPointUserData> {
+    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());
-        debug_assert_eq!(self.0.vertices.len(), self.0.user_data.len());
 
         let vertex_index = self.0.vertices.len() as u32;
         self.0.vertices.push(PointCloudVertex {
             position,
             radius: Size::AUTO,
         });
-        self.0.user_data.push(Default::default());
         self.batch_mut().point_count += 1;
 
         PointBuilder {
             vertex: self.0.vertices.last_mut().unwrap(),
             color: &mut self.0.color_buffer,
-            user_data: self.0.user_data.last_mut().unwrap(),
+            picking_instance_id: &mut self.0.picking_instance_ids_buffer,
             vertex_index,
             additional_outline_mask_ids: &mut self
                 .0
@@ -308,13 +253,13 @@ where
         &mut self,
         size_hint: usize,
         positions: impl Iterator<Item = glam::Vec2>,
-    ) -> PointsBuilder<'_, PerPointUserData> {
+    ) -> PointsBuilder<'_> {
         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<'_, PerPointUserData> {
+    pub fn add_point_2d(&mut self, position: glam::Vec2) -> PointBuilder<'_> {
         self.add_point(position.extend(0.0))
     }
 
@@ -331,19 +276,17 @@ where
 }
 
 // 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, PerPointUserData> {
+pub struct PointBuilder<'a> {
     vertex: &'a mut PointCloudVertex,
     color: &'a mut CpuWriteGpuReadBuffer<Color32>,
-    user_data: &'a mut PerPointUserData,
+    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, PerPointUserData> PointBuilder<'a, PerPointUserData>
-where
-    PerPointUserData: Clone,
-{
+impl<'a> PointBuilder<'a> {
     #[inline]
     pub fn radius(self, radius: Size) -> Self {
         self.vertex.radius = radius;
@@ -357,21 +300,24 @@ where
         self
     }
 
-    pub fn user_data(self, data: PerPointUserData) -> Self {
-        *self.user_data = data;
-        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, PerPointUserData> Drop for PointBuilder<'a, PerPointUserData> {
+impl<'a> Drop for PointBuilder<'a> {
     fn drop(&mut self) {
         if self.outline_mask_id.is_some() {
             self.additional_outline_mask_ids.push((
@@ -382,21 +328,17 @@ impl<'a, PerPointUserData> Drop for PointBuilder<'a, PerPointUserData> {
     }
 }
 
-pub struct PointsBuilder<'a, PerPointUserData> {
+pub struct PointsBuilder<'a> {
     // Vertices is a slice, which radii will update
     vertices: &'a mut [PointCloudVertex],
     max_points: usize,
     colors: &'a mut CpuWriteGpuReadBuffer<Color32>,
     picking_instance_ids: &'a mut CpuWriteGpuReadBuffer<PickingLayerInstanceId>,
-    user_data: &'a mut Vec<PerPointUserData>,
     additional_outline_mask_ids: &'a mut Vec<(std::ops::Range<u32>, OutlineMaskPreference)>,
     start_vertex_index: u32,
 }
 
-impl<'a, PerPointUserData> PointsBuilder<'a, PerPointUserData>
-where
-    PerPointUserData: Clone,
-{
+impl<'a> PointsBuilder<'a> {
     /// Assigns radii to all points.
     ///
     /// This mustn't call this more than once.
@@ -440,19 +382,6 @@ where
         self
     }
 
-    /// Assigns user data for all points in this builder.
-    ///
-    /// This mustn't call this more than once.
-    ///
-    /// User data is currently not available on the GPU.
-    #[inline]
-    pub fn user_data(self, data: impl Iterator<Item = PerPointUserData>) -> Self {
-        crate::profile_function!();
-        self.user_data
-            .extend(data.take(self.max_points - self.user_data.len()));
-        self
-    }
-
     /// Pushes additional outline mask ids for a specific range of points.
     /// The range is relative to this builder's range, not the entire batch.
     ///