From 0de0edf934992ae0952370f60b7a844712f07436 Mon Sep 17 00:00:00 2001
From: Andreas Reich <andreas@rerun.io>
Date: Wed, 19 Apr 2023 12:14:00 +0200
Subject: [PATCH] Fix picking entities with image + another object (or label)
 twice (#1908)

* Fix picking entities with image + another object twice

* also avoid duplicate hits with ui labels
---
 .../src/ui/view_spatial/scene/picking.rs      | 42 +++++++++++++++----
 crates/re_viewer/src/ui/view_spatial/ui.rs    | 19 ++++++---
 2 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/crates/re_viewer/src/ui/view_spatial/scene/picking.rs b/crates/re_viewer/src/ui/view_spatial/scene/picking.rs
index 3feb71285d4..ddf73c2fdfb 100644
--- a/crates/re_viewer/src/ui/view_spatial/scene/picking.rs
+++ b/crates/re_viewer/src/ui/view_spatial/scene/picking.rs
@@ -1,5 +1,6 @@
 //! Handles picking in 2D & 3D spaces.
 
+use ahash::HashSet;
 use re_data_store::InstancePathHash;
 use re_log_types::{component_types::InstanceKey, EntityPathHash};
 use re_renderer::PickingLayerProcessor;
@@ -108,27 +109,52 @@ impl PickingContext {
     ) -> PickingResult {
         crate::profile_function!();
 
-        let mut hits = Vec::new();
-
-        // Start with gpu based picking as baseline. This is our prime source of picking information.
-        hits.extend(picking_gpu(
+        // Gather picking results from different sources.
+        let gpu_pick = picking_gpu(
             render_ctx,
             gpu_readback_identifier,
             self,
             previous_picking_result,
-        ));
-
-        // We also never throw away any textured rects, even if they're behind other objects.
+        );
         let mut rect_hits = picking_textured_rects(
             self,
             &primitives.textured_rectangles,
             &primitives.textured_rectangles_ids,
         );
         rect_hits.sort_by(|a, b| b.depth_offset.cmp(&a.depth_offset));
+        let ui_rect_hits = picking_ui_rects(self, ui_data);
+
+        let mut hits = Vec::new();
+
+        // Start with gpu based picking as baseline. This is our prime source of picking information.
+        //
+        // ..unless the same object got also picked as part of a textured rect.
+        // Textured rect picks also know where on the rect, making this the better source!
+        // Note that whenever this happens, it means that the same object path has a textured rect and something else
+        // e.g. a camera.
+        if let Some(gpu_pick) = gpu_pick {
+            if rect_hits.iter().all(|rect_hit| {
+                rect_hit.instance_path_hash.entity_path_hash
+                    != gpu_pick.instance_path_hash.entity_path_hash
+            }) {
+                hits.push(gpu_pick);
+            }
+        }
+
+        // We never throw away any textured rects, even if they're behind other objects.
         hits.extend(rect_hits);
 
         // UI rects are overlaid on top, but we don't let them hide other picking results either.
-        hits.extend(picking_ui_rects(self, ui_data));
+        // Give any other previous hits precedence.
+        let previously_hit_objects: HashSet<_> = hits
+            .iter()
+            .map(|prev_hit| prev_hit.instance_path_hash)
+            .collect();
+        hits.extend(
+            ui_rect_hits
+                .into_iter()
+                .filter(|ui_hit| !previously_hit_objects.contains(&ui_hit.instance_path_hash)),
+        );
 
         PickingResult { hits }
     }
diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs
index f0c8e14ac30..49209bb1b32 100644
--- a/crates/re_viewer/src/ui/view_spatial/ui.rs
+++ b/crates/re_viewer/src/ui/view_spatial/ui.rs
@@ -760,13 +760,20 @@ pub fn picking(
                 .iter()
                 .find(|image| image.ent_path == instance_path.entity_path)
                 .and_then(|image| {
+                    // If we're here because of back-projection, but this wasn't actually a depth image, drop out.
+                    // (the back-projection property may be true despite this not being a depth image!)
+                    if hit.hit_type != PickingHitType::TexturedRect
+                        && *ent_properties.backproject_depth.get()
+                        && image.tensor.meaning != TensorDataMeaning::Depth
+                    {
+                        return None;
+                    }
                     image.tensor.image_height_width_channels().map(|[_, w, _]| {
-                        (
-                            image,
-                            hit.instance_path_hash
-                                .instance_key
-                                .to_2d_image_coordinate(w),
-                        )
+                        let coordinates = hit
+                            .instance_path_hash
+                            .instance_key
+                            .to_2d_image_coordinate(w);
+                        (image, coordinates)
                     })
                 })
         } else {