Skip to content

Commit

Permalink
Fix double clicking camera no longer focusing on said camera (#1911)
Browse files Browse the repository at this point in the history
Also cleans up how a camera/pinhole transform is referenced in scene/ui code: Only by its path now since several pinholes on the same path don't make sense (in fact we warn when you try to log this)
  • Loading branch information
Wumpf committed Apr 19, 2023
1 parent e3b09af commit 6127e02
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 44 deletions.
6 changes: 3 additions & 3 deletions crates/re_viewer/src/misc/selection_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use egui::NumExt;
use lazy_static::lazy_static;
use nohash_hasher::IntMap;

use re_data_store::{EntityPath, InstancePath, InstancePathHash};
use re_data_store::EntityPath;
use re_log_types::{component_types::InstanceKey, EntityPathHash};
use re_renderer::OutlineMaskPreference;

Expand Down Expand Up @@ -34,10 +34,10 @@ pub enum HoveredSpace {

/// Path of a space camera, this 3D space is viewed through.
/// (None for a free floating Eye)
tracked_space_camera: Option<InstancePath>,
tracked_space_camera: Option<EntityPath>,

/// Corresponding 2D spaces and pixel coordinates (with Z=depth)
point_in_space_cameras: Vec<(InstancePathHash, Option<glam::Vec3>)>,
point_in_space_cameras: Vec<(EntityPath, Option<glam::Vec3>)>,
},
}

Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewer/src/ui/view_spatial/scene/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ impl SceneSpatial {
if self
.space_cameras
.iter()
.any(|camera| camera.instance_path_hash.entity_path_hash != space_info_path.hash())
.any(|camera| &camera.ent_path != space_info_path)
{
return SpatialNavigationMode::ThreeD;
}
Expand Down
22 changes: 10 additions & 12 deletions crates/re_viewer/src/ui/view_spatial/scene/primitives.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use egui::Color32;
use re_data_store::InstancePathHash;
use re_log_types::EntityPathHash;
use re_data_store::EntityPath;
use re_log_types::{component_types::InstanceKey, EntityPathHash};
use re_renderer::{
renderer::{DepthClouds, MeshInstance},
LineStripSeriesBuilder, PointCloudBuilder,
Expand Down Expand Up @@ -169,7 +169,7 @@ impl SceneSpatialPrimitives {
pub fn add_axis_lines(
&mut self,
transform: macaw::IsoTransform,
instance_path_hash: InstancePathHash,
ent_path: Option<&EntityPath>,
axis_length: f32,
) {
use re_renderer::renderer::LineStripFlags;
Expand All @@ -178,12 +178,10 @@ impl SceneSpatialPrimitives {
let line_radius = re_renderer::Size::new_scene(axis_length * 0.05);
let origin = transform.translation();

let picking_layer_id = picking_layer_id_from_instance_path_hash(instance_path_hash);

let mut line_batch = self
.line_strips
.batch("origin axis")
.picking_object_id(picking_layer_id.object);
let mut line_batch = self.line_strips.batch("origin axis").picking_object_id(
re_renderer::PickingLayerObjectId(ent_path.map_or(0, |p| p.hash64())),
);
let picking_instance_id = re_renderer::PickingLayerInstanceId(InstanceKey::SPLAT.0);

line_batch
.add_segment(
Expand All @@ -193,7 +191,7 @@ impl SceneSpatialPrimitives {
.radius(line_radius)
.color(AXIS_COLOR_X)
.flags(LineStripFlags::CAP_END_TRIANGLE | LineStripFlags::CAP_START_ROUND)
.picking_instance_id(picking_layer_id.instance);
.picking_instance_id(picking_instance_id);
line_batch
.add_segment(
origin,
Expand All @@ -202,7 +200,7 @@ impl SceneSpatialPrimitives {
.radius(line_radius)
.color(AXIS_COLOR_Y)
.flags(LineStripFlags::CAP_END_TRIANGLE | LineStripFlags::CAP_START_ROUND)
.picking_instance_id(picking_layer_id.instance);
.picking_instance_id(picking_instance_id);
line_batch
.add_segment(
origin,
Expand All @@ -211,6 +209,6 @@ impl SceneSpatialPrimitives {
.radius(line_radius)
.color(AXIS_COLOR_Z)
.flags(LineStripFlags::CAP_END_TRIANGLE | LineStripFlags::CAP_START_ROUND)
.picking_instance_id(picking_layer_id.instance);
.picking_instance_id(picking_instance_id);
}
}
10 changes: 5 additions & 5 deletions crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use re_data_store::{EntityPath, EntityProperties, InstancePathHash};
use re_data_store::{EntityPath, EntityProperties};
use re_log_types::{
component_types::InstanceKey,
coordinates::{Handedness, SignedAxis3},
Expand Down Expand Up @@ -60,7 +60,7 @@ impl CamerasPart {
fn visit_instance(
scene: &mut SceneSpatial,
entity_view: &EntityView<Transform>,
entity_path: &EntityPath,
ent_path: &EntityPath,
instance_key: InstanceKey,
props: &EntityProperties,
transforms: &TransformCache,
Expand All @@ -74,7 +74,7 @@ impl CamerasPart {
//
// Note that currently a transform on an object can't have both a pinhole AND a rigid transform,
// which makes this rather well defined here.
let parent_path = entity_path
let parent_path = ent_path
.parent()
.expect("root path can't be part of scene query");
let Some(world_from_parent) = transforms.reference_from_entity(&parent_path) else {
Expand All @@ -90,7 +90,7 @@ impl CamerasPart {
let frustum_length = *props.pinhole_image_plane_distance.get();

scene.space_cameras.push(SpaceCamera3D {
instance_path_hash: InstancePathHash::instance(entity_path, instance_key),
ent_path: ent_path.clone(),
view_coordinates,
world_from_camera,
pinhole: Some(pinhole),
Expand Down Expand Up @@ -144,7 +144,7 @@ impl CamerasPart {
let radius = re_renderer::Size::new_points(1.0);
let color = SceneSpatial::CAMERA_COLOR;
let instance_path_for_picking = instance_path_hash_for_picking(
entity_path,
ent_path,
instance_key,
entity_view,
entity_highlight.any_selection_highlight,
Expand Down
9 changes: 4 additions & 5 deletions crates/re_viewer/src/ui/view_spatial/space_camera_3d.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
use glam::{vec3, Affine3A, Mat3, Quat, Vec2, Vec3};
use macaw::{IsoTransform, Ray3};

use re_data_store::InstancePathHash;
use re_log_types::ViewCoordinates;
use re_log_types::{EntityPath, ViewCoordinates};

/// A logged camera that connects spaces.
#[derive(Clone)]
pub struct SpaceCamera3D {
/// Path to the instance which has the projection (pinhole, ortho or otherwise) transforms.
/// Path to the entity which has the projection (pinhole, ortho or otherwise) transforms.
///
/// We expect the camera transform to apply to this instance and every path below it.
pub instance_path_hash: InstancePathHash,
pub ent_path: EntityPath,

/// The coordinate system of the camera ("view-space").
pub view_coordinates: ViewCoordinates,
Expand Down Expand Up @@ -46,7 +45,7 @@ impl SpaceCamera3D {
match from_rub_quat(self.view_coordinates) {
Ok(from_rub) => Some(self.world_from_camera * IsoTransform::from_quat(from_rub)),
Err(err) => {
re_log::warn_once!("Camera {:?}: {err}", self.instance_path_hash);
re_log::warn_once!("Camera {:?}: {err}", self.ent_path);
None
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewer/src/ui/view_spatial/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ pub fn picking(
.iter()
.map(|cam| {
(
cam.instance_path_hash,
cam.ent_path.clone(),
hovered_point.and_then(|pos| cam.project_onto_2d(pos)),
)
})
Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewer/src/ui/view_spatial/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ fn show_projections_from_3d_space(
} = ctx.selection_state().hovered_space()
{
for (space_2d, pos_2d) in target_spaces {
if space_2d.entity_path_hash == space.hash() {
if space_2d == space {
if let Some(pos_2d) = pos_2d {
// User is hovering a 2D point inside a 3D view.
let pos_in_ui = ui_from_space.transform_pos(pos2(pos_2d.x, pos_2d.y));
Expand Down
30 changes: 14 additions & 16 deletions crates/re_viewer/src/ui/view_spatial/ui_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use egui::NumExt as _;
use glam::Affine3A;
use macaw::{vec3, BoundingBox, Quat, Vec3};

use re_data_store::{EntityPropertyMap, InstancePath, InstancePathHash};
use re_data_store::EntityPropertyMap;
use re_log_types::{EntityPath, ViewCoordinates};
use re_renderer::{
view_builder::{Projection, TargetConfiguration, ViewBuilder},
Expand Down Expand Up @@ -38,7 +38,7 @@ pub struct View3DState {
pub orbit_eye: Option<OrbitEye>,

/// Currently tracked camera.
pub tracked_camera: Option<InstancePath>,
pub tracked_camera: Option<EntityPath>,

/// Camera pose just before we took over another camera via [Self::tracked_camera].
camera_before_tracked_camera: Option<Eye>,
Expand Down Expand Up @@ -99,7 +99,7 @@ impl View3DState {
let tracking_camera = self
.tracked_camera
.as_ref()
.and_then(|c| find_camera(space_cameras, &c.hash()));
.and_then(|c| find_camera(space_cameras, c));

if let Some(tracking_camera) = tracking_camera {
if let Some(cam_interpolation) = &mut self.eye_interpolation {
Expand Down Expand Up @@ -224,11 +224,11 @@ impl SpaceSpecs {
}
}

fn find_camera(space_cameras: &[SpaceCamera3D], needle: &InstancePathHash) -> Option<Eye> {
fn find_camera(space_cameras: &[SpaceCamera3D], needle: &EntityPath) -> Option<Eye> {
let mut found_camera = None;

for camera in space_cameras {
if &camera.instance_path_hash == needle {
if &camera.ent_path == needle {
if found_camera.is_some() {
return None; // More than one camera
} else {
Expand Down Expand Up @@ -309,7 +309,7 @@ pub fn view_3d(
eye.approx_pixel_world_size_at(transform.translation(), rect.size()) * 32.0;
scene
.primitives
.add_axis_lines(transform, camera.instance_path_hash, axis_length);
.add_axis_lines(transform, Some(&camera.ent_path), axis_length);
}
}

Expand Down Expand Up @@ -377,11 +377,11 @@ pub fn view_3d(

// While hovering an entity, focuses the camera on it.
if let Some(Item::InstancePath(_, instance_path)) = ctx.hovered().first() {
if let Some(camera) = find_camera(&scene.space_cameras, &instance_path.hash()) {
if let Some(camera) = find_camera(&scene.space_cameras, &instance_path.entity_path) {
state.state_3d.camera_before_tracked_camera =
state.state_3d.orbit_eye.map(|eye| eye.to_eye());
state.state_3d.interpolate_to_eye(camera);
state.state_3d.tracked_camera = Some(instance_path.clone());
state.state_3d.tracked_camera = Some(instance_path.entity_path.clone());
} else if let Some(clicked_point) = state.state_3d.hovered_point {
if let Some(mut new_orbit_eye) = state.state_3d.orbit_eye {
// TODO(andreas): It would be nice if we could focus on the center of the entity rather than the clicked point.
Expand Down Expand Up @@ -428,11 +428,9 @@ pub fn view_3d(

if state.state_3d.show_axes {
let axis_length = 1.0; // The axes are also a measuring stick
scene.primitives.add_axis_lines(
macaw::IsoTransform::IDENTITY,
InstancePathHash::NONE,
axis_length,
);
scene
.primitives
.add_axis_lines(macaw::IsoTransform::IDENTITY, None, axis_length);
}

if state.state_3d.show_bbox {
Expand Down Expand Up @@ -519,15 +517,15 @@ pub fn view_3d(
fn show_projections_from_2d_space(
ctx: &mut ViewerContext<'_>,
scene: &mut SceneSpatial,
tracked_space_camera: &Option<InstancePath>,
tracked_space_camera: &Option<EntityPath>,
scene_bbox_accum: &BoundingBox,
) {
match ctx.selection_state().hovered_space() {
HoveredSpace::TwoD { space_2d, pos } => {
if let Some(cam) = scene
.space_cameras
.iter()
.find(|cam| cam.instance_path_hash.entity_path_hash == space_2d.hash())
.find(|cam| &cam.ent_path == space_2d)
{
if let Some(ray) = cam.unproject_as_ray(glam::vec2(pos.x, pos.y)) {
// Render a thick line to the actual z value if any and a weaker one as an extension
Expand Down Expand Up @@ -559,7 +557,7 @@ fn show_projections_from_2d_space(
if let Some(cam) = scene
.space_cameras
.iter()
.find(|cam| cam.instance_path_hash == camera_path.hash())
.find(|cam| &cam.ent_path == camera_path)
{
let cam_to_pos = *pos - cam.position();
let distance = cam_to_pos.length();
Expand Down

0 comments on commit 6127e02

Please sign in to comment.