From b0a6f7751ddfb9fd1c8aa99e625515eb9b7103e2 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 17 Apr 2023 16:53:13 +0200 Subject: [PATCH 01/37] transform cache now deals with Affine3 matrices only --- crates/re_viewer/src/misc/transform_cache.rs | 79 ++++++------------- .../view_spatial/scene/scene_part/arrows3d.rs | 5 +- .../view_spatial/scene/scene_part/boxes2d.rs | 5 +- .../view_spatial/scene/scene_part/boxes3d.rs | 6 +- .../view_spatial/scene/scene_part/cameras.rs | 4 +- .../view_spatial/scene/scene_part/images.rs | 9 ++- .../view_spatial/scene/scene_part/lines2d.rs | 6 +- .../view_spatial/scene/scene_part/lines3d.rs | 6 +- .../view_spatial/scene/scene_part/meshes.rs | 7 +- .../view_spatial/scene/scene_part/points2d.rs | 6 +- .../view_spatial/scene/scene_part/points3d.rs | 8 +- 11 files changed, 47 insertions(+), 94 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 59290e1e94aa..434c9c1ad71d 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -20,7 +20,7 @@ pub struct TransformCache { reference_path: EntityPath, /// All reachable entities. - reference_from_entity_per_entity: IntMap, + reference_from_entity_per_entity: IntMap, /// All unreachable descendant paths of `reference_path`. unreachable_descendants: Vec<(EntityPath, UnreachableTransform)>, @@ -61,6 +61,13 @@ impl std::fmt::Display for UnreachableTransform { } } +fn transform_affine3(a: glam::Affine3A, b: glam::Affine3A) -> glam::Affine3A { + glam::Affine3A { + matrix3: a.matrix3.mul_mat3(&b.matrix3), + translation: a.matrix3.mul_vec3a(b.translation) + a.translation, + } +} + impl TransformCache { /// Determines transforms for all entities relative to a space path which serves as the "reference". /// I.e. the resulting transforms are "reference from scene" @@ -98,13 +105,13 @@ impl TransformCache { entity_db, &query, entity_prop_map, - glam::Mat4::IDENTITY, + glam::Affine3A::IDENTITY, false, ); // Walk up from the reference to the highest reachable parent. let mut encountered_pinhole = false; - let mut reference_from_ancestor = glam::Mat4::IDENTITY; + let mut reference_from_ancestor = glam::Affine3A::IDENTITY; while let Some(parent_path) = current_tree.path.parent() { let Some(parent_tree) = &entity_db.tree.subtree(&parent_path) else { // Unlike not having the space path in the hierarchy, this should be impossible. @@ -117,9 +124,10 @@ impl TransformCache { // Note that the transform at the reference is the first that needs to be inverted to "break out" of its hierarchy. // Generally, the transform _at_ a node isn't relevant to it's children, but only to get to its parent in turn! - match inverse_transform_at( + match transform_at( ¤t_tree.path, entity_db, + entity_prop_map, &query, &mut encountered_pinhole, ) { @@ -129,8 +137,9 @@ impl TransformCache { break; } Ok(None) => {} - Ok(Some(child_from_parent)) => { - reference_from_ancestor *= child_from_parent; + Ok(Some(parent_from_child)) => { + reference_from_ancestor = + transform_affine3(reference_from_ancestor, parent_from_child.inverse()); } } @@ -156,7 +165,7 @@ impl TransformCache { entity_db: &EntityDb, query: &LatestAtQuery, entity_properties: &EntityPropertyMap, - reference_from_entity: glam::Mat4, + reference_from_entity: glam::Affine3A, encountered_pinhole: bool, ) { match self @@ -186,7 +195,9 @@ impl TransformCache { continue; } Ok(None) => reference_from_entity, - Ok(Some(child_from_parent)) => reference_from_entity * child_from_parent, + Ok(Some(child_from_parent)) => { + transform_affine3(reference_from_entity, child_from_parent) + } }; self.gather_descendants_transforms( child_tree, @@ -202,7 +213,7 @@ impl TransformCache { /// Retrieves the transform of on entity from its local system to the space of the reference. /// /// Returns None if the path is not reachable. - pub fn reference_from_entity(&self, entity_path: &EntityPath) -> Option { + pub fn reference_from_entity(&self, entity_path: &EntityPath) -> Option { self.reference_from_entity_per_entity .get(entity_path) .cloned() @@ -223,10 +234,10 @@ fn transform_at( entity_properties: &EntityPropertyMap, query: &LatestAtQuery, encountered_pinhole: &mut bool, -) -> Result, UnreachableTransform> { +) -> Result, UnreachableTransform> { if let Some(transform) = query_latest_single(entity_db, entity_path, query) { match transform { - re_log_types::Transform::Rigid3(rigid) => Ok(Some(rigid.parent_from_child().to_mat4())), + re_log_types::Transform::Rigid3(rigid) => Ok(Some(rigid.parent_from_child().into())), // If we're connected via 'unknown' it's not reachable re_log_types::Transform::Unknown => Err(UnreachableTransform::UnknownTransform), @@ -246,7 +257,7 @@ fn transform_at( let focal_length = glam::vec2(focal_length.x(), focal_length.y()); let scale = distance / focal_length; let translation = (-pinhole.principal_point() * scale).extend(distance); - let parent_from_child = glam::Mat4::from_scale_rotation_translation( + let parent_from_child = glam::Affine3A::from_scale_rotation_translation( // We want to preserve any depth that might be on the pinhole image. // Use harmonic mean of x/y scale for those. scale.extend(1.0 / (1.0 / scale.x + 1.0 / scale.y)), @@ -262,47 +273,3 @@ fn transform_at( Ok(None) } } - -fn inverse_transform_at( - entity_path: &EntityPath, - entity_db: &EntityDb, - query: &LatestAtQuery, - encountered_pinhole: &mut bool, -) -> Result, UnreachableTransform> { - if let Some(parent_transform) = query_latest_single(entity_db, entity_path, query) { - match parent_transform { - re_log_types::Transform::Rigid3(rigid) => Ok(Some(rigid.child_from_parent().to_mat4())), - // If we're connected via 'unknown', everything except whats under `parent_tree` is unreachable - re_log_types::Transform::Unknown => Err(UnreachableTransform::UnknownTransform), - - re_log_types::Transform::Pinhole(pinhole) => { - if *encountered_pinhole { - Err(UnreachableTransform::NestedPinholeCameras) - } else { - *encountered_pinhole = true; - - // TODO(andreas): If we don't have a resolution we don't know the FOV ergo we don't know how to project. Unclear what to do. - if let Some(resolution) = pinhole.resolution() { - let translation = pinhole.principal_point().extend(-100.0); // Large Y offset so this is in front of all 2d that came so far. TODO(andreas): Find better solution - Ok(Some( - glam::Mat4::from_scale_rotation_translation( - // Scaled with 0.5 since perspective_infinite_lh uses NDC, i.e. [-1; 1] range. - (resolution * 0.5).extend(1.0), - glam::Quat::IDENTITY, - translation, - ) * glam::Mat4::perspective_infinite_lh( - pinhole.fov_y().unwrap(), - pinhole.aspect_ratio().unwrap_or(1.0), - 0.0, - ), - )) - } else { - Err(UnreachableTransform::InversePinholeCameraWithoutResolution) - } - } - } - } - } else { - Ok(None) - } -} diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs index f40ca9025684..48652e9c99cd 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs @@ -1,4 +1,3 @@ -use glam::Mat4; use re_data_store::EntityPath; use re_log_types::{ component_types::{ColorRGBA, InstanceKey, Label, Radius}, @@ -21,7 +20,7 @@ impl Arrows3DPart { scene: &mut SceneSpatial, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, highlights: &SpaceViewHighlights, ) -> Result<(), QueryError> { scene.num_logged_3d_objects += 1; @@ -35,7 +34,7 @@ impl Arrows3DPart { .primitives .line_strips .batch("arrows") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs index 8849d7a2de7a..59e2398a46d2 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs @@ -1,4 +1,3 @@ -use glam::Mat4; use re_data_store::EntityPath; use re_log_types::{ component_types::{ClassId, ColorRGBA, InstanceKey, Label, Radius, Rect2D}, @@ -27,7 +26,7 @@ impl Boxes2DPart { scene: &mut SceneSpatial, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, highlights: &SpaceViewHighlights, ) -> Result<(), QueryError> { scene.num_logged_2d_objects += 1; @@ -41,7 +40,7 @@ impl Boxes2DPart { .primitives .line_strips .batch("2d boxes") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs index 6f06a36f440f..5352d667f240 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs @@ -1,5 +1,3 @@ -use glam::Mat4; - use re_data_store::EntityPath; use re_log_types::{ component_types::{Box3D, ClassId, ColorRGBA, InstanceKey, Label, Quaternion, Radius, Vec3D}, @@ -26,7 +24,7 @@ impl Boxes3DPart { scene: &mut SceneSpatial, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, entity_highlight: &SpaceViewOutlineMasks, ) -> Result<(), QueryError> { scene.num_logged_3d_objects += 1; @@ -37,7 +35,7 @@ impl Boxes3DPart { .primitives .line_strips .batch("box 3d") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index 6f59283ad5c0..e719d968d3d2 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -83,7 +83,7 @@ impl CamerasPart { // If this transform is not representable as rigid transform, the camera is probably under another camera transform, // in which case we don't (yet) know how to deal with this! - let Some(world_from_camera) = macaw::IsoTransform::from_mat4(&world_from_parent) else { + let Some(world_from_camera) = macaw::IsoTransform::from_mat4(&world_from_parent.into()) else { return; }; @@ -155,7 +155,7 @@ impl CamerasPart { .primitives .line_strips .batch("camera frustum") - .world_from_obj(world_from_parent) + .world_from_obj(world_from_parent.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(instance_layer_id.object); let lines = batch diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs index 5893e2b3c2be..bbac68f00e30 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs @@ -29,7 +29,7 @@ fn push_tensor_texture( scene: &mut SceneSpatial, ctx: &mut ViewerContext<'_>, annotations: &Annotations, - world_from_obj: glam::Mat4, + world_from_obj: glam::Affine3A, ent_path: &EntityPath, tensor: &Tensor, multiplicative_tint: egui::Rgba, @@ -144,7 +144,7 @@ impl ImagesPart { transforms: &TransformCache, properties: &EntityProperties, ent_path: &EntityPath, - world_from_obj: glam::Mat4, + world_from_obj: glam::Affine3A, highlights: &SpaceViewHighlights, ) -> Result<(), QueryError> { crate::profile_function!(); @@ -180,6 +180,7 @@ impl ImagesPart { crate::misc::queries::closest_pinhole_transform(ctx, ent_path, &query); if let Some(pinhole_ent_path) = pinhole_ent_path { + // TODO: is this still true?? // NOTE: we don't pass in `world_from_obj` because this corresponds to the // transform of the projection plane, which is of no use to us here. // What we want are the extrinsics of the depth camera! @@ -222,7 +223,7 @@ impl ImagesPart { scene: &mut SceneSpatial, ctx: &mut ViewerContext<'_>, ent_path: &EntityPath, - world_from_obj: glam::Mat4, + world_from_obj: glam::Affine3A, entity_highlight: &SpaceViewOutlineMasks, tensor: Tensor, color: Option, @@ -339,7 +340,7 @@ impl ImagesPart { }; scene.primitives.depth_clouds.clouds.push(DepthCloud { - world_from_obj, + world_from_obj: world_from_obj.into(), depth_camera_intrinsics: intrinsics.image_from_cam.into(), world_depth_from_data_depth, point_radius_from_world_depth, diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs index f07375679530..64ef4ae8a0fe 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs @@ -1,5 +1,3 @@ -use glam::Mat4; - use re_data_store::EntityPath; use re_log_types::{ component_types::{ColorRGBA, InstanceKey, LineStrip2D, Radius}, @@ -22,7 +20,7 @@ impl Lines2DPart { scene: &mut SceneSpatial, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, entity_highlight: &SpaceViewOutlineMasks, ) -> Result<(), QueryError> { scene.num_logged_2d_objects += 1; @@ -34,7 +32,7 @@ impl Lines2DPart { .primitives .line_strips .batch("lines 2d") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs index 4a527a291fec..b6a2bb70b320 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs @@ -1,5 +1,3 @@ -use glam::Mat4; - use re_data_store::EntityPath; use re_log_types::{ component_types::{ColorRGBA, InstanceKey, LineStrip3D, Radius}, @@ -22,7 +20,7 @@ impl Lines3DPart { scene: &mut SceneSpatial, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, entity_highlight: &SpaceViewOutlineMasks, ) -> Result<(), QueryError> { scene.num_logged_3d_objects += 1; @@ -33,7 +31,7 @@ impl Lines3DPart { .primitives .line_strips .batch("lines 3d") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/meshes.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/meshes.rs index 4e935a2663f4..ec9f452ca039 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/meshes.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/meshes.rs @@ -1,5 +1,3 @@ -use glam::Mat4; - use re_data_store::EntityPath; use re_log_types::{ component_types::{ColorRGBA, InstanceKey}, @@ -25,14 +23,13 @@ impl MeshPart { scene: &mut SceneSpatial, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, ctx: &mut ViewerContext<'_>, highlights: &SpaceViewHighlights, ) -> Result<(), QueryError> { scene.num_logged_3d_objects += 1; let _default_color = DefaultColor::EntityPath(ent_path); - let world_from_obj_affine = glam::Affine3A::from_mat4(world_from_obj); let entity_highlight = highlights.entity_outline_mask(ent_path.hash()); let visitor = @@ -56,7 +53,7 @@ impl MeshPart { ) .map(|cpu_mesh| MeshSource { picking_instance_hash, - world_from_mesh: world_from_obj_affine, + world_from_mesh: world_from_obj, mesh: cpu_mesh, outline_mask_ids, }) diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs index 41bdc4a88534..5f9459f0ae1a 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs @@ -1,5 +1,3 @@ -use glam::Mat4; - use re_data_store::{EntityPath, InstancePathHash}; use re_log_types::{ component_types::{ClassId, ColorRGBA, InstanceKey, KeypointId, Label, Point2D, Radius}, @@ -64,7 +62,7 @@ impl Points2DPart { query: &SceneQuery<'_>, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, entity_highlight: &SpaceViewOutlineMasks, ) -> Result<(), QueryError> { crate::profile_function!(); @@ -112,7 +110,7 @@ impl Points2DPart { .primitives .points .batch("2d points") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs index 1b6ca61f9669..1845757bb442 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs @@ -1,5 +1,3 @@ -use glam::Mat4; - use re_data_store::{EntityPath, InstancePathHash}; use re_log_types::{ component_types::{ClassId, ColorRGBA, InstanceKey, KeypointId, Label, Point3D, Radius}, @@ -35,7 +33,7 @@ impl Points3DPart { instance_path_hashes: &'a [InstancePathHash], colors: &'a [egui::Color32], annotation_infos: &'a [ResolvedAnnotationInfo], - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, ) -> Result + 'a, QueryError> { let labels = itertools::izip!( annotation_infos.iter(), @@ -70,7 +68,7 @@ impl Points3DPart { query: &SceneQuery<'_>, entity_view: &EntityView, ent_path: &EntityPath, - world_from_obj: Mat4, + world_from_obj: glam::Affine3A, entity_highlight: &SpaceViewOutlineMasks, ) -> Result<(), QueryError> { crate::profile_function!(); @@ -119,7 +117,7 @@ impl Points3DPart { .primitives .points .batch("3d points") - .world_from_obj(world_from_obj) + .world_from_obj(world_from_obj.into()) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); From 40e6e1dc26621a76665235f3a8327f635b3ea4e2 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 17 Apr 2023 18:55:19 +0200 Subject: [PATCH 02/37] use a perspective camera in 2d views that sit at a space camera --- crates/re_viewer/src/misc/transform_cache.rs | 5 +- .../view_spatial/scene/scene_part/cameras.rs | 16 ++++-- .../src/ui/view_spatial/space_camera_3d.rs | 4 +- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 56 +++++++++++++++---- crates/re_viewer/src/ui/view_spatial/ui_3d.rs | 40 ++++++------- 5 files changed, 80 insertions(+), 41 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 434c9c1ad71d..3ee773e10983 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -16,7 +16,6 @@ use crate::misc::TimeControl; #[derive(Clone)] pub struct TransformCache { /// All transforms provided are relative to this reference path. - #[allow(dead_code)] reference_path: EntityPath, /// All reachable entities. @@ -210,6 +209,10 @@ impl TransformCache { } } + pub fn reference_path(&self) -> &EntityPath { + &self.reference_path + } + /// Retrieves the transform of on entity from its local system to the space of the reference. /// /// Returns None if the path is not reachable. diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index e719d968d3d2..4ce9192c1d46 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -82,9 +82,17 @@ impl CamerasPart { }; // If this transform is not representable as rigid transform, the camera is probably under another camera transform, - // in which case we don't (yet) know how to deal with this! - let Some(world_from_camera) = macaw::IsoTransform::from_mat4(&world_from_parent.into()) else { - return; + // in which case we don't (yet) know how to deal with this!] + // TODO: what is this special case + let world_from_camera = if transforms.reference_path() == entity_path { + macaw::IsoTransform::IDENTITY + } else { + match macaw::IsoTransform::from_mat4(&world_from_parent.into()) { + Some(world_from_camera) => world_from_camera, + None => { + return; + } + } }; let frustum_length = *props.pinhole_image_plane_distance.get(); @@ -94,7 +102,7 @@ impl CamerasPart { view_coordinates, world_from_camera, pinhole: Some(pinhole), - picture_plane_distance: Some(frustum_length), + picture_plane_distance: frustum_length, }); // TODO(andreas): FOV fallback doesn't make much sense. What does pinhole without fov mean? diff --git a/crates/re_viewer/src/ui/view_spatial/space_camera_3d.rs b/crates/re_viewer/src/ui/view_spatial/space_camera_3d.rs index fc687bd958df..e1cefec66b02 100644 --- a/crates/re_viewer/src/ui/view_spatial/space_camera_3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/space_camera_3d.rs @@ -23,8 +23,8 @@ pub struct SpaceCamera3D { /// The projection transform of a child-entity. pub pinhole: Option, - /// Optional distance of a picture plane from the camera. - pub picture_plane_distance: Option, + /// Distance of a picture plane from the camera. + pub picture_plane_distance: f32, } impl SpaceCamera3D { diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index ca790381a47a..fcdc0daedaf0 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -7,7 +7,7 @@ use re_renderer::view_builder::{TargetConfiguration, ViewBuilder}; use super::{ eye::Eye, ui::{create_labels, picking, screenshot_context_menu}, - SpatialNavigationMode, ViewSpatialState, + SpaceCamera3D, SpatialNavigationMode, ViewSpatialState, }; use crate::{ gpu_bridge, @@ -308,6 +308,11 @@ fn view_2d_scrollable( fov_y: None, }; + let space_camera = scene + .space_cameras + .iter() + .find(|c| c.instance_path_hash.entity_path_hash == space.hash()); + let Ok(target_config) = setup_target_config( &painter, space_from_ui, @@ -317,6 +322,7 @@ fn view_2d_scrollable( scene .primitives .any_outlines, + space_camera, ) else { return response; }; @@ -404,27 +410,57 @@ fn setup_target_config( space_name: &str, auto_size_config: re_renderer::AutoSizeConfig, any_outlines: bool, + space_camera: Option<&SpaceCamera3D>, ) -> anyhow::Result { let pixels_from_points = painter.ctx().pixels_per_point(); let resolution_in_pixel = gpu_bridge::viewport_resolution_in_pixels(painter.clip_rect(), pixels_from_points); anyhow::ensure!(resolution_in_pixel[0] > 0 && resolution_in_pixel[1] > 0); - let camera_position_space = space_from_ui.transform_pos(painter.clip_rect().min); + // TODO: This should be in the eye? - Ok({ - let name = space_name.into(); - let top_left_position = glam::vec2(camera_position_space.x, camera_position_space.y); - TargetConfiguration { - name, - resolution_in_pixel, - view_from_world: macaw::IsoTransform::from_translation(-top_left_position.extend(0.0)), - projection_from_view: re_renderer::view_builder::Projection::Orthographic { + let (projection_from_view, view_from_world) = if let Some(space_camera) = space_camera { + let pinhole = space_camera.pinhole.unwrap(); // TODO: + + // Put the camera at the position where it sees the entire image plane as defined + // by the pinhole camera. + let camera_distance = pinhole.focal_length_in_pixels()[0]; // We don't support non-square pixels, pick 0. + ( + re_renderer::view_builder::Projection::Perspective { + vertical_fov: pinhole.fov_y().unwrap(), // TODO: no unwrap + near_plane_distance: 0.01, // TODO: + }, + macaw::IsoTransform::look_at_rh( + pinhole.principal_point().extend(-camera_distance), + pinhole.principal_point().extend(0.0), + -glam::Vec3::Y, + ) + .unwrap_or(macaw::IsoTransform::IDENTITY), + ) + } else { + let top_left_pos = space_from_ui.transform_pos(painter.clip_rect().min); + ( + re_renderer::view_builder::Projection::Orthographic { camera_mode: re_renderer::view_builder::OrthographicCameraMode::TopLeftCornerAndExtendZ, vertical_world_size: space_from_pixel * resolution_in_pixel[1] as f32, far_plane_distance: 1000.0, }, + macaw::IsoTransform::from_translation(glam::vec3( + -top_left_pos.x, + -top_left_pos.y, + 0.0, + )), + ) + }; + + Ok({ + let name = space_name.into(); + TargetConfiguration { + name, + resolution_in_pixel, + view_from_world, + projection_from_view, pixels_from_point: pixels_from_points, auto_size_config, outline_config: any_outlines.then(|| outline_config(painter.ctx())), diff --git a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs index 86cb34e808f4..d64e3c8f8ccb 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs @@ -533,7 +533,7 @@ fn show_projections_from_2d_space( // Render a thick line to the actual z value if any and a weaker one as an extension // If we don't have a z value, we only render the thick one. let thick_ray_length = if pos.z.is_finite() && pos.z > 0.0 { - Some(pos.z) + pos.z } else { cam.picture_plane_distance }; @@ -564,7 +564,7 @@ fn show_projections_from_2d_space( let cam_to_pos = *pos - cam.position(); let distance = cam_to_pos.length(); let ray = macaw::Ray3::from_origin_dir(cam.position(), cam_to_pos / distance); - add_picking_ray(&mut scene.primitives, ray, scene_bbox_accum, Some(distance)); + add_picking_ray(&mut scene.primitives, ray, scene_bbox_accum, distance); } } } @@ -576,34 +576,26 @@ fn add_picking_ray( primitives: &mut SceneSpatialPrimitives, ray: macaw::Ray3, scene_bbox_accum: &BoundingBox, - thick_ray_length: Option, + thick_ray_length: f32, ) { let mut line_batch = primitives.line_strips.batch("picking ray"); let origin = ray.point_along(0.0); // No harm in making this ray _very_ long. (Infinite messes with things though!) let fallback_ray_end = ray.point_along(scene_bbox_accum.size().length() * 10.0); - - if let Some(line_length) = thick_ray_length { - let main_ray_end = ray.point_along(line_length); - line_batch - .add_segment(origin, main_ray_end) - .color(egui::Color32::WHITE) - .flags(re_renderer::renderer::LineStripFlags::NO_COLOR_GRADIENT) - .radius(Size::new_points(1.0)); - line_batch - .add_segment(main_ray_end, fallback_ray_end) - .color(egui::Color32::DARK_GRAY) - // TODO(andreas): Make this dashed. - .flags(re_renderer::renderer::LineStripFlags::NO_COLOR_GRADIENT) - .radius(Size::new_points(0.5)); - } else { - line_batch - .add_segment(origin, fallback_ray_end) - .color(egui::Color32::WHITE) - .flags(re_renderer::renderer::LineStripFlags::NO_COLOR_GRADIENT) - .radius(Size::new_points(1.0)); - } + let main_ray_end = ray.point_along(thick_ray_length); + + line_batch + .add_segment(origin, main_ray_end) + .color(egui::Color32::WHITE) + .flags(re_renderer::renderer::LineStripFlags::NO_COLOR_GRADIENT) + .radius(Size::new_points(1.0)); + line_batch + .add_segment(main_ray_end, fallback_ray_end) + .color(egui::Color32::DARK_GRAY) + // TODO(andreas): Make this dashed. + .flags(re_renderer::renderer::LineStripFlags::NO_COLOR_GRADIENT) + .radius(Size::new_points(0.5)); } fn default_eye(scene_bbox: &macaw::BoundingBox, space_specs: &SpaceSpecs) -> OrbitEye { From a252372562bcd048fb0b7fcc3dc3f3bd765ce1b0 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 17 Apr 2023 19:11:45 +0200 Subject: [PATCH 03/37] clarify/simplify use of focal length --- crates/re_log_types/src/component_types/transform.rs | 6 ++++-- crates/re_viewer/src/misc/transform_cache.rs | 8 ++------ crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 9 +++++---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/re_log_types/src/component_types/transform.rs b/crates/re_log_types/src/component_types/transform.rs index c57e70d1a9aa..e708ef671cc8 100644 --- a/crates/re_log_types/src/component_types/transform.rs +++ b/crates/re_log_types/src/component_types/transform.rs @@ -156,13 +156,15 @@ impl Pinhole { /// /// [see definition of intrinsic matrix](https://en.wikipedia.org/wiki/Camera_resectioning#Intrinsic_parameters) #[inline] - pub fn focal_length_in_pixels(&self) -> Vec2D { - [self.image_from_cam[0][0], self.image_from_cam[1][1]].into() + pub fn focal_length_in_pixels(&self) -> f32 { + // Use only the first element of the focal length vector, as we don't support non-square pixels. + self.image_from_cam[0][0] } /// Focal length. #[inline] pub fn focal_length(&self) -> Option { + // Use only the first element of the focal length vector, as we don't support non-square pixels. self.resolution.map(|r| self.image_from_cam[0][0] / r[0]) } diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 3ee773e10983..1b8ea4cd0d6d 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -256,14 +256,10 @@ fn transform_at( let props = entity_properties.get(entity_path); let distance = *props.pinhole_image_plane_distance.get(); - let focal_length = pinhole.focal_length_in_pixels(); - let focal_length = glam::vec2(focal_length.x(), focal_length.y()); - let scale = distance / focal_length; + let scale = distance / pinhole.focal_length_in_pixels(); let translation = (-pinhole.principal_point() * scale).extend(distance); let parent_from_child = glam::Affine3A::from_scale_rotation_translation( - // We want to preserve any depth that might be on the pinhole image. - // Use harmonic mean of x/y scale for those. - scale.extend(1.0 / (1.0 / scale.x + 1.0 / scale.y)), + glam::Vec3::splat(scale), glam::Quat::IDENTITY, translation, ); diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index fcdc0daedaf0..09d7e5c607c7 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -422,16 +422,17 @@ fn setup_target_config( let (projection_from_view, view_from_world) = if let Some(space_camera) = space_camera { let pinhole = space_camera.pinhole.unwrap(); // TODO: - // Put the camera at the position where it sees the entire image plane as defined - // by the pinhole camera. - let camera_distance = pinhole.focal_length_in_pixels()[0]; // We don't support non-square pixels, pick 0. ( re_renderer::view_builder::Projection::Perspective { vertical_fov: pinhole.fov_y().unwrap(), // TODO: no unwrap near_plane_distance: 0.01, // TODO: }, + // Put the camera at the position where it sees the entire image plane as defined + // by the pinhole camera. macaw::IsoTransform::look_at_rh( - pinhole.principal_point().extend(-camera_distance), + pinhole + .principal_point() + .extend(-pinhole.focal_length_in_pixels()), pinhole.principal_point().extend(0.0), -glam::Vec3::Y, ) From 23cfc6660f11288bfc2f2b2d5fbc8db546fb6bea Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 17 Apr 2023 19:45:13 +0200 Subject: [PATCH 04/37] refine camera plane distance heuristic for 2D scenes and make it configurable again --- crates/re_viewer/src/ui/selection_panel.rs | 32 +++++----------------- crates/re_viewer/src/ui/view_spatial/ui.rs | 28 +++++++++---------- 2 files changed, 20 insertions(+), 40 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index f84b282b4b6a..dcb0f3d1287a 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -7,12 +7,9 @@ use re_log_types::{ TimeType, Transform, }; -use crate::{ - ui::{view_spatial::SpatialNavigationMode, Blueprint}, - Item, UiVerbosity, ViewerContext, -}; +use crate::{ui::Blueprint, Item, UiVerbosity, ViewerContext}; -use super::{data_ui::DataUi, space_view::ViewState}; +use super::data_ui::DataUi; // --- @@ -293,13 +290,7 @@ fn blueprint_ui( // splat - the whole entity let data_blueprint = space_view.data_blueprint.data_blueprints_individual(); let mut props = data_blueprint.get(&instance_path.entity_path); - entity_props_ui( - ctx, - ui, - Some(&instance_path.entity_path), - &mut props, - &space_view.view_state, - ); + entity_props_ui(ctx, ui, Some(&instance_path.entity_path), &mut props); data_blueprint.set(instance_path.entity_path.clone(), props); } } else { @@ -313,13 +304,7 @@ fn blueprint_ui( .data_blueprint .group_mut(*data_blueprint_group_handle) { - entity_props_ui( - ctx, - ui, - None, - &mut group.properties_individual, - &space_view.view_state, - ); + entity_props_ui(ctx, ui, None, &mut group.properties_individual); } else { ctx.selection_state_mut().clear_current(); } @@ -364,7 +349,6 @@ fn entity_props_ui( ui: &mut egui::Ui, entity_path: Option<&EntityPath>, entity_props: &mut EntityProperties, - view_state: &ViewState, ) { ui.checkbox(&mut entity_props.visible, "Visible"); ui.checkbox(&mut entity_props.interactive, "Interactive") @@ -400,11 +384,9 @@ fn entity_props_ui( } ui.end_row(); - if *view_state.state_spatial.nav_mode.get() == SpatialNavigationMode::ThreeD { - if let Some(entity_path) = entity_path { - pinhole_props_ui(ctx, ui, entity_path, entity_props); - depth_props_ui(ctx, ui, entity_path, entity_props); - } + if let Some(entity_path) = entity_path { + pinhole_props_ui(ctx, ui, entity_path, entity_props); + depth_props_ui(ctx, ui, entity_path, entity_props); } }); } diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index 19a31bbf4e0c..3920a336cba0 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -152,29 +152,21 @@ impl ViewSpatialState { ) { crate::profile_function!(); - let scene_size = self.scene_bbox_accum.size().length(); - let query = ctx.current_query(); let entity_paths = data_blueprint.entity_paths().clone(); // TODO(andreas): Workaround borrow checker for entity_path in entity_paths { - Self::update_pinhole_property_heuristics( - ctx, - data_blueprint, - &query, - &entity_path, - scene_size, - ); + self.update_pinhole_property_heuristics(ctx, data_blueprint, &query, &entity_path); self.update_depth_cloud_property_heuristics(ctx, data_blueprint, &query, &entity_path); } } fn update_pinhole_property_heuristics( + &self, ctx: &mut ViewerContext<'_>, data_blueprint: &mut DataBlueprintTree, query: &re_arrow_store::LatestAtQuery, entity_path: &EntityPath, - scene_size: f32, ) { if let Some(re_log_types::Transform::Pinhole(_)) = query_latest_single::( @@ -183,11 +175,17 @@ impl ViewSpatialState { query, ) { - let default_image_plane_distance = if scene_size.is_finite() && scene_size > 0.0 { - scene_size * 0.05 - } else { - 1.0 - }; + let scene_size = self.scene_bbox_accum.size().length(); + let default_image_plane_distance = + if *self.nav_mode.get() == SpatialNavigationMode::TwoD { + // TODO(andreas): For 2D views, the extent of the scene _without_ everything under the camera would be a better heuristic. + // Use the diagonal of the entire scene, so everything is visible, making the actual 2D scene the "backdrop" + scene_size + } else if scene_size.is_finite() && scene_size > 0.0 { + scene_size * 0.05 + } else { + 1.0 + }; let mut properties = data_blueprint.data_blueprints_individual().get(entity_path); if properties.pinhole_image_plane_distance.is_auto() { From c272debba8606adf125b94211fa99e708be1f26b Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 18 Apr 2023 14:47:54 +0200 Subject: [PATCH 05/37] comment wip --- crates/re_viewer/src/misc/transform_cache.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 1b8ea4cd0d6d..5f6e90cf3ceb 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -250,6 +250,8 @@ fn transform_at( } else { *encountered_pinhole = true; + // TODO: update this comment a bit. + TODO: fix the heuristic for distance on 2d scenes!! // A pinhole camera means that we're looking at an image. // Images are spanned in their local x/y space. // Center it and move it along z, scaling the further we move. @@ -264,6 +266,14 @@ fn transform_at( translation, ); + // Let's think the inverse of this through, in the context of a 2D view that sits at a pinhole camera: + // In this situation, we have a camera that is `focal_length_in_pixels` away from the XY image plane + // and peering down on it from the `principal_point`. + // + // In this case, any 3D object that is up to `pinhole_image_plane_distance` away from the camera should be visible! + // This means we need to apply the scaling factor of `pinhole.focal_length_in_pixels() / distance`, + // as well as undo the translation of the principal point. + Ok(Some(parent_from_child)) } } From e8cf284b25f69e71b622304001808eb47edb0f4c Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 26 Apr 2023 15:38:03 +0200 Subject: [PATCH 06/37] merge fixup and comment improvements --- crates/re_viewer/src/misc/transform_cache.rs | 9 ++++----- .../src/ui/view_spatial/scene/scene_part/cameras.rs | 2 +- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 5 +---- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 5f6e90cf3ceb..8a9ccf631662 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -250,11 +250,8 @@ fn transform_at( } else { *encountered_pinhole = true; - // TODO: update this comment a bit. - TODO: fix the heuristic for distance on 2d scenes!! - // A pinhole camera means that we're looking at an image. - // Images are spanned in their local x/y space. - // Center it and move it along z, scaling the further we move. + // A pinhole camera means that we're looking at some 2D image plane from a single point (the pinhole). + // Center the image plane and move it along z, scaling the further the image plane is. let props = entity_properties.get(entity_path); let distance = *props.pinhole_image_plane_distance.get(); @@ -271,8 +268,10 @@ fn transform_at( // and peering down on it from the `principal_point`. // // In this case, any 3D object that is up to `pinhole_image_plane_distance` away from the camera should be visible! + // (everything behind, is clipped by the image plane) // This means we need to apply the scaling factor of `pinhole.focal_length_in_pixels() / distance`, // as well as undo the translation of the principal point. + // .. which is just what the inverse transformation does ◼️. Ok(Some(parent_from_child)) } diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index db57e57842af..ad1a03f4aef9 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -84,7 +84,7 @@ impl CamerasPart { // If this transform is not representable as rigid transform, the camera is probably under another camera transform, // in which case we don't (yet) know how to deal with this!] // TODO: what is this special case - let world_from_camera = if transforms.reference_path() == entity_path { + let world_from_camera = if transforms.reference_path() == ent_path { macaw::IsoTransform::IDENTITY } else { match macaw::IsoTransform::from_mat4(&world_from_parent.into()) { diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index c99f54947ed2..30cba86852ff 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -308,10 +308,7 @@ fn view_2d_scrollable( fov_y: None, }; - let space_camera = scene - .space_cameras - .iter() - .find(|c| c.instance_path_hash.entity_path_hash == space.hash()); + let space_camera = scene.space_cameras.iter().find(|c| &c.ent_path == space); let Ok(target_config) = setup_target_config( &painter, From 0791702abb50766780b6f6e37d9b24dc276eae27 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 26 Apr 2023 16:32:29 +0200 Subject: [PATCH 07/37] line & point builder now work with affine transforms --- crates/re_renderer/examples/depth_cloud.rs | 7 ++++--- crates/re_renderer/examples/multiview.rs | 6 ++++-- crates/re_renderer/src/line_strip_builder.rs | 4 ++-- crates/re_renderer/src/point_cloud_builder.rs | 4 ++-- crates/re_renderer/src/renderer/lines.rs | 9 +++++---- crates/re_renderer/src/renderer/point_cloud.rs | 10 +++++----- .../src/ui/view_spatial/scene/primitives.rs | 18 ++++-------------- .../view_spatial/scene/scene_part/arrows3d.rs | 2 +- .../view_spatial/scene/scene_part/boxes2d.rs | 2 +- .../view_spatial/scene/scene_part/boxes3d.rs | 2 +- .../view_spatial/scene/scene_part/cameras.rs | 2 +- .../view_spatial/scene/scene_part/lines2d.rs | 2 +- .../view_spatial/scene/scene_part/lines3d.rs | 2 +- .../view_spatial/scene/scene_part/points2d.rs | 2 +- .../view_spatial/scene/scene_part/points3d.rs | 2 +- 15 files changed, 34 insertions(+), 40 deletions(-) diff --git a/crates/re_renderer/examples/depth_cloud.rs b/crates/re_renderer/examples/depth_cloud.rs index dddc40d1ef71..38fb6c35c95d 100644 --- a/crates/re_renderer/examples/depth_cloud.rs +++ b/crates/re_renderer/examples/depth_cloud.rs @@ -287,9 +287,10 @@ impl framework::Example for RenderDepthClouds { let splits = framework::split_resolution(resolution, 1, 2).collect::>(); let frame_size = albedo.dimensions.as_vec2().extend(0.0) / 15.0; - let scale = glam::Mat4::from_scale(frame_size); - let rotation = glam::Mat4::IDENTITY; - let translation_center = glam::Mat4::from_translation(-glam::Vec3::splat(0.5) * frame_size); + let scale = glam::Affine3A::from_scale(frame_size); + let rotation = glam::Affine3A::IDENTITY; + let translation_center = + glam::Affine3A::from_translation(-glam::Vec3::splat(0.5) * frame_size); let world_from_model = rotation * translation_center * scale; let frame_draw_data = { diff --git a/crates/re_renderer/examples/multiview.rs b/crates/re_renderer/examples/multiview.rs index 48b77e24388f..bc6bc1783a3b 100644 --- a/crates/re_renderer/examples/multiview.rs +++ b/crates/re_renderer/examples/multiview.rs @@ -113,7 +113,9 @@ fn build_lines(re_ctx: &mut RenderContext, seconds_since_startup: f32) -> LineDr // Blue spiral, rotating builder .batch("blue spiral") - .world_from_obj(glam::Mat4::from_rotation_x(seconds_since_startup * 10.0)) + .world_from_obj(glam::Affine3A::from_rotation_x( + seconds_since_startup * 10.0, + )) .add_strip((0..1000).map(|i| { glam::vec3( (i as f32 * 0.01).sin() * 2.0, @@ -318,7 +320,7 @@ impl Example for Multiview { let mut builder = PointCloudBuilder::new(re_ctx); builder .batch("Random Points") - .world_from_obj(glam::Mat4::from_rotation_x(seconds_since_startup)) + .world_from_obj(glam::Affine3A::from_rotation_x(seconds_since_startup)) .add_points( self.random_points_positions.len(), self.random_points_positions.iter().cloned(), diff --git a/crates/re_renderer/src/line_strip_builder.rs b/crates/re_renderer/src/line_strip_builder.rs index 9fee0afdb964..f6e6ee650e6f 100644 --- a/crates/re_renderer/src/line_strip_builder.rs +++ b/crates/re_renderer/src/line_strip_builder.rs @@ -65,7 +65,7 @@ impl LineStripSeriesBuilder { pub fn batch(&mut self, label: impl Into) -> LineBatchBuilder<'_> { self.batches.push(LineBatchInfo { label: label.into(), - world_from_obj: glam::Mat4::IDENTITY, + world_from_obj: glam::Affine3A::IDENTITY, line_vertex_count: 0, overall_outline_mask_ids: OutlineMaskPreference::NONE, additional_outline_mask_ids_vertex_ranges: Vec::new(), @@ -138,7 +138,7 @@ impl<'a> LineBatchBuilder<'a> { /// Sets the `world_from_obj` matrix for the *entire* batch. #[inline] - pub fn world_from_obj(mut self, world_from_obj: glam::Mat4) -> Self { + pub fn world_from_obj(mut self, world_from_obj: glam::Affine3A) -> Self { self.batch_mut().world_from_obj = world_from_obj; self } diff --git a/crates/re_renderer/src/point_cloud_builder.rs b/crates/re_renderer/src/point_cloud_builder.rs index 63278d5c7e69..2c07aebb8191 100644 --- a/crates/re_renderer/src/point_cloud_builder.rs +++ b/crates/re_renderer/src/point_cloud_builder.rs @@ -63,7 +63,7 @@ impl PointCloudBuilder { pub fn batch(&mut self, label: impl Into) -> PointCloudBatchBuilder<'_> { self.batches.push(PointCloudBatchInfo { label: label.into(), - world_from_obj: glam::Mat4::IDENTITY, + world_from_obj: glam::Affine3A::IDENTITY, flags: PointCloudBatchFlags::ENABLE_SHADING, point_count: 0, overall_outline_mask_ids: OutlineMaskPreference::NONE, @@ -128,7 +128,7 @@ impl<'a> PointCloudBatchBuilder<'a> { /// Sets the `world_from_obj` matrix for the *entire* batch. #[inline] - pub fn world_from_obj(mut self, world_from_obj: glam::Mat4) -> Self { + pub fn world_from_obj(mut self, world_from_obj: glam::Affine3A) -> Self { self.batch_mut().world_from_obj = world_from_obj; self } diff --git a/crates/re_renderer/src/renderer/lines.rs b/crates/re_renderer/src/renderer/lines.rs index a46571928c63..f645bc5f523d 100644 --- a/crates/re_renderer/src/renderer/lines.rs +++ b/crates/re_renderer/src/renderer/lines.rs @@ -239,7 +239,7 @@ pub struct LineBatchInfo { /// /// TODO(andreas): We don't apply scaling to the radius yet. Need to pass a scaling factor like this in /// `let scale = Mat3::from(world_from_obj).determinant().abs().cbrt()` - pub world_from_obj: glam::Mat4, + pub world_from_obj: glam::Affine3A, /// Number of vertices covered by this batch. /// @@ -351,7 +351,7 @@ impl LineDrawData { let batches = if batches.is_empty() { vec![LineBatchInfo { - world_from_obj: glam::Mat4::IDENTITY, + world_from_obj: glam::Affine3A::IDENTITY, label: "LineDrawData::fallback_batch".into(), line_vertex_count: vertices.len() as _, overall_outline_mask_ids: OutlineMaskPreference::NONE, @@ -602,7 +602,7 @@ impl LineDrawData { batches .iter() .map(|batch_info| gpu_data::BatchUniformBuffer { - world_from_obj: batch_info.world_from_obj.into(), + world_from_obj: glam::Mat4::from(batch_info.world_from_obj).into(), outline_mask_ids: batch_info .overall_outline_mask_ids .0 @@ -626,7 +626,8 @@ impl LineDrawData { .additional_outline_mask_ids_vertex_ranges .iter() .map(|(_, mask)| gpu_data::BatchUniformBuffer { - world_from_obj: batch_info.world_from_obj.into(), + world_from_obj: glam::Mat4::from(batch_info.world_from_obj) + .into(), outline_mask_ids: mask.0.unwrap_or_default().into(), picking_object_id: batch_info.picking_object_id, end_padding: Default::default(), diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index 03611b80a256..fb205d5b4127 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -112,10 +112,9 @@ pub struct PointCloudBatchInfo { /// Transformation applies to point positions /// - /// TODO(andreas): Since we blindly apply this to positions only there is no restriction on this matrix. /// TODO(andreas): We don't apply scaling to the radius yet. Need to pass a scaling factor like this in /// `let scale = Mat3::from(world_from_obj).determinant().abs().cbrt()` - pub world_from_obj: glam::Mat4, + pub world_from_obj: glam::Affine3A, /// Additional properties of this point cloud batch. pub flags: PointCloudBatchFlags, @@ -201,7 +200,7 @@ impl PointCloudDrawData { let fallback_batches = [PointCloudBatchInfo { label: "fallback_batches".into(), - world_from_obj: glam::Mat4::IDENTITY, + world_from_obj: glam::Affine3A::IDENTITY, flags: PointCloudBatchFlags::empty(), point_count: vertices.len() as _, overall_outline_mask_ids: OutlineMaskPreference::NONE, @@ -397,7 +396,7 @@ impl PointCloudDrawData { batches .iter() .map(|batch_info| gpu_data::BatchUniformBuffer { - world_from_obj: batch_info.world_from_obj.into(), + world_from_obj: glam::Mat4::from(batch_info.world_from_obj).into(), flags: batch_info.flags.bits.into(), outline_mask_ids: batch_info .overall_outline_mask_ids @@ -422,7 +421,8 @@ impl PointCloudDrawData { .additional_outline_mask_ids_vertex_ranges .iter() .map(|(_, mask)| gpu_data::BatchUniformBuffer { - world_from_obj: batch_info.world_from_obj.into(), + world_from_obj: glam::Mat4::from(batch_info.world_from_obj) + .into(), flags: batch_info.flags.bits.into(), outline_mask_ids: mask.0.unwrap_or_default().into(), end_padding: Default::default(), diff --git a/crates/re_viewer/src/ui/view_spatial/scene/primitives.rs b/crates/re_viewer/src/ui/view_spatial/scene/primitives.rs index 407d1eec47fa..5a2e2afa4955 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/primitives.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/primitives.rs @@ -105,22 +105,12 @@ impl SceneSpatialPrimitives { // we calculate a per batch bounding box for lines and points. // TODO(andreas): We should keep these around to speed up picking! for (batch, vertex_iter) in points.iter_vertices_by_batch() { - // Only use points which are an IsoTransform to update the bounding box - // This prevents crazy bounds-increases when projecting 3d to 2d - // See: https://github.com/rerun-io/rerun/issues/1203 - if let Some(transform) = macaw::IsoTransform::from_mat4(&batch.world_from_obj) { - let batch_bb = macaw::BoundingBox::from_points(vertex_iter.map(|v| v.position)); - *bounding_box = bounding_box.union(batch_bb.transform_affine3(&transform.into())); - } + let batch_bb = macaw::BoundingBox::from_points(vertex_iter.map(|v| v.position)); + *bounding_box = bounding_box.union(batch_bb.transform_affine3(&batch.world_from_obj)); } for (batch, vertex_iter) in line_strips.iter_vertices_by_batch() { - // Only use points which are an IsoTransform to update the bounding box - // This prevents crazy bounds-increases when projecting 3d to 2d - // See: https://github.com/rerun-io/rerun/issues/1203 - if let Some(transform) = macaw::IsoTransform::from_mat4(&batch.world_from_obj) { - let batch_bb = macaw::BoundingBox::from_points(vertex_iter.map(|v| v.position)); - *bounding_box = bounding_box.union(batch_bb.transform_affine3(&transform.into())); - } + let batch_bb = macaw::BoundingBox::from_points(vertex_iter.map(|v| v.position)); + *bounding_box = bounding_box.union(batch_bb.transform_affine3(&batch.world_from_obj)); } for mesh in meshes { diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs index 48652e9c99cd..60d4e5fccf11 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/arrows3d.rs @@ -34,7 +34,7 @@ impl Arrows3DPart { .primitives .line_strips .batch("arrows") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs index 59e2398a46d2..3de91ba5ad94 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs @@ -40,7 +40,7 @@ impl Boxes2DPart { .primitives .line_strips .batch("2d boxes") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs index 5352d667f240..991a45e18bc0 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes3d.rs @@ -35,7 +35,7 @@ impl Boxes3DPart { .primitives .line_strips .batch("box 3d") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index ad1a03f4aef9..6b96d76c809f 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -163,7 +163,7 @@ impl CamerasPart { .primitives .line_strips .batch("camera frustum") - .world_from_obj(world_from_parent.into()) + .world_from_obj(world_from_parent) .outline_mask_ids(entity_highlight.overall) .picking_object_id(instance_layer_id.object); let lines = batch diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs index 64ef4ae8a0fe..0a9b47b9b5ed 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs @@ -32,7 +32,7 @@ impl Lines2DPart { .primitives .line_strips .batch("lines 2d") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs index b6a2bb70b320..492e45391f15 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines3d.rs @@ -31,7 +31,7 @@ impl Lines3DPart { .primitives .line_strips .batch("lines 3d") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs index 5f9459f0ae1a..6da3de364805 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs @@ -110,7 +110,7 @@ impl Points2DPart { .primitives .points .batch("2d points") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs index 1845757bb442..1da202dd38fc 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points3d.rs @@ -117,7 +117,7 @@ impl Points3DPart { .primitives .points .batch("3d points") - .world_from_obj(world_from_obj.into()) + .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); From 6d5acba89610bbbfb1285b3c524277f85da89cf6 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 26 Apr 2023 18:46:41 +0200 Subject: [PATCH 08/37] wip --- .../view_spatial/scene/scene_part/cameras.rs | 32 +++++++++++-------- crates/re_viewer/src/ui/view_spatial/ui.rs | 13 ++++---- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 14 +++++--- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index 6b96d76c809f..7a000c0f9e70 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -81,22 +81,26 @@ impl CamerasPart { return; }; - // If this transform is not representable as rigid transform, the camera is probably under another camera transform, - // in which case we don't (yet) know how to deal with this!] - // TODO: what is this special case - let world_from_camera = if transforms.reference_path() == ent_path { - macaw::IsoTransform::IDENTITY - } else { - match macaw::IsoTransform::from_mat4(&world_from_parent.into()) { - Some(world_from_camera) => world_from_camera, - None => { - return; - } - } - }; - let frustum_length = *props.pinhole_image_plane_distance.get(); + // If the camera is our reference, there is nothing for us to display. + if transforms.reference_path() == ent_path { + scene.space_cameras.push(SpaceCamera3D { + ent_path: ent_path.clone(), + view_coordinates, + world_from_camera: macaw::IsoTransform::IDENTITY, + pinhole: Some(pinhole), + picture_plane_distance: frustum_length, + }); + return; + } + + // If this transform is not representable an iso transform transform we can't display it yet. + // This would happen if the camera is under another camera or under a transform with non-uniform scale. + let Some(world_from_camera) = macaw::IsoTransform::from_mat4(&world_from_parent.into()) else { + return; + }; + scene.space_cameras.push(SpaceCamera3D { ent_path: ent_path.clone(), view_coordinates, diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index 4667045763a3..99f3059ecdfd 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -175,14 +175,15 @@ impl ViewSpatialState { query, ) { - let scene_size = self.scene_bbox_accum.size().length(); + let scene_size = self.scene_bbox_accum.size(); + let scene_diagonal = scene_size.length(); let default_image_plane_distance = if *self.nav_mode.get() == SpatialNavigationMode::TwoD { - // TODO(andreas): For 2D views, the extent of the scene _without_ everything under the camera would be a better heuristic. - // Use the diagonal of the entire scene, so everything is visible, making the actual 2D scene the "backdrop" - scene_size - } else if scene_size.is_finite() && scene_size > 0.0 { - scene_size * 0.05 + // If we're in 2D mode things get tricky! + // TODO: describe why + 1000.0 + } else if scene_diagonal.is_finite() && scene_diagonal > 0.0 { + scene_diagonal * 0.05 } else { 1.0 }; diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 30cba86852ff..7e93bd741654 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -414,15 +414,21 @@ fn setup_target_config( gpu_bridge::viewport_resolution_in_pixels(painter.clip_rect(), pixels_from_points); anyhow::ensure!(resolution_in_pixel[0] > 0 && resolution_in_pixel[1] > 0); - // TODO: This should be in the eye? + // TODO: Determine earlier and let (projection_from_view, view_from_world) = if let Some(space_camera) = space_camera { - let pinhole = space_camera.pinhole.unwrap(); // TODO: + // Looking through a space camera at the space root in a 2D view is *similar* than + // looking through that same camera in a 3D view. + // + // There is once crucial difference though: the XY coordinates are supposed to be "pixels" + // I.e. if I place an image without any scaling (!), I expect the XY coordinates to be + // in pixels of that image. + let pinhole = space_camera.pinhole.unwrap(); // TODO: ( re_renderer::view_builder::Projection::Perspective { - vertical_fov: pinhole.fov_y().unwrap(), // TODO: no unwrap - near_plane_distance: 0.01, // TODO: + vertical_fov: pinhole.fov_y().unwrap_or(Eye::DEFAULT_FOV_Y), + near_plane_distance: 0.01, }, // Put the camera at the position where it sees the entire image plane as defined // by the pinhole camera. From 7d8ee3ba16df18026e47725265ffd3d51455ced5 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 27 Apr 2023 09:03:09 +0200 Subject: [PATCH 09/37] improved image plane heuristic for 2D --- crates/re_viewer/src/ui/view_spatial/ui.rs | 47 ++++++++++++++----- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 7 +-- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index 99f3059ecdfd..c0834722df83 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -175,21 +175,42 @@ impl ViewSpatialState { query, ) { - let scene_size = self.scene_bbox_accum.size(); - let scene_diagonal = scene_size.length(); - let default_image_plane_distance = - if *self.nav_mode.get() == SpatialNavigationMode::TwoD { - // If we're in 2D mode things get tricky! - // TODO: describe why - 1000.0 - } else if scene_diagonal.is_finite() && scene_diagonal > 0.0 { - scene_diagonal * 0.05 - } else { - 1.0 - }; - let mut properties = data_blueprint.data_blueprints_individual().get(entity_path); if properties.pinhole_image_plane_distance.is_auto() { + let default_image_plane_distance = + if *self.nav_mode.get() == SpatialNavigationMode::TwoD { + // If we're in 2D mode things get tricky! + // The issue is that position of any object not directly under the camera is influenced + // by the image plane distance itself! The bounding box is therefore not useful at all. + // + // To make matters worse, we're running into accuracy issue surprisingly quickly! + // Typically, at about 1000 units we can already see flickering in points when points are close. + // (probably because pixel units are large and a high plane distance leads to a very small scale factor + // also as of writing we don't have a good heuristic for the near plane either, making matters worse) + // + // What we *do* know is that as long the bounding box pierces through the image plane, + // we apparently didn't choose a big enough image plane distance. + // So let's enlarge it a bit. + // (We could do so more accurately by taking the camera distance into account, but this would complicate things further.) + // + // Note that we wouldn't have any of these issues if the image plane wouldn't clip the scene! + + // TODO: Another issue with this is that it gets XY extents wrong at first and only works when the camera was added. + if self.scene_bbox.max.z > 0.0 { + properties.pinhole_image_plane_distance.get().at_least(1.0) * 1.1 + } else { + properties.pinhole_image_plane_distance.get().at_least(1.0) + } + } else { + let scene_size = self.scene_bbox_accum.size(); + let scene_diagonal = scene_size.length(); + if scene_diagonal.is_finite() && scene_diagonal > 0.0 { + scene_diagonal * 0.05 + } else { + 1.0 + } + }; + properties.pinhole_image_plane_distance = EditableAutoValue::Auto(default_image_plane_distance); data_blueprint diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 7e93bd741654..4e6e6744970d 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -420,9 +420,10 @@ fn setup_target_config( // Looking through a space camera at the space root in a 2D view is *similar* than // looking through that same camera in a 3D view. // - // There is once crucial difference though: the XY coordinates are supposed to be "pixels" - // I.e. if I place an image without any scaling (!), I expect the XY coordinates to be - // in pixels of that image. + // There is once crucial difference though: + // Images are supposed to be on the XY plane and scaled in pixels. + // I.e. if I place an image without 2D transform, it gets an identify transform + // and X/Y coordinates correspond to pixels. let pinhole = space_camera.pinhole.unwrap(); // TODO: ( From 2398efe927c2c112459e98d405707a3bc72258a0 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 27 Apr 2023 10:45:51 +0200 Subject: [PATCH 10/37] hack for image plane distance for in inverse pinhole transforms. remove setting from ui again for 2d views --- crates/re_viewer/src/misc/transform_cache.rs | 13 ++++--- crates/re_viewer/src/ui/selection_panel.rs | 27 ++++++++++--- crates/re_viewer/src/ui/view_spatial/ui.rs | 40 +++----------------- 3 files changed, 35 insertions(+), 45 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 8a9ccf631662..adbd62178da0 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -126,8 +126,11 @@ impl TransformCache { match transform_at( ¤t_tree.path, entity_db, - entity_prop_map, &query, + // TODO(andreas): For inverse pinhole cameras we'd like the image plane to be indefinitely far away. + // High values quickly run into precision issues though. + // What we really want is to render everything under the inverse pinhole on top, independent of everything else. + |_| 500.0, &mut encountered_pinhole, ) { Err(unreachable_reason) => { @@ -181,11 +184,12 @@ impl TransformCache { for child_tree in tree.children.values() { let mut encountered_pinhole = encountered_pinhole; + let reference_from_child = match transform_at( &child_tree.path, entity_db, - entity_properties, query, + |p| *entity_properties.get(p).pinhole_image_plane_distance.get(), &mut encountered_pinhole, ) { Err(unreachable_reason) => { @@ -234,8 +238,8 @@ impl TransformCache { fn transform_at( entity_path: &EntityPath, entity_db: &EntityDb, - entity_properties: &EntityPropertyMap, query: &LatestAtQuery, + pinhole_image_plane_distance: impl Fn(&EntityPath) -> f32, encountered_pinhole: &mut bool, ) -> Result, UnreachableTransform> { if let Some(transform) = query_latest_single(entity_db, entity_path, query) { @@ -252,8 +256,7 @@ fn transform_at( // A pinhole camera means that we're looking at some 2D image plane from a single point (the pinhole). // Center the image plane and move it along z, scaling the further the image plane is. - let props = entity_properties.get(entity_path); - let distance = *props.pinhole_image_plane_distance.get(); + let distance = pinhole_image_plane_distance(entity_path); let scale = distance / pinhole.focal_length_in_pixels(); let translation = (-pinhole.principal_point() * scale).extend(distance); diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index dcb0f3d1287a..841ea082c5f6 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -9,7 +9,7 @@ use re_log_types::{ use crate::{ui::Blueprint, Item, UiVerbosity, ViewerContext}; -use super::data_ui::DataUi; +use super::{data_ui::DataUi, space_view::ViewState, view_spatial::SpatialNavigationMode}; // --- @@ -290,7 +290,13 @@ fn blueprint_ui( // splat - the whole entity let data_blueprint = space_view.data_blueprint.data_blueprints_individual(); let mut props = data_blueprint.get(&instance_path.entity_path); - entity_props_ui(ctx, ui, Some(&instance_path.entity_path), &mut props); + entity_props_ui( + ctx, + ui, + Some(&instance_path.entity_path), + &mut props, + &space_view.view_state, + ); data_blueprint.set(instance_path.entity_path.clone(), props); } } else { @@ -304,7 +310,13 @@ fn blueprint_ui( .data_blueprint .group_mut(*data_blueprint_group_handle) { - entity_props_ui(ctx, ui, None, &mut group.properties_individual); + entity_props_ui( + ctx, + ui, + None, + &mut group.properties_individual, + &space_view.view_state, + ); } else { ctx.selection_state_mut().clear_current(); } @@ -349,6 +361,7 @@ fn entity_props_ui( ui: &mut egui::Ui, entity_path: Option<&EntityPath>, entity_props: &mut EntityProperties, + view_state: &ViewState, ) { ui.checkbox(&mut entity_props.visible, "Visible"); ui.checkbox(&mut entity_props.interactive, "Interactive") @@ -384,9 +397,11 @@ fn entity_props_ui( } ui.end_row(); - if let Some(entity_path) = entity_path { - pinhole_props_ui(ctx, ui, entity_path, entity_props); - depth_props_ui(ctx, ui, entity_path, entity_props); + if *view_state.state_spatial.nav_mode.get() == SpatialNavigationMode::ThreeD { + if let Some(entity_path) = entity_path { + pinhole_props_ui(ctx, ui, entity_path, entity_props); + depth_props_ui(ctx, ui, entity_path, entity_props); + } } }); } diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index c0834722df83..df20dec00e2d 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -177,40 +177,12 @@ impl ViewSpatialState { { let mut properties = data_blueprint.data_blueprints_individual().get(entity_path); if properties.pinhole_image_plane_distance.is_auto() { - let default_image_plane_distance = - if *self.nav_mode.get() == SpatialNavigationMode::TwoD { - // If we're in 2D mode things get tricky! - // The issue is that position of any object not directly under the camera is influenced - // by the image plane distance itself! The bounding box is therefore not useful at all. - // - // To make matters worse, we're running into accuracy issue surprisingly quickly! - // Typically, at about 1000 units we can already see flickering in points when points are close. - // (probably because pixel units are large and a high plane distance leads to a very small scale factor - // also as of writing we don't have a good heuristic for the near plane either, making matters worse) - // - // What we *do* know is that as long the bounding box pierces through the image plane, - // we apparently didn't choose a big enough image plane distance. - // So let's enlarge it a bit. - // (We could do so more accurately by taking the camera distance into account, but this would complicate things further.) - // - // Note that we wouldn't have any of these issues if the image plane wouldn't clip the scene! - - // TODO: Another issue with this is that it gets XY extents wrong at first and only works when the camera was added. - if self.scene_bbox.max.z > 0.0 { - properties.pinhole_image_plane_distance.get().at_least(1.0) * 1.1 - } else { - properties.pinhole_image_plane_distance.get().at_least(1.0) - } - } else { - let scene_size = self.scene_bbox_accum.size(); - let scene_diagonal = scene_size.length(); - if scene_diagonal.is_finite() && scene_diagonal > 0.0 { - scene_diagonal * 0.05 - } else { - 1.0 - } - }; - + let scene_size = self.scene_bbox_accum.size().length(); + let default_image_plane_distance = if scene_size.is_finite() && scene_size > 0.0 { + scene_size * 0.05 + } else { + 1.0 + }; properties.pinhole_image_plane_distance = EditableAutoValue::Auto(default_image_plane_distance); data_blueprint From 338c1dabc4e19fe97c11dfe5418abec917684d51 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 27 Apr 2023 17:37:33 +0200 Subject: [PATCH 11/37] add viewport transformation to viewbuilder --- .vscode/settings.json | 1 + crates/re_renderer/examples/picking.rs | 6 +- .../src/draw_phases/picking_layer.rs | 38 ++++--------- crates/re_renderer/src/lib.rs | 4 +- crates/re_renderer/src/rect.rs | 50 ++++++++++++++++- .../re_renderer/src/renderer/debug_overlay.rs | 4 +- crates/re_renderer/src/transform.rs | 56 +++++++++++++++++++ crates/re_renderer/src/view_builder.rs | 40 ++++++++++++- crates/re_viewer/src/gpu_bridge/mod.rs | 1 + crates/re_viewer/src/ui/view_spatial/ui.rs | 2 +- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 1 + crates/re_viewer/src/ui/view_spatial/ui_3d.rs | 1 + 12 files changed, 167 insertions(+), 37 deletions(-) create mode 100644 crates/re_renderer/src/transform.rs diff --git a/.vscode/settings.json b/.vscode/settings.json index 17f3e3d71095..d2c500877355 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -40,6 +40,7 @@ "Skybox", "smallvec", "swapchain", + "texcoord", "texcoords", "Tonemapper", "tonemapping", diff --git a/crates/re_renderer/examples/picking.rs b/crates/re_renderer/examples/picking.rs index afcb442ffbba..fbcfc3888f53 100644 --- a/crates/re_renderer/examples/picking.rs +++ b/crates/re_renderer/examples/picking.rs @@ -3,8 +3,8 @@ use rand::Rng; use re_renderer::{ renderer::MeshInstance, view_builder::{Projection, TargetConfiguration, ViewBuilder}, - Color32, GpuReadbackIdentifier, IntRect, PickingLayerId, PickingLayerInstanceId, - PickingLayerProcessor, PointCloudBuilder, Size, + Color32, GpuReadbackIdentifier, PickingLayerId, PickingLayerInstanceId, PickingLayerProcessor, + PointCloudBuilder, RectInt, Size, }; mod framework; @@ -145,7 +145,7 @@ impl framework::Example for Picking { // Use an uneven number of pixels for the picking rect so that there is a clearly defined middle-pixel. // (for this sample a size of 1 would be sufficient, but for a real application you'd want to use a larger size to allow snapping) let picking_rect_size = 31; - let picking_rect = IntRect::from_middle_and_extent( + let picking_rect = RectInt::from_middle_and_extent( self.picking_position.as_ivec2(), glam::uvec2(picking_rect_size, picking_rect_size), ); diff --git a/crates/re_renderer/src/draw_phases/picking_layer.rs b/crates/re_renderer/src/draw_phases/picking_layer.rs index cd4d6601ebb0..9d698d2ce6d6 100644 --- a/crates/re_renderer/src/draw_phases/picking_layer.rs +++ b/crates/re_renderer/src/draw_phases/picking_layer.rs @@ -13,14 +13,16 @@ use crate::{ allocator::create_and_fill_uniform_buffer, global_bindings::FrameUniformBuffer, include_shader_module, + rect::RectF32, texture_info::Texture2DBufferInfo, + transform::{ndc_from_pixel, region_of_interest_from_ndc}, view_builder::ViewBuilder, wgpu_resources::{ BindGroupDesc, BindGroupEntry, BindGroupLayoutDesc, GpuBindGroup, GpuRenderPipelineHandle, GpuTexture, GpuTextureHandle, PipelineLayoutDesc, PoolError, RenderPipelineDesc, TextureDesc, WgpuResourcePools, }, - DebugLabel, GpuReadbackBuffer, GpuReadbackIdentifier, IntRect, RenderContext, + DebugLabel, GpuReadbackBuffer, GpuReadbackIdentifier, RectInt, RenderContext, }; use smallvec::smallvec; @@ -32,7 +34,7 @@ pub struct PickingResult { /// Picking rect supplied on picking request. /// Describes the area of the picking layer that was read back. - pub rect: IntRect, + pub rect: RectInt, /// Picking id data for the requested rectangle. /// @@ -64,8 +66,7 @@ impl PickingResult { [(pos_on_picking_rect.y * self.rect.width() + pos_on_picking_rect.x) as usize]; self.world_from_cropped_projection.project_point3( - pixel_coord_to_ndc(pos_on_picking_rect.as_vec2(), self.rect.extent.as_vec2()) - .extend(raw_depth), + ndc_from_pixel(pos_on_picking_rect.as_vec2(), self.rect.extent).extend(raw_depth), ) } @@ -81,7 +82,7 @@ impl PickingResult { /// Type used as user data on the gpu readback belt. struct ReadbackBeltMetadata { - picking_rect: IntRect, + picking_rect: RectInt, world_from_cropped_projection: glam::Mat4, user_data: T, @@ -125,14 +126,6 @@ impl From for [u32; 4] { } } -/// Converts a pixel coordinate to normalized device coordinates. -pub fn pixel_coord_to_ndc(coord: glam::Vec2, target_resolution: glam::Vec2) -> glam::Vec2 { - glam::vec2( - coord.x / target_resolution.x * 2.0 - 1.0, - 1.0 - coord.y / target_resolution.y * 2.0, - ) -} - #[derive(thiserror::Error, Debug)] pub enum PickingLayerError { #[error(transparent)] @@ -184,7 +177,7 @@ impl PickingLayerProcessor { ctx: &mut RenderContext, view_name: &DebugLabel, screen_resolution: glam::UVec2, - picking_rect: IntRect, + picking_rect: RectInt, frame_uniform_buffer_content: &FrameUniformBuffer, enable_picking_target_sampling: bool, readback_identifier: GpuReadbackIdentifier, @@ -234,18 +227,11 @@ impl PickingLayerProcessor { DepthReadbackWorkaround::new(ctx, picking_rect.extent, picking_depth_target.handle) }); - let rect_min = picking_rect.left_top.as_vec2(); - let rect_max = rect_min + picking_rect.extent.as_vec2(); - let screen_resolution = screen_resolution.as_vec2(); - // y axis is flipped in NDC, therefore we need to flip the y axis of the rect. - let rect_min_ndc = - pixel_coord_to_ndc(glam::vec2(rect_min.x, rect_max.y), screen_resolution); - let rect_max_ndc = - pixel_coord_to_ndc(glam::vec2(rect_max.x, rect_min.y), screen_resolution); - let scale = 2.0 / (rect_max_ndc - rect_min_ndc); - let translation = -0.5 * (rect_min_ndc + rect_max_ndc); - let cropped_projection_from_projection = glam::Mat4::from_scale(scale.extend(1.0)) - * glam::Mat4::from_translation(translation.extend(0.0)); + let pixel_size = screen_resolution.as_vec2().recip(); + let cropped_projection_from_projection = region_of_interest_from_ndc(RectF32 { + left_top: picking_rect.left_top.as_vec2() * pixel_size, + extent: picking_rect.extent.as_vec2() * pixel_size, + }); // Setup frame uniform buffer let previous_projection_from_world: glam::Mat4 = diff --git a/crates/re_renderer/src/lib.rs b/crates/re_renderer/src/lib.rs index 1932e1aeb9e2..9c11b90100a5 100644 --- a/crates/re_renderer/src/lib.rs +++ b/crates/re_renderer/src/lib.rs @@ -25,6 +25,7 @@ mod global_bindings; mod line_strip_builder; mod point_cloud_builder; mod size; +mod transform; mod wgpu_buffer_types; mod wgpu_resources; @@ -41,6 +42,7 @@ pub use debug_label::DebugLabel; pub use depth_offset::DepthOffset; pub use line_strip_builder::{LineStripBuilder, LineStripSeriesBuilder}; pub use point_cloud_builder::{PointCloudBatchBuilder, PointCloudBuilder}; +pub use rect::RectF32; pub use size::Size; pub use view_builder::{AutoSizeConfig, ViewBuilder}; pub use wgpu_resources::WgpuResourcePoolStatistics; @@ -67,7 +69,7 @@ mod file_server; pub use self::file_server::FileServer; mod rect; -pub use rect::IntRect; +pub use rect::RectInt; #[cfg(not(all(not(target_arch = "wasm32"), debug_assertions)))] // wasm or release builds #[rustfmt::skip] // it's auto-generated diff --git a/crates/re_renderer/src/rect.rs b/crates/re_renderer/src/rect.rs index 8b70e81ac357..acbea7a13930 100644 --- a/crates/re_renderer/src/rect.rs +++ b/crates/re_renderer/src/rect.rs @@ -2,7 +2,7 @@ /// /// Typically used for texture cutouts etc. #[derive(Clone, Copy, Debug)] -pub struct IntRect { +pub struct RectInt { /// The top left corner of the rectangle. pub left_top: glam::IVec2, @@ -10,7 +10,7 @@ pub struct IntRect { pub extent: glam::UVec2, } -impl IntRect { +impl RectInt { #[inline] pub fn from_middle_and_extent(middle: glam::IVec2, size: glam::UVec2) -> Self { Self { @@ -38,3 +38,49 @@ impl IntRect { } } } + +/// A 2D rectangle with float coordinates. +#[derive(Clone, Copy, Debug)] +pub struct RectF32 { + /// The top left corner of the rectangle. + pub left_top: glam::Vec2, + + /// The size of the rectangle. Supposed to be positive. + pub extent: glam::Vec2, +} + +impl RectF32 { + /// The unit rectangle, defined as (0, 0) - (1, 1). + pub const UNIT: RectF32 = RectF32 { + left_top: glam::Vec2::ZERO, + extent: glam::Vec2::ONE, + }; + + #[inline] + pub fn min(self) -> glam::Vec2 { + self.left_top + } + + #[inline] + pub fn max(self) -> glam::Vec2 { + self.left_top + self.extent + } + + #[inline] + pub fn scale_extent(self, factor: f32) -> RectF32 { + RectF32 { + left_top: self.left_top * factor, + extent: self.extent * factor, + } + } +} + +impl From for RectF32 { + #[inline] + fn from(rect: RectInt) -> Self { + Self { + left_top: rect.left_top.as_vec2(), + extent: rect.extent.as_vec2(), + } + } +} diff --git a/crates/re_renderer/src/renderer/debug_overlay.rs b/crates/re_renderer/src/renderer/debug_overlay.rs index 5b78ac3e3139..263e75bfbcdc 100644 --- a/crates/re_renderer/src/renderer/debug_overlay.rs +++ b/crates/re_renderer/src/renderer/debug_overlay.rs @@ -8,7 +8,7 @@ use crate::{ GpuRenderPipelineHandle, GpuTexture, PipelineLayoutDesc, RenderPipelineDesc, WgpuResourcePools, }, - IntRect, + RectInt, }; use super::{DrawData, FileResolver, FileSystem, RenderContext, Renderer}; @@ -75,7 +75,7 @@ impl DebugOverlayDrawData { ctx: &mut RenderContext, debug_texture: &GpuTexture, screen_resolution: glam::UVec2, - overlay_rect: IntRect, + overlay_rect: RectInt, ) -> Result { let mut renderers = ctx.renderers.write(); let debug_overlay = renderers.get_or_create::<_, DebugOverlayRenderer>( diff --git a/crates/re_renderer/src/transform.rs b/crates/re_renderer/src/transform.rs new file mode 100644 index 000000000000..1beda279ed11 --- /dev/null +++ b/crates/re_renderer/src/transform.rs @@ -0,0 +1,56 @@ +//! Transformation utilities. +//! +//! Some space definitions to keep in mind: +//! +//! Texture coordinates: +//! * origin top left +//! * full texture ([left; right], [top; bottom]): +//! ([0; 1], [0; 1]) +//! +//! NDC: +//! * origin center +//! * full screen ([left; right], [top; bottom]): +//! ([-1; 1], [1; -1]) +//! +//! Pixel coordinates: +//! * origin top left +//! * full screen ([left; right], [top; bottom]): +//! ([0; `screen_extent.x`], [0; `screen_extent.y`]) + +use crate::rect::RectF32; + +/// Transforms texture coordinates to normalized device coordinates (NDC). +#[inline] +pub fn ndc_from_texcoord(texcoord: glam::Vec2) -> glam::Vec2 { + glam::vec2(texcoord.x * 2.0 - 1.0, 1.0 - texcoord.y * 2.0) +} + +/// Transforms texture coordinates to normalized device coordinates (NDC). +#[inline] +pub fn ndc_from_pixel(pixel_coord: glam::Vec2, screen_extent: glam::UVec2) -> glam::Vec2 { + glam::vec2( + pixel_coord.x / screen_extent.x as f32 * 2.0 - 1.0, + 1.0 - pixel_coord.y / screen_extent.y as f32 * 2.0, + ) +} + +/// Computes a transformation matrix that transforms normalized device coordinates +/// to a region of interest. +/// +/// Note that this be used for zooming out/in & panning in NDC space. +/// For ease of use, the region of interest that defines the entire screen is defined in a texture coordinate +/// space. Meaning that the identity transform is defined by the unit rectangle, `RectF32::UNIT`. +pub fn region_of_interest_from_ndc(texcoord_region_of_interest: RectF32) -> glam::Mat4 { + let rect_min = texcoord_region_of_interest.min(); + let rect_max = texcoord_region_of_interest.max(); + + // y axis is flipped in NDC, therefore we need to flip the y axis of the rect. + let rect_min_ndc = ndc_from_texcoord(glam::vec2(rect_min.x, rect_max.y)); + let rect_max_ndc = ndc_from_texcoord(glam::vec2(rect_max.x, rect_min.y)); + + let scale = 2.0 / (rect_max_ndc - rect_min_ndc); + let translation = -0.5 * (rect_min_ndc + rect_max_ndc); + + glam::Mat4::from_scale(scale.extend(1.0)) + * glam::Mat4::from_translation(translation.extend(0.0)) +} diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index 86d8fc817b59..ce9c1a08e708 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -10,9 +10,11 @@ use crate::{ ScreenshotProcessor, }, global_bindings::FrameUniformBuffer, + rect::RectF32, renderer::{CompositorDrawData, DebugOverlayDrawData, DrawData, Renderer}, + transform::region_of_interest_from_ndc, wgpu_resources::{GpuBindGroup, GpuTexture, PoolError, TextureDesc}, - DebugLabel, IntRect, Rgba, Size, + DebugLabel, RectInt, Rgba, Size, }; type DrawFn = dyn for<'a, 'b> Fn( @@ -152,10 +154,35 @@ impl Default for AutoSizeConfig { #[derive(Debug, Clone)] pub struct TargetConfiguration { pub name: DebugLabel, + pub resolution_in_pixel: [u32; 2], pub view_from_world: macaw::IsoTransform, pub projection_from_view: Projection, + /// Defines a viewport transformation from the projected space to the final image space. + /// + /// The scale & translation defined by transforming this rectangle to + /// a unit rectangle with the origin in the top left corner and the right bottom corner at (1, 1) + /// is applied to the projection matrix as a final step. + /// I.e. this transform uses a texture coordinate system! + /// + /// This can be used to implement pan & zoom independent of the camera projection. + /// Meaning that this transform allows you to zoom in on a portion of a perspectively projected + /// scene. + /// + /// To apply no transform, you'd set this this to the unit rectangle, `RectF32::UNIT`. + /// In order to for example zoom in on the bottom-left quadrant of the projected space you'd set this to: + /// ``` + /// RectF32 { left_top: glam::vec2(0.0, 0.5), extent: glam::vec2(0.5, 0.5) } + /// ``` + /// This quadrant will then fill the entire target space. + /// + /// Note that it is perfectly valid to have rectangles that include areas outside of the 0-1 range. + /// + /// Internally, this is used to transform the normalized device coordinates to the given portion. + /// This transform is applied to the projection matrix. + pub viewport_transformation: RectF32, + /// How many pixels are there per point. /// I.e. the ui scaling factor. pub pixels_from_point: f32, @@ -176,6 +203,10 @@ impl Default for TargetConfiguration { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, }, + viewport_transformation: RectF32 { + left_top: glam::Vec2::ZERO, + extent: glam::Vec2::ONE, + }, pixels_from_point: 1.0, auto_size_config: Default::default(), outline_config: None, @@ -376,6 +407,10 @@ impl ViewBuilder { } }; + // Finally, apply a viewport transformation to the projection. + let projection_from_view = + region_of_interest_from_ndc(config.viewport_transformation) * projection_from_view; + let mut view_from_world = config.view_from_world.to_mat4(); // For OrthographicCameraMode::TopLeftCorner, we want Z facing forward. match config.projection_from_view { @@ -387,6 +422,7 @@ impl ViewBuilder { }, Projection::Perspective { .. } => {} }; + let camera_position = config.view_from_world.inverse().translation(); let camera_forward = -view_from_world.row(2).truncate(); let projection_from_world = projection_from_view * view_from_world; @@ -683,7 +719,7 @@ impl ViewBuilder { pub fn schedule_picking_rect( &mut self, ctx: &mut RenderContext, - picking_rect: IntRect, + picking_rect: RectInt, readback_identifier: GpuReadbackIdentifier, readback_user_data: T, show_debug_view: bool, diff --git a/crates/re_viewer/src/gpu_bridge/mod.rs b/crates/re_viewer/src/gpu_bridge/mod.rs index 9cf35e1d4c01..7de6a430e2af 100644 --- a/crates/re_viewer/src/gpu_bridge/mod.rs +++ b/crates/re_viewer/src/gpu_bridge/mod.rs @@ -198,6 +198,7 @@ pub fn render_image( vertical_world_size: space_from_pixel * resolution_in_pixel[1] as f32, far_plane_distance: 1000.0, }, + viewport_transformation: re_renderer::RectF32::UNIT, pixels_from_point: pixels_from_points, auto_size_config: Default::default(), outline_config: None, diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index df20dec00e2d..72c047798c3b 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -710,7 +710,7 @@ pub fn picking( let _ = view_builder.schedule_picking_rect( ctx.render_ctx, - re_renderer::IntRect::from_middle_and_extent( + re_renderer::RectInt::from_middle_and_extent( picking_context.pointer_in_pixel.as_ivec2(), glam::uvec2(picking_rect_size, picking_rect_size), ), diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 4e6e6744970d..91c1534e67d0 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -466,6 +466,7 @@ fn setup_target_config( resolution_in_pixel, view_from_world, projection_from_view, + viewport_transformation: re_renderer::RectF32::UNIT, pixels_from_point: pixels_from_points, auto_size_config, outline_config: any_outlines.then(|| outline_config(painter.ctx())), diff --git a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs index 31c47e1c0fe6..6d3988a58364 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs @@ -330,6 +330,7 @@ pub fn view_3d( vertical_fov: eye.fov_y.unwrap(), near_plane_distance: eye.near(), }, + viewport_transformation: re_renderer::RectF32::UNIT, pixels_from_point: ui.ctx().pixels_per_point(), auto_size_config: state.auto_size_config(), From bf28f872c164d135ff62d4476d8e3660460b8e6e Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Fri, 28 Apr 2023 10:51:47 +0200 Subject: [PATCH 12/37] better viewport transform --- crates/re_renderer/examples/2d.rs | 1 + crates/re_renderer/examples/depth_cloud.rs | 2 + crates/re_renderer/examples/multiview.rs | 1 + crates/re_renderer/examples/outlines.rs | 1 + crates/re_renderer/examples/picking.rs | 1 + .../src/draw_phases/picking_layer.rs | 15 +-- crates/re_renderer/src/lib.rs | 1 + crates/re_renderer/src/transform.rs | 61 ++++++++---- crates/re_renderer/src/view_builder.rs | 47 +++++---- crates/re_viewer/src/gpu_bridge/mod.rs | 2 +- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 98 ++++++++++--------- crates/re_viewer/src/ui/view_spatial/ui_3d.rs | 3 +- 12 files changed, 134 insertions(+), 99 deletions(-) diff --git a/crates/re_renderer/examples/2d.rs b/crates/re_renderer/examples/2d.rs index eb17d6955cfd..de356a95eedc 100644 --- a/crates/re_renderer/examples/2d.rs +++ b/crates/re_renderer/examples/2d.rs @@ -284,6 +284,7 @@ impl framework::Example for Render2D { projection_from_view: Projection::Perspective { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: resolution[0] as f32 / resolution[1] as f32, }, pixels_from_point, ..Default::default() diff --git a/crates/re_renderer/examples/depth_cloud.rs b/crates/re_renderer/examples/depth_cloud.rs index 38fb6c35c95d..e83706f991fc 100644 --- a/crates/re_renderer/examples/depth_cloud.rs +++ b/crates/re_renderer/examples/depth_cloud.rs @@ -123,6 +123,7 @@ impl RenderDepthClouds { projection_from_view: Projection::Perspective { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: resolution_in_pixel[0] as f32 / resolution_in_pixel[1] as f32, }, pixels_from_point, ..Default::default() @@ -202,6 +203,7 @@ impl RenderDepthClouds { projection_from_view: Projection::Perspective { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: resolution_in_pixel[0] as f32 / resolution_in_pixel[1] as f32, }, pixels_from_point, ..Default::default() diff --git a/crates/re_renderer/examples/multiview.rs b/crates/re_renderer/examples/multiview.rs index bc6bc1783a3b..ddcf793d3d3c 100644 --- a/crates/re_renderer/examples/multiview.rs +++ b/crates/re_renderer/examples/multiview.rs @@ -343,6 +343,7 @@ impl Example for Multiview { Projection::Perspective { vertical_fov: 70.0 * TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: resolution[0] as f32 / resolution[1] as f32, } } else { Projection::Orthographic { diff --git a/crates/re_renderer/examples/outlines.rs b/crates/re_renderer/examples/outlines.rs index 2cdc2ed6e87c..f8449ac86351 100644 --- a/crates/re_renderer/examples/outlines.rs +++ b/crates/re_renderer/examples/outlines.rs @@ -61,6 +61,7 @@ impl framework::Example for Outlines { projection_from_view: Projection::Perspective { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: resolution[0] as f32 / resolution[1] as f32, }, pixels_from_point, outline_config: Some(OutlineConfig { diff --git a/crates/re_renderer/examples/picking.rs b/crates/re_renderer/examples/picking.rs index fbcfc3888f53..b3ea0f8a42d2 100644 --- a/crates/re_renderer/examples/picking.rs +++ b/crates/re_renderer/examples/picking.rs @@ -135,6 +135,7 @@ impl framework::Example for Picking { projection_from_view: Projection::Perspective { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: resolution[0] as f32 / resolution[1] as f32, }, pixels_from_point, outline_config: None, diff --git a/crates/re_renderer/src/draw_phases/picking_layer.rs b/crates/re_renderer/src/draw_phases/picking_layer.rs index 9d698d2ce6d6..d0fe73852d1c 100644 --- a/crates/re_renderer/src/draw_phases/picking_layer.rs +++ b/crates/re_renderer/src/draw_phases/picking_layer.rs @@ -15,7 +15,7 @@ use crate::{ include_shader_module, rect::RectF32, texture_info::Texture2DBufferInfo, - transform::{ndc_from_pixel, region_of_interest_from_ndc}, + transform::{ndc_from_pixel, RectTransform}, view_builder::ViewBuilder, wgpu_resources::{ BindGroupDesc, BindGroupEntry, BindGroupLayoutDesc, GpuBindGroup, GpuRenderPipelineHandle, @@ -227,11 +227,14 @@ impl PickingLayerProcessor { DepthReadbackWorkaround::new(ctx, picking_rect.extent, picking_depth_target.handle) }); - let pixel_size = screen_resolution.as_vec2().recip(); - let cropped_projection_from_projection = region_of_interest_from_ndc(RectF32 { - left_top: picking_rect.left_top.as_vec2() * pixel_size, - extent: picking_rect.extent.as_vec2() * pixel_size, - }); + let cropped_projection_from_projection = RectTransform { + from: picking_rect.into(), + to: RectF32 { + left_top: glam::Vec2::ZERO, + extent: screen_resolution.as_vec2(), + }, + } + .to_ndc_transform(); // Setup frame uniform buffer let previous_projection_from_world: glam::Mat4 = diff --git a/crates/re_renderer/src/lib.rs b/crates/re_renderer/src/lib.rs index 9c11b90100a5..74896d13d3c5 100644 --- a/crates/re_renderer/src/lib.rs +++ b/crates/re_renderer/src/lib.rs @@ -44,6 +44,7 @@ pub use line_strip_builder::{LineStripBuilder, LineStripSeriesBuilder}; pub use point_cloud_builder::{PointCloudBatchBuilder, PointCloudBuilder}; pub use rect::RectF32; pub use size::Size; +pub use transform::RectTransform; pub use view_builder::{AutoSizeConfig, ViewBuilder}; pub use wgpu_resources::WgpuResourcePoolStatistics; diff --git a/crates/re_renderer/src/transform.rs b/crates/re_renderer/src/transform.rs index 1beda279ed11..58f48b1a4d9d 100644 --- a/crates/re_renderer/src/transform.rs +++ b/crates/re_renderer/src/transform.rs @@ -34,23 +34,46 @@ pub fn ndc_from_pixel(pixel_coord: glam::Vec2, screen_extent: glam::UVec2) -> gl ) } -/// Computes a transformation matrix that transforms normalized device coordinates -/// to a region of interest. -/// -/// Note that this be used for zooming out/in & panning in NDC space. -/// For ease of use, the region of interest that defines the entire screen is defined in a texture coordinate -/// space. Meaning that the identity transform is defined by the unit rectangle, `RectF32::UNIT`. -pub fn region_of_interest_from_ndc(texcoord_region_of_interest: RectF32) -> glam::Mat4 { - let rect_min = texcoord_region_of_interest.min(); - let rect_max = texcoord_region_of_interest.max(); - - // y axis is flipped in NDC, therefore we need to flip the y axis of the rect. - let rect_min_ndc = ndc_from_texcoord(glam::vec2(rect_min.x, rect_max.y)); - let rect_max_ndc = ndc_from_texcoord(glam::vec2(rect_max.x, rect_min.y)); - - let scale = 2.0 / (rect_max_ndc - rect_min_ndc); - let translation = -0.5 * (rect_min_ndc + rect_max_ndc); - - glam::Mat4::from_scale(scale.extend(1.0)) - * glam::Mat4::from_translation(translation.extend(0.0)) +#[derive(Clone, Debug)] +pub struct RectTransform { + pub from: RectF32, + pub to: RectF32, +} + +impl RectTransform { + /// No-op rect transform that transforms from a unit rectangle to a unit rectangle. + pub const IDENTITY: RectTransform = RectTransform { + from: RectF32::UNIT, + to: RectF32::UNIT, + }; + + /// Computes a transformation matrix that applies the rect transform to the NDC space. + /// + /// + /// Note only the relation of the rectangles in `RectTransform` is important. + /// Scaling or moving both rectangles by the same amount does not change the result. + pub fn to_ndc_transform(&self) -> glam::Mat4 { + // It's easier to think in texcoord space, and then transform to NDC. + // This texcoord rect specifies the portion of the screen that should become the entire range of the NDC screen. + let texcoord_rect = RectF32 { + left_top: (self.from.left_top - self.to.left_top) / self.to.extent, + extent: self.from.extent / self.to.extent, + }; + let texcoord_rect_min = texcoord_rect.min(); + let texcoord_rect_max = texcoord_rect.max(); + + // y axis is flipped in NDC, therefore we need to flip the y axis of the rect. + let rect_min_ndc = ndc_from_texcoord(glam::vec2(texcoord_rect_min.x, texcoord_rect_max.y)); + let rect_max_ndc = ndc_from_texcoord(glam::vec2(texcoord_rect_max.x, texcoord_rect_min.y)); + + let scale = 2.0 / (rect_max_ndc - rect_min_ndc); + let translation = -0.5 * (rect_min_ndc + rect_max_ndc); + + glam::Mat4::from_scale(scale.extend(1.0)) + * glam::Mat4::from_translation(translation.extend(0.0)) + } + + pub fn scale(&self) -> glam::Vec2 { + self.to.extent / self.from.extent + } } diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index ce9c1a08e708..513b16f1ecbf 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -10,9 +10,8 @@ use crate::{ ScreenshotProcessor, }, global_bindings::FrameUniformBuffer, - rect::RectF32, renderer::{CompositorDrawData, DebugOverlayDrawData, DrawData, Renderer}, - transform::region_of_interest_from_ndc, + transform::RectTransform, wgpu_resources::{GpuBindGroup, GpuTexture, PoolError, TextureDesc}, DebugLabel, RectInt, Rgba, Size, }; @@ -111,6 +110,12 @@ pub enum Projection { /// Distance of the near plane. near_plane_distance: f32, + + /// Aspect ratio of the perspective transformation. + /// + /// This is typically just resolution.y / resolution.x. + /// Setting this to anything else is mostly useful when panning & zooming within a fixed transformation. + aspect_ratio: f32, }, /// Orthographic projection with the camera position at the near plane's center, @@ -161,30 +166,22 @@ pub struct TargetConfiguration { /// Defines a viewport transformation from the projected space to the final image space. /// - /// The scale & translation defined by transforming this rectangle to - /// a unit rectangle with the origin in the top left corner and the right bottom corner at (1, 1) - /// is applied to the projection matrix as a final step. - /// I.e. this transform uses a texture coordinate system! - /// /// This can be used to implement pan & zoom independent of the camera projection. /// Meaning that this transform allows you to zoom in on a portion of a perspectively projected /// scene. /// - /// To apply no transform, you'd set this this to the unit rectangle, `RectF32::UNIT`. - /// In order to for example zoom in on the bottom-left quadrant of the projected space you'd set this to: - /// ``` - /// RectF32 { left_top: glam::vec2(0.0, 0.5), extent: glam::vec2(0.5, 0.5) } - /// ``` - /// This quadrant will then fill the entire target space. - /// - /// Note that it is perfectly valid to have rectangles that include areas outside of the 0-1 range. + /// Note only the relation of the rectangles in `RectTransform` is important. + /// Scaling or moving both rectangles by the same amount does not change the result. /// /// Internally, this is used to transform the normalized device coordinates to the given portion. /// This transform is applied to the projection matrix. - pub viewport_transformation: RectF32, + pub viewport_transformation: RectTransform, /// How many pixels are there per point. + /// /// I.e. the ui scaling factor. + /// Note that this does not affect any of the camera & projection properties and is only used + /// whenever point sizes were explicitly specified. pub pixels_from_point: f32, /// How [`Size::AUTO`] is interpreted. @@ -202,11 +199,9 @@ impl Default for TargetConfiguration { projection_from_view: Projection::Perspective { vertical_fov: 70.0 * std::f32::consts::TAU / 360.0, near_plane_distance: 0.01, + aspect_ratio: 1.0, }, - viewport_transformation: RectF32 { - left_top: glam::Vec2::ZERO, - extent: glam::Vec2::ONE, - }, + viewport_transformation: RectTransform::IDENTITY, pixels_from_point: 1.0, auto_size_config: Default::default(), outline_config: None, @@ -323,14 +318,12 @@ impl ViewBuilder { }, ); - let aspect_ratio = - config.resolution_in_pixel[0] as f32 / config.resolution_in_pixel[1] as f32; - let (projection_from_view, tan_half_fov, pixel_world_size_from_camera_distance) = match config.projection_from_view.clone() { Projection::Perspective { vertical_fov, near_plane_distance, + aspect_ratio, } => { // We use infinite reverse-z projection matrix // * great precision both with floating point and integer: https://developer.nvidia.com/content/depth-precision-visualized @@ -372,6 +365,8 @@ impl ViewBuilder { vertical_world_size, far_plane_distance, } => { + let aspect_ratio = + config.resolution_in_pixel[0] as f32 / config.resolution_in_pixel[1] as f32; let horizontal_world_size = vertical_world_size * aspect_ratio; // Note that we inverse z (by swapping near and far plane) to be consistent with our perspective projection. let projection_from_view = match camera_mode { @@ -409,7 +404,11 @@ impl ViewBuilder { // Finally, apply a viewport transformation to the projection. let projection_from_view = - region_of_interest_from_ndc(config.viewport_transformation) * projection_from_view; + config.viewport_transformation.to_ndc_transform() * projection_from_view; + let pixel_scale_factor = config.viewport_transformation.scale().min_element(); + // TODO: this seems super weird. + let pixel_world_size_from_camera_distance = + pixel_world_size_from_camera_distance / pixel_scale_factor; let mut view_from_world = config.view_from_world.to_mat4(); // For OrthographicCameraMode::TopLeftCorner, we want Z facing forward. diff --git a/crates/re_viewer/src/gpu_bridge/mod.rs b/crates/re_viewer/src/gpu_bridge/mod.rs index 7de6a430e2af..18e3a8e0dcd4 100644 --- a/crates/re_viewer/src/gpu_bridge/mod.rs +++ b/crates/re_viewer/src/gpu_bridge/mod.rs @@ -198,7 +198,7 @@ pub fn render_image( vertical_world_size: space_from_pixel * resolution_in_pixel[1] as f32, far_plane_distance: 1000.0, }, - viewport_transformation: re_renderer::RectF32::UNIT, + viewport_transformation: re_renderer::RectTransform::IDENTITY, pixels_from_point: pixels_from_points, auto_size_config: Default::default(), outline_config: None, diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 91c1534e67d0..891924b3d355 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -295,9 +295,6 @@ fn view_2d_scrollable( // Create our transforms. let ui_from_space = egui::emath::RectTransform::from_to(scene_rect_accum, response.rect); let space_from_ui = ui_from_space.inverse(); - let space_from_points = space_from_ui.scale().y; - let points_from_pixels = 1.0 / painter.ctx().pixels_per_point(); - let space_from_pixel = space_from_points * points_from_pixels; state .state_2d @@ -313,7 +310,6 @@ fn view_2d_scrollable( let Ok(target_config) = setup_target_config( &painter, space_from_ui, - space_from_pixel, &space.to_string(), state.auto_size_config(), scene @@ -403,7 +399,6 @@ fn view_2d_scrollable( fn setup_target_config( painter: &egui::Painter, space_from_ui: RectTransform, - space_from_pixel: f32, space_name: &str, auto_size_config: re_renderer::AutoSizeConfig, any_outlines: bool, @@ -414,49 +409,56 @@ fn setup_target_config( gpu_bridge::viewport_resolution_in_pixels(painter.clip_rect(), pixels_from_points); anyhow::ensure!(resolution_in_pixel[0] > 0 && resolution_in_pixel[1] > 0); - // TODO: Determine earlier and - - let (projection_from_view, view_from_world) = if let Some(space_camera) = space_camera { - // Looking through a space camera at the space root in a 2D view is *similar* than - // looking through that same camera in a 3D view. - // - // There is once crucial difference though: - // Images are supposed to be on the XY plane and scaled in pixels. - // I.e. if I place an image without 2D transform, it gets an identify transform - // and X/Y coordinates correspond to pixels. - - let pinhole = space_camera.pinhole.unwrap(); // TODO: - ( - re_renderer::view_builder::Projection::Perspective { - vertical_fov: pinhole.fov_y().unwrap_or(Eye::DEFAULT_FOV_Y), - near_plane_distance: 0.01, - }, - // Put the camera at the position where it sees the entire image plane as defined - // by the pinhole camera. - macaw::IsoTransform::look_at_rh( - pinhole - .principal_point() - .extend(-pinhole.focal_length_in_pixels()), - pinhole.principal_point().extend(0.0), - -glam::Vec3::Y, + let default_pinhole_image_plane_size = + glam::vec2(space_from_ui.to().width(), space_from_ui.to().height()); + let default_focal_length_in_pixels = default_pinhole_image_plane_size.x; + + // TODO: don't use space camera but do a query. + let pinhole = space_camera + .and_then(|c| c.pinhole) + .unwrap_or(re_log_types::Pinhole { + image_from_cam: glam::Mat3::from_cols( + glam::vec3(default_focal_length_in_pixels, 0.0, 0.0), + glam::vec3(0.0, default_focal_length_in_pixels, 0.0), + (default_pinhole_image_plane_size * 0.5 + glam::Vec2::splat(0.5)).extend(1.0), ) - .unwrap_or(macaw::IsoTransform::IDENTITY), - ) - } else { - let top_left_pos = space_from_ui.transform_pos(painter.clip_rect().min); - ( - re_renderer::view_builder::Projection::Orthographic { - camera_mode: - re_renderer::view_builder::OrthographicCameraMode::TopLeftCornerAndExtendZ, - vertical_world_size: space_from_pixel * resolution_in_pixel[1] as f32, - far_plane_distance: 1000.0, - }, - macaw::IsoTransform::from_translation(glam::vec3( - -top_left_pos.x, - -top_left_pos.y, - 0.0, - )), - ) + .into(), + // Look at the entire space by default. + resolution: Some(default_pinhole_image_plane_size.into()), + }); + + let projection_from_view = re_renderer::view_builder::Projection::Perspective { + vertical_fov: pinhole.fov_y().unwrap_or(Eye::DEFAULT_FOV_Y), + near_plane_distance: 0.01, + aspect_ratio: pinhole.aspect_ratio().unwrap(), // TODO: + }; + + // Put the camera at the position where it sees the entire image plane as defined + // by the pinhole camera. + let view_from_world = macaw::IsoTransform::look_at_rh( + pinhole + .principal_point() + .extend(-pinhole.focal_length_in_pixels()), + pinhole.principal_point().extend(0.0), + -glam::Vec3::Y, + ) + .unwrap_or(macaw::IsoTransform::IDENTITY); + + // What portion of the space are we looking at? + // TODO: cleanup spaces + // TODO: Need to take principal point into account + let total_ui_rect = space_from_ui.from(); + let visible_ui_rect = painter.clip_rect(); + + let viewport_transformation = re_renderer::RectTransform { + from: re_renderer::RectF32 { + left_top: glam::vec2(visible_ui_rect.left(), visible_ui_rect.top()), + extent: glam::vec2(visible_ui_rect.width(), visible_ui_rect.height()), + }, + to: re_renderer::RectF32 { + left_top: glam::vec2(total_ui_rect.left(), total_ui_rect.top()), + extent: glam::vec2(total_ui_rect.width(), total_ui_rect.height()), + }, }; Ok({ @@ -466,7 +468,7 @@ fn setup_target_config( resolution_in_pixel, view_from_world, projection_from_view, - viewport_transformation: re_renderer::RectF32::UNIT, + viewport_transformation, pixels_from_point: pixels_from_points, auto_size_config, outline_config: any_outlines.then(|| outline_config(painter.ctx())), diff --git a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs index 6d3988a58364..1a74d7cf07a4 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs @@ -329,8 +329,9 @@ pub fn view_3d( projection_from_view: Projection::Perspective { vertical_fov: eye.fov_y.unwrap(), near_plane_distance: eye.near(), + aspect_ratio: resolution_in_pixel[0] as f32 / resolution_in_pixel[1] as f32, }, - viewport_transformation: re_renderer::RectF32::UNIT, + viewport_transformation: re_renderer::RectTransform::IDENTITY, pixels_from_point: ui.ctx().pixels_per_point(), auto_size_config: state.auto_size_config(), From 118c8c862c007bff56b8313c1c34d3e55e3b064a Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Fri, 28 Apr 2023 16:01:29 +0200 Subject: [PATCH 13/37] limit zoom, correctly handle ui scale under viewport zoom --- crates/re_renderer/src/draw_phases/picking_layer.rs | 2 +- crates/re_renderer/src/transform.rs | 2 +- crates/re_renderer/src/view_builder.rs | 11 ++++++----- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 3 ++- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/re_renderer/src/draw_phases/picking_layer.rs b/crates/re_renderer/src/draw_phases/picking_layer.rs index d0fe73852d1c..414bf1092c7c 100644 --- a/crates/re_renderer/src/draw_phases/picking_layer.rs +++ b/crates/re_renderer/src/draw_phases/picking_layer.rs @@ -234,7 +234,7 @@ impl PickingLayerProcessor { extent: screen_resolution.as_vec2(), }, } - .to_ndc_transform(); + .to_ndc_scale_and_translation(); // Setup frame uniform buffer let previous_projection_from_world: glam::Mat4 = diff --git a/crates/re_renderer/src/transform.rs b/crates/re_renderer/src/transform.rs index 58f48b1a4d9d..4904bce0c065 100644 --- a/crates/re_renderer/src/transform.rs +++ b/crates/re_renderer/src/transform.rs @@ -52,7 +52,7 @@ impl RectTransform { /// /// Note only the relation of the rectangles in `RectTransform` is important. /// Scaling or moving both rectangles by the same amount does not change the result. - pub fn to_ndc_transform(&self) -> glam::Mat4 { + pub fn to_ndc_scale_and_translation(&self) -> glam::Mat4 { // It's easier to think in texcoord space, and then transform to NDC. // This texcoord rect specifies the portion of the screen that should become the entire range of the NDC screen. let texcoord_rect = RectF32 { diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index 513b16f1ecbf..5d26483986b8 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -403,12 +403,13 @@ impl ViewBuilder { }; // Finally, apply a viewport transformation to the projection. - let projection_from_view = - config.viewport_transformation.to_ndc_transform() * projection_from_view; - let pixel_scale_factor = config.viewport_transformation.scale().min_element(); - // TODO: this seems super weird. + let ndc_scale_and_translation = config + .viewport_transformation + .to_ndc_scale_and_translation(); + let projection_from_view = ndc_scale_and_translation * projection_from_view; + // Need to take into account that a smaller portion of the world scale is visible now. let pixel_world_size_from_camera_distance = - pixel_world_size_from_camera_distance / pixel_scale_factor; + pixel_world_size_from_camera_distance / ndc_scale_and_translation.col(0).x; let mut view_from_world = config.view_from_world.to_mat4(); // For OrthographicCameraMode::TopLeftCorner, we want Z facing forward. diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 891924b3d355..e71266a9ca5d 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -153,7 +153,8 @@ impl View2DState { // Moving the center in the direction of the desired shift center += shift_in_space; } - scale = new_scale; + // Don't show less than one horizontal scene unit in the entire screen. + scale = new_scale.at_most(available_size.x); accepting_scroll = false; } From e31277d677104659873c4c2b00e7dfe41eb3f8b5 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Fri, 28 Apr 2023 17:53:05 +0200 Subject: [PATCH 14/37] 2D points now draw as real 2D circles --- crates/re_renderer/shader/depth_cloud.wgsl | 3 +- crates/re_renderer/shader/point_cloud.wgsl | 20 +++++++++--- .../re_renderer/shader/utils/sphere_quad.wgsl | 32 ++++++++++++------- .../re_renderer/src/renderer/point_cloud.rs | 3 ++ .../view_spatial/scene/scene_part/points2d.rs | 4 +++ 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/crates/re_renderer/shader/depth_cloud.wgsl b/crates/re_renderer/shader/depth_cloud.wgsl index 0c7050815052..b29ddf97778f 100644 --- a/crates/re_renderer/shader/depth_cloud.wgsl +++ b/crates/re_renderer/shader/depth_cloud.wgsl @@ -133,7 +133,8 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { if 0.0 < point_data.unresolved_radius { // Span quad - let quad = sphere_quad_span(vertex_idx, point_data.pos_in_world, point_data.unresolved_radius, depth_cloud_info.radius_boost_in_ui_points); + let quad = sphere_or_circle_quad_span(vertex_idx, point_data.pos_in_world, point_data.unresolved_radius, + depth_cloud_info.radius_boost_in_ui_points, false); out.pos_in_clip = frame.projection_from_world * Vec4(quad.pos_in_world, 1.0); out.pos_in_world = quad.pos_in_world; out.point_radius = quad.point_resolved_radius; diff --git a/crates/re_renderer/shader/point_cloud.wgsl b/crates/re_renderer/shader/point_cloud.wgsl index a55404230692..881b6dd05a57 100644 --- a/crates/re_renderer/shader/point_cloud.wgsl +++ b/crates/re_renderer/shader/point_cloud.wgsl @@ -36,6 +36,8 @@ var batch: BatchUniformBuffer; // Flags // See point_cloud.rs#PointCloudBatchFlags const ENABLE_SHADING: u32 = 1u; +const DRAW_AS_CIRCLES: u32 = 2u; + const TEXTURE_SIZE: u32 = 2048u; struct VertexOut { @@ -94,7 +96,8 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { let point_data = read_data(quad_idx); // Span quad - let quad = sphere_quad_span(vertex_idx, point_data.pos, point_data.unresolved_radius, draw_data.radius_boost_in_ui_points); + let quad = sphere_or_circle_quad_span(vertex_idx, point_data.pos, point_data.unresolved_radius, + draw_data.radius_boost_in_ui_points, has_any_flag(batch.flags, DRAW_AS_CIRCLES)); // Output, transform to projection space and done. var out: VertexOut; @@ -108,9 +111,18 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { return out; } +fn coverage(world_position: Vec3, radius: f32, point_center: Vec3) -> f32 { + if is_camera_orthographic() || has_any_flag(batch.flags, DRAW_AS_CIRCLES) { + return circle_quad_coverage(world_position, radius, point_center); + } else { + return sphere_quad_coverage(world_position, radius, point_center); + } +} + + @fragment fn fs_main(in: VertexOut) -> @location(0) Vec4 { - let coverage = sphere_quad_coverage(in.world_position, in.radius, in.point_center); + let coverage = coverage(in.world_position, in.radius, in.point_center); if coverage < 0.001 { discard; } @@ -127,7 +139,7 @@ fn fs_main(in: VertexOut) -> @location(0) Vec4 { @fragment fn fs_main_picking_layer(in: VertexOut) -> @location(0) UVec4 { - let coverage = sphere_quad_coverage(in.world_position, in.radius, in.point_center); + let coverage = coverage(in.world_position, in.radius, in.point_center); if coverage <= 0.5 { discard; } @@ -139,7 +151,7 @@ fn fs_main_outline_mask(in: VertexOut) -> @location(0) UVec2 { // Output is an integer target, can't use coverage therefore. // But we still want to discard fragments where coverage is low. // Since the outline extends a bit, a very low cut off tends to look better. - let coverage = sphere_quad_coverage(in.world_position, in.radius, in.point_center); + let coverage = coverage(in.world_position, in.radius, in.point_center); if coverage < 1.0 { discard; } diff --git a/crates/re_renderer/shader/utils/sphere_quad.wgsl b/crates/re_renderer/shader/utils/sphere_quad.wgsl index 00937d8e7701..22e98cb52e49 100644 --- a/crates/re_renderer/shader/utils/sphere_quad.wgsl +++ b/crates/re_renderer/shader/utils/sphere_quad.wgsl @@ -4,7 +4,7 @@ /// Span a quad in a way that guarantees that we'll be able to draw a perspective correct sphere /// on it. -fn sphere_quad_span_perspective( +fn sphere_quad( point_pos: Vec3, point_radius: f32, top_bottom: f32, @@ -42,7 +42,7 @@ fn sphere_quad_span_perspective( /// Span a quad in a way that guarantees that we'll be able to draw an orthographic correct sphere /// on it. -fn sphere_quad_span_orthographic(point_pos: Vec3, point_radius: f32, top_bottom: f32, left_right: f32) -> Vec3 { +fn circle_quad(point_pos: Vec3, point_radius: f32, top_bottom: f32, left_right: f32) -> Vec3 { let quad_normal = frame.camera_forward; let quad_right = normalize(cross(quad_normal, frame.view_from_world[1].xyz)); // It's spheres so any orthogonal vector would do. let quad_up = cross(quad_right, quad_normal); @@ -50,7 +50,8 @@ fn sphere_quad_span_orthographic(point_pos: Vec3, point_radius: f32, top_bottom: // Add half a pixel of margin for the feathering we do for antialiasing the spheres. // It's fairly subtle but if we don't do this our spheres look slightly squarish - let radius = point_radius + 0.5 * frame.pixel_world_size_from_camera_distance; + // TODO(andreas): Computing distance to camera here is a bit excessive, should get distance more easily - keep in mind this code runs for ortho & perspective. + let radius = point_radius + 0.5 * approx_pixel_world_size_at(distance(point_pos, frame.camera_position)); return point_pos + pos_in_quad * radius; } @@ -65,10 +66,11 @@ struct SphereQuadData { point_resolved_radius: f32, } -/// Span a quad onto which perspective correct spheres can be drawn. +/// Span a quad onto which circles or perspective correct spheres can be drawn. /// -/// Spanning is done in perspective or orthographically depending of the state of the global cam. -fn sphere_quad_span(vertex_idx: u32, point_pos: Vec3, point_unresolved_radius: f32, radius_boost_in_ui_points: f32) -> SphereQuadData { +/// Note that in orthographic mode, spheres are always circles. +fn sphere_or_circle_quad_span(vertex_idx: u32, point_pos: Vec3, point_unresolved_radius: f32, + radius_boost_in_ui_points: f32, force_circle: bool) -> SphereQuadData { // Resolve radius to a world size. We need the camera distance for this, which is useful later on. let to_camera = frame.camera_position - point_pos; let camera_distance = length(to_camera); @@ -82,18 +84,16 @@ fn sphere_quad_span(vertex_idx: u32, point_pos: Vec3, point_unresolved_radius: f // Span quad var pos: Vec3; - if is_camera_perspective() { - pos = sphere_quad_span_perspective(point_pos, radius, top_bottom, left_right, to_camera, camera_distance); + if is_camera_orthographic() || force_circle { + pos = circle_quad(point_pos, radius, top_bottom, left_right); } else { - pos = sphere_quad_span_orthographic(point_pos, radius, top_bottom, left_right); + pos = sphere_quad(point_pos, radius, top_bottom, left_right, to_camera, camera_distance); } return SphereQuadData(pos, radius); } fn sphere_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) -> f32 { - // There's easier ways to compute anti-aliasing for when we are in ortho mode since it's just circles. - // But it's very nice to have mostly the same code path and this gives us the sphere world position along the way. let ray = camera_ray_to_world_pos(world_position); // Sphere intersection with anti-aliasing as described by Iq here @@ -110,3 +110,13 @@ fn sphere_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) - // Note that we have signed distances to the sphere surface. return saturate(0.5 - distance_to_surface_in_pixels); } + +/// Computes coverage of a sphere or circle on a quad. +/// +/// Note that in orthographic mode, spheres are always circles. +fn circle_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) -> f32 { + let to_center = point_center - world_position; + let distance = length(to_center); + let distance_pixel_difference = fwidth(distance); + return smoothstep(radius + distance_pixel_difference, radius - distance_pixel_difference, distance); +} diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index fb205d5b4127..3101bc3d9122 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -49,6 +49,9 @@ bitflags! { pub struct PointCloudBatchFlags : u32 { /// If true, we shade all points in the batch like spheres. const ENABLE_SHADING = 0b0001; + + /// If true, draw circles instead of spheres. + const DRAW_AS_CIRCLES = 0b0010; } } diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs index 6da3de364805..c91c3d55055c 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/points2d.rs @@ -110,6 +110,10 @@ impl Points2DPart { .primitives .points .batch("2d points") + .flags( + re_renderer::renderer::PointCloudBatchFlags::DRAW_AS_CIRCLES + | re_renderer::renderer::PointCloudBatchFlags::ENABLE_SHADING, + ) .world_from_obj(world_from_obj) .outline_mask_ids(entity_highlight.overall) .picking_object_id(re_renderer::PickingLayerObjectId(ent_path.hash64())); From 2f3bcf0525f91bcc9f2f2e8c31bb004aa0a63b67 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Fri, 28 Apr 2023 18:18:33 +0200 Subject: [PATCH 15/37] better 2D rendering for lines with perspective camera around --- crates/re_renderer/shader/lines.wgsl | 8 ++++- crates/re_renderer/shader/utils/camera.wgsl | 32 ++++++++++++------- crates/re_renderer/src/line_strip_builder.rs | 16 ++++++---- crates/re_renderer/src/renderer/lines.rs | 7 ++++ .../view_spatial/scene/scene_part/boxes2d.rs | 6 +++- .../view_spatial/scene/scene_part/lines2d.rs | 2 +- 6 files changed, 50 insertions(+), 21 deletions(-) diff --git a/crates/re_renderer/shader/lines.wgsl b/crates/re_renderer/shader/lines.wgsl index 6a143a8233c8..74be45083851 100644 --- a/crates/re_renderer/shader/lines.wgsl +++ b/crates/re_renderer/shader/lines.wgsl @@ -44,6 +44,7 @@ const CAP_START_TRIANGLE: u32 = 8u; const CAP_START_ROUND: u32 = 16u; const CAP_START_EXTEND_OUTWARDS: u32 = 32u; const NO_COLOR_GRADIENT: u32 = 64u; +const FORCE_ORTHO_SPANNING: u32 = 128u; // A lot of the attributes don't need to be interpolated across triangles. // To document that and safe some time we mark them up with @interpolate(flat) @@ -199,7 +200,12 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { // Resolve radius. // (slight inaccuracy: End caps are going to adjust their center_position) - let camera_ray = camera_ray_to_world_pos(center_position); + var camera_ray: Ray; + if has_any_flag(strip_data.flags, FORCE_ORTHO_SPANNING) || is_camera_orthographic() { + camera_ray = camera_ray_to_world_pos_orthographic(center_position); + } else { + camera_ray = camera_ray_to_world_pos_perspective(center_position); + } let camera_distance = distance(camera_ray.origin, center_position); var strip_radius = unresolved_size_to_world(strip_data.unresolved_radius, camera_distance, frame.auto_size_lines); diff --git a/crates/re_renderer/shader/utils/camera.wgsl b/crates/re_renderer/shader/utils/camera.wgsl index 7f1c71a3d460..280ff765c5c1 100644 --- a/crates/re_renderer/shader/utils/camera.wgsl +++ b/crates/re_renderer/shader/utils/camera.wgsl @@ -15,22 +15,32 @@ struct Ray { direction: Vec3, } -// Returns the ray from the camera to a given world position. -fn camera_ray_to_world_pos(world_pos: Vec3) -> Ray { +// Returns the ray from the camera to a given world position, assuming the camera is perspective +fn camera_ray_to_world_pos_perspective(world_pos: Vec3) -> Ray { + var ray: Ray; + ray.origin = frame.camera_position; + ray.direction = normalize(world_pos - frame.camera_position); + return ray; +} + +// Returns the ray from the camera to a given world position, assuming the camera is orthographic +fn camera_ray_to_world_pos_orthographic(world_pos: Vec3) -> Ray { var ray: Ray; + // The ray originates on the camera plane, not from the camera position + let to_pos = world_pos - frame.camera_position; + let camera_plane_distance = dot(to_pos, frame.camera_forward); + ray.origin = world_pos - frame.camera_forward * camera_plane_distance; + ray.direction = frame.camera_forward; + return ray; +} +// Returns the ray from the camera to a given world position. +fn camera_ray_to_world_pos(world_pos: Vec3) -> Ray { if is_camera_perspective() { - ray.origin = frame.camera_position; - ray.direction = normalize(world_pos - frame.camera_position); + return camera_ray_to_world_pos_perspective(world_pos); } else { - // The ray originates on the camera plane, not from the camera position - let to_pos = world_pos - frame.camera_position; - let camera_plane_distance = dot(to_pos, frame.camera_forward); - ray.origin = world_pos - frame.camera_forward * camera_plane_distance; - ray.direction = frame.camera_forward; + return camera_ray_to_world_pos_orthographic(world_pos); } - - return ray; } // Returns the camera ray direction through a given screen uv coordinates (ranging from 0 to 1, i.e. NOT ndc coordinates) diff --git a/crates/re_renderer/src/line_strip_builder.rs b/crates/re_renderer/src/line_strip_builder.rs index f6e6ee650e6f..1526150128aa 100644 --- a/crates/re_renderer/src/line_strip_builder.rs +++ b/crates/re_renderer/src/line_strip_builder.rs @@ -104,6 +104,14 @@ impl LineStripSeriesBuilder { pub fn is_empty(&self) -> bool { self.strips.is_empty() } + + pub fn default_rectangle_flags() -> LineStripFlags { + LineStripFlags::CAP_END_ROUND + | LineStripFlags::CAP_START_ROUND + | LineStripFlags::NO_COLOR_GRADIENT + | LineStripFlags::CAP_END_EXTEND_OUTWARDS + | LineStripFlags::CAP_START_EXTEND_OUTWARDS + } } pub struct LineBatchBuilder<'a>(&'a mut LineStripSeriesBuilder); @@ -292,13 +300,7 @@ impl<'a> LineBatchBuilder<'a> { ] .into_iter(), ) - .flags( - LineStripFlags::CAP_END_ROUND - | LineStripFlags::CAP_START_ROUND - | LineStripFlags::NO_COLOR_GRADIENT - | LineStripFlags::CAP_END_EXTEND_OUTWARDS - | LineStripFlags::CAP_START_EXTEND_OUTWARDS, - ) + .flags(LineStripSeriesBuilder::default_rectangle_flags()) } /// Adds a 2D series of line connected points. diff --git a/crates/re_renderer/src/renderer/lines.rs b/crates/re_renderer/src/renderer/lines.rs index f645bc5f523d..9d6336773965 100644 --- a/crates/re_renderer/src/renderer/lines.rs +++ b/crates/re_renderer/src/renderer/lines.rs @@ -227,7 +227,14 @@ bitflags! { const CAP_START_EXTEND_OUTWARDS = 0b0010_0000; /// Disable color gradient which is on by default + /// + /// TODO(andreas): Could be moved to per batch flags. const NO_COLOR_GRADIENT = 0b0100_0000; + + /// Forces spanning the line's quads as-if the camera was orthographic. + /// + /// TODO(andreas): Could be moved to per batch flags. + const FORCE_ORTHO_SPANNING = 0b1000_0000; } } diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs index 3de91ba5ad94..e612fe64b482 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs @@ -4,7 +4,7 @@ use re_log_types::{ Component, }; use re_query::{query_primary_with_history, EntityView, QueryError}; -use re_renderer::Size; +use re_renderer::{renderer::LineStripFlags, LineStripSeriesBuilder, Size}; use crate::{ misc::{SpaceViewHighlights, TransformCache, ViewerContext}, @@ -70,6 +70,10 @@ impl Boxes2DPart { glam::vec2(rect.width(), 0.0), glam::vec2(0.0, rect.height()), ) + .flags( + LineStripSeriesBuilder::default_rectangle_flags() + | LineStripFlags::FORCE_ORTHO_SPANNING, + ) .color(color) .radius(radius) .picking_instance_id(instance_key_to_picking_id( diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs index 0a9b47b9b5ed..7206ca856037 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs @@ -50,7 +50,7 @@ impl Lines2DPart { .add_strip_2d(strip.0.into_iter().map(|v| v.into())) .color(color) .radius(radius) - .flags(LineStripFlags::NO_COLOR_GRADIENT) + .flags(LineStripFlags::NO_COLOR_GRADIENT | LineStripFlags::FORCE_ORTHO_SPANNING) .picking_instance_id(instance_key_to_picking_id( instance_key, entity_view, From 53d5ff02affad7f1b7bd73a5de59c363a54bb1ac Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Fri, 28 Apr 2023 18:26:32 +0200 Subject: [PATCH 16/37] disable 3D labels in 2D views --- crates/re_viewer/src/ui/view_spatial/ui.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index 72c047798c3b..4894bafa4c2e 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -565,6 +565,10 @@ pub fn create_labels( (f32::INFINITY, pos_in_ui + egui::vec2(0.0, 3.0)) } UiLabelTarget::Position3D(pos) => { + // TODO(#1640): 3D labels are not visible in 2D for now. + if nav_mode == SpatialNavigationMode::TwoD { + continue; + } let pos_in_ui = ui_from_world_3d * pos.extend(1.0); if pos_in_ui.w <= 0.0 { continue; // behind camera From 4fea9db8d53ea0bf062f8c5b43821b2950d50701 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sat, 29 Apr 2023 11:21:16 +0200 Subject: [PATCH 17/37] space camera no longer required for correct pinhole camera in ui_2d --- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index e71266a9ca5d..753001a980a6 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -1,13 +1,14 @@ use eframe::emath::RectTransform; use egui::{pos2, vec2, Align2, Color32, NumExt as _, Pos2, Rect, ScrollArea, Shape, Vec2}; use macaw::IsoTransform; -use re_data_store::{EntityPath, EntityPropertyMap}; +use re_data_store::{query_latest_single, EntityPath, EntityPropertyMap}; +use re_log_types::Pinhole; use re_renderer::view_builder::{TargetConfiguration, ViewBuilder}; use super::{ eye::Eye, ui::{create_labels, picking, screenshot_context_menu}, - SpaceCamera3D, SpatialNavigationMode, ViewSpatialState, + SpatialNavigationMode, ViewSpatialState, }; use crate::{ gpu_bridge, @@ -306,7 +307,18 @@ fn view_2d_scrollable( fov_y: None, }; - let space_camera = scene.space_cameras.iter().find(|c| &c.ent_path == space); + // Query for a pinhole camera at the space view's root that we should take over. + // Note that we can't rely on the camera being part of scene.space_cameras since that requires + // the camera to be added to the scene! + let pinhole = query_latest_single( + &ctx.log_db.entity_db, + space, + &ctx.rec_cfg.time_ctrl.current_query(), + ) + .and_then(|transform| match transform { + re_log_types::Transform::Pinhole(pinhole) => Some(pinhole), + _ => None, + }); let Ok(target_config) = setup_target_config( &painter, @@ -316,7 +328,7 @@ fn view_2d_scrollable( scene .primitives .any_outlines, - space_camera, + pinhole, ) else { return response; }; @@ -403,30 +415,28 @@ fn setup_target_config( space_name: &str, auto_size_config: re_renderer::AutoSizeConfig, any_outlines: bool, - space_camera: Option<&SpaceCamera3D>, + pinhole: Option, ) -> anyhow::Result { let pixels_from_points = painter.ctx().pixels_per_point(); let resolution_in_pixel = gpu_bridge::viewport_resolution_in_pixels(painter.clip_rect(), pixels_from_points); anyhow::ensure!(resolution_in_pixel[0] > 0 && resolution_in_pixel[1] > 0); + // For simplicity (and to reduce surprises!) we always render with a pinhole camera. + // Make up a default pinhole camera if we don't have one placing it in a way to look at the entire space. let default_pinhole_image_plane_size = glam::vec2(space_from_ui.to().width(), space_from_ui.to().height()); let default_focal_length_in_pixels = default_pinhole_image_plane_size.x; - - // TODO: don't use space camera but do a query. - let pinhole = space_camera - .and_then(|c| c.pinhole) - .unwrap_or(re_log_types::Pinhole { - image_from_cam: glam::Mat3::from_cols( - glam::vec3(default_focal_length_in_pixels, 0.0, 0.0), - glam::vec3(0.0, default_focal_length_in_pixels, 0.0), - (default_pinhole_image_plane_size * 0.5 + glam::Vec2::splat(0.5)).extend(1.0), - ) - .into(), - // Look at the entire space by default. - resolution: Some(default_pinhole_image_plane_size.into()), - }); + let pinhole = pinhole.unwrap_or(re_log_types::Pinhole { + image_from_cam: glam::Mat3::from_cols( + glam::vec3(default_focal_length_in_pixels, 0.0, 0.0), + glam::vec3(0.0, default_focal_length_in_pixels, 0.0), + (default_pinhole_image_plane_size * 0.5 + glam::Vec2::splat(0.5)).extend(1.0), + ) + .into(), + // Look at the entire space by default. + resolution: Some(default_pinhole_image_plane_size.into()), + }); let projection_from_view = re_renderer::view_builder::Projection::Perspective { vertical_fov: pinhole.fov_y().unwrap_or(Eye::DEFAULT_FOV_Y), From 2584ce47b75caf70a384836b2f17478621680626 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 30 Apr 2023 11:54:47 +0200 Subject: [PATCH 18/37] consistent canvas rect handling, take principal point into account when displaying 2d canvas --- crates/re_viewer/src/ui/view_spatial/ui.rs | 11 +- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 369 +++++++++--------- crates/re_viewer/src/ui/view_spatial/ui_3d.rs | 1 - 3 files changed, 182 insertions(+), 199 deletions(-) diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index 4894bafa4c2e..1a78d99bdf02 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -529,8 +529,7 @@ fn axis_name(axis: Option) -> String { pub fn create_labels( scene_ui: &mut SceneSpatialUiData, - ui_from_space2d: egui::emath::RectTransform, - space2d_from_ui: egui::emath::RectTransform, + ui_from_canvas: egui::emath::RectTransform, eye3d: &Eye, parent_ui: &mut egui::Ui, highlights: &SpaceViewHighlights, @@ -540,7 +539,7 @@ pub fn create_labels( let mut label_shapes = Vec::with_capacity(scene_ui.labels.len() * 2); - let ui_from_world_3d = eye3d.ui_from_world(*ui_from_space2d.to()); + let ui_from_world_3d = eye3d.ui_from_world(*ui_from_canvas.to()); for label in &scene_ui.labels { let (wrap_width, text_anchor_pos) = match label.target { @@ -549,7 +548,7 @@ pub fn create_labels( if nav_mode == SpatialNavigationMode::ThreeD { continue; } - let rect_in_ui = ui_from_space2d.transform_rect(rect); + let rect_in_ui = ui_from_canvas.transform_rect(rect); ( // Place the text centered below the rect (rect_in_ui.width() - 4.0).at_least(60.0), @@ -561,7 +560,7 @@ pub fn create_labels( if nav_mode == SpatialNavigationMode::ThreeD { continue; } - let pos_in_ui = ui_from_space2d.transform_pos(pos); + let pos_in_ui = ui_from_canvas.transform_pos(pos); (f32::INFINITY, pos_in_ui + egui::vec2(0.0, 3.0)) } UiLabelTarget::Position3D(pos) => { @@ -623,7 +622,7 @@ pub fn create_labels( label_shapes.push(egui::Shape::galley(text_rect.center_top(), galley)); scene_ui.pickable_ui_rects.push(( - space2d_from_ui.transform_rect(bg_rect), + ui_from_canvas.inverse().transform_rect(bg_rect), label.labeled_instance, )); } diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 753001a980a6..7a58fb3ed93f 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -60,26 +60,21 @@ impl View2DState { /// Returns `(desired_size, scroll_offset)` where: /// - `desired_size` is the size of the painter necessary to capture the zoomed view in ui points /// - `scroll_offset` is the position of the `ScrollArea` offset in ui points - fn desired_size_and_offset( - &self, - available_size: Vec2, - scene_rect_accum: Rect, - ) -> (Vec2, Vec2) { + fn desired_size_and_offset(&self, available_size: Vec2, canvas_rect: Rect) -> (Vec2, Vec2) { match self.zoom { ZoomState2D::Scaled { scale, center, .. } => { - let desired_size = scene_rect_accum.size() * scale; + let desired_size = canvas_rect.size() * scale; // Try to keep the center of the scene in the middle of the available size - let scroll_offset = (center.to_vec2() - scene_rect_accum.left_top().to_vec2()) - * scale + let scroll_offset = (center.to_vec2() - canvas_rect.left_top().to_vec2()) * scale - available_size / 2.0; (desired_size, scroll_offset) } ZoomState2D::Auto => { // Otherwise, we autoscale the space to fit available area while maintaining aspect ratio - let scene_bbox = if scene_rect_accum.is_positive() { - scene_rect_accum + let scene_bbox = if canvas_rect.is_positive() { + canvas_rect } else { Rect::from_min_max(Pos2::ZERO, Pos2::new(1.0, 1.0)) }; @@ -102,7 +97,7 @@ impl View2DState { &mut self, response: &egui::Response, ui_to_space: egui::emath::RectTransform, - scene_rect_accum: Rect, + canvas_rect: Rect, available_size: Vec2, ) { // Determine if we are zooming @@ -118,14 +113,14 @@ impl View2DState { if let Some(input_zoom) = hovered_zoom { if input_zoom > 1.0 { let scale = response.rect.height() / ui_to_space.to().height(); - let center = scene_rect_accum.center(); + let center = canvas_rect.center(); self.zoom = ZoomState2D::Scaled { scale, center, accepting_scroll: false, }; // Recursively update now that we have initialized `ZoomState` to `Scaled` - self.update(response, ui_to_space, scene_rect_accum, available_size); + self.update(response, ui_to_space, canvas_rect, available_size); } } } @@ -183,8 +178,8 @@ impl View2DState { } // If our zoomed region is smaller than the available size - if scene_rect_accum.size().x * scale < available_size.x - && scene_rect_accum.size().y * scale < available_size.y + if canvas_rect.size().x * scale < available_size.x + && canvas_rect.size().y * scale < available_size.y { self.zoom = ZoomState2D::Auto; } @@ -193,7 +188,7 @@ impl View2DState { /// Take the offset from the `ScrollArea` and apply it back to center so that other /// scroll interfaces work as expected. - fn capture_scroll(&mut self, offset: Vec2, available_size: Vec2, scene_rect_accum: Rect) { + fn capture_scroll(&mut self, offset: Vec2, available_size: Vec2, canvas_rect: Rect) { if let ZoomState2D::Scaled { scale, accepting_scroll, @@ -201,7 +196,7 @@ impl View2DState { } = self.zoom { if accepting_scroll { - let center = scene_rect_accum.left_top() + (available_size / 2.0 + offset) / scale; + let center = canvas_rect.left_top() + (available_size / 2.0 + offset) / scale; self.zoom = ZoomState2D::Scaled { scale, center, @@ -224,7 +219,7 @@ pub fn view_2d( ui: &mut egui::Ui, state: &mut ViewSpatialState, space: &EntityPath, - scene: SceneSpatial, + mut scene: SceneSpatial, scene_rect_accum: Rect, space_view_id: SpaceViewId, highlights: &SpaceViewHighlights, @@ -235,9 +230,34 @@ pub fn view_2d( // Save off the available_size since this is used for some of the layout updates later let available_size = ui.available_size(); + // Determine the canvas which determines the extent of the explorable scene coordinates, + // and thus the size of the scroll area. + // + // TODO(andreas): We want to move away from the scroll area and instead work with open ended 2D scene coordinates! + // The term canvas might then refer to the area in scene coordinates visible at a given moment. + // Orthogonally, we'll want to visualize the resolution rectangle of the pinhole camera. + // + // For that we need to check if this is defined by a pinhole camera. + // Note that we can't rely on the camera being part of scene.space_cameras since that requires + // the camera to be added to the scene! + let pinhole = query_latest_single( + &ctx.log_db.entity_db, + space, + &ctx.rec_cfg.time_ctrl.current_query(), + ) + .and_then(|transform| match transform { + re_log_types::Transform::Pinhole(pinhole) => Some(pinhole), + _ => None, + }); + let canvas_rect = pinhole + .and_then(|p| p.resolution()) + .map_or(scene_rect_accum, |res| { + Rect::from_min_max(egui::Pos2::ZERO, egui::pos2(res.x, res.y)) + }); + let (desired_size, offset) = state .state_2d - .desired_size_and_offset(available_size, scene_rect_accum); + .desired_size_and_offset(available_size, canvas_rect); // Bound the offset based on sizes // TODO(jleibs): can we derive this from the ScrollArea shape? @@ -249,169 +269,127 @@ pub fn view_2d( .auto_shrink([false, false]); let scroll_out = scroll_area.show(ui, |ui| { - view_2d_scrollable( - desired_size, - available_size, - ctx, - ui, - state, - space, - scene, - scene_rect_accum, - space_view_id, - highlights, - entity_properties, - ) - }); + let (mut response, painter) = + ui.allocate_painter(desired_size, egui::Sense::click_and_drag()); - // Update the scroll area based on the computed offset - // This handles cases of dragging/zooming the space - state - .state_2d - .capture_scroll(scroll_out.state.offset, available_size, scene_rect_accum); - scroll_out.inner -} + if !response.rect.is_positive() { + return response; // protect against problems with zero-sized views + } -/// Create the real 2D view inside the scrollable area -#[allow(clippy::too_many_arguments)] -fn view_2d_scrollable( - desired_size: Vec2, - available_size: Vec2, - ctx: &mut ViewerContext<'_>, - parent_ui: &mut egui::Ui, - state: &mut ViewSpatialState, - space: &EntityPath, - mut scene: SceneSpatial, - scene_rect_accum: Rect, - space_view_id: SpaceViewId, - highlights: &SpaceViewHighlights, - entity_properties: &EntityPropertyMap, -) -> egui::Response { - let (mut response, painter) = - parent_ui.allocate_painter(desired_size, egui::Sense::click_and_drag()); + let ui_from_canvas = egui::emath::RectTransform::from_to(canvas_rect, response.rect); + let canvas_from_ui = ui_from_canvas.inverse(); - if !response.rect.is_positive() { - return response; // protect against problems with zero-sized views - } + state + .state_2d + .update(&response, canvas_from_ui, canvas_rect, available_size); - // Create our transforms. - let ui_from_space = egui::emath::RectTransform::from_to(scene_rect_accum, response.rect); - let space_from_ui = ui_from_space.inverse(); + let eye = Eye { + world_from_view: IsoTransform::IDENTITY, + fov_y: None, + }; - state - .state_2d - .update(&response, space_from_ui, scene_rect_accum, available_size); + let Ok(target_config) = setup_target_config( + &painter, + canvas_from_ui, + &space.to_string(), + state.auto_size_config(), + scene + .primitives + .any_outlines, + pinhole, + ) else { + return response; + }; - let eye = Eye { - world_from_view: IsoTransform::IDENTITY, - fov_y: None, - }; + let mut view_builder = ViewBuilder::new(ctx.render_ctx, target_config); - // Query for a pinhole camera at the space view's root that we should take over. - // Note that we can't rely on the camera being part of scene.space_cameras since that requires - // the camera to be added to the scene! - let pinhole = query_latest_single( - &ctx.log_db.entity_db, - space, - &ctx.rec_cfg.time_ctrl.current_query(), - ) - .and_then(|transform| match transform { - re_log_types::Transform::Pinhole(pinhole) => Some(pinhole), - _ => None, - }); + // Create labels now since their shapes participate are added to scene.ui for picking. + let label_shapes = create_labels( + &mut scene.ui, + ui_from_canvas, + &eye, + ui, + highlights, + SpatialNavigationMode::TwoD, + ); - let Ok(target_config) = setup_target_config( - &painter, - space_from_ui, - &space.to_string(), - state.auto_size_config(), - scene - .primitives - .any_outlines, - pinhole, - ) else { - return response; - }; + if !re_ui::egui_helpers::is_anything_being_dragged(ui.ctx()) { + response = picking( + ctx, + response, + canvas_from_ui, + painter.clip_rect(), + ui, + eye, + &mut view_builder, + space_view_id, + state, + &scene, + space, + entity_properties, + ); + } - let mut view_builder = ViewBuilder::new(ctx.render_ctx, target_config); - - // Create labels now since their shapes participate are added to scene.ui for picking. - let label_shapes = create_labels( - &mut scene.ui, - ui_from_space, - space_from_ui, - &eye, - parent_ui, - highlights, - SpatialNavigationMode::TwoD, - ); - - if !re_ui::egui_helpers::is_anything_being_dragged(parent_ui.ctx()) { - response = picking( - ctx, - response, - space_from_ui, - painter.clip_rect(), - parent_ui, - eye, - &mut view_builder, - space_view_id, - state, - &scene, - space, - entity_properties, - ); - } + // ------------------------------------------------------------------------ - // ------------------------------------------------------------------------ + // Screenshot context menu. + let (response, screenshot_mode) = screenshot_context_menu(ctx, response); + if let Some(mode) = screenshot_mode { + let _ = view_builder.schedule_screenshot( + ctx.render_ctx, + space_view_id.gpu_readback_id(), + mode, + ); + } - // Screenshot context menu. - let (response, screenshot_mode) = screenshot_context_menu(ctx, response); - if let Some(mode) = screenshot_mode { - let _ = - view_builder.schedule_screenshot(ctx.render_ctx, space_view_id.gpu_readback_id(), mode); - } + // Draw a re_renderer driven view. + // Camera & projection are configured to ingest space coordinates directly. + { + let command_buffer = match fill_view_builder( + ctx.render_ctx, + &mut view_builder, + scene.primitives, + &ScreenBackground::ClearColor(ui.visuals().extreme_bg_color.into()), + ) { + Ok(command_buffer) => command_buffer, + Err(err) => { + re_log::error!("Failed to fill view builder: {}", err); + return response; + } + }; + painter.add(gpu_bridge::renderer_paint_callback( + ctx.render_ctx, + command_buffer, + view_builder, + painter.clip_rect(), + painter.ctx().pixels_per_point(), + )); + } - // Draw a re_renderer driven view. - // Camera & projection are configured to ingest space coordinates directly. - { - let command_buffer = match fill_view_builder( - ctx.render_ctx, - &mut view_builder, - scene.primitives, - &ScreenBackground::ClearColor(parent_ui.visuals().extreme_bg_color.into()), - ) { - Ok(command_buffer) => command_buffer, - Err(err) => { - re_log::error!("Failed to fill view builder: {}", err); - return response; - } - }; - painter.add(gpu_bridge::renderer_paint_callback( - ctx.render_ctx, - command_buffer, - view_builder, - painter.clip_rect(), - painter.ctx().pixels_per_point(), + painter.extend(show_projections_from_3d_space( + ctx, + ui, + space, + &ui_from_canvas, )); - } - painter.extend(show_projections_from_3d_space( - ctx, - parent_ui, - space, - &ui_from_space, - )); + // Add egui driven labels on top of re_renderer content. + painter.extend(label_shapes); - // Add egui driven labels on top of re_renderer content. - painter.extend(label_shapes); + response + }); - response + // Update the scroll area based on the computed offset + // This handles cases of dragging/zooming the space + state + .state_2d + .capture_scroll(scroll_out.state.offset, available_size, scene_rect_accum); + scroll_out.inner } fn setup_target_config( painter: &egui::Painter, - space_from_ui: RectTransform, + canvas_from_ui: RectTransform, space_name: &str, auto_size_config: re_renderer::AutoSizeConfig, any_outlines: bool, @@ -424,24 +402,28 @@ fn setup_target_config( // For simplicity (and to reduce surprises!) we always render with a pinhole camera. // Make up a default pinhole camera if we don't have one placing it in a way to look at the entire space. - let default_pinhole_image_plane_size = - glam::vec2(space_from_ui.to().width(), space_from_ui.to().height()); - let default_focal_length_in_pixels = default_pinhole_image_plane_size.x; - let pinhole = pinhole.unwrap_or(re_log_types::Pinhole { - image_from_cam: glam::Mat3::from_cols( - glam::vec3(default_focal_length_in_pixels, 0.0, 0.0), - glam::vec3(0.0, default_focal_length_in_pixels, 0.0), - (default_pinhole_image_plane_size * 0.5 + glam::Vec2::splat(0.5)).extend(1.0), - ) - .into(), - // Look at the entire space by default. - resolution: Some(default_pinhole_image_plane_size.into()), + let canvas_size = glam::vec2(canvas_from_ui.to().width(), canvas_from_ui.to().height()); + let default_principal_point = canvas_size * 0.5; + let pinhole = pinhole.unwrap_or_else(|| { + let focal_length_in_pixels = canvas_size.x; + + re_log_types::Pinhole { + image_from_cam: glam::Mat3::from_cols( + glam::vec3(focal_length_in_pixels, 0.0, 0.0), + glam::vec3(0.0, focal_length_in_pixels, 0.0), + default_principal_point.extend(1.0), + ) + .into(), + resolution: Some(canvas_size.into()), + } }); let projection_from_view = re_renderer::view_builder::Projection::Perspective { vertical_fov: pinhole.fov_y().unwrap_or(Eye::DEFAULT_FOV_Y), near_plane_distance: 0.01, - aspect_ratio: pinhole.aspect_ratio().unwrap(), // TODO: + aspect_ratio: pinhole + .aspect_ratio() + .unwrap_or(canvas_size.x / canvas_size.y), }; // Put the camera at the position where it sees the entire image plane as defined @@ -455,23 +437,19 @@ fn setup_target_config( ) .unwrap_or(macaw::IsoTransform::IDENTITY); - // What portion of the space are we looking at? - // TODO: cleanup spaces - // TODO: Need to take principal point into account - let total_ui_rect = space_from_ui.from(); - let visible_ui_rect = painter.clip_rect(); - - let viewport_transformation = re_renderer::RectTransform { - from: re_renderer::RectF32 { - left_top: glam::vec2(visible_ui_rect.left(), visible_ui_rect.top()), - extent: glam::vec2(visible_ui_rect.width(), visible_ui_rect.height()), - }, - to: re_renderer::RectF32 { - left_top: glam::vec2(total_ui_rect.left(), total_ui_rect.top()), - extent: glam::vec2(total_ui_rect.width(), total_ui_rect.height()), - }, + // Cut to the portion of the currently visible ui area. + let mut viewport_transformation = re_renderer::RectTransform { + from: egui_rect_to_re_renderer(painter.clip_rect()), + to: egui_rect_to_re_renderer(*canvas_from_ui.from()), }; + // The principal point might not be quite centered. + // We need to account for this translation in the viewport transformation. + let principal_point_offset = default_principal_point - pinhole.principal_point(); + let ui_from_canvas_scale = canvas_from_ui.inverse().scale(); + viewport_transformation.from.left_top += + principal_point_offset * glam::vec2(ui_from_canvas_scale.x, ui_from_canvas_scale.y); + Ok({ let name = space_name.into(); TargetConfiguration { @@ -487,13 +465,20 @@ fn setup_target_config( }) } +fn egui_rect_to_re_renderer(rect: egui::Rect) -> re_renderer::RectF32 { + re_renderer::RectF32 { + left_top: glam::vec2(rect.left(), rect.top()), + extent: glam::vec2(rect.width(), rect.height()), + } +} + // ------------------------------------------------------------------------ fn show_projections_from_3d_space( ctx: &ViewerContext<'_>, ui: &egui::Ui, space: &EntityPath, - ui_from_space: &RectTransform, + ui_from_canvas: &RectTransform, ) -> Vec { let mut shapes = Vec::new(); if let HoveredSpace::ThreeD { @@ -505,7 +490,7 @@ fn show_projections_from_3d_space( 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)); + let pos_in_ui = ui_from_canvas.transform_pos(pos2(pos_2d.x, pos_2d.y)); let radius = 4.0; shapes.push(Shape::circle_filled( pos_in_ui, diff --git a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs index 1a74d7cf07a4..7e4f4751f779 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs @@ -348,7 +348,6 @@ pub fn view_3d( let label_shapes = create_labels( &mut scene.ui, RectTransform::from_to(rect, rect), - RectTransform::from_to(rect, rect), &eye, ui, highlights, From 65788f2bc2a3d2c5ba7615cfa8844c1606a57eaf Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 30 Apr 2023 12:11:45 +0200 Subject: [PATCH 19/37] comments on the nature of our interim 3D->2D solution --- crates/re_viewer/src/misc/transform_cache.rs | 19 ++++++++++--------- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index adbd62178da0..87a3d555dfa9 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -266,15 +266,16 @@ fn transform_at( translation, ); - // Let's think the inverse of this through, in the context of a 2D view that sits at a pinhole camera: - // In this situation, we have a camera that is `focal_length_in_pixels` away from the XY image plane - // and peering down on it from the `principal_point`. - // - // In this case, any 3D object that is up to `pinhole_image_plane_distance` away from the camera should be visible! - // (everything behind, is clipped by the image plane) - // This means we need to apply the scaling factor of `pinhole.focal_length_in_pixels() / distance`, - // as well as undo the translation of the principal point. - // .. which is just what the inverse transformation does ◼️. + // Above calculation is nice for a certain kind of visualizing a projected image plane, + // but the image plane distance is arbitrary and there might be other, better visualizations! + + // TODO(#1988): + // As such we don't ever want to invert this matrix! + // However, currently our 2D views require do to exactly that since we're forced to + // build a relationship between the 2D plane and the 3D world, when actually the 2D plane + // should have infinite depth! + // The inverse of this matrix *is* working for this, but quickly runs into precision issues. + // See also `ui_2d.rs#setup_target_config` Ok(Some(parent_from_child)) } diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 7a58fb3ed93f..77bc02b53552 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -400,6 +400,20 @@ fn setup_target_config( gpu_bridge::viewport_resolution_in_pixels(painter.clip_rect(), pixels_from_points); anyhow::ensure!(resolution_in_pixel[0] > 0 && resolution_in_pixel[1] > 0); + // TODO(#1988): + // The camera setup is done in a way that works well with the way we inverse pinhole camera transformations right now. + // This has a lot of issues though, mainly because we pretend that the 2D plane has a defined depth. + // * very bad depth precision as we limit the depth range from 0 to focal_length_in_pixels + // * depth values in depth buffer are almost non-sensical and can't be used easily for picking + // * 2D rendering can use depth buffer for layering only in a very limited way + // + // Instead we should treat 2D objects as pre-projected with their depth information already lost. + // + // We would define two cameras then: + // * an orthographic camera for handling 2D rendering + // * a perspective camera *at the origin* for 3D rendering + // Both share the same view-builder and the same viewport transformation but are independent otherwise. + // For simplicity (and to reduce surprises!) we always render with a pinhole camera. // Make up a default pinhole camera if we don't have one placing it in a way to look at the entire space. let canvas_size = glam::vec2(canvas_from_ui.to().width(), canvas_from_ui.to().height()); From 39f46f4e144a572194f00382114c31dcbda9d201 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 30 Apr 2023 12:15:57 +0200 Subject: [PATCH 20/37] point out that picking should use same transforms --- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 77bc02b53552..9e48f4fde7a8 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -283,6 +283,7 @@ pub fn view_2d( .state_2d .update(&response, canvas_from_ui, canvas_rect, available_size); + // TODO(andreas): Use the same eye & transformations as in `setup_target_config`. let eye = Eye { world_from_view: IsoTransform::IDENTITY, fov_y: None, From b89d4eff2ae807e20e62adacb6dfc42e74091fe2 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 30 Apr 2023 12:24:52 +0200 Subject: [PATCH 21/37] easier point/line flag building --- crates/re_renderer/src/line_strip_builder.rs | 16 ++++++++-------- crates/re_renderer/src/point_cloud_builder.rs | 5 +++-- .../ui/view_spatial/scene/scene_part/boxes2d.rs | 6 +----- .../ui/view_spatial/scene/scene_part/lines2d.rs | 2 +- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/crates/re_renderer/src/line_strip_builder.rs b/crates/re_renderer/src/line_strip_builder.rs index 1526150128aa..e315b9abadad 100644 --- a/crates/re_renderer/src/line_strip_builder.rs +++ b/crates/re_renderer/src/line_strip_builder.rs @@ -265,13 +265,7 @@ impl<'a> LineBatchBuilder<'a> { ] .into_iter(), ) - .flags( - LineStripFlags::CAP_END_ROUND - | LineStripFlags::CAP_START_ROUND - | LineStripFlags::NO_COLOR_GRADIENT - | LineStripFlags::CAP_END_EXTEND_OUTWARDS - | LineStripFlags::CAP_START_EXTEND_OUTWARDS, - ) + .flags(LineStripSeriesBuilder::default_rectangle_flags()) } /// Add rectangle outlines. @@ -312,12 +306,14 @@ impl<'a> LineBatchBuilder<'a> { points: impl Iterator, ) -> LineStripBuilder<'_> { self.add_strip(points.map(|p| p.extend(0.0))) + .flags(LineStripFlags::FORCE_ORTHO_SPANNING) } /// Adds a single 2D line segment connecting two points. Uses autogenerated depth value. #[inline] pub fn add_segment_2d(&mut self, a: glam::Vec2, b: glam::Vec2) -> LineStripBuilder<'_> { self.add_strip_2d([a, b].into_iter()) + .flags(LineStripFlags::FORCE_ORTHO_SPANNING) } /// Adds a series of unconnected 2D line segments. @@ -329,6 +325,7 @@ impl<'a> LineBatchBuilder<'a> { segments: impl Iterator, ) -> LineStripBuilder<'_> { self.add_segments(segments.map(|(a, b)| (a.extend(0.0), b.extend(0.0)))) + .flags(LineStripFlags::FORCE_ORTHO_SPANNING) } /// Add 2D rectangle outlines. @@ -347,6 +344,7 @@ impl<'a> LineBatchBuilder<'a> { extent_u.extend(0.0), extent_v.extend(0.0), ) + .flags(LineStripFlags::FORCE_ORTHO_SPANNING) } /// Add 2D rectangle outlines with axis along X and Y. @@ -364,6 +362,7 @@ impl<'a> LineBatchBuilder<'a> { glam::Vec3::X * (max.x - min.x), glam::Vec3::Y * (max.y - min.y), ) + .flags(LineStripFlags::FORCE_ORTHO_SPANNING) } } @@ -392,10 +391,11 @@ impl<'a> LineStripBuilder<'a> { self } + /// Adds (!) flags to the line strip. #[inline] pub fn flags(self, flags: LineStripFlags) -> Self { for strip in self.builder.strips[self.strip_range.clone()].iter_mut() { - strip.flags = flags; + strip.flags |= flags; } self } diff --git a/crates/re_renderer/src/point_cloud_builder.rs b/crates/re_renderer/src/point_cloud_builder.rs index 2c07aebb8191..4862748996aa 100644 --- a/crates/re_renderer/src/point_cloud_builder.rs +++ b/crates/re_renderer/src/point_cloud_builder.rs @@ -244,11 +244,12 @@ impl<'a> PointCloudBatchBuilder<'a> { colors, picking_instance_ids, ) + .flags(PointCloudBatchFlags::DRAW_AS_CIRCLES) } - /// Set flags for this batch. + /// Adds (!) flags for this batch. pub fn flags(mut self, flags: PointCloudBatchFlags) -> Self { - self.batch_mut().flags = flags; + self.batch_mut().flags |= flags; self } diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs index e612fe64b482..3de91ba5ad94 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/boxes2d.rs @@ -4,7 +4,7 @@ use re_log_types::{ Component, }; use re_query::{query_primary_with_history, EntityView, QueryError}; -use re_renderer::{renderer::LineStripFlags, LineStripSeriesBuilder, Size}; +use re_renderer::Size; use crate::{ misc::{SpaceViewHighlights, TransformCache, ViewerContext}, @@ -70,10 +70,6 @@ impl Boxes2DPart { glam::vec2(rect.width(), 0.0), glam::vec2(0.0, rect.height()), ) - .flags( - LineStripSeriesBuilder::default_rectangle_flags() - | LineStripFlags::FORCE_ORTHO_SPANNING, - ) .color(color) .radius(radius) .picking_instance_id(instance_key_to_picking_id( diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs index 7206ca856037..0a9b47b9b5ed 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/lines2d.rs @@ -50,7 +50,7 @@ impl Lines2DPart { .add_strip_2d(strip.0.into_iter().map(|v| v.into())) .color(color) .radius(radius) - .flags(LineStripFlags::NO_COLOR_GRADIENT | LineStripFlags::FORCE_ORTHO_SPANNING) + .flags(LineStripFlags::NO_COLOR_GRADIENT) .picking_instance_id(instance_key_to_picking_id( instance_key, entity_view, From 85414a0b831e6d62943fec838ed962a3bbedf4c0 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 30 Apr 2023 12:40:04 +0200 Subject: [PATCH 22/37] minor cleanup --- crates/re_renderer/src/lib.rs | 3 +-- crates/re_renderer/src/line_strip_builder.rs | 6 +++--- crates/re_viewer/src/misc/transform_cache.rs | 6 ++---- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/re_renderer/src/lib.rs b/crates/re_renderer/src/lib.rs index 74896d13d3c5..f6e07b5839c1 100644 --- a/crates/re_renderer/src/lib.rs +++ b/crates/re_renderer/src/lib.rs @@ -42,7 +42,7 @@ pub use debug_label::DebugLabel; pub use depth_offset::DepthOffset; pub use line_strip_builder::{LineStripBuilder, LineStripSeriesBuilder}; pub use point_cloud_builder::{PointCloudBatchBuilder, PointCloudBuilder}; -pub use rect::RectF32; +pub use rect::{RectF32, RectInt}; pub use size::Size; pub use transform::RectTransform; pub use view_builder::{AutoSizeConfig, ViewBuilder}; @@ -70,7 +70,6 @@ mod file_server; pub use self::file_server::FileServer; mod rect; -pub use rect::RectInt; #[cfg(not(all(not(target_arch = "wasm32"), debug_assertions)))] // wasm or release builds #[rustfmt::skip] // it's auto-generated diff --git a/crates/re_renderer/src/line_strip_builder.rs b/crates/re_renderer/src/line_strip_builder.rs index e315b9abadad..2514f621b24b 100644 --- a/crates/re_renderer/src/line_strip_builder.rs +++ b/crates/re_renderer/src/line_strip_builder.rs @@ -105,7 +105,7 @@ impl LineStripSeriesBuilder { self.strips.is_empty() } - pub fn default_rectangle_flags() -> LineStripFlags { + pub fn default_box_flags() -> LineStripFlags { LineStripFlags::CAP_END_ROUND | LineStripFlags::CAP_START_ROUND | LineStripFlags::NO_COLOR_GRADIENT @@ -265,7 +265,7 @@ impl<'a> LineBatchBuilder<'a> { ] .into_iter(), ) - .flags(LineStripSeriesBuilder::default_rectangle_flags()) + .flags(LineStripSeriesBuilder::default_box_flags()) } /// Add rectangle outlines. @@ -294,7 +294,7 @@ impl<'a> LineBatchBuilder<'a> { ] .into_iter(), ) - .flags(LineStripSeriesBuilder::default_rectangle_flags()) + .flags(LineStripSeriesBuilder::default_box_flags()) } /// Adds a 2D series of line connected points. diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 87a3d555dfa9..afcd8048c0c8 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -127,9 +127,8 @@ impl TransformCache { ¤t_tree.path, entity_db, &query, - // TODO(andreas): For inverse pinhole cameras we'd like the image plane to be indefinitely far away. - // High values quickly run into precision issues though. - // What we really want is to render everything under the inverse pinhole on top, independent of everything else. + // TODO(#1988): See comment in transform_at. This is a workaround for precision issues + // and the fact that there is no meaningful image plane distance for 3D->2D views. |_| 500.0, &mut encountered_pinhole, ) { @@ -184,7 +183,6 @@ impl TransformCache { for child_tree in tree.children.values() { let mut encountered_pinhole = encountered_pinhole; - let reference_from_child = match transform_at( &child_tree.path, entity_db, From 3c7ee6a04038bff80165564099165939b796e15c Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 09:47:41 +0200 Subject: [PATCH 23/37] doc test fix --- crates/re_renderer/src/view_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index 5d26483986b8..677201231a38 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -698,11 +698,11 @@ impl ViewBuilder { /// Data from the picking rect needs to be retrieved via [`crate::PickingLayerProcessor::next_readback_result`]. /// To do so, you need to pass the exact same `identifier` and type of user data as you've done here: /// ```no_run - /// use re_renderer::{view_builder::ViewBuilder, IntRect, PickingLayerProcessor, RenderContext}; + /// use re_renderer::{view_builder::ViewBuilder, RectInt, PickingLayerProcessor, RenderContext}; /// fn schedule_picking_readback( /// ctx: &mut RenderContext, /// view_builder: &mut ViewBuilder, - /// picking_rect: IntRect, + /// picking_rect: RectInt, /// ) { /// view_builder.schedule_picking_rect( /// ctx, picking_rect, 42, "My screenshot".to_owned(), false, From 8c08a82b1dd89e7dc15b920f167b84795f675b13 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 17:37:52 +0200 Subject: [PATCH 24/37] clarify what sphere_quad's coverage methods do --- crates/re_renderer/shader/utils/sphere_quad.wgsl | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/re_renderer/shader/utils/sphere_quad.wgsl b/crates/re_renderer/shader/utils/sphere_quad.wgsl index 22e98cb52e49..6416c40aec31 100644 --- a/crates/re_renderer/shader/utils/sphere_quad.wgsl +++ b/crates/re_renderer/shader/utils/sphere_quad.wgsl @@ -93,13 +93,14 @@ fn sphere_or_circle_quad_span(vertex_idx: u32, point_pos: Vec3, point_unresolved return SphereQuadData(pos, radius); } -fn sphere_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) -> f32 { +/// Computes coverage of a 3D sphere placed at `sphere_center` in the fragment shader using the currently set camera. +fn sphere_quad_coverage(world_position: Vec3, radius: f32, sphere_center: Vec3) -> f32 { let ray = camera_ray_to_world_pos(world_position); // Sphere intersection with anti-aliasing as described by Iq here // https://www.shadertoy.com/view/MsSSWV // (but rearranged and labeled to it's easier to understand!) - let d = ray_sphere_distance(ray, point_center, radius); + let d = ray_sphere_distance(ray, sphere_center, radius); let distance_to_sphere_surface = d.x; let closest_ray_dist = d.y; let pixel_world_size = approx_pixel_world_size_at(closest_ray_dist); @@ -111,11 +112,12 @@ fn sphere_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) - return saturate(0.5 - distance_to_surface_in_pixels); } -/// Computes coverage of a sphere or circle on a quad. +/// Computes coverage of a 2D sphere placed at `circle_center` in the fragment shader using the currently set camera. /// -/// Note that in orthographic mode, spheres are always circles. -fn circle_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) -> f32 { - let to_center = point_center - world_position; +/// 2D primitives are always facing the camera - the difference to sphere_quad_coverage is that +/// perspective projection is not taken into account. +fn circle_quad_coverage(world_position: Vec3, radius: f32, circle_center: Vec3) -> f32 { + let to_center = circle_center - world_position; let distance = length(to_center); let distance_pixel_difference = fwidth(distance); return smoothstep(radius + distance_pixel_difference, radius - distance_pixel_difference, distance); From 5e984fad829cee32388546868a8d04c44de7c8e7 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:19:29 +0200 Subject: [PATCH 25/37] fix taking only one axis into account for pixel size approximation --- crates/re_renderer/src/view_builder.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/re_renderer/src/view_builder.rs b/crates/re_renderer/src/view_builder.rs index 677201231a38..65a606963c75 100644 --- a/crates/re_renderer/src/view_builder.rs +++ b/crates/re_renderer/src/view_builder.rs @@ -391,8 +391,8 @@ impl ViewBuilder { }; let tan_half_fov = glam::vec2(f32::MAX, f32::MAX); - let pixel_world_size_from_camera_distance = - vertical_world_size / config.resolution_in_pixel[1] as f32; + let pixel_world_size_from_camera_distance = vertical_world_size + / config.resolution_in_pixel[0].max(config.resolution_in_pixel[1]) as f32; ( projection_from_view, @@ -408,8 +408,11 @@ impl ViewBuilder { .to_ndc_scale_and_translation(); let projection_from_view = ndc_scale_and_translation * projection_from_view; // Need to take into account that a smaller portion of the world scale is visible now. - let pixel_world_size_from_camera_distance = - pixel_world_size_from_camera_distance / ndc_scale_and_translation.col(0).x; + let pixel_world_size_from_camera_distance = pixel_world_size_from_camera_distance + / ndc_scale_and_translation + .col(0) + .x + .max(ndc_scale_and_translation.col(1).y); let mut view_from_world = config.view_from_world.to_mat4(); // For OrthographicCameraMode::TopLeftCorner, we want Z facing forward. From 9693389eb15b027bb1aa11291808eb92a00ebf17 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:20:19 +0200 Subject: [PATCH 26/37] comment explaining how to use FORCE_ORTHO_SPANNING --- crates/re_renderer/src/renderer/lines.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/re_renderer/src/renderer/lines.rs b/crates/re_renderer/src/renderer/lines.rs index 9d6336773965..1d51b5d841ee 100644 --- a/crates/re_renderer/src/renderer/lines.rs +++ b/crates/re_renderer/src/renderer/lines.rs @@ -233,6 +233,10 @@ bitflags! { /// Forces spanning the line's quads as-if the camera was orthographic. /// + /// This is useful for lines that are on a plane that is parallel to the camera: + /// Without this flag, the lines will poke through the camera plane as they orient themselves towards the camera. + /// Note that since distances to the camera are computed differently in orthographic mode, this changes how screen space sizes are computed. + /// /// TODO(andreas): Could be moved to per batch flags. const FORCE_ORTHO_SPANNING = 0b1000_0000; } From 45df7a5f9e502ee5bd2993523a1195f8e680b1fc Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:21:30 +0200 Subject: [PATCH 27/37] remove unnecessary affine3a multiply method --- crates/re_viewer/src/misc/transform_cache.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index afcd8048c0c8..a379078917cb 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -60,13 +60,6 @@ impl std::fmt::Display for UnreachableTransform { } } -fn transform_affine3(a: glam::Affine3A, b: glam::Affine3A) -> glam::Affine3A { - glam::Affine3A { - matrix3: a.matrix3.mul_mat3(&b.matrix3), - translation: a.matrix3.mul_vec3a(b.translation) + a.translation, - } -} - impl TransformCache { /// Determines transforms for all entities relative to a space path which serves as the "reference". /// I.e. the resulting transforms are "reference from scene" @@ -139,8 +132,7 @@ impl TransformCache { } Ok(None) => {} Ok(Some(parent_from_child)) => { - reference_from_ancestor = - transform_affine3(reference_from_ancestor, parent_from_child.inverse()); + reference_from_ancestor = reference_from_ancestor * parent_from_child.inverse(); } } @@ -196,9 +188,7 @@ impl TransformCache { continue; } Ok(None) => reference_from_entity, - Ok(Some(child_from_parent)) => { - transform_affine3(reference_from_entity, child_from_parent) - } + Ok(Some(child_from_parent)) => reference_from_entity * child_from_parent, }; self.gather_descendants_transforms( child_tree, From c48fe19399b1709bb6ee62a7ec29b246108d3858 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:29:21 +0200 Subject: [PATCH 28/37] impl From for wgpu_buffer_types::Mat4 --- crates/re_renderer/src/renderer/lines.rs | 5 ++--- crates/re_renderer/src/renderer/point_cloud.rs | 7 +++---- crates/re_renderer/src/wgpu_buffer_types.rs | 7 +++++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/re_renderer/src/renderer/lines.rs b/crates/re_renderer/src/renderer/lines.rs index 1d51b5d841ee..759f66984e7a 100644 --- a/crates/re_renderer/src/renderer/lines.rs +++ b/crates/re_renderer/src/renderer/lines.rs @@ -613,7 +613,7 @@ impl LineDrawData { batches .iter() .map(|batch_info| gpu_data::BatchUniformBuffer { - world_from_obj: glam::Mat4::from(batch_info.world_from_obj).into(), + world_from_obj: batch_info.world_from_obj.into(), outline_mask_ids: batch_info .overall_outline_mask_ids .0 @@ -637,8 +637,7 @@ impl LineDrawData { .additional_outline_mask_ids_vertex_ranges .iter() .map(|(_, mask)| gpu_data::BatchUniformBuffer { - world_from_obj: glam::Mat4::from(batch_info.world_from_obj) - .into(), + world_from_obj: batch_info.world_from_obj.into(), outline_mask_ids: mask.0.unwrap_or_default().into(), picking_object_id: batch_info.picking_object_id, end_padding: Default::default(), diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index 3101bc3d9122..efe9006df69e 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -50,7 +50,7 @@ bitflags! { /// If true, we shade all points in the batch like spheres. const ENABLE_SHADING = 0b0001; - /// If true, draw circles instead of spheres. + /// If true, draw 2D camera facing circles instead of spheres. const DRAW_AS_CIRCLES = 0b0010; } } @@ -399,7 +399,7 @@ impl PointCloudDrawData { batches .iter() .map(|batch_info| gpu_data::BatchUniformBuffer { - world_from_obj: glam::Mat4::from(batch_info.world_from_obj).into(), + world_from_obj: batch_info.world_from_obj.into(), flags: batch_info.flags.bits.into(), outline_mask_ids: batch_info .overall_outline_mask_ids @@ -424,8 +424,7 @@ impl PointCloudDrawData { .additional_outline_mask_ids_vertex_ranges .iter() .map(|(_, mask)| gpu_data::BatchUniformBuffer { - world_from_obj: glam::Mat4::from(batch_info.world_from_obj) - .into(), + world_from_obj: batch_info.world_from_obj.into(), flags: batch_info.flags.bits.into(), outline_mask_ids: mask.0.unwrap_or_default().into(), end_padding: Default::default(), diff --git a/crates/re_renderer/src/wgpu_buffer_types.rs b/crates/re_renderer/src/wgpu_buffer_types.rs index ee7276337b55..e4207aaed0fc 100644 --- a/crates/re_renderer/src/wgpu_buffer_types.rs +++ b/crates/re_renderer/src/wgpu_buffer_types.rs @@ -289,6 +289,13 @@ impl From for Mat4 { } } +impl From for Mat4 { + #[inline] + fn from(m: glam::Affine3A) -> Self { + glam::Mat4::from(m).into() + } +} + impl From for glam::Mat4 { #[inline] fn from(val: Mat4) -> Self { From d2564b7e593ad10ee2d01f9524f80738c41d5052 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:40:59 +0200 Subject: [PATCH 29/37] rename rect top_left to min --- .../src/draw_phases/picking_layer.rs | 2 +- crates/re_renderer/src/rect.rs | 27 +++++++++---------- .../re_renderer/src/renderer/debug_overlay.rs | 2 +- crates/re_renderer/src/transform.rs | 4 +-- .../src/ui/view_spatial/scene/picking.rs | 2 +- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 4 +-- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/crates/re_renderer/src/draw_phases/picking_layer.rs b/crates/re_renderer/src/draw_phases/picking_layer.rs index 414bf1092c7c..931f8489189e 100644 --- a/crates/re_renderer/src/draw_phases/picking_layer.rs +++ b/crates/re_renderer/src/draw_phases/picking_layer.rs @@ -230,7 +230,7 @@ impl PickingLayerProcessor { let cropped_projection_from_projection = RectTransform { from: picking_rect.into(), to: RectF32 { - left_top: glam::Vec2::ZERO, + min: glam::Vec2::ZERO, extent: screen_resolution.as_vec2(), }, } diff --git a/crates/re_renderer/src/rect.rs b/crates/re_renderer/src/rect.rs index acbea7a13930..65fcd9cbfe9a 100644 --- a/crates/re_renderer/src/rect.rs +++ b/crates/re_renderer/src/rect.rs @@ -3,8 +3,10 @@ /// Typically used for texture cutouts etc. #[derive(Clone, Copy, Debug)] pub struct RectInt { - /// The top left corner of the rectangle. - pub left_top: glam::IVec2, + /// The corner with the smallest coordinates. + /// + /// In most coordinate spaces this is the to top left corner of the rectangle. + pub min: glam::IVec2, /// The size of the rectangle. pub extent: glam::UVec2, @@ -14,7 +16,7 @@ impl RectInt { #[inline] pub fn from_middle_and_extent(middle: glam::IVec2, size: glam::UVec2) -> Self { Self { - left_top: middle - size.as_ivec2() / 2, + min: middle - size.as_ivec2() / 2, extent: size, } } @@ -42,8 +44,10 @@ impl RectInt { /// A 2D rectangle with float coordinates. #[derive(Clone, Copy, Debug)] pub struct RectF32 { - /// The top left corner of the rectangle. - pub left_top: glam::Vec2, + /// The corner with the smallest coordinates. + /// + /// In most coordinate spaces this is the to top left corner of the rectangle. + pub min: glam::Vec2, /// The size of the rectangle. Supposed to be positive. pub extent: glam::Vec2, @@ -52,24 +56,19 @@ pub struct RectF32 { impl RectF32 { /// The unit rectangle, defined as (0, 0) - (1, 1). pub const UNIT: RectF32 = RectF32 { - left_top: glam::Vec2::ZERO, + min: glam::Vec2::ZERO, extent: glam::Vec2::ONE, }; - #[inline] - pub fn min(self) -> glam::Vec2 { - self.left_top - } - #[inline] pub fn max(self) -> glam::Vec2 { - self.left_top + self.extent + self.min + self.extent } #[inline] pub fn scale_extent(self, factor: f32) -> RectF32 { RectF32 { - left_top: self.left_top * factor, + min: self.min * factor, extent: self.extent * factor, } } @@ -79,7 +78,7 @@ impl From for RectF32 { #[inline] fn from(rect: RectInt) -> Self { Self { - left_top: rect.left_top.as_vec2(), + min: rect.min.as_vec2(), extent: rect.extent.as_vec2(), } } diff --git a/crates/re_renderer/src/renderer/debug_overlay.rs b/crates/re_renderer/src/renderer/debug_overlay.rs index 263e75bfbcdc..d121957ff739 100644 --- a/crates/re_renderer/src/renderer/debug_overlay.rs +++ b/crates/re_renderer/src/renderer/debug_overlay.rs @@ -108,7 +108,7 @@ impl DebugOverlayDrawData { "DebugOverlayDrawData".into(), gpu_data::DebugOverlayUniformBuffer { screen_resolution: screen_resolution.as_vec2().into(), - position_in_pixel: overlay_rect.left_top.as_vec2().into(), + position_in_pixel: overlay_rect.min.as_vec2().into(), extent_in_pixel: overlay_rect.extent.as_vec2().into(), mode: mode as u32, _padding: 0, diff --git a/crates/re_renderer/src/transform.rs b/crates/re_renderer/src/transform.rs index 4904bce0c065..81fbb3838822 100644 --- a/crates/re_renderer/src/transform.rs +++ b/crates/re_renderer/src/transform.rs @@ -56,10 +56,10 @@ impl RectTransform { // It's easier to think in texcoord space, and then transform to NDC. // This texcoord rect specifies the portion of the screen that should become the entire range of the NDC screen. let texcoord_rect = RectF32 { - left_top: (self.from.left_top - self.to.left_top) / self.to.extent, + min: (self.from.min - self.to.min) / self.to.extent, extent: self.from.extent / self.to.extent, }; - let texcoord_rect_min = texcoord_rect.min(); + let texcoord_rect_min = texcoord_rect.min; let texcoord_rect_max = texcoord_rect.max(); // y axis is flipped in NDC, therefore we need to flip the y axis of the rect. 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 b2230142eb39..cc5ec43ce7a4 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/picking.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/picking.rs @@ -182,7 +182,7 @@ fn picking_gpu( // First, figure out where on the rect the cursor is by now. // (for simplicity, we assume the screen hasn't been resized) let pointer_on_picking_rect = - context.pointer_in_pixel - gpu_picking_result.rect.left_top.as_vec2(); + context.pointer_in_pixel - gpu_picking_result.rect.min.as_vec2(); // The cursor might have moved outside of the rect. Clamp it back in. let pointer_on_picking_rect = pointer_on_picking_rect.clamp( glam::Vec2::ZERO, diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 9e48f4fde7a8..673780a101c0 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -462,7 +462,7 @@ fn setup_target_config( // We need to account for this translation in the viewport transformation. let principal_point_offset = default_principal_point - pinhole.principal_point(); let ui_from_canvas_scale = canvas_from_ui.inverse().scale(); - viewport_transformation.from.left_top += + viewport_transformation.from.min += principal_point_offset * glam::vec2(ui_from_canvas_scale.x, ui_from_canvas_scale.y); Ok({ @@ -482,7 +482,7 @@ fn setup_target_config( fn egui_rect_to_re_renderer(rect: egui::Rect) -> re_renderer::RectF32 { re_renderer::RectF32 { - left_top: glam::vec2(rect.left(), rect.top()), + min: glam::vec2(rect.left(), rect.top()), extent: glam::vec2(rect.width(), rect.height()), } } From a20b9a3ec5201cae873e2eabe0568a558d517a56 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:42:02 +0200 Subject: [PATCH 30/37] remove unnecessary quaternion on pinhole transform calc --- crates/re_viewer/src/misc/transform_cache.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index a379078917cb..43114d1e3825 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -248,11 +248,8 @@ fn transform_at( let scale = distance / pinhole.focal_length_in_pixels(); let translation = (-pinhole.principal_point() * scale).extend(distance); - let parent_from_child = glam::Affine3A::from_scale_rotation_translation( - glam::Vec3::splat(scale), - glam::Quat::IDENTITY, - translation, - ); + let parent_from_child = glam::Affine3A::from_translation(translation) + * glam::Affine3A::from_scale(glam::Vec3::splat(scale)); // Above calculation is nice for a certain kind of visualizing a projected image plane, // but the image plane distance is arbitrary and there might be other, better visualizations! From 1efac9f22daccdc8316be71bd2f626e1a6069e41 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:42:52 +0200 Subject: [PATCH 31/37] make error swallowing on screenshots more explicit --- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 8 +++----- crates/re_viewer/src/ui/view_spatial/ui_3d.rs | 5 +++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 673780a101c0..2aa6073b2c8d 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -336,11 +336,9 @@ pub fn view_2d( // Screenshot context menu. let (response, screenshot_mode) = screenshot_context_menu(ctx, response); if let Some(mode) = screenshot_mode { - let _ = view_builder.schedule_screenshot( - ctx.render_ctx, - space_view_id.gpu_readback_id(), - mode, - ); + view_builder + .schedule_screenshot(ctx.render_ctx, space_view_id.gpu_readback_id(), mode) + .ok(); } // Draw a re_renderer driven view. diff --git a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs index 8b2a35889a0b..e1c60ca72e09 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_3d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_3d.rs @@ -416,8 +416,9 @@ pub fn view_3d( // Screenshot context menu. let (_, screenshot_mode) = screenshot_context_menu(ctx, response); if let Some(mode) = screenshot_mode { - let _ = - view_builder.schedule_screenshot(ctx.render_ctx, space_view_id.gpu_readback_id(), mode); + view_builder + .schedule_screenshot(ctx.render_ctx, space_view_id.gpu_readback_id(), mode) + .ok(); } show_projections_from_2d_space( From 4cfbd75403b6c0694b4d83f22da9ff125e6687a1 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 18:45:30 +0200 Subject: [PATCH 32/37] failure to compute camera now logs error and stops from rendering --- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 2aa6073b2c8d..5de80dbb05ea 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -441,14 +441,15 @@ fn setup_target_config( // Put the camera at the position where it sees the entire image plane as defined // by the pinhole camera. - let view_from_world = macaw::IsoTransform::look_at_rh( + let Some(view_from_world) = macaw::IsoTransform::look_at_rh( pinhole .principal_point() .extend(-pinhole.focal_length_in_pixels()), pinhole.principal_point().extend(0.0), -glam::Vec3::Y, - ) - .unwrap_or(macaw::IsoTransform::IDENTITY); + ) else { + anyhow::bail!("Failed to compute camera transform for 2D view."); + }; // Cut to the portion of the currently visible ui area. let mut viewport_transformation = re_renderer::RectTransform { From 0bc8089dd9b9435089bbd10d9bf8988f27736105 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 19:03:57 +0200 Subject: [PATCH 33/37] note on non-square pixels --- crates/re_log_types/src/component_types/transform.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/re_log_types/src/component_types/transform.rs b/crates/re_log_types/src/component_types/transform.rs index e708ef671cc8..9c01d325ac54 100644 --- a/crates/re_log_types/src/component_types/transform.rs +++ b/crates/re_log_types/src/component_types/transform.rs @@ -157,7 +157,8 @@ impl Pinhole { /// [see definition of intrinsic matrix](https://en.wikipedia.org/wiki/Camera_resectioning#Intrinsic_parameters) #[inline] pub fn focal_length_in_pixels(&self) -> f32 { - // Use only the first element of the focal length vector, as we don't support non-square pixels. + // TODO(andreas): We should support anamorphic formats! https://en.wikipedia.org/wiki/Anamorphic_format + // The required changes trickle out to a few places and imply different fov in x & y direction that are not dependent on the self.image_from_cam[0][0] } From 8c71d379017175cc2417aadaa20c30000a0f289f Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 2 May 2023 19:18:07 +0200 Subject: [PATCH 34/37] better handle different x & y focal length + comment --- crates/re_log_types/src/component_types/transform.rs | 6 ++---- crates/re_viewer/src/misc/transform_cache.rs | 11 +++++++++-- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 5 ++++- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/re_log_types/src/component_types/transform.rs b/crates/re_log_types/src/component_types/transform.rs index 9c01d325ac54..f6b5fc4ece8e 100644 --- a/crates/re_log_types/src/component_types/transform.rs +++ b/crates/re_log_types/src/component_types/transform.rs @@ -156,10 +156,8 @@ impl Pinhole { /// /// [see definition of intrinsic matrix](https://en.wikipedia.org/wiki/Camera_resectioning#Intrinsic_parameters) #[inline] - pub fn focal_length_in_pixels(&self) -> f32 { - // TODO(andreas): We should support anamorphic formats! https://en.wikipedia.org/wiki/Anamorphic_format - // The required changes trickle out to a few places and imply different fov in x & y direction that are not dependent on the - self.image_from_cam[0][0] + pub fn focal_length_in_pixels(&self) -> Vec2D { + [self.image_from_cam[0][0], self.image_from_cam[1][1]].into() } /// Focal length. diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 43114d1e3825..e5c0504ab25e 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -246,10 +246,17 @@ fn transform_at( // Center the image plane and move it along z, scaling the further the image plane is. let distance = pinhole_image_plane_distance(entity_path); - let scale = distance / pinhole.focal_length_in_pixels(); + let focal_length = pinhole.focal_length_in_pixels(); + let focal_length = glam::vec2(focal_length.x(), focal_length.y()); + let scale = distance / focal_length; let translation = (-pinhole.principal_point() * scale).extend(distance); let parent_from_child = glam::Affine3A::from_translation(translation) - * glam::Affine3A::from_scale(glam::Vec3::splat(scale)); + + // We want to preserve any depth that might be on the pinhole image. + // Use harmonic mean of x/y scale for those. + * glam::Affine3A::from_scale( + scale.extend(2.0 / (1.0 / scale.x + 1.0 / scale.y)), + ); // Above calculation is nice for a certain kind of visualizing a projected image plane, // but the image plane distance is arbitrary and there might be other, better visualizations! diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 5de80dbb05ea..0417ae659e95 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -441,10 +441,13 @@ fn setup_target_config( // Put the camera at the position where it sees the entire image plane as defined // by the pinhole camera. + // TODO(andreas): Support anamorphic pinhole cameras properly. + let focal_length = pinhole.focal_length_in_pixels(); + let focal_length = 2.0 / (1.0 / focal_length.x() + 1.0 / focal_length.y()); // harmonic mean let Some(view_from_world) = macaw::IsoTransform::look_at_rh( pinhole .principal_point() - .extend(-pinhole.focal_length_in_pixels()), + .extend(-focal_length), pinhole.principal_point().extend(0.0), -glam::Vec3::Y, ) else { From f92a10e9d0eba5e68443307140514d7b73841cea Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 3 May 2023 11:15:23 +0200 Subject: [PATCH 35/37] renaming and tests around RectTransform --- .../src/draw_phases/picking_layer.rs | 4 +- crates/re_renderer/src/rect.rs | 5 + crates/re_renderer/src/transform.rs | 108 ++++++++++++++++-- crates/re_viewer/src/ui/view_spatial/ui_2d.rs | 6 +- 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/crates/re_renderer/src/draw_phases/picking_layer.rs b/crates/re_renderer/src/draw_phases/picking_layer.rs index 931f8489189e..d2466a1d7329 100644 --- a/crates/re_renderer/src/draw_phases/picking_layer.rs +++ b/crates/re_renderer/src/draw_phases/picking_layer.rs @@ -228,8 +228,8 @@ impl PickingLayerProcessor { }); let cropped_projection_from_projection = RectTransform { - from: picking_rect.into(), - to: RectF32 { + region_of_interest: picking_rect.into(), + region: RectF32 { min: glam::Vec2::ZERO, extent: screen_resolution.as_vec2(), }, diff --git a/crates/re_renderer/src/rect.rs b/crates/re_renderer/src/rect.rs index 65fcd9cbfe9a..f62d59364a2d 100644 --- a/crates/re_renderer/src/rect.rs +++ b/crates/re_renderer/src/rect.rs @@ -65,6 +65,11 @@ impl RectF32 { self.min + self.extent } + #[inline] + pub fn center(self) -> glam::Vec2 { + self.min + self.extent / 2.0 + } + #[inline] pub fn scale_extent(self, factor: f32) -> RectF32 { RectF32 { diff --git a/crates/re_renderer/src/transform.rs b/crates/re_renderer/src/transform.rs index 81fbb3838822..dfff2f7b93ef 100644 --- a/crates/re_renderer/src/transform.rs +++ b/crates/re_renderer/src/transform.rs @@ -34,30 +34,36 @@ pub fn ndc_from_pixel(pixel_coord: glam::Vec2, screen_extent: glam::UVec2) -> gl ) } +/// Defines a transformation from a rectangular region of interest into a rectangular target region. +/// +/// Transforms map the range of `region_of_interest` to the range of `region`. #[derive(Clone, Debug)] pub struct RectTransform { - pub from: RectF32, - pub to: RectF32, + pub region_of_interest: RectF32, + pub region: RectF32, } impl RectTransform { /// No-op rect transform that transforms from a unit rectangle to a unit rectangle. pub const IDENTITY: RectTransform = RectTransform { - from: RectF32::UNIT, - to: RectF32::UNIT, + region_of_interest: RectF32::UNIT, + region: RectF32::UNIT, }; /// Computes a transformation matrix that applies the rect transform to the NDC space. /// + /// This matrix is expected to be the left most transformation in the vertex transformation chain. + /// It causes the area described by `region_of_interest` to be mapped to the area described by `region`. + /// Meaning, that `region` represents the full screen of the NDC space. /// - /// Note only the relation of the rectangles in `RectTransform` is important. + /// This means that only the relation of the rectangles in `RectTransform` is important. /// Scaling or moving both rectangles by the same amount does not change the result. pub fn to_ndc_scale_and_translation(&self) -> glam::Mat4 { // It's easier to think in texcoord space, and then transform to NDC. // This texcoord rect specifies the portion of the screen that should become the entire range of the NDC screen. let texcoord_rect = RectF32 { - min: (self.from.min - self.to.min) / self.to.extent, - extent: self.from.extent / self.to.extent, + min: (self.region_of_interest.min - self.region.min) / self.region.extent, + extent: self.region_of_interest.extent / self.region.extent, }; let texcoord_rect_min = texcoord_rect.min; let texcoord_rect_max = texcoord_rect.max(); @@ -74,6 +80,92 @@ impl RectTransform { } pub fn scale(&self) -> glam::Vec2 { - self.to.extent / self.from.extent + self.region.extent / self.region_of_interest.extent + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + pub fn to_ndc_scale_and_translation() { + let region = RectF32 { + min: glam::vec2(1.0, 1.0), + extent: glam::vec2(2.0, 3.0), + }; + + // Identity + { + let rect_transform = RectTransform { + region_of_interest: region, + region, + }; + let identity = rect_transform.to_ndc_scale_and_translation(); + assert_eq!(identity, glam::Mat4::IDENTITY); + } + + // Scale + { + let scale_factor = glam::vec2(2.0, 0.25); + + let rect_transform = RectTransform { + region_of_interest: RectF32 { + // Move the roi to the middle of the region. + min: region.center() - region.extent * scale_factor * 0.5, + extent: region.extent * scale_factor, + }, + region, + }; + let scale = rect_transform.to_ndc_scale_and_translation(); + assert_eq!( + scale, + glam::Mat4::from_scale(1.0 / scale_factor.extend(1.0)) + ); + } + + // Translation + { + let translation_vec = glam::vec2(1.0, 2.0); + + let rect_transform = RectTransform { + region_of_interest: RectF32 { + min: region.min + translation_vec * region.extent, + extent: region.extent, + }, + region, + }; + let translation = rect_transform.to_ndc_scale_and_translation(); + assert_eq!( + translation, + glam::Mat4::from_translation( + glam::vec3(-translation_vec.x, translation_vec.y, 0.0) * 2.0 + ) + ); + } + + // Scale + translation + { + let scale_factor = glam::vec2(2.0, 0.25); + let translation_vec = glam::vec2(1.0, 2.0); + + let rect_transform = RectTransform { + region_of_interest: RectF32 { + // Move the roi to the middle of the region and then apply translation + min: region.center() - region.extent * scale_factor * 0.5 + + translation_vec * region.extent, + extent: region.extent * scale_factor, + }, + region, + }; + let scale_and_translation = rect_transform.to_ndc_scale_and_translation(); + assert_eq!( + scale_and_translation, + glam::Mat4::from_scale(1.0 / scale_factor.extend(1.0)) + * glam::Mat4::from_translation( + glam::vec3(-translation_vec.x, translation_vec.y, 0.0) * 2.0 + ) + ); + } } } diff --git a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs index 0417ae659e95..e2123b513994 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui_2d.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui_2d.rs @@ -456,15 +456,15 @@ fn setup_target_config( // Cut to the portion of the currently visible ui area. let mut viewport_transformation = re_renderer::RectTransform { - from: egui_rect_to_re_renderer(painter.clip_rect()), - to: egui_rect_to_re_renderer(*canvas_from_ui.from()), + region_of_interest: egui_rect_to_re_renderer(painter.clip_rect()), + region: egui_rect_to_re_renderer(*canvas_from_ui.from()), }; // The principal point might not be quite centered. // We need to account for this translation in the viewport transformation. let principal_point_offset = default_principal_point - pinhole.principal_point(); let ui_from_canvas_scale = canvas_from_ui.inverse().scale(); - viewport_transformation.from.min += + viewport_transformation.region_of_interest.min += principal_point_offset * glam::vec2(ui_from_canvas_scale.x, ui_from_canvas_scale.y); Ok({ From ec0a980cb49a72eab6ebb523f5f61ff052bd6069 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 3 May 2023 11:59:48 +0200 Subject: [PATCH 36/37] yet another workaround for https://github.com/gfx-rs/naga/issues/1743 --- crates/re_renderer/shader/point_cloud.wgsl | 14 +++++++++++++ .../re_renderer/shader/utils/sphere_quad.wgsl | 11 ---------- .../re_renderer/src/renderer/point_cloud.rs | 21 +++++++++++++------ .../src/wgpu_resources/shader_module_pool.rs | 10 ++++++++- crates/re_web_viewer_server/build.rs | 1 + 5 files changed, 39 insertions(+), 18 deletions(-) diff --git a/crates/re_renderer/shader/point_cloud.wgsl b/crates/re_renderer/shader/point_cloud.wgsl index 881b6dd05a57..3fff3e6e61b9 100644 --- a/crates/re_renderer/shader/point_cloud.wgsl +++ b/crates/re_renderer/shader/point_cloud.wgsl @@ -111,6 +111,20 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut { return out; } +// TODO(andreas): move this to sphere_quad.wgsl once https://github.com/gfx-rs/naga/issues/1743 is resolved +// point_cloud.rs has a specific workaround in place so we don't need to split vertex/fragment shader here +// +/// Computes coverage of a 2D sphere placed at `circle_center` in the fragment shader using the currently set camera. +/// +/// 2D primitives are always facing the camera - the difference to sphere_quad_coverage is that +/// perspective projection is not taken into account. +fn circle_quad_coverage(world_position: Vec3, radius: f32, circle_center: Vec3) -> f32 { + let to_center = circle_center - world_position; + let distance = length(to_center); + let distance_pixel_difference = fwidth(distance); + return smoothstep(radius + distance_pixel_difference, radius - distance_pixel_difference, distance); +} + fn coverage(world_position: Vec3, radius: f32, point_center: Vec3) -> f32 { if is_camera_orthographic() || has_any_flag(batch.flags, DRAW_AS_CIRCLES) { return circle_quad_coverage(world_position, radius, point_center); diff --git a/crates/re_renderer/shader/utils/sphere_quad.wgsl b/crates/re_renderer/shader/utils/sphere_quad.wgsl index 6416c40aec31..83fc190496f8 100644 --- a/crates/re_renderer/shader/utils/sphere_quad.wgsl +++ b/crates/re_renderer/shader/utils/sphere_quad.wgsl @@ -111,14 +111,3 @@ fn sphere_quad_coverage(world_position: Vec3, radius: f32, sphere_center: Vec3) // Note that we have signed distances to the sphere surface. return saturate(0.5 - distance_to_surface_in_pixels); } - -/// Computes coverage of a 2D sphere placed at `circle_center` in the fragment shader using the currently set camera. -/// -/// 2D primitives are always facing the camera - the difference to sphere_quad_coverage is that -/// perspective projection is not taken into account. -fn circle_quad_coverage(world_position: Vec3, radius: f32, circle_center: Vec3) -> f32 { - let to_center = circle_center - world_position; - let distance = length(to_center); - let distance_pixel_difference = fwidth(distance); - return smoothstep(radius + distance_pixel_difference, radius - distance_pixel_difference, distance); -} diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index efe9006df69e..81e1a4c198e1 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -624,17 +624,26 @@ impl Renderer for PointCloudRenderer { &pools.bind_group_layouts, ); - let shader_module = pools.shader_modules.get_or_create( - device, - resolver, - &include_shader_module!("../../shader/point_cloud.wgsl"), - ); + let shader_module_desc = include_shader_module!("../../shader/point_cloud.wgsl"); + let shader_module = + pools + .shader_modules + .get_or_create(device, resolver, &shader_module_desc); + + // HACK/WORKAROUND for https://github.com/gfx-rs/naga/issues/1743 + let mut shader_module_desc_vertex = shader_module_desc.clone(); + shader_module_desc_vertex.extra_workaround_replacements = + vec![("fwidth(".to_owned(), "f32(".to_owned())]; + let shader_module_vertex = + pools + .shader_modules + .get_or_create(device, resolver, &shader_module_desc_vertex); let render_pipeline_desc_color = RenderPipelineDesc { label: "PointCloudRenderer::render_pipeline_color".into(), pipeline_layout, vertex_entrypoint: "vs_main".into(), - vertex_handle: shader_module, + vertex_handle: shader_module_vertex, fragment_entrypoint: "fs_main".into(), fragment_handle: shader_module, vertex_buffers: smallvec![], diff --git a/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs b/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs index 9b42f87d1788..df0f4de5c715 100644 --- a/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs +++ b/crates/re_renderer/src/wgpu_resources/shader_module_pool.rs @@ -21,6 +21,7 @@ macro_rules! include_shader_module { $crate::wgpu_resources::ShaderModuleDesc { label: $crate::DebugLabel::from(stringify!($path).strip_prefix("../../shader/")), source: $crate::include_file!($path), + extra_workaround_replacements: Vec::new(), } }}; } @@ -33,6 +34,9 @@ pub struct ShaderModuleDesc { /// Path to the source code of this shader module. pub source: PathBuf, + + /// Additional text replacement workarounds that may be added on top of globally known workarounds. + pub extra_workaround_replacements: Vec<(String, String)>, } impl PartialEq for ShaderModuleDesc { @@ -46,6 +50,7 @@ impl Hash for ShaderModuleDesc { // NOTE: for a shader, the only thing that should matter is the source // code since we can have many entrypoints referring to the same file! self.source.hash(state); + self.extra_workaround_replacements.hash(state); } } @@ -62,7 +67,10 @@ impl ShaderModuleDesc { .map_err(|err| re_log::error!(err=%re_error::format(err))) .unwrap_or_default(); - for (from, to) in shader_text_workaround_replacements { + for (from, to) in shader_text_workaround_replacements + .iter() + .chain(self.extra_workaround_replacements.iter()) + { source_interpolated.contents = source_interpolated.contents.replace(from, to); } diff --git a/crates/re_web_viewer_server/build.rs b/crates/re_web_viewer_server/build.rs index 3837410ac6a2..945cab819778 100644 --- a/crates/re_web_viewer_server/build.rs +++ b/crates/re_web_viewer_server/build.rs @@ -57,6 +57,7 @@ impl<'a> Packages<'a> { // account for locally patched dependencies. rerun_if_changed(path.join("Cargo.toml").as_ref()); rerun_if_changed(path.join("**/*.rs").as_ref()); + rerun_if_changed(path.join("**/*.wgsl").as_ref()); } // Track all direct and indirect dependencies of that root package From ee59962e8832f8f21282b4ab427a673ca76fc59d Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 3 May 2023 12:04:54 +0200 Subject: [PATCH 37/37] remove h word --- crates/re_renderer/src/renderer/point_cloud.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_renderer/src/renderer/point_cloud.rs b/crates/re_renderer/src/renderer/point_cloud.rs index 81e1a4c198e1..4845541f2619 100644 --- a/crates/re_renderer/src/renderer/point_cloud.rs +++ b/crates/re_renderer/src/renderer/point_cloud.rs @@ -630,7 +630,7 @@ impl Renderer for PointCloudRenderer { .shader_modules .get_or_create(device, resolver, &shader_module_desc); - // HACK/WORKAROUND for https://github.com/gfx-rs/naga/issues/1743 + // WORKAROUND for https://github.com/gfx-rs/naga/issues/1743 let mut shader_module_desc_vertex = shader_module_desc.clone(); shader_module_desc_vertex.extra_workaround_replacements = vec![("fwidth(".to_owned(), "f32(".to_owned())];