diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index cd0006c0f..b8fb4bb2b 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -838,9 +838,12 @@ fn phase_menu(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, v: &m // A right-click an overlay took (the Cryptomatte eyedropper's) opens the menu too. let forwarded = ui.data_mut(|d| d.remove_temp::(egui::Id::unique(FORWARDED_SECONDARY))); if let Some(pos) = resp.interact_pointer_pos().filter(|_| resp.secondary_clicked()).or(forwarded) { - // Picking an already selected layer preserves the group; an unselected hit - // becomes the target, using the same visible/unlocked hit test as left-click. - pick(app, ectx, map.to_comp(pos), false); + // The layer a left click there would select: the topmost one, keeping the group when it + // is already selected (#1535), or the selected layer whose shown path is under the + // pointer. The menu's list reaches the layers under it. + if ov::segment_at(&v.paths, &map, pos, 6.0).is_none() { + click_pick(app, ectx, map.to_comp(pos), false); + } let point = map.to_comp(pos); let hits: Vec<(u64, String)> = layers_at(ectx, point).map(|l| (l.id.0, l.name.clone())).collect(); ui.data_mut(|d| d.insert_temp(menu_hits_id, hits)); @@ -1845,15 +1848,19 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response } // The Roto Brush paints in the Layer panel only: select as usual and say so. rt if rt.roto_kind(false).is_some() => { - if pick(app, ectx, cpt, mods.shift).is_none() { + if click_pick(app, ectx, cpt, mods.shift).is_none() { let _ = app.session.execute("edit.deselectAll", json!({})); } app.ui.status = crate::i18n::tr(ROTO_HINT).to_string(); } _ => { - if pick(app, ectx, cpt, mods.shift).is_none() { + let on_path = ov::segment_at(&paths, &map, pos, 6.0).is_some(); + if on_path && !mods.shift { + // A click on a path of a selected layer stays on that layer, even where a + // layer in front covers it, so a double-click there free-transforms the path. + } else if click_pick(app, ectx, cpt, mods.shift).is_none() { let _ = app.session.execute("edit.deselectAll", json!({})); - } else if !mods.shift && !app.session.state.selected_vertices.is_empty() && ov::segment_at(&paths, &map, pos, 6.0).is_none() { + } else if !mods.shift && !app.session.state.selected_vertices.is_empty() && !on_path { // A click away from the paths, even on their layer, deselects the mask // points (the layer stays selected), as in After Effects (#513). let _ = app.session.execute("mask.selectVertices", json!({"vertices": []})); @@ -1904,7 +1911,8 @@ fn phase_dblclick(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Respons { let cpt = map.to_comp(pos); let roto = app.ui.tool.roto_kind(false).is_some(); - if let Some(l) = pick(app, ectx, cpt, false).and_then(|l| comp.layer(l)) { + // The topmost layer, as the double-click's first click selected it (#1535). + if let Some(l) = click_pick(app, ectx, cpt, false).and_then(|l| comp.layer(l)) { match &l.source { effectcraft_engine::project::LayerSource::Comp { item } => app.session.open_comp(*item), // With a Roto Brush tool, double-clicking a layer opens it in the Layer panel, @@ -2002,8 +2010,8 @@ use geometry::{ use geometry::{box_corners, project_clipped}; 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}; +pub(crate) use shapes::{click_pick, layer_at, layer_at_including_locked, layers_at, pick, selectable_layers}; use shapes::{create_shape, draw_shape_ghost, shape_drag}; -pub(crate) use shapes::{layer_at, layer_at_including_locked, layers_at, pick, selectable_layers}; #[cfg(test)] use tools::constrain_45; use tools::{add_motion_key, axis_at, axis_drag, drag_axes, draw_rigs, parent_world_inverse, pen_cursor, pen_cursor_icon, pen_press, select_motion_key}; diff --git a/crates/ui-egui/src/panels/viewer/shapes.rs b/crates/ui-egui/src/panels/viewer/shapes.rs index 76892cbaf..ce9b256cd 100644 --- a/crates/ui-egui/src/panels/viewer/shapes.rs +++ b/crates/ui-egui/src/panels/viewer/shapes.rs @@ -39,10 +39,24 @@ fn quad_area(q: &[[f64; 2]; 4]) -> f64 { (0..4).map(|i| q[i][0] * q[(i + 1) % 4][1] - q[(i + 1) % 4][0] * q[i][1]).sum::().abs() / 2.0 } -/// The layer a press at a comp point acts on, selected (Shift toggles the topmost layer). As in +/// The layer a drag from a comp point acts on, selected (Shift toggles the topmost layer). As in /// After Effects, a selected layer under the point comes before the layers in front of it, so it -/// can be dragged where they cover it; otherwise the topmost layer (#230). +/// can be dragged where they cover it; otherwise the topmost layer (#230). Clicks use +/// [`click_pick`], except a Puppet tool click while no layer has pins, which uses this so a pin +/// can go on a selected layer behind others. pub(crate) fn pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool) -> Option { + pick_layer(app, ectx, cpt, toggle, true) +} + +/// The layer a click (press and release without a drag) at a comp point acts on: always the +/// topmost layer, selected unless it already is (Shift toggles it). Only a drag goes to a selected +/// layer behind others ([`pick`]), so a click can still select a layer in front of it (#1535). +/// The viewer doesn't call it for a click on a shown path, which stays on the path's layer. +pub(crate) fn click_pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool) -> Option { + pick_layer(app, ectx, cpt, toggle, false) +} + +fn pick_layer(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool, prefer_selected: bool) -> Option { if toggle { let hit = layer_at(ectx, cpt)?.id; let _ = app.session.execute("layer.select", json!({"layers": [hit.0], "toggle": true})); @@ -50,7 +64,8 @@ pub(crate) fn pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], togg } let selected = &app.session.state.selected_layers; let hits: Vec = layers_at(ectx, cpt).map(|l| l.id).collect(); - let hit = *hits.iter().find(|id| selected.contains(id)).or(hits.first())?; + let preferred = if prefer_selected { hits.iter().find(|id| selected.contains(id)) } else { None }; + let hit = *preferred.or(hits.first())?; if !selected.contains(&hit) { let _ = app.session.execute("layer.select", json!({"layers": [hit.0]})); } diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index e01608e16..7934168cd 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -198,11 +198,10 @@ fn viewer_click_picks_nothing_while_the_solo_layer_is_inactive() { assert!(h.state().session.state.selected_layers.is_empty(), "{back:?} stays excluded by solo"); } -/// A selected layer under the pointer takes the press before the layers in front of it, as in -/// After Effects: it stays selected and a drag moves it. With none selected there, the topmost -/// layer does (#230). +/// A click, left or right, selects the topmost layer under the pointer, even over a selected +/// layer behind it (#1535); a drag still moves a selected layer behind others (#230). #[test] -fn viewer_press_prefers_a_selected_layer_under_the_pointer() { +fn viewer_click_selects_the_topmost_layer_and_a_drag_moves_the_selected_one() { let mut h = selection_harness(); let comp = h.state().session.active_comp().unwrap(); let (front, back) = (comp.layers[0].id, comp.layers[1].id); @@ -210,7 +209,20 @@ fn viewer_press_prefers_a_selected_layer_under_the_pointer() { h.run_steps(3); let at = screen(&h, [320.0, 180.0]); click(&mut h, at); - assert_eq!(h.state().session.state.selected_layers, vec![back], "a click keeps the selected layer behind"); + assert_eq!(h.state().session.state.selected_layers, vec![front], "a click selects the layer in front of the selected one (#1535)"); + // A right-click selects what a left click would, so the menu acts on the layer in front, + // here where both layers cover the pointer and the layer behind is selected. + h.state_mut().session.execute("layer.select", json!({"layers": [back.0]})).unwrap(); + h.run_steps(3); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed, modifiers: Default::default() }); + h.step(); + } + h.run_steps(2); + assert_eq!(h.state().session.state.selected_layers, vec![front], "a right-click selects the layer in front"); + h.key_press(egui::Key::Escape); + h.run_steps(3); + h.state_mut().session.execute("layer.select", json!({"layers": [back.0]})).unwrap(); h.run_steps(40); let (front0, back0) = (position_and_anchor(&h, front).0, position_and_anchor(&h, back).0); drag(&mut h, at, at + vec2(60.0, 0.0)); @@ -224,6 +236,71 @@ fn viewer_press_prefers_a_selected_layer_under_the_pointer() { assert_eq!(h.state().session.state.selected_layers, vec![front], "nothing selected: the topmost layer"); } +/// A click, left or right, on a path of the selected layer, where a layer in front covers it, stays +/// on that path's layer rather than selecting the layer in front, so a double-click there +/// free-transforms the path (#1535 keeps this from `pick`). +#[test] +fn viewer_double_click_on_a_covered_path_of_the_selected_layer_free_transforms_it() { + let mut h = selection_harness(); + let comp = h.state().session.active_comp().unwrap(); + let back = comp.layers[1].id; + let s = &mut h.state_mut().session; + s.execute("layer.select", json!({"layers": [back.0]})).unwrap(); + // In Back's own space (80×80): its top edge runs through comp (320, 150), under Front. + s.execute("mask.new", json!({"layer": back.0, "vertices": [[10, 10], [70, 10], [70, 70], [10, 70]], "closed": true})).unwrap(); + h.run_steps(40); + let at = screen(&h, [320.0, 150.0]); + h.input_mut().events.push(Event::PointerMoved(at)); + h.step(); + // A right-click there acts on the path's layer too, as a left click does. + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Secondary, pressed, modifiers: Default::default() }); + h.step(); + } + h.run_steps(2); + assert_eq!(h.state().session.state.selected_layers, vec![back], "a right-click on the path keeps its layer"); + h.key_press(egui::Key::Escape); + h.run_steps(40); + // Two clicks a frame apart: the first selects as a click does, the second double-clicks. + for _ in 0..2 { + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + } + h.step(); + } + h.run_steps(3); + assert_eq!(h.state().session.state.selected_layers, vec![back], "the path's layer stays selected"); + assert_eq!(h.state().session.state.selected_vertices.len(), 4, "all the path's points are selected"); + assert!(h.state().auto.find("viewer.freeTransform.handle.0").is_some(), "the free-transform box is up"); +} + +/// A double-click where a precomp layer covers the selected layer opens the precomp: like a single +/// click, it acts on the topmost layer, which its first click selects (#1535). +#[test] +fn viewer_double_click_opens_the_topmost_layer_over_a_selected_one() { + let mut s = Session::default(); + let inner = s.execute("comp.new", json!({"name": "Inner", "width": 80, "height": 80, "duration": 4})).unwrap()["comp"].as_u64().unwrap(); + s.execute("comp.new", json!({"name": "Main", "width": 640, "height": 360, "duration": 4})).unwrap(); + s.execute("layer.newSolid", json!({"name": "Back", "color": "#406080", "width": 200, "height": 200})).unwrap(); + s.execute("layer.addItem", json!({"item": "Inner"})).unwrap(); + let back = s.active_comp().unwrap().layers[1].id; + s.execute("layer.select", json!({"layers": [back.0]})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.run_steps(40); + let at = screen(&h, [320.0, 180.0]); + h.input_mut().events.push(Event::PointerMoved(at)); + h.step(); + // Two clicks a frame apart: the first selects as a click does, the second double-clicks. + for _ in 0..2 { + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + } + h.step(); + } + h.run_steps(3); + assert_eq!(h.state().session.active_comp_id().map(|c| c.0), Some(inner), "the precomp in front opens"); +} + /// Each composition keeps its Resolution, saved with the project as in After Effects: chosen for /// comp A in the viewer's popup, it is still A's after showing comp B, costs no undo step, keeps /// A's cached frames and is read back from the saved file (#417).