From 325c029d067312059dad8d28a958dd3ce4013e2b Mon Sep 17 00:00:00 2001 From: Bar Borer Date: Tue, 3 Nov 2026 04:59:16 +0200 Subject: [PATCH] Viewer: the Rotation tool measures its angle on screen about the anchor (#2870) One comp-space path: the pivot is the anchor where it shows and a flipped parent reverses the turn, so the separate parent-space frame and the moved parent_to_comp go. Under uneven or skewed parents the layer's edge tracks the pointer slightly less exactly. The empty-layer marker goes back to main's. Co-Authored-By: Claude Opus 5.5 --- crates/render/src/eval.rs | 9 +--- crates/ui-egui/src/panels/viewer.rs | 29 +++++++------ crates/ui-egui/src/panels/viewer/geometry.rs | 43 ++------------------ crates/ui-egui/src/panels/viewer/gesture.rs | 7 ++-- crates/ui-egui/src/panels/viewer/tests.rs | 16 +------- crates/ui-egui/src/panels/viewer_overlays.rs | 13 +++++- crates/ui-egui/tests/it/ui_viewer.rs | 22 ---------- 7 files changed, 38 insertions(+), 101 deletions(-) diff --git a/crates/render/src/eval.rs b/crates/render/src/eval.rs index 1aaf02d76..81fe45d6c 100644 --- a/crates/render/src/eval.rs +++ b/crates/render/src/eval.rs @@ -214,14 +214,7 @@ impl<'a> EvalCtx<'a> { /// Layer space → comp (world) space, including parents. pub fn world_matrix(&self, layer: &Layer) -> Mat4 { - self.parent_matrix(layer) * self.local_matrix_par(layer, self.par_ratio(layer)) - } - - /// The space a layer's Position and Rotation live in → comp (world) space: its parents' - /// transforms, without their pixel aspect, which children don't inherit (identity without a - /// parent). - pub fn parent_matrix(&self, layer: &Layer) -> Mat4 { - let mut m = Mat4::IDENTITY; + let mut m = self.local_matrix_par(layer, self.par_ratio(layer)); let mut cur = layer.parent; let mut guard = 0; while let Some(pid) = cur { diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 52dd47567..4fdac5ae4 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -565,8 +565,10 @@ fn phase_layers(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, v: continue; } // Cameras, lights, empty layers: just the anchor. - if let Some(c) = anchor_in_comp(ectx, l) { - painter.circle_stroke(map.to_screen(c), 6.0, Stroke::new(1.0, col)); + if let Some(tr) = l.transform() { + let pos = ectx.v3(l, tr, "position", [0.0; 3]); + let s = map.to_screen([pos[0], pos[1]]); + painter.circle_stroke(s, 6.0, Stroke::new(1.0, col)); } continue; }; @@ -1130,9 +1132,10 @@ fn phase_dragstart(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, .into_iter() .filter_map(|o| Some((o.id, ectx.f(o, o.transform()?, "rotation", 0.0), rotation_sign(ectx, o) * sign))) .collect(); - let (c2p, pivot) = layer.as_ref().and_then(|l| rotate_frame(ectx, l)).unwrap_or((Mat3::IDENTITY, cpt)); + let pivot = layer.as_ref().and_then(|l| anchor_in_comp(ectx, l)).unwrap_or(cpt); let rot = layer.as_ref().and_then(|l| l.transform().map(|tr| ectx.f(l, tr, "rotation", 0.0))).unwrap_or(0.0); - Gesture::Rotate { layer: l, c2p, pivot, last_angle: pointer_angle(&c2p, pivot, cpt), turned: 0.0, start_rot: rot, others } + let last_angle = (cpt[1] - pivot[1]).atan2(cpt[0] - pivot[0]).to_degrees(); + Gesture::Rotate { layer: l, pivot, sign, last_angle, turned: 0.0, start_rot: rot, others } }), Tool::PanBehind => pick(app, ectx, cpt, false).and_then(|l| { let layer = comp.layer(l)?; @@ -1390,17 +1393,17 @@ fn phase_drag(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, resp: let _ = app.session.execute("prop.set", json!({"layer": lid.0, "path": "transform/scale", "value": v, "merge": merge})); } } - Gesture::Rotate { layer, c2p, pivot, last_angle, turned, start_rot, others } => { + Gesture::Rotate { layer, pivot, sign, last_angle, turned, start_rot, others } => { // Count the turn incrementally: the absolute angle wraps at ±180°, so writing it // straight would jump a revolution back each time the pointer went round. - let ang = pointer_angle(&c2p, pivot, cpt); - let turned = turned + turn_delta(last_angle, ang); - ui.data_mut(|dd| dd.insert_temp(gid, Gesture::Rotate { layer, c2p, pivot, last_angle: ang, turned, start_rot, others: others.clone() })); + let ang = (cpt[1] - pivot[1]).atan2(cpt[0] - pivot[0]).to_degrees(); + let turned = turned + sign * turn_delta(last_angle, ang); + ui.data_mut(|dd| dd.insert_temp(gid, Gesture::Rotate { layer, pivot, sign, last_angle: ang, turned, start_rot, others: others.clone() })); // Shift snaps the accumulated value (so 405° snaps to 1x +45°, not to 45°). let r = rotate_value(start_rot, turned, mods.shift); let _ = app.session.execute("prop.set", json!({"layer": layer.0, "path": "transform/rotation", "value": r, "merge": merge})); - for (lid, r0, sign) in others { - let v = r0 + sign * (r - start_rot); + for (lid, r0, other) in others { + let v = r0 + other * (r - start_rot); let _ = app.session.execute("prop.set", json!({"layer": lid.0, "path": "transform/rotation", "value": v, "merge": merge})); } } @@ -1893,12 +1896,12 @@ mod view; use frame::{empty_state, rect_of, show_frame}; pub(crate) use frame::{extended_region, minify_factor, snap_to_pixels, styled_line, zoom_sampler, zoom_texture_options}; use geometry::{ - anchor_in_comp, cam_state, drag_ratio, handle_axes, handle_cursor, handle_point, one_axis, pointer_angle, project_segment, quad_intersects_box, - rotate_frame, rotate_value, rotation_sign, selected_others, snap_handle, turn_delta, + anchor_in_comp, cam_state, drag_ratio, handle_axes, handle_cursor, handle_point, one_axis, project_segment, quad_intersects_box, rotate_value, + rotation_sign, selected_others, snap_handle, turn_delta, }; #[cfg(test)] use geometry::{box_corners, project_clipped}; -pub(crate) use geometry::{from_effect_space, fx2c, l2c, layer_outline, layer_quad, parent_to_comp}; +pub(crate) use geometry::{from_effect_space, fx2c, l2c, layer_outline, layer_quad}; use gesture::{Gesture, HANDLE, ROTO_HINT, pen_id, roi_drag, roi_handles, vertex_grab_radius}; use shapes::{create_shape, draw_shape_ghost, shape_drag}; pub(crate) use shapes::{layer_at, layer_at_including_locked, layers_at, pick, selectable_layers}; diff --git a/crates/ui-egui/src/panels/viewer/geometry.rs b/crates/ui-egui/src/panels/viewer/geometry.rs index 39809e247..144047f32 100644 --- a/crates/ui-egui/src/panels/viewer/geometry.rs +++ b/crates/ui-egui/src/panels/viewer/geometry.rs @@ -1,3 +1,4 @@ +use super::ov::parent_to_comp; use super::{CameraState, Comp, EffectcraftApp, EvalCtx, Layer, LayerId, Mat3, Pos2, VIEW_CAM, gv2, three_d, vec2}; /// The camera the viewer shows (view camera or the comp's active camera). @@ -120,12 +121,6 @@ pub(crate) fn layer_outline(ctx: &EvalCtx, layer: &Layer) -> Vec<[f64; 2]> { project_clipped(&l2c(ctx, layer).0, &box_corners(b)) } -/// Parent space → comp matrix of a 2D layer at the context time. -pub(crate) fn parent_to_comp(ctx: &EvalCtx, layer: &Layer) -> Mat3 { - let w = ctx.parent_matrix(layer).0; - Mat3([[w[0][0], w[0][1], w[0][3]], [w[1][0], w[1][1], w[1][3]], [0.0, 0.0, 1.0]]) -} - /// Where a layer's anchor point shows in the viewer (comp pixels), through the current 3D view for /// a 3D layer. None without a transform, or when it is at or behind the camera. pub(super) fn anchor_in_comp(ctx: &EvalCtx, layer: &Layer) -> Option<[f64; 2]> { @@ -138,40 +133,10 @@ pub(super) fn anchor_in_comp(ctx: &EvalCtx, layer: &Layer) -> Option<[f64; 2]> { Some([p.x, p.y]) } -/// −1 when a layer's Rotation turns it counter-clockwise on screen: its parent is flipped or, for -/// a 3D layer, the view sees its parent's XY plane from behind. Otherwise 1. +/// −1 when a layer's Rotation turns it counter-clockwise on screen, its parent being flipped. +/// Otherwise 1. pub(super) fn rotation_sign(ctx: &EvalCtx, layer: &Layer) -> f64 { - let plane = match layer.transform() { - Some(tr) if layer.is_3d() => { - let at = effectcraft_engine::geom::Mat4::translate(ctx.position(layer, tr).into()); - (cam_state(ctx).projection(ctx.comp.width as f64, ctx.comp.height as f64) * ctx.parent_matrix(layer) * at).plane_to_mat3() - } - _ => parent_to_comp(ctx, layer), - }; - if plane.determinant() < 0.0 { -1.0 } else { 1.0 } -} - -/// The space the Rotation tool measures the pointer's angle in: comp → that space, and the layer's -/// pivot there. A 2D layer turns in parent space about its Position, so it follows the pointer -/// through any parent (a flip or skew needs no special case). A 3D layer, or a parent that collapses -/// the plane, measures on screen about the anchor, mirrored when the turn is reversed. -pub(super) fn rotate_frame(ctx: &EvalCtx, layer: &Layer) -> Option<(Mat3, [f64; 2])> { - if !layer.is_3d() - && let Some(c2p) = parent_to_comp(ctx, layer).inverse() - { - let p = ctx.position(layer, layer.transform()?); - return Some((c2p, [p[0], p[1]])); - } - let a = anchor_in_comp(ctx, layer)?; - let c2p = Mat3::scale(gv2(rotation_sign(ctx, layer), 1.0)); - let p = c2p.apply(gv2(a[0], a[1])); - Some((c2p, [p.x, p.y])) -} - -/// The pointer's angle (degrees, as atan2 gives it) about `pivot`, in the space of [`rotate_frame`]. -pub(super) fn pointer_angle(c2p: &Mat3, pivot: [f64; 2], cpt: [f64; 2]) -> f64 { - let p = c2p.apply(gv2(cpt[0], cpt[1])); - (p.y - pivot[1]).atan2(p.x - pivot[0]).to_degrees() + if parent_to_comp(ctx, layer).determinant() < 0.0 { -1.0 } else { 1.0 } } /// Which axes a bounding-box handle scales: corners (0–3) both; the top and bottom edges (4, 6) diff --git a/crates/ui-egui/src/panels/viewer/gesture.rs b/crates/ui-egui/src/panels/viewer/gesture.rs index 797828618..72ef22a84 100644 --- a/crates/ui-egui/src/panels/viewer/gesture.rs +++ b/crates/ui-egui/src/panels/viewer/gesture.rs @@ -34,10 +34,11 @@ pub(super) enum Gesture { }, Rotate { layer: LayerId, - /// Comp → the space the pointer's angle is measured in, and the pivot there (see - /// `rotate_frame`): comp pixels, not screen points, so zooming mid-drag doesn't turn it. - c2p: Mat3, + /// The anchor where it shows, in comp pixels, not screen points, so zooming mid-drag + /// doesn't turn it. pivot: [f64; 2], + /// −1 when the layer's value runs opposite to the pointer (see `rotation_sign`). + sign: f64, /// The pointer's angle about the pivot on the last frame (degrees, as atan2 gives it). last_angle: f64, /// The turn so far, summed frame by frame so it keeps counting past a full revolution in diff --git a/crates/ui-egui/src/panels/viewer/tests.rs b/crates/ui-egui/src/panels/viewer/tests.rs index 591e02f86..9cc8afc4c 100644 --- a/crates/ui-egui/src/panels/viewer/tests.rs +++ b/crates/ui-egui/src/panels/viewer/tests.rs @@ -160,21 +160,7 @@ fn with_ctx(s: &effectcraft_engine::Session, f: impl FnOnce(&EvalCtx) -> T) - f(&EvalCtx::new(&s.project, comp.id, comp, Tick::ZERO)) } -/// #2870: a parent scaled to nothing has no inverse: the Rotation tool measures on screen about -/// where the anchor collapsed to, with no panic and no NaN. -#[test] -fn rotate_frame_falls_back_on_screen_when_the_parent_collapses() { - let (s, c) = parented_session(json!({"position": [400.0, 200.0], "scale": [0.0, 100.0]})); - with_ctx(&s, |ctx| { - let l = ctx.comp.layer(LayerId(c)).unwrap(); - let (c2p, pivot) = rotate_frame(ctx, l).unwrap(); - assert_eq!(c2p, Mat3::IDENTITY); - assert_eq!(pivot, [400.0, 210.0]); - assert_eq!(pointer_angle(&c2p, pivot, [400.0, 310.0]), 90.0); - }); -} - -/// #2870: the marker and pivot of a child follow its parent, through the camera for a 3D layer +/// #2870: the pivot of a child follows its parent, through the camera for a 3D layer /// and none behind it. #[test] fn anchor_in_comp_follows_the_parent_and_the_camera() { diff --git a/crates/ui-egui/src/panels/viewer_overlays.rs b/crates/ui-egui/src/panels/viewer_overlays.rs index ba16abcc8..00c07425d 100644 --- a/crates/ui-egui/src/panels/viewer_overlays.rs +++ b/crates/ui-egui/src/panels/viewer_overlays.rs @@ -369,6 +369,17 @@ pub(crate) fn motion_frame_at(frames: &[PathFrame], hits: &[KeyHit], pos: Pos2) (!hits.iter().any(|h| h.tangent.is_none() && h.layer == near.layer && h.prop == near.prop && h.time == near.time)).then_some(*near) } +/// Parent space → comp matrix of a 2D layer at the context time. +pub(crate) fn parent_to_comp(ectx: &EvalCtx, l: &Layer) -> Mat3 { + match l.parent.and_then(|p| ectx.comp.layer(p)) { + Some(p) => { + let w = ectx.world_matrix(p).0; + Mat3([[w[0][0], w[0][1], w[0][3]], [w[1][0], w[1][1], w[1][3]], [0.0, 0.0, 1.0]]) + } + None => Mat3::IDENTITY, + } +} + /// Draw the position motion path of a selected 2D layer: frame dots along the path, keyframe /// squares (filled when selected) and spatial Bezier tangent handles. Works through parents. #[allow(clippy::too_many_arguments)] @@ -399,7 +410,7 @@ pub(crate) fn motion_path( Some((x, y)) => separated_value(x, y, t), None => pos.value_at(t).as_vec3(), }; - let p2c = super::viewer::parent_to_comp(ectx, l); + let p2c = parent_to_comp(ectx, l); let to_scr = |v: [f64; 3]| { let c = p2c.apply(gv2(v[0], v[1])); map.to_screen([c.x, c.y]) diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index cfd44dc1a..71ea2ed79 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -2962,28 +2962,6 @@ fn rotate_tool_turns_a_child_of_a_rotated_parent_smoothly() { assert_turns_by(&rotate_round(&mut h, [380.0, 60.0], &[child]), 0, 10.0); } -/// #2870: under a parent scaled unevenly the angle is the one in parent space, so the layer's edge -/// stays under the pointer, which covers uneven degrees per frame. -#[test] -fn rotate_tool_measures_in_parent_space_under_a_stretched_parent() { - let (mut h, _, child) = rotate_child_harness(json!({"scale": [200.0, 100.0]})); - let seen = rotate_round(&mut h, [80.0, 120.0], &[child]); - let mut turned = 0.0_f64; - let mut last = 0.0_f64; - let want: Vec = (0..=40) - .map(|i| { - let a = (i as f64 * 10.0).to_radians(); - let ang = a.sin().atan2(a.cos() / 2.0).to_degrees(); - turned += (ang - last + 180.0).rem_euclid(360.0) - 180.0; - last = ang; - turned - }) - .collect(); - let end = seen.last().unwrap()[0]; - assert!((end - want[40]).abs() < 1.0, "the layer follows the pointer in parent space: {end}, want {}", want[40]); - assert!(seen.windows(2).all(|w| (w[1][0] - w[0][0]).abs() < 30.0), "no jump: {seen:?}"); -} - /// #2870: a parent's pixel aspect isn't inherited by its child, so it doesn't move the pivot either. #[test] fn rotate_tool_ignores_a_parents_pixel_aspect() {