From a0c1b60d619b0c5d31df1c068add999fe999e650 Mon Sep 17 00:00:00 2001 From: Jonas Neubauer <108294796+user1342554@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:01:41 +0200 Subject: [PATCH] G2 (#66): respect solo and visibility in viewer selection --- crates/ui-egui/src/panels/viewer.rs | 24 ++++---- crates/ui-egui/src/panels/viewer_text.rs | 6 +- crates/ui-egui/tests/ui_text_edit.rs | 25 ++++++++ crates/ui-egui/tests/ui_viewer.rs | 76 ++++++++++++++++++++++++ docs/gaps.md | 4 ++ 5 files changed, 119 insertions(+), 16 deletions(-) diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index c1d90dd2..973d83af 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -6,7 +6,7 @@ use effectcraft_engine::effects::puppet::PinKind; use effectcraft_engine::geom::{Mat3, vec2 as gv2}; -use effectcraft_engine::project::{Layer, LayerId}; +use effectcraft_engine::project::{Comp, Layer, LayerId}; use effectcraft_engine::render::EvalCtx; use effectcraft_engine::render::three_d::{self, CameraState, View3D}; use effectcraft_engine::time::Tick; @@ -1373,10 +1373,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let b = map.to_comp(pos); let (x0, x1) = (a[0].min(b[0]), a[0].max(b[0])); let (y0, y1) = (a[1].min(b[1]), a[1].max(b[1])); - let hits: Vec = comp - .layers - .iter() - .filter(|l| l.is_active_at(time) && !l.switches.locked) + let hits: Vec = selectable_layers(&comp, time) .filter(|l| layer_quad(&ectx, l).is_some_and(|(_, q, _)| q.iter().any(|p| p[0] >= x0 && p[0] <= x1 && p[1] >= y0 && p[1] <= y1))) .map(|l| l.id.0) .collect(); @@ -1683,15 +1680,18 @@ fn pen_press(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, map: & }); } +/// Visible, unlocked layers eligible for viewer hit tests, in front-to-back order. +/// Solo scope matches the renderer: only active AV layers activate it, even when hidden or locked. +pub(crate) fn selectable_layers(comp: &Comp, time: Tick) -> impl Iterator { + let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av() && l.is_active_at(time)); + comp.layers + .iter() + .filter(move |l| l.is_active_at(time) && l.switches.video && !l.switches.locked && !l.is_camera() && !l.is_light() && (!any_solo || l.switches.solo)) +} + /// Topmost layer under a comp point (selects it; shift toggles). Returns the hit layer. pub(crate) fn pick(app: &mut EffectcraftApp, ectx: &EvalCtx, cpt: [f64; 2], toggle: bool) -> Option { - let hit = ectx - .comp - .layers - .iter() - .filter(|l| l.is_active_at(ectx.time) && l.switches.video && !l.switches.locked && !l.is_camera() && !l.is_light()) - .find(|l| layer_quad(ectx, l).is_some_and(|(_, q, _)| point_in_quad(cpt, &q))) - .map(|l| l.id)?; + let hit = selectable_layers(ectx.comp, ectx.time).find(|l| layer_quad(ectx, l).is_some_and(|(_, q, _)| point_in_quad(cpt, &q))).map(|l| l.id)?; if toggle { let _ = app.session.execute("layer.select", json!({"layers": [hit.0], "toggle": true})); } else if !app.session.state.selected_layers.contains(&hit) { diff --git a/crates/ui-egui/src/panels/viewer_text.rs b/crates/ui-egui/src/panels/viewer_text.rs index dd0a28a5..37f7d082 100644 --- a/crates/ui-egui/src/panels/viewer_text.rs +++ b/crates/ui-egui/src/panels/viewer_text.rs @@ -119,10 +119,8 @@ fn edited(app: &EffectcraftApp, ectx: &EvalCtx, lid: LayerId) -> Option /// The topmost text layer whose text is under screen point `s`. fn text_layer_at(app: &EffectcraftApp, ectx: &EvalCtx, map: &ViewerMap, s: Pos2) -> Option { - ectx.comp - .layers - .iter() - .filter(|l| matches!(l.source, LayerSource::Text) && l.is_active_at(ectx.time) && l.switches.video && !l.switches.locked) + super::viewer::selectable_layers(ectx.comp, ectx.time) + .filter(|l| matches!(l.source, LayerSource::Text)) .filter_map(|l| edited(app, ectx, l.id)) .find(|e| e.contains(map, s)) } diff --git a/crates/ui-egui/tests/ui_text_edit.rs b/crates/ui-egui/tests/ui_text_edit.rs index 2c9af608..86d966db 100644 --- a/crates/ui-egui/tests/ui_text_edit.rs +++ b/crates/ui-egui/tests/ui_text_edit.rs @@ -46,6 +46,31 @@ fn edited(h: &Harness<'_, EffectcraftApp>) -> (u64, String, usize, usize) { (e.layer.0, layer_doc(s, e.layer).unwrap().text, e.anchor, e.caret) } +#[test] +fn type_tool_skips_hidden_and_unsoloed_text_layers() { + for switch in ["video", "solo"] { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Type", "width": 640, "height": 360, "duration": 2})).unwrap(); + let mut ids = vec![]; + for _ in 0..2 { + let lid = s.execute("layer.newText", json!({"text": "alpha beta", "size": 80, "position": [100, 200], "justify": "left"})).unwrap()["layer"] + .as_u64() + .unwrap(); + ids.push(lid); + } + let (target, value) = if switch == "video" { (ids[1], false) } else { (ids[0], true) }; + s.execute("layer.setSwitch", json!({"layers": [target], "switch": switch, "value": value})).unwrap(); + s.execute("edit.deselectAll", json!({})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.state_mut().ui.tool = effectcraft_ui_egui::state::Tool::Type; + h.run_steps(3); + let comp = rect(&h, "viewer.comp"); + let k = comp.width() / 640.0; + click(&mut h, comp.min + egui::vec2(330.0 * k, 180.0 * k), Modifiers::NONE); + assert_eq!(edited(&h).0, ids[0], "{switch}"); + } +} + #[test] fn type_tool_click_type_edit_and_commit() { let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).with_step_dt(1.0 / 60.0).build_eframe(|_| app()); diff --git a/crates/ui-egui/tests/ui_viewer.rs b/crates/ui-egui/tests/ui_viewer.rs index 835536bb..164affcb 100644 --- a/crates/ui-egui/tests/ui_viewer.rs +++ b/crates/ui-egui/tests/ui_viewer.rs @@ -58,6 +58,82 @@ fn screen(h: &Harness<'_, EffectcraftApp>, p: [f32; 2]) -> Pos2 { c.min + vec2(p[0] * z, p[1] * z) } +fn selection_harness() -> Harness<'static, EffectcraftApp> { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Selection", "width": 640, "height": 360, "duration": 4})).unwrap(); + for name in ["Back", "Front"] { + s.execute("layer.newSolid", json!({"name": name, "color": "#406080", "width": 80, "height": 80})).unwrap(); + } + s.execute("edit.deselectAll", json!({})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.run_steps(3); + h +} + +#[test] +fn viewer_click_skips_hidden_and_unsoloed_layers() { + for switch in ["video", "solo"] { + let mut h = selection_harness(); + let comp = h.state().session.active_comp().unwrap(); + let (front, back) = (comp.layers[0].id, comp.layers[1].id); + let at = screen(&h, [320.0, 180.0]); + click(&mut h, at); + assert_eq!(h.state().session.state.selected_layers, vec![front]); + let s = &mut h.state_mut().session; + let (target, value) = if switch == "video" { (front, false) } else { (back, true) }; + s.execute("layer.setSwitch", json!({"layers": [target.0], "switch": switch, "value": value})).unwrap(); + s.execute("edit.deselectAll", json!({})).unwrap(); + h.run_steps(40); // Start a new click rather than a double-click. + click(&mut h, at); + assert_eq!(h.state().session.state.selected_layers, vec![back], "{switch}"); + } +} + +#[test] +fn viewer_marquee_skips_hidden_and_unsoloed_layers() { + for switch in ["video", "solo"] { + let mut h = selection_harness(); + let comp = h.state().session.active_comp().unwrap(); + let (front, back) = (comp.layers[0].id, comp.layers[1].id); + let (target, value) = if switch == "video" { (front, false) } else { (back, true) }; + h.state_mut().session.execute("layer.setSwitch", json!({"layers": [target.0], "switch": switch, "value": value})).unwrap(); + h.run_steps(3); + let (from, to) = (screen(&h, [250.0, 110.0]), screen(&h, [390.0, 250.0])); + drag(&mut h, from, to); + assert_eq!(h.state().session.state.selected_layers, vec![back], "{switch}"); + } +} + +#[test] +fn viewer_click_does_not_pick_through_hidden_or_locked_solo_layers() { + for switch in ["video", "lock"] { + let mut h = selection_harness(); + let front = h.state().session.active_comp().unwrap().layers[0].id; + let s = &mut h.state_mut().session; + s.execute("layer.setSwitch", json!({"layers": [front.0], "switch": "solo", "value": true})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [front.0], "switch": switch, "value": switch == "lock"})).unwrap(); + h.run_steps(3); + let at = screen(&h, [320.0, 180.0]); + click(&mut h, at); + assert!(h.state().session.state.selected_layers.is_empty(), "{switch}"); + } +} + +#[test] +fn viewer_click_ignores_inactive_solo_layers() { + let mut h = selection_harness(); + let comp = h.state().session.active_comp().unwrap(); + let (front, back) = (comp.layers[0].id, comp.layers[1].id); + let s = &mut h.state_mut().session; + s.execute("layer.setSwitch", json!({"layers": [front.0], "switch": "solo", "value": true})).unwrap(); + let cid = s.active_comp_id().unwrap(); + std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(front).unwrap().in_point = effectcraft_engine::time::Tick::from_seconds_f64(1.0); + 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]); +} + #[test] fn bottom_bar_in_after_effects_order() { let h = harness(); diff --git a/docs/gaps.md b/docs/gaps.md index 062d2b95..4bceace8 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -67,6 +67,10 @@ The most important missing piece: it turns every other estimate here into a meas - Run the headless snapshot and control-channel checks on Linux (X11 and Wayland) and Windows, not only macOS. Interactions that only fail with real input (docking drags, viewer pan, drag-and-drop import, inline rename) need scripted input tests. +- Regression evidence for [#66](https://github.com/storytold/effectcraft/issues/66): scripted + pointer tests in `ui_viewer` and `ui_text_edit`, checked headlessly on Linux, cover viewer clicks, + marquee selection and text picking when layers are hidden or excluded by solo. They also check + hidden/locked solo layers and solo layers outside their active time range. - Done when: no open bug blocks a basic workflow (import, arrange, animate, preview, render) on any of the three desktop platforms, and every bug users report gets triaged within a day.