Use gpu picking for points, streamline/share picking code some more

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.
  ///