fix temporal gaps in gpu picking report

Author: Wumpf

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

@@ -247,10 +247,12 @@ impl SceneSpatial {
         SpatialNavigationMode::ThreeD
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn picking(
         &self,
         render_ctx: &re_renderer::RenderContext,
         gpu_readback_identifier: re_renderer::GpuReadbackIdentifier,
+        previous_picking_result: &Option<PickingResult>,
         pointer_in_ui: glam::Vec2,
         ui_rect: &egui::Rect,
         eye: &Eye,
@@ -259,6 +261,7 @@ impl SceneSpatial {
         picking::picking(
             render_ctx,
             gpu_readback_identifier,
+            previous_picking_result,
             pointer_in_ui,
             ui_rect,
             eye,

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

@@ -10,6 +10,7 @@ use crate::{
     ui::view_spatial::eye::Eye,
 };
 
+#[derive(Clone)]
 pub enum AdditionalPickingInfo {
     /// No additional picking information.
     None,
@@ -21,6 +22,7 @@ pub enum AdditionalPickingInfo {
     GuiOverlay,
 }
 
+#[derive(Clone)]
 pub struct PickingRayHit {
     /// What entity or instance got hit by the picking ray.
     ///
@@ -34,6 +36,9 @@ pub struct PickingRayHit {
 
     /// Any additional information about the picking hit.
     pub info: AdditionalPickingInfo,
+
+    /// True if this picking result came from a GPU picking pass.
+    pub used_gpu_picking: bool,
 }
 
 impl PickingRayHit {
@@ -43,10 +48,12 @@ impl PickingRayHit {
             ray_t: t,
             info: AdditionalPickingInfo::None,
             depth_offset: 0,
+            used_gpu_picking: false,
         }
     }
 }
 
+#[derive(Clone)]
 pub struct PickingResult {
     /// Picking ray hit for an opaque object (if any).
     pub opaque_hit: Option<PickingRayHit>,
@@ -131,11 +138,11 @@ impl PickingState {
     }
 }
 
-// TODO(andreas): This should be temporary. As we switch over (almost) everything over to gpu based picking this should go away.
 #[allow(clippy::too_many_arguments)]
 pub fn picking(
     render_ctx: &re_renderer::RenderContext,
     gpu_readback_identifier: re_renderer::GpuReadbackIdentifier,
+    previous_picking_result: &Option<PickingResult>,
     pointer_in_ui: glam::Vec2,
     ui_rect: &egui::Rect,
     eye: &Eye,
@@ -160,6 +167,7 @@ pub fn picking(
             ray_t: f32::INFINITY,
             info: AdditionalPickingInfo::None,
             depth_offset: 0,
+            used_gpu_picking: false,
         },
         // Combined, sorted (and partially "hidden") by opaque results later.
         transparent_hits: Vec::new(),
@@ -206,9 +214,20 @@ pub fn picking(
 
             // TODO(andreas): We're lacking depth information!
             state.closest_opaque_pick.instance_path_hash = picked_object;
+            state.closest_opaque_pick.used_gpu_picking = true;
         } else {
-            // TODO(andreas): It is *possible* that some frames we don't get a picking result and the frame after we get several.
+            // It is possible that some frames we don't get a picking result and the frame after we get several.
             // We need to cache the last picking result and use it until we get a new one or the mouse leaves the screen.
+            // (Andreas: On my mac this *actually* happens in very simple scenes, I get occasional frames with 0 and then with 2 picking results!)
+            if let Some(PickingResult {
+                opaque_hit: Some(previous_opaque_hit),
+                ..
+            }) = previous_picking_result
+            {
+                if previous_opaque_hit.used_gpu_picking {
+                    state.closest_opaque_pick = previous_opaque_hit.clone();
+                }
+            }
         }
     }
 
@@ -368,6 +387,7 @@ fn picking_textured_rects(
                 ray_t: t,
                 info: AdditionalPickingInfo::TexturedRect(glam::vec2(u, v)),
                 depth_offset: rect.depth_offset,
+                used_gpu_picking: false,
             };
             state.check_hit(0.0, picking_hit, rect.multiplicative_tint.a() < 1.0);
         }
@@ -391,6 +411,7 @@ fn picking_ui_rects(
                 ray_t: 0.0,
                 info: AdditionalPickingInfo::GuiOverlay,
                 depth_offset: 0,
+                used_gpu_picking: false,
             },
             false,
         );

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

@@ -18,8 +18,11 @@ use crate::{
 };
 
 use super::{
-    eye::Eye, scene::SceneSpatialUiData, ui_2d::View2DState, ui_3d::View3DState, SceneSpatial,
-    SpaceSpecs,
+    eye::Eye,
+    scene::{PickingResult, SceneSpatialUiData},
+    ui_2d::View2DState,
+    ui_3d::View3DState,
+    SceneSpatial, SpaceSpecs,
 };
 
 /// Describes how the scene is navigated, determining if it is a 2D or 3D experience.
@@ -77,6 +80,10 @@ pub struct ViewSpatialState {
     #[serde(skip)]
     pub scene_num_primitives: usize,
 
+    /// Last frame's picking result.
+    #[serde(skip)]
+    pub previous_picking_result: Option<PickingResult>,
+
     pub(super) state_2d: View2DState,
     pub(super) state_3d: View3DState,
 
@@ -97,6 +104,7 @@ impl Default for ViewSpatialState {
                 point_radius: re_renderer::Size::AUTO, // let re_renderer decide
                 line_radius: re_renderer::Size::AUTO,  // let re_renderer decide
             },
+            previous_picking_result: None,
         }
     }
 }

crates/re_viewer/src/ui/view_spatial/ui_2d.rs

@@ -367,11 +367,13 @@ fn view_2d_scrollable(
         let picking_result = scene.picking(
             ctx.render_ctx,
             space_view_id.gpu_readback_id(),
+            &state.previous_picking_result,
             glam::vec2(pointer_pos_space.x, pointer_pos_space.y),
             &scene_rect_accum,
             &eye,
             hover_radius,
         );
+        state.previous_picking_result = Some(picking_result.clone());
 
         for hit in picking_result.iter_hits() {
             let Some(instance_path) = hit.instance_path_hash.resolve(&ctx.log_db.entity_db)
@@ -462,6 +464,8 @@ fn view_2d_scrollable(
                     .map(|instance_path| Item::InstancePath(Some(space_view_id), instance_path))
             }));
         }
+    } else {
+        state.previous_picking_result = None;
     }
 
     ctx.select_hovered_on_click(&response);

crates/re_viewer/src/ui/view_spatial/ui_3d.rs

@@ -377,11 +377,13 @@ pub fn view_3d(
         let picking_result = scene.picking(
             ctx.render_ctx,
             space_view_id.gpu_readback_id(),
+            &state.previous_picking_result,
             glam::vec2(pointer_pos.x, pointer_pos.y),
             &rect,
             &eye,
             5.0,
         );
+        state.previous_picking_result = Some(picking_result.clone());
 
         for hit in picking_result.iter_hits() {
             let Some(instance_path) = hit.instance_path_hash.resolve(&ctx.log_db.entity_db)
@@ -459,6 +461,8 @@ pub fn view_3d(
             .map(|hit| picking_result.space_position(hit));
 
         project_onto_other_spaces(ctx, &scene.space_cameras, &mut state.state_3d, space);
+    } else {
+        state.previous_picking_result = None;
     }
 
     ctx.select_hovered_on_click(&response);