Skip to content

Commit

Permalink
Fix picking entities with image + another object (or label) twice (#1908
Browse files Browse the repository at this point in the history
)

* Fix picking entities with image + another object twice

* also avoid duplicate hits with ui labels
  • Loading branch information
Wumpf committed Apr 19, 2023
1 parent 6127e02 commit 0de0edf
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 14 deletions.
42 changes: 34 additions & 8 deletions crates/re_viewer/src/ui/view_spatial/scene/picking.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 }
}
Expand Down
19 changes: 13 additions & 6 deletions crates/re_viewer/src/ui/view_spatial/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 0de0edf

Please sign in to comment.