Skip to content

Commit

Permalink
Double-click to select entity (#2504)
Browse files Browse the repository at this point in the history
Closes #2214

Hovering a point cloud now hovers individual points. Clicking once
selects a point (a single instance). Double-clicking selects the entire
point-cloud (the entire entity).

### What
With this PR, we always set the full instance key in the GPU picking
buffer. When reading we decide whether to look at both entity and
instance (hover/click single instances) or to only look at the entity
path (select whole point-cloud on double-click).

Previously what was put in the GPU picking buffer depended on what was
already selected. The new code is a lot simpler.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2504

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/c7f9332/docs
Examples preview: https://rerun.io/preview/c7f9332/examples
<!-- pr-link-docs:end -->
  • Loading branch information
emilk committed Jun 26, 2023
1 parent 7eccb46 commit c792540
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 164 deletions.
4 changes: 2 additions & 2 deletions crates/re_data_ui/src/item_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,9 @@ pub fn select_hovered_on_click(

if response.clicked() {
if response.ctx.input(|i| i.modifiers.command) {
selection_state.toggle_selection(selection_state.hovered().to_vec());
selection_state.toggle_selection(items.to_vec());
} else {
selection_state.set_selection(selection_state.hovered().clone().into_iter());
selection_state.set_selection(items.iter().cloned());
}
}
}
23 changes: 12 additions & 11 deletions crates/re_log_types/src/instance_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,7 @@ use crate::Component;
/// assert_eq!(InstanceKey::data_type(), DataType::UInt64);
/// ```
#[derive(
Copy,
Clone,
Debug,
Hash,
PartialEq,
Eq,
PartialOrd,
Ord,
ArrowField,
ArrowSerialize,
ArrowDeserialize,
Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, ArrowField, ArrowSerialize, ArrowDeserialize,
)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[arrow_field(transparent)]
Expand Down Expand Up @@ -79,6 +69,17 @@ impl InstanceKey {
}
}

impl std::fmt::Debug for InstanceKey {
#[inline]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if self.is_splat() {
"splat".fmt(f)
} else {
self.0.fmt(f)
}
}
}

impl std::fmt::Display for InstanceKey {
#[inline]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand Down
11 changes: 9 additions & 2 deletions crates/re_renderer/src/view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ use crate::{
DebugLabel, RectInt, Rgba, Size,
};

// These are only used if the point sizes are not already set.
// Rerun always sets explicit sizes, so these are only used for other
// users of re_renderer (mostly examples).
// Unit: UI points.
const AUTO_POINT_RADIUS: f32 = 2.5;
const AUTO_LINE_RADIUS: f32 = 1.5;

#[derive(thiserror::Error, Debug)]
pub enum ViewBuilderError {
#[error("Screenshot was already scheduled.")]
Expand Down Expand Up @@ -418,12 +425,12 @@ impl ViewBuilder {
let projection_from_world = projection_from_view * view_from_world;

let auto_size_points = if config.auto_size_config.point_radius.is_auto() {
Size::new_points(2.5)
Size::new_points(AUTO_POINT_RADIUS)
} else {
config.auto_size_config.point_radius
};
let auto_size_lines = if config.auto_size_config.line_radius.is_auto() {
Size::new_points(1.5)
Size::new_points(AUTO_LINE_RADIUS)
} else {
config.auto_size_config.line_radius
};
Expand Down
8 changes: 2 additions & 6 deletions crates/re_space_view_spatial/src/scene/parts/arrows3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use re_viewer_context::{
ArchetypeDefinition, DefaultColor, ScenePart, SceneQuery, SpaceViewHighlights, ViewerContext,
};

use super::{instance_key_to_picking_id, SpatialScenePartData, SpatialSpaceViewState};
use super::{picking_id_from_instance_key, SpatialScenePartData, SpatialSpaceViewState};
use crate::{
scene::{
contexts::{SpatialSceneContext, SpatialSceneEntityContext},
Expand Down Expand Up @@ -74,11 +74,7 @@ impl Arrows3DPart {
| LineStripFlags::FLAG_CAP_START_ROUND
| LineStripFlags::FLAG_CAP_START_EXTEND_OUTWARDS,
)
.picking_instance_id(instance_key_to_picking_id(
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
));
.picking_instance_id(picking_id_from_instance_key(instance_key));

if let Some(outline_mask_ids) = ent_context.highlight.instances.get(&instance_key) {
segment.outline_mask_ids(*outline_mask_ids);
Expand Down
20 changes: 5 additions & 15 deletions crates/re_space_view_spatial/src/scene/parts/boxes2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ use re_viewer_context::{
ArchetypeDefinition, DefaultColor, ScenePart, SceneQuery, SpaceViewHighlights, ViewerContext,
};

use super::{
instance_key_to_picking_id, instance_path_hash_for_picking, SpatialScenePartData,
SpatialSpaceViewState,
};
use crate::{
scene::{
contexts::{SpatialSceneContext, SpatialSceneEntityContext},
Expand All @@ -19,6 +15,8 @@ use crate::{
SpatialSpaceView,
};

use super::{picking_id_from_instance_key, SpatialScenePartData, SpatialSpaceViewState};

#[derive(Default)]
pub struct Boxes2DPart(SpatialScenePartData);

Expand Down Expand Up @@ -47,12 +45,8 @@ impl Boxes2DPart {
radius: Option<Radius>,
label: Option<Label>,
class_id: Option<ClassId>| {
let instance_hash = instance_path_hash_for_picking(
ent_path,
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
);
let instance_hash =
re_data_store::InstancePathHash::instance(ent_path, instance_key);

let annotation_info = ent_context
.annotations
Expand Down Expand Up @@ -81,11 +75,7 @@ impl Boxes2DPart {
)
.color(color)
.radius(radius)
.picking_instance_id(instance_key_to_picking_id(
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
));
.picking_instance_id(picking_id_from_instance_key(instance_key));

if let Some(outline_mask_ids) = ent_context
.highlight
Expand Down
15 changes: 3 additions & 12 deletions crates/re_space_view_spatial/src/scene/parts/boxes3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ use crate::{
SpatialSpaceView,
};

use super::{
instance_key_to_picking_id, instance_path_hash_for_picking, SpatialScenePartData,
SpatialSpaceViewState,
};
use super::{picking_id_from_instance_key, SpatialScenePartData, SpatialSpaceViewState};

#[derive(Default)]
pub struct Boxes3DPart(SpatialScenePartData);
Expand Down Expand Up @@ -67,11 +64,7 @@ impl Boxes3DPart {
.add_box_outline(transform)
.radius(radius)
.color(color)
.picking_instance_id(instance_key_to_picking_id(
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
));
.picking_instance_id(picking_id_from_instance_key(instance_key));

if let Some(outline_mask_ids) = ent_context.highlight.instances.get(&instance_key) {
box_lines.outline_mask_ids(*outline_mask_ids);
Expand All @@ -84,11 +77,9 @@ impl Boxes3DPart {
ent_context.world_from_obj.transform_point3(tran),
),
color,
labeled_instance: instance_path_hash_for_picking(
labeled_instance: re_data_store::InstancePathHash::instance(
ent_path,
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
),
});
}
Expand Down
12 changes: 4 additions & 8 deletions crates/re_space_view_spatial/src/scene/parts/cameras.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ use re_viewer_context::{ArchetypeDefinition, ScenePart, TimeControl};
use re_viewer_context::{SceneQuery, ViewerContext};
use re_viewer_context::{SpaceViewHighlights, SpaceViewOutlineMasks};

use super::{instance_path_hash_for_picking, SpatialScenePartData, SpatialSpaceViewState};
use crate::{
instance_hash_conversions::picking_layer_id_from_instance_path_hash,
scene::contexts::SpatialSceneContext, space_camera_3d::SpaceCamera3D, SpatialSpaceView,
};

use super::{SpatialScenePartData, SpatialSpaceViewState};

const CAMERA_COLOR: re_renderer::Color32 = re_renderer::Color32::from_rgb(150, 150, 150);

/// Determine the view coordinates (i.e.) the axis semantics.
Expand Down Expand Up @@ -152,13 +153,8 @@ impl CamerasPart {
];

let radius = re_renderer::Size::new_points(1.0);
let num_instances = 1; // There is only ever one instance of `Transform` per entity.
let instance_path_for_picking = instance_path_hash_for_picking(
ent_path,
instance_key,
num_instances,
entity_highlight.any_selection_highlight,
);
let instance_path_for_picking =
re_data_store::InstancePathHash::instance(ent_path, instance_key);
let instance_layer_id = picking_layer_id_from_instance_path_hash(instance_path_for_picking);

let mut line_builder = scene_context.shared_render_builders.lines();
Expand Down
8 changes: 2 additions & 6 deletions crates/re_space_view_spatial/src/scene/parts/lines2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
SpatialSpaceView,
};

use super::{instance_key_to_picking_id, SpatialScenePartData, SpatialSpaceViewState};
use super::{picking_id_from_instance_key, SpatialScenePartData, SpatialSpaceViewState};

#[derive(Default)]
pub struct Lines2DPart(SpatialScenePartData);
Expand Down Expand Up @@ -54,11 +54,7 @@ impl Lines2DPart {
.add_strip_2d(strip.0.into_iter().map(|v| v.into()))
.color(color)
.radius(radius)
.picking_instance_id(instance_key_to_picking_id(
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
));
.picking_instance_id(picking_id_from_instance_key(instance_key));

if let Some(outline_mask_ids) = ent_context.highlight.instances.get(&instance_key) {
lines.outline_mask_ids(*outline_mask_ids);
Expand Down
8 changes: 2 additions & 6 deletions crates/re_space_view_spatial/src/scene/parts/lines3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
SpatialSpaceView,
};

use super::{instance_key_to_picking_id, SpatialScenePartData, SpatialSpaceViewState};
use super::{picking_id_from_instance_key, SpatialScenePartData, SpatialSpaceViewState};

#[derive(Default)]
pub struct Lines3DPart(SpatialScenePartData);
Expand Down Expand Up @@ -54,11 +54,7 @@ impl Lines3DPart {
.add_strip(strip.0.into_iter().map(|v| v.into()))
.color(color)
.radius(radius)
.picking_instance_id(instance_key_to_picking_id(
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
));
.picking_instance_id(picking_id_from_instance_key(instance_key));

if let Some(outline_mask_ids) = ent_context.highlight.instances.get(&instance_key) {
lines.outline_mask_ids(*outline_mask_ids);
Expand Down
10 changes: 3 additions & 7 deletions crates/re_space_view_spatial/src/scene/parts/meshes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::scene::{
};
use crate::SpatialSpaceView;

use super::{instance_path_hash_for_picking, SpatialScenePartData, SpatialSpaceViewState};
use super::{SpatialScenePartData, SpatialSpaceViewState};

#[derive(Default)]
pub struct MeshPart(SpatialScenePartData);
Expand All @@ -33,12 +33,8 @@ impl MeshPart {
let visitor = |instance_key: InstanceKey,
mesh: re_components::Mesh3D,
_color: Option<ColorRGBA>| {
let picking_instance_hash = instance_path_hash_for_picking(
ent_path,
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
);
let picking_instance_hash =
re_data_store::InstancePathHash::instance(ent_path, instance_key);

let outline_mask_ids = ent_context.highlight.index_outline_mask(instance_key);

Expand Down
50 changes: 3 additions & 47 deletions crates/re_space_view_spatial/src/scene/parts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use ahash::HashMap;
use std::sync::Arc;

use re_components::{ClassId, ColorRGBA, KeypointId, Radius};
use re_data_store::{EntityPath, InstancePathHash};
use re_data_store::EntityPath;
use re_viewer_context::{
Annotations, DefaultColor, ResolvedAnnotationInfo, ScenePartCollection, SceneQuery,
};
Expand Down Expand Up @@ -109,54 +109,10 @@ impl SpatialScenePartCollection {
}
}

/// Computes the instance hash that should be used for picking (in turn for selecting/hover)
///
/// TODO(andreas): Resolve the hash-for-picking when retrieving the picking result instead of doing it ahead of time here to speed up things.
/// (gpu picking would always get the "most fine grained hash" which we could then resolve to groups etc. depending on selection state)
/// Right now this is a bit hard to do since number of instances depends on the Primary. This is expected to change soon.
pub fn instance_path_hash_for_picking(
ent_path: &EntityPath,
instance_key: re_log_types::InstanceKey,
num_instances: usize,
any_part_selected: bool,
) -> InstancePathHash {
InstancePathHash::instance(
ent_path,
instance_key_for_picking(instance_key, num_instances, any_part_selected),
)
}

/// Computes the instance key that should be used for picking (in turn for selecting/hover)
///
/// Assumes the entity is interactive.
///
/// TODO(andreas): Resolve the hash-for-picking when retrieving the picking result instead of doing it ahead of time here to speed up things.
/// (gpu picking would always get the "most fine grained hash" which we could then resolve to groups etc. depending on selection state)
/// Right now this is a bit hard to do since number of instances depends on the Primary. This is expected to change soon.
pub fn instance_key_for_picking(
pub fn picking_id_from_instance_key(
instance_key: re_log_types::InstanceKey,
num_instances: usize,
any_part_selected: bool,
) -> re_log_types::InstanceKey {
// If no part of the entity is selected or if there is only one instance, selecting
// should select the entire entity, not the specific instance.
// (the splat key means that no particular instance is selected but all at once instead)
if num_instances == 1 || !any_part_selected {
re_log_types::InstanceKey::SPLAT
} else {
instance_key
}
}

/// See [`instance_key_for_picking`]
pub fn instance_key_to_picking_id(
instance_key: re_log_types::InstanceKey,
num_instances: usize,
any_part_selected: bool,
) -> re_renderer::PickingLayerInstanceId {
re_renderer::PickingLayerInstanceId(
instance_key_for_picking(instance_key, num_instances, any_part_selected).0,
)
re_renderer::PickingLayerInstanceId(instance_key.0)
}

/// Process [`ColorRGBA`] components using annotations and default colors.
Expand Down
23 changes: 6 additions & 17 deletions crates/re_space_view_spatial/src/scene/parts/points2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use crate::{
};

use super::{
instance_key_to_picking_id, instance_path_hash_for_picking, process_annotations_and_keypoints,
process_colors, process_radii, SpatialScenePartData, SpatialSpaceViewState,
picking_id_from_instance_key, process_annotations_and_keypoints, process_colors, process_radii,
SpatialScenePartData, SpatialSpaceViewState,
};

pub struct Points2DPart {
Expand Down Expand Up @@ -92,14 +92,7 @@ impl Points2DPart {
re_tracing::profile_scope!("instance_hashes");
ent_view
.iter_instance_keys()
.map(|instance_key| {
instance_path_hash_for_picking(
ent_path,
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
)
})
.map(|instance_key| InstancePathHash::instance(ent_path, instance_key))
.collect::<Vec<_>>()
};

Expand Down Expand Up @@ -131,13 +124,9 @@ impl Points2DPart {
.filter_map(|pt| pt.map(glam::Vec2::from))
};

let picking_instance_ids = ent_view.iter_instance_keys().map(|instance_key| {
instance_key_to_picking_id(
instance_key,
ent_view.num_instances(),
ent_context.highlight.any_selection_highlight,
)
});
let picking_instance_ids = ent_view
.iter_instance_keys()
.map(picking_id_from_instance_key);

let mut point_range_builder = point_batch.add_points_2d(
ent_view.num_instances(),
Expand Down

0 comments on commit c792540

Please sign in to comment.