From 632ab4a292eb31559975b439ef66459effb8aaf2 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Fri, 16 Oct 2026 12:03:01 +0530 Subject: [PATCH 1/2] fix(viewer): drag a 3D layer along a gizmo axis (#1618) The 3D gizmo's X/Y/Z arrows were drawn but not interactive, so pushing a layer in depth meant typing Z Position by hand. With the Selection tool, dragging an arrow now moves the layer along that axis only (no snapping), as one undo step, reusing the viewer's Move gesture. An axis pointing at the camera (Z in the default front view) projected to a dot; it is now drawn as a short diagonal stub so it can be grabbed. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/viewer.rs | 74 +++++++++++++++++++++++++---- crates/ui-egui/tests/ui_3d.rs | 54 +++++++++++++++++++++ 2 files changed, 118 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index f4bb5a88..008a29c7 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -976,6 +976,8 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let selected = app.session.state.selected_layers.clone(); // Bounding-box handles: (layer, handle, screen point, its resize cursor). let mut handle_hits: Vec<(LayerId, usize, Pos2, egui::CursorIcon)> = Vec::new(); + // 3D gizmo arrows: (layer, grabbable screen segment, position change per comp px in x and y). + let mut axis_hits: Vec<(LayerId, [Pos2; 2], [f64; 3], [f64; 3])> = Vec::new(); let mut vertex_hits: Vec = Vec::new(); let mut vertex_marks: Vec = Vec::new(); let mut key_hits: Vec = Vec::new(); @@ -1068,14 +1070,29 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let pc = cam_state(&ectx).projection(cw as f64, ch as f64) * w; let a3 = effectcraft_engine::geom::vec3(a[0], a[1], a[2]); let len = 60.0 / zoom as f64; - for (axis, col) in [ - (effectcraft_engine::geom::vec3(len, 0.0, 0.0), Color32::from_rgb(0xe0, 0x50, 0x50)), - (effectcraft_engine::geom::vec3(0.0, len, 0.0), Color32::from_rgb(0x60, 0xd0, 0x60)), - (effectcraft_engine::geom::vec3(0.0, 0.0, len), Color32::from_rgb(0x50, 0x8c, 0xf0)), + let pinv = parent_world_inverse(&ectx, l); + for (axis, col, name) in [ + (effectcraft_engine::geom::vec3(len, 0.0, 0.0), Color32::from_rgb(0xe0, 0x50, 0x50), "x"), + (effectcraft_engine::geom::vec3(0.0, len, 0.0), Color32::from_rgb(0x60, 0xd0, 0x60), "y"), + (effectcraft_engine::geom::vec3(0.0, 0.0, len), Color32::from_rgb(0x50, 0x8c, 0xf0), "z"), ] { // (clipped at the camera's near plane, #572) - if let Some([o, e]) = project_segment(&pc, a3, a3 + axis) { - painter.arrow(map.to_screen(o), map.to_screen(e) - map.to_screen(o), Stroke::new(2.0, col)); + let Some([o, e]) = project_segment(&pc, a3, a3 + axis) else { continue }; + // An axis pointing at the camera (Z in the default front view) projects to + // a dot: draw it as a short diagonal stub so it can still be grabbed. + let mut s = [e[0] - o[0], e[1] - o[1]]; + if (s[0] * s[0] + s[1] * s[1]).sqrt() * (zoom as f64) < 12.0 { + let k = 24.0 / zoom as f64; + s = [k, -k]; + } + let (so, se) = (map.to_screen(o), map.to_screen([o[0] + s[0], o[1] + s[1]])); + painter.arrow(so, se - so, Stroke::new(2.0, col)); + if !l.switches.locked { + let dir = pinv.apply_vec(w.apply_vec(axis)); + let (ax, ay) = axis_drag([dir.x, dir.y, dir.z], s); + // The inner part stays on the anchor point, which drags the layer freely. + axis_hits.push((l.id, [so + (se - so) * 0.35, se], ax, ay)); + app.auto.add(&format!("viewer.gizmo.{}.{name}", l.id.0), Rect::from_center_size(se, vec2(12.0, 12.0)), "3D axis"); } } } @@ -1342,6 +1359,8 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { egui::CursorIcon::PointingHand } else if let Some((.., cursor)) = handle_hits.iter().find(|(_, _, h, _)| h.distance(hp) < HANDLE) { *cursor + } else if axis_at(&axis_hits, hp).is_some() { + egui::CursorIcon::Move } else { egui::CursorIcon::Default } @@ -1549,6 +1568,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { .collect(), }) }) + } else if let Some((lid, _, ax, ay)) = axis_at(&axis_hits, press) { + // A gizmo arrow: move along that axis only, without snapping. + let p0 = comp.layer(lid).and_then(|l| Some(ectx.v3(l, l.transform()?, "position", [0.0; 3]))); + p0.map(|p0| Gesture::Move { layers: vec![(lid, p0, ax, ay)], start: cpt, snap_src: None }) } else if let Some(l) = pick(app, &ectx, cpt, mods.shift) { let layers: Vec<(LayerId, [f64; 3], [f64; 3], [f64; 3])> = app .session @@ -2033,6 +2056,31 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } } +/// The position change per comp pixel of drag (x, y) that moves a layer by `dir` when the +/// pointer covers the axis' on-screen vector `s` (comp px); drag across the axis is ignored. +fn axis_drag(dir: [f64; 3], s: [f64; 2]) -> ([f64; 3], [f64; 3]) { + let n = (s[0] * s[0] + s[1] * s[1]).max(1e-12); + (dir.map(|v| v * s[0] / n), dir.map(|v| v * s[1] / n)) +} + +/// The gizmo arrow under screen point `p`. +fn axis_at(hits: &[(LayerId, [Pos2; 2], [f64; 3], [f64; 3])], p: Pos2) -> Option<(LayerId, [Pos2; 2], [f64; 3], [f64; 3])> { + let dist = |[a, b]: [Pos2; 2]| { + let ab = b - a; + let t = ((p - a).dot(ab) / ab.length_sq().max(1e-6)).clamp(0.0, 1.0); + p.distance(a + ab * t) + }; + hits.iter().filter(|h| dist(h.1) < 6.0).min_by(|a, b| dist(a.1).total_cmp(&dist(b.1))).copied() +} + +/// World to parent space for a 3D layer (identity without a parent). +fn parent_world_inverse(ctx: &EvalCtx, l: &Layer) -> effectcraft_engine::geom::Mat4 { + match l.parent.and_then(|p| ctx.comp.layer(p)) { + Some(p) => ctx.world_matrix(p).inverse().unwrap_or_default(), + None => effectcraft_engine::geom::Mat4::IDENTITY, + } +} + /// Parent-space position change per comp pixel of drag (x and y) for a layer: 2D layers map /// through the parent transform; 3D layers move in the view plane at their depth. fn drag_axes(ctx: &EvalCtx, l: &Layer) -> ([f64; 3], [f64; 3]) { @@ -2046,10 +2094,7 @@ fn drag_axes(ctx: &EvalCtx, l: &Layer) -> ([f64; 3], [f64; 3]) { let anchor = l.transform().map(|tr| ctx.v3(l, tr, "anchor", [0.0; 3])).unwrap_or([0.0; 3]); let wp = ctx.world_matrix(l).apply(anchor.into()); let k = if cam.ortho { 1.0 / cam.zoom.max(1e-9) } else { cam.depth(wp).max(1.0) / cam.zoom.max(1e-9) }; - let pinv = match l.parent.and_then(|p| ctx.comp.layer(p)) { - Some(p) => ctx.world_matrix(p).inverse().unwrap_or_default(), - None => effectcraft_engine::geom::Mat4::IDENTITY, - }; + let pinv = parent_world_inverse(ctx, l); let a = pinv.apply_vec(cam.right() * k); let b = pinv.apply_vec(cam.down() * k); ([a.x, a.y, a.z], [b.x, b.y, b.z]) @@ -2792,6 +2837,15 @@ mod tests { assert!(project_clipped(&m, &box_corners([-50.0, -400.0, 50.0, -200.0])).is_empty(), "wholly behind"); } + #[test] + fn axis_drag_follows_the_arrow_and_ignores_drag_across_it() { + let (ax, ay) = axis_drag([0.0, 0.0, 60.0], [24.0, -24.0]); + let at = |d: [f64; 2]| [0, 1, 2].map(|i| ax[i] * d[0] + ay[i] * d[1]); + assert_eq!(at([24.0, -24.0]), [0.0, 0.0, 60.0]); + assert_eq!(at([-12.0, 12.0]), [0.0, 0.0, -30.0]); + assert_eq!(at([10.0, 10.0]), [0.0, 0.0, 0.0]); + } + #[test] fn gizmo_segments_clip_at_the_near_plane() { // w = 100 + z. diff --git a/crates/ui-egui/tests/ui_3d.rs b/crates/ui-egui/tests/ui_3d.rs index 876035a9..2ba5265a 100644 --- a/crates/ui-egui/tests/ui_3d.rs +++ b/crates/ui-egui/tests/ui_3d.rs @@ -265,3 +265,57 @@ fn each_layout_pane_chooses_its_own_view() { settle(&mut h); assert!(present(&h, "activeCamera") && !has_camera(&h, "activeCamera")); } + +fn drag(h: &mut Harness<'_, EffectcraftApp>, from: egui::Pos2, to: egui::Pos2) { + h.event(egui::Event::PointerMoved(from)); + h.step(); + h.event(egui::Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.step(); + for k in 1..=8 { + h.event(egui::Event::PointerMoved(from + (to - from) * (k as f32 / 8.0))); + h.step(); + } + h.event(egui::Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(3); +} + +/// #1618: the gizmo's arrows drag a 3D layer along one axis. Z, which points at the default +/// camera, still has a grabbable arrow and pushes the layer in depth, as one undo step. +#[test] +fn gizmo_arrows_move_a_3d_layer_along_one_axis() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 640, "height": 360, "duration": 2})).unwrap(); + let solid = s.execute("layer.newSolid", json!({"color": "#ff0000", "width": 200, "height": 200})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.setSwitch", json!({"layer": solid, "switch": "threeD", "value": true})).unwrap(); + s.execute("layer.select", json!({"layers": [solid]})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + settle(&mut h); + let pos = |h: &mut Harness<'_, EffectcraftApp>| -> Vec { + let v = h.state_mut().session.execute("prop.get", json!({"layer": solid, "path": "transform/position"})).unwrap()["value"].clone(); + serde_json::from_value(v).unwrap() + }; + let tip = |h: &Harness<'_, EffectcraftApp>, axis: &str| { + let id = format!("viewer.gizmo.{solid}.{axis}"); + let [x, y, w, hh] = h.state().auto.find(&id).unwrap_or_else(|| panic!("{id} is registered")).rect; + egui::pos2(x + w / 2.0, y + hh / 2.0) + }; + let p0 = pos(&mut h); + + let z = tip(&h, "z"); + drag(&mut h, z, z + egui::vec2(30.0, -30.0)); + let p1 = pos(&mut h); + assert_eq!(&p1[..2], &p0[..2], "x and y stay put"); + assert!(p1[2] > p0[2] + 10.0, "pushed back in depth: {p0:?} -> {p1:?}"); + + settle(&mut h); + let x = tip(&h, "x"); + drag(&mut h, x, x + egui::vec2(40.0, 25.0)); + let p2 = pos(&mut h); + assert!(p2[0] > p1[0] + 10.0, "moved along x: {p1:?} -> {p2:?}"); + assert_eq!(&p2[1..], &p1[1..], "only x changes"); + + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(pos(&mut h), p1, "the x drag is one undo step"); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(pos(&mut h), p0, "the z drag is one undo step"); +} From 1443ad0177214ec0967ed7bcc11d1bf4385ffc35 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Fri, 16 Oct 2026 13:53:12 +0530 Subject: [PATCH 2/2] fix(viewer): gizmo arrows take precedence over box handles under them Review on #1627: the X arrow tip can sit on a bounding-box handle (a 120 px wide layer at 100%). The arrow is painted on top, but the press and hover tests checked the handles first, so dragging it scaled the layer. Check the arrows first in both, and cover the overlap with a pointer-input test. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/viewer.rs | 15 +++--- crates/ui-egui/tests/ui_3d.rs | 76 ++++++++++++++++++++--------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 008a29c7..93fcaa2f 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -1357,10 +1357,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } else if vertex_at(hp, true).or_else(|| vertex_at(hp, false)).is_some() { // A path point under a box handle is grabbed first: no resize cursor. egui::CursorIcon::PointingHand - } else if let Some((.., cursor)) = handle_hits.iter().find(|(_, _, h, _)| h.distance(hp) < HANDLE) { - *cursor } else if axis_at(&axis_hits, hp).is_some() { egui::CursorIcon::Move + } else if let Some((.., cursor)) = handle_hits.iter().find(|(_, _, h, _)| h.distance(hp) < HANDLE) { + *cursor } else { egui::CursorIcon::Default } @@ -1544,7 +1544,12 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { // far enough to count as a drag it may have left a small handle. Tool::Selection => { let cpt = map.to_comp(press); - if let Some((lid, hi, ..)) = handle_hits.iter().find(|(_, _, h, _)| h.distance(press) < HANDLE + 2.0).cloned() { + // A gizmo arrow is drawn over the box handles, so it is grabbed first: move along + // that axis only, without snapping. + if let Some((lid, _, ax, ay)) = axis_at(&axis_hits, press) { + let p0 = comp.layer(lid).and_then(|l| Some(ectx.v3(l, l.transform()?, "position", [0.0; 3]))); + p0.map(|p0| Gesture::Move { layers: vec![(lid, p0, ax, ay)], start: cpt, snap_src: None }) + } else if let Some((lid, hi, ..)) = handle_hits.iter().find(|(_, _, h, _)| h.distance(press) < HANDLE + 2.0).cloned() { let layer = comp.layer(lid).cloned(); layer.and_then(|layer| { let tr = layer.transform()?; @@ -1568,10 +1573,6 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { .collect(), }) }) - } else if let Some((lid, _, ax, ay)) = axis_at(&axis_hits, press) { - // A gizmo arrow: move along that axis only, without snapping. - let p0 = comp.layer(lid).and_then(|l| Some(ectx.v3(l, l.transform()?, "position", [0.0; 3]))); - p0.map(|p0| Gesture::Move { layers: vec![(lid, p0, ax, ay)], start: cpt, snap_src: None }) } else if let Some(l) = pick(app, &ectx, cpt, mods.shift) { let layers: Vec<(LayerId, [f64; 3], [f64; 3], [f64; 3])> = app .session diff --git a/crates/ui-egui/tests/ui_3d.rs b/crates/ui-egui/tests/ui_3d.rs index 2ba5265a..223d9960 100644 --- a/crates/ui-egui/tests/ui_3d.rs +++ b/crates/ui-egui/tests/ui_3d.rs @@ -279,43 +279,73 @@ fn drag(h: &mut Harness<'_, EffectcraftApp>, from: egui::Pos2, to: egui::Pos2) { h.run_steps(3); } +/// A selected 3D solid (`w` x `h`) in an empty 640x360 comp seen by the default camera. +fn gizmo_harness(w: u32, h: u32) -> (Harness<'static, EffectcraftApp>, u64) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 640, "height": 360, "duration": 2})).unwrap(); + let solid = s.execute("layer.newSolid", json!({"color": "#ff0000", "width": w, "height": h})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.setSwitch", json!({"layer": solid, "switch": "threeD", "value": true})).unwrap(); + s.execute("layer.select", json!({"layers": [solid]})).unwrap(); + let mut harness = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + settle(&mut harness); + (harness, solid) +} + +fn transform(h: &mut Harness<'_, EffectcraftApp>, layer: u64, prop: &str) -> Vec { + let v = h.state_mut().session.execute("prop.get", json!({"layer": layer, "path": format!("transform/{prop}")})).unwrap()["value"].clone(); + serde_json::from_value(v).unwrap() +} + +/// The centre of the automation element `id`. +fn centre(h: &Harness<'_, EffectcraftApp>, id: &str) -> egui::Pos2 { + let [x, y, w, hh] = h.state().auto.find(id).unwrap_or_else(|| panic!("{id} is registered")).rect; + egui::pos2(x + w / 2.0, y + hh / 2.0) +} + /// #1618: the gizmo's arrows drag a 3D layer along one axis. Z, which points at the default /// camera, still has a grabbable arrow and pushes the layer in depth, as one undo step. #[test] fn gizmo_arrows_move_a_3d_layer_along_one_axis() { - let mut s = Session::default(); - s.execute("comp.new", json!({"name": "C", "width": 640, "height": 360, "duration": 2})).unwrap(); - let solid = s.execute("layer.newSolid", json!({"color": "#ff0000", "width": 200, "height": 200})).unwrap()["layer"].as_u64().unwrap(); - s.execute("layer.setSwitch", json!({"layer": solid, "switch": "threeD", "value": true})).unwrap(); - s.execute("layer.select", json!({"layers": [solid]})).unwrap(); - let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); - settle(&mut h); - let pos = |h: &mut Harness<'_, EffectcraftApp>| -> Vec { - let v = h.state_mut().session.execute("prop.get", json!({"layer": solid, "path": "transform/position"})).unwrap()["value"].clone(); - serde_json::from_value(v).unwrap() - }; - let tip = |h: &Harness<'_, EffectcraftApp>, axis: &str| { - let id = format!("viewer.gizmo.{solid}.{axis}"); - let [x, y, w, hh] = h.state().auto.find(&id).unwrap_or_else(|| panic!("{id} is registered")).rect; - egui::pos2(x + w / 2.0, y + hh / 2.0) - }; - let p0 = pos(&mut h); + let (mut h, solid) = gizmo_harness(200, 200); + let p0 = transform(&mut h, solid, "position"); - let z = tip(&h, "z"); + let z = centre(&h, &format!("viewer.gizmo.{solid}.z")); drag(&mut h, z, z + egui::vec2(30.0, -30.0)); - let p1 = pos(&mut h); + let p1 = transform(&mut h, solid, "position"); assert_eq!(&p1[..2], &p0[..2], "x and y stay put"); assert!(p1[2] > p0[2] + 10.0, "pushed back in depth: {p0:?} -> {p1:?}"); settle(&mut h); - let x = tip(&h, "x"); + let x = centre(&h, &format!("viewer.gizmo.{solid}.x")); drag(&mut h, x, x + egui::vec2(40.0, 25.0)); - let p2 = pos(&mut h); + let p2 = transform(&mut h, solid, "position"); assert!(p2[0] > p1[0] + 10.0, "moved along x: {p1:?} -> {p2:?}"); assert_eq!(&p2[1..], &p1[1..], "only x changes"); h.state_mut().session.execute("edit.undo", json!({})).unwrap(); - assert_eq!(pos(&mut h), p1, "the x drag is one undo step"); + assert_eq!(transform(&mut h, solid, "position"), p1, "the x drag is one undo step"); h.state_mut().session.execute("edit.undo", json!({})).unwrap(); - assert_eq!(pos(&mut h), p0, "the z drag is one undo step"); + assert_eq!(transform(&mut h, solid, "position"), p0, "the z drag is one undo step"); +} + +/// #1627 review: an arrow drawn over a box handle is what the pointer grabs (it scaled the +/// layer). At 100% a 120 px wide layer has its right-edge handle under the X arrow's tip. +#[test] +fn a_gizmo_arrow_wins_over_the_box_handle_under_it() { + let (mut h, solid) = gizmo_harness(120, 200); + h.state_mut().ui.viewer.zoom = Some(1.0); + settle(&mut h); + let x = centre(&h, &format!("viewer.gizmo.{solid}.x")); + assert!(x.distance(centre(&h, &format!("viewer.handle.{solid}.5"))) < 2.0, "the right-edge handle is under the X tip"); + + h.event(egui::Event::PointerMoved(x)); + h.run_steps(2); + assert_eq!(h.output().platform_output.cursor_icon, egui::CursorIcon::Move, "hover shows the move cursor, not resize"); + + let p0 = transform(&mut h, solid, "position"); + drag(&mut h, x, x + egui::vec2(30.0, 0.0)); + let p1 = transform(&mut h, solid, "position"); + assert_eq!(transform(&mut h, solid, "scale"), vec![100.0, 100.0, 100.0], "not scaled"); + assert!((p1[0] - p0[0] - 30.0).abs() < 1e-6, "moved 30 px along x: {p0:?} -> {p1:?}"); + assert_eq!(&p1[1..], &p0[1..]); }