Skip to content

Commit 1e5f933

Browse files
emilkteh-cmc
andauthored
Improve the orbit eye to always maintain an up-axis (#5193)
* Continuing the work in #3817 ### What - The orbit eye now always maintains an up axis internally (+Z by default, if there is no `ViewCoordinates`). - When a user rolls the camera (middle mouse drag), we now change the internal up-axis, which makes rolling then orbiting a much better experience as you don't end up in a weird state with no up axis. - The orbit eye center visualization now indicates the current up-axis https://github.com/rerun-io/rerun/assets/1148717/daeb1e57-8fe3-4ac9-84fc-c673bb70ad09 - Fixes #3539 - Fixes #3420 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using newly built examples: [app.rerun.io](https://app.rerun.io/pr/5193/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5193/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [app.rerun.io](https://app.rerun.io/pr/5193/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/5193) - [Docs preview](https://rerun.io/preview/c4c8821a33c3df282e31c64ca1d719358666b72a/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/c4c8821a33c3df282e31c64ca1d719358666b72a/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --------- Co-authored-by: Clement Rey <[email protected]>
1 parent 5c7dabe commit 1e5f933

File tree

5 files changed

+189
-139
lines changed

5 files changed

+189
-139
lines changed

clippy.toml

+2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ disallowed-macros = [
4141
# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
4242
disallowed-methods = [
4343
{ path = "egui_extras::TableBody::row", reason = "`row` doesn't scale. Use `rows` instead." },
44+
{ path = "glam::Vec2::normalize", reason = "normalize() can create NaNs. Use try_normalize or normalize_or_zero" },
45+
{ path = "glam::Vec3::normalize", reason = "normalize() can create NaNs. Use try_normalize or normalize_or_zero" },
4446
{ path = "sha1::Digest::new", reason = "SHA1 is cryptographically broken" },
4547
{ path = "std::env::temp_dir", reason = "Use the tempdir crate instead" },
4648
{ path = "std::panic::catch_unwind", reason = "We compile with `panic = 'abort'`" },

crates/re_space_view_spatial/src/eye.rs

+57-71
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use egui::{lerp, NumExt as _, Rect};
2-
use glam::Affine3A;
32
use macaw::{vec3, IsoTransform, Mat4, Quat, Vec3};
43

54
use re_space_view::controls::{
@@ -96,7 +95,7 @@ impl Eye {
9695
let ray_dir = self
9796
.world_from_rub_view
9897
.transform_vector3(glam::vec3(px, py, -1.0));
99-
macaw::Ray3::from_origin_dir(self.pos_in_world(), ray_dir.normalize())
98+
macaw::Ray3::from_origin_dir(self.pos_in_world(), ray_dir.normalize_or_zero())
10099
} else {
101100
// The ray originates on the camera plane, not from the camera position
102101
let ray_dir = self.world_from_rub_view.rotation().mul_vec3(glam::Vec3::Z);
@@ -159,23 +158,37 @@ pub struct OrbitEye {
159158
pub world_from_view_rot: Quat,
160159
pub fov_y: f32,
161160

162-
/// Zero = no up (3dof rotation)
163-
pub up: Vec3,
161+
/// The up-axis of the eye itself, in world-space.
162+
///
163+
/// Initially, the up-axis of the eye will be the same as the up-axis of the scene (or +Z if
164+
/// the scene has no up axis defined).
165+
/// Rolling the camera (e.g. middle-click) will permanently modify the eye's up axis, until the
166+
/// next reset.
167+
///
168+
/// A value of `Vec3::ZERO` is valid and will result in 3 degrees of freedom, although we never
169+
/// use it at the moment.
170+
pub eye_up: Vec3,
164171

165172
/// For controlling the eye with WSAD in a smooth way.
166173
pub velocity: Vec3,
167174
}
168175

169176
impl OrbitEye {
170-
const MAX_PITCH: f32 = 0.999 * 0.25 * std::f32::consts::TAU;
171-
172-
pub fn new(orbit_center: Vec3, orbit_radius: f32, world_from_view_rot: Quat, up: Vec3) -> Self {
177+
/// Avoids zentith/nadir singularity.
178+
const MAX_PITCH: f32 = 0.99 * 0.25 * std::f32::consts::TAU;
179+
180+
pub fn new(
181+
orbit_center: Vec3,
182+
orbit_radius: f32,
183+
world_from_view_rot: Quat,
184+
eye_up: Vec3,
185+
) -> Self {
173186
OrbitEye {
174187
orbit_center,
175188
orbit_radius,
176189
world_from_view_rot,
177190
fov_y: Eye::DEFAULT_FOV_Y,
178-
up,
191+
eye_up,
179192
velocity: Vec3::ZERO,
180193
}
181194
}
@@ -206,6 +219,7 @@ impl OrbitEye {
206219
self.world_from_view_rot = eye.world_from_rub_view.rotation();
207220
self.fov_y = eye.fov_y.unwrap_or(Eye::DEFAULT_FOV_Y);
208221
self.velocity = Vec3::ZERO;
222+
self.eye_up = eye.world_from_rub_view.rotation() * glam::Vec3::Y;
209223
}
210224

211225
pub fn lerp(&self, other: &Self, t: f32) -> Self {
@@ -219,54 +233,28 @@ impl OrbitEye {
219233
orbit_radius: lerp(self.orbit_radius..=other.orbit_radius, t),
220234
world_from_view_rot: self.world_from_view_rot.slerp(other.world_from_view_rot, t),
221235
fov_y: egui::lerp(self.fov_y..=other.fov_y, t),
222-
up: self.up.lerp(other.up, t).normalize_or_zero(),
236+
// A slerp would technically be nicer for eye_up, but it only really
237+
// matters if the user starts interacting half-way through the lerp,
238+
// and even then it's not a big deal.
239+
eye_up: self.eye_up.lerp(other.eye_up, t).normalize_or_zero(),
223240
velocity: self.velocity.lerp(other.velocity, t),
224241
}
225242
}
226243
}
227244

228-
/// Direction we are looking at
245+
/// World-direction we are looking at
229246
fn fwd(&self) -> Vec3 {
230-
self.world_from_view_rot * -Vec3::Z
247+
self.world_from_view_rot * -Vec3::Z // view-coordinates are RUB
231248
}
232249

233250
/// Only valid if we have an up vector.
234251
///
235252
/// `[-tau/4, +tau/4]`
236253
fn pitch(&self) -> Option<f32> {
237-
if self.up == Vec3::ZERO {
254+
if self.eye_up == Vec3::ZERO {
238255
None
239256
} else {
240-
Some(self.fwd().dot(self.up).clamp(-1.0, 1.0).asin())
241-
}
242-
}
243-
244-
fn set_fwd(&mut self, fwd: Vec3) {
245-
if let Some(pitch) = self.pitch() {
246-
let pitch = pitch.clamp(-Self::MAX_PITCH, Self::MAX_PITCH);
247-
248-
let fwd = project_onto(fwd, self.up).normalize(); // Remove pitch
249-
let right = fwd.cross(self.up).normalize();
250-
let fwd = Quat::from_axis_angle(right, pitch) * fwd; // Tilt up/down
251-
let fwd = fwd.normalize(); // Prevent drift
252-
253-
let world_from_view_rot =
254-
Quat::from_affine3(&Affine3A::look_at_rh(Vec3::ZERO, fwd, self.up).inverse());
255-
256-
if world_from_view_rot.is_finite() {
257-
self.world_from_view_rot = world_from_view_rot;
258-
}
259-
} else {
260-
self.world_from_view_rot = Quat::from_rotation_arc(-Vec3::Z, fwd);
261-
}
262-
}
263-
264-
#[allow(unused)]
265-
pub fn set_up(&mut self, up: Vec3) {
266-
self.up = up.normalize_or_zero();
267-
268-
if self.up != Vec3::ZERO {
269-
self.set_fwd(self.fwd()); // this will clamp the rotation
257+
Some(self.fwd().dot(self.eye_up).clamp(-1.0, 1.0).asin())
270258
}
271259
}
272260

@@ -278,12 +266,12 @@ impl OrbitEye {
278266
let mut did_interact = response.drag_delta().length() > 0.0;
279267

280268
if response.drag_delta().length() > drag_threshold {
281-
if response.dragged_by(ROLL_MOUSE)
269+
let roll = response.dragged_by(ROLL_MOUSE)
282270
|| (response.dragged_by(ROLL_MOUSE_ALT)
283271
&& response
284272
.ctx
285-
.input(|i| i.modifiers.contains(ROLL_MOUSE_MODIFIER)))
286-
{
273+
.input(|i| i.modifiers.contains(ROLL_MOUSE_MODIFIER)));
274+
if roll {
287275
if let Some(pointer_pos) = response.ctx.pointer_latest_pos() {
288276
self.roll(&response.rect, pointer_pos, response.drag_delta());
289277
}
@@ -388,31 +376,26 @@ impl OrbitEye {
388376
let sensitivity = 0.004; // radians-per-point. TODO(emilk): take fov_y and canvas size into account
389377
let delta = sensitivity * delta;
390378

391-
if self.up == Vec3::ZERO {
392-
// 3-dof rotation
393-
let rot_delta = Quat::from_rotation_y(-delta.x) * Quat::from_rotation_x(-delta.y);
394-
self.world_from_view_rot *= rot_delta;
395-
} else {
379+
if let Some(old_pitch) = self.pitch() {
396380
// 2-dof rotation
397-
let fwd = Quat::from_axis_angle(self.up, -delta.x) * self.fwd();
398-
let fwd = fwd.normalize(); // Prevent drift
399381

400-
let pitch = self.pitch().unwrap() - delta.y;
401-
let pitch = pitch.clamp(-Self::MAX_PITCH, Self::MAX_PITCH);
382+
// Apply change in heading:
383+
self.world_from_view_rot =
384+
Quat::from_axis_angle(self.eye_up, -delta.x) * self.world_from_view_rot;
402385

403-
let fwd = project_onto(fwd, self.up).normalize(); // Remove pitch
404-
let right = fwd.cross(self.up).normalize();
405-
let fwd = Quat::from_axis_angle(right, pitch) * fwd; // Tilt up/down
406-
let fwd = fwd.normalize(); // Prevent drift
386+
// We need to clamp pitch to avoid nadir/zenith singularity:
387+
let new_pitch = (old_pitch - delta.y).clamp(-Self::MAX_PITCH, Self::MAX_PITCH);
388+
let pitch_delta = new_pitch - old_pitch;
407389

408-
let new_world_from_view_rot =
409-
Quat::from_affine3(&Affine3A::look_at_rh(Vec3::ZERO, fwd, self.up).inverse());
390+
// Apply change in pitch:
391+
self.world_from_view_rot *= Quat::from_rotation_x(pitch_delta);
410392

411-
if new_world_from_view_rot.is_finite() {
412-
self.world_from_view_rot = new_world_from_view_rot;
413-
} else {
414-
re_log::debug_once!("Failed to rotate camera: got non-finites");
415-
}
393+
// Avoid numeric drift:
394+
self.world_from_view_rot = self.world_from_view_rot.normalize();
395+
} else {
396+
// no up-axis -> no pitch -> 3-dof rotation
397+
let rot_delta = Quat::from_rotation_y(-delta.x) * Quat::from_rotation_x(-delta.y);
398+
self.world_from_view_rot *= rot_delta;
416399
}
417400
}
418401

@@ -422,9 +405,17 @@ impl OrbitEye {
422405
let rel = pointer_pos - rect.center();
423406
let delta_angle = delta.rot90().dot(rel) / rel.length_sq();
424407
let rot_delta = Quat::from_rotation_z(delta_angle);
408+
409+
let up_in_view = self.world_from_view_rot.inverse() * self.eye_up;
410+
425411
self.world_from_view_rot *= rot_delta;
426412

427-
self.up = Vec3::ZERO; // forget about this until user resets the eye
413+
// Permanently change our up-axis, at least until the user resets the view:
414+
self.eye_up = self.world_from_view_rot * up_in_view;
415+
416+
// Prevent numeric drift:
417+
self.world_from_view_rot = self.world_from_view_rot.normalize();
418+
self.eye_up = self.eye_up.normalize_or_zero();
428419
}
429420

430421
/// Translate based on a certain number of pixel delta.
@@ -439,8 +430,3 @@ impl OrbitEye {
439430
self.orbit_center += translate;
440431
}
441432
}
442-
443-
/// e.g. up is `[0,0,1]`, we return things like `[x,y,0]`
444-
fn project_onto(v: Vec3, up: Vec3) -> Vec3 {
445-
v - up * v.dot(up)
446-
}

crates/re_space_view_spatial/src/picking.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,11 @@ fn picking_textured_rects(context: &PickingContext, images: &[ViewerImage]) -> V
247247

248248
for image in images {
249249
let rect = &image.textured_rect;
250-
let rect_plane = macaw::Plane3::from_normal_point(
251-
rect.extent_u.cross(rect.extent_v).normalize(),
252-
rect.top_left_corner_position,
253-
);
250+
let Some(normal) = rect.extent_u.cross(rect.extent_v).try_normalize() else {
251+
continue; // extent_u and extent_v are parallel. Shouldn't happen.
252+
};
253+
254+
let rect_plane = macaw::Plane3::from_normal_point(normal, rect.top_left_corner_position);
254255

255256
// TODO(andreas): Interaction radius is currently ignored for rects.
256257
let (intersect, t) =

crates/re_space_view_spatial/src/ui.rs

+35-13
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use re_entity_db::EntityPath;
77
use re_format::format_f32;
88
use re_renderer::OutlineConfig;
99
use re_space_view::ScreenshotMode;
10-
use re_types::components::{DepthMeter, InstanceKey, TensorData};
10+
use re_types::components::{DepthMeter, InstanceKey, TensorData, ViewCoordinates};
1111
use re_types::tensor_data::TensorDataMeaning;
1212
use re_viewer_context::{
1313
HoverHighlight, Item, SelectedSpaceContext, SelectionHighlight, SpaceViewHighlights,
@@ -97,10 +97,10 @@ impl SpatialSpaceViewState {
9797
) {
9898
let re_ui = ctx.re_ui;
9999

100-
let view_coordinates = ctx
100+
let scene_view_coordinates = ctx
101101
.entity_db
102102
.store()
103-
.query_latest_component(space_origin, &ctx.current_query())
103+
.query_latest_component::<ViewCoordinates>(space_origin, &ctx.current_query())
104104
.map(|c| c.value);
105105

106106
ctx.re_ui.selection_grid(ui, "spatial_settings_ui")
@@ -145,7 +145,7 @@ impl SpatialSpaceViewState {
145145
.clicked()
146146
{
147147
self.bounding_boxes.accumulated = self.bounding_boxes.current;
148-
self.state_3d.reset_camera(&self.bounding_boxes.accumulated, &view_coordinates);
148+
self.state_3d.reset_camera(&self.bounding_boxes.accumulated, scene_view_coordinates);
149149
}
150150
let mut spin = self.state_3d.spin();
151151
if re_ui.checkbox(ui, &mut spin, "Spin")
@@ -160,19 +160,21 @@ impl SpatialSpaceViewState {
160160
ctx.re_ui.grid_left_hand_label(ui, "Coordinates")
161161
.on_hover_text("The world coordinate system used for this view");
162162
ui.vertical(|ui|{
163-
let up_description = if let Some(up) = view_coordinates.and_then(|v| v.up()) {
164-
format!("Up is {up}")
163+
let up_description = if let Some(scene_up) = scene_view_coordinates.and_then(|vc| vc.up()) {
164+
format!("Scene up is {scene_up}")
165165
} else {
166-
"Up is unspecified".to_owned()
166+
"Scene up is unspecified".to_owned()
167167
};
168168
ui.label(up_description).on_hover_ui(|ui| {
169-
ui.horizontal(|ui| {
170-
ui.spacing_mut().item_spacing.x = 0.0;
171-
ui.label("Set with ");
172-
ui.code("rerun.log_view_coordinates");
173-
ui.label(".");
174-
});
169+
re_ui::markdown_ui(ui, egui::Id::new("view_coordinates_tooltip"), "Set with `rerun.ViewCoordinates`.");
175170
});
171+
172+
if let Some(eye) = &self.state_3d.orbit_eye {
173+
if eye.eye_up != glam::Vec3::ZERO {
174+
ui.label(format!("Current camera-eye up-axis is {}", format_vector(eye.eye_up)));
175+
}
176+
}
177+
176178
re_ui.checkbox(ui, &mut self.state_3d.show_axes, "Show origin axes").on_hover_text("Show X-Y-Z axes");
177179
re_ui.checkbox(ui, &mut self.state_3d.show_bbox, "Show bounding box").on_hover_text("Show the current scene bounding box");
178180
re_ui.checkbox(ui, &mut self.state_3d.show_accumulated_bbox, "Show accumulated bounding box").on_hover_text("Show bounding box accumulated over all rendered frames");
@@ -769,3 +771,23 @@ fn hit_ui(ui: &mut egui::Ui, hit: &crate::picking::PickingRayHit) {
769771
ui.label(format!("Hover position: [{x:.5}, {y:.5}, {z:.5}]"));
770772
}
771773
}
774+
775+
fn format_vector(v: glam::Vec3) -> String {
776+
use glam::Vec3;
777+
778+
if v == Vec3::X {
779+
"+X".to_owned()
780+
} else if v == -Vec3::X {
781+
"-X".to_owned()
782+
} else if v == Vec3::Y {
783+
"+Y".to_owned()
784+
} else if v == -Vec3::Y {
785+
"-Y".to_owned()
786+
} else if v == Vec3::Z {
787+
"+Z".to_owned()
788+
} else if v == -Vec3::Z {
789+
"-Z".to_owned()
790+
} else {
791+
format!("[{:.02}, {:.02}, {:.02}]", v.x, v.y, v.z)
792+
}
793+
}

0 commit comments

Comments
 (0)