diff --git a/crates/ui-egui/src/icons.rs b/crates/ui-egui/src/icons.rs index eec054fff..3f6a22445 100644 --- a/crates/ui-egui/src/icons.rs +++ b/crates/ui-egui/src/icons.rs @@ -745,6 +745,12 @@ pub fn keyframe( painter.extend(keyframe_shapes(center, size, left, right, fill)); } +/// The screen rect a [`keyframe`] glyph covers, read off the shapes it draws. +pub fn keyframe_bounds(center: Pos2, size: f32, left: effectcraft_engine::keyframe::KeyHalf, right: effectcraft_engine::keyframe::KeyHalf) -> Rect { + // Any opaque fill: a transparent shape would report no bounds. + egui::Shape::Vec(keyframe_shapes(center, size, left, right, Color32::WHITE)).visual_bounding_rect() +} + /// The halves of a [`keyframe`] glyph. Flat, without a dark outline (as in After Effects), so /// keys that overlap read as one shape instead of a tangle of borders (#284); the hairline in /// the fill colour closes the seam between the halves. @@ -812,6 +818,23 @@ mod tests { } } + /// Each half reaches as far as it is drawn, plus the hairline around it, and the halves are + /// independent (a key can come in linear and go out on hold). + #[test] + fn keyframe_bounds_follow_each_half() { + let c = egui::pos2(100.0, 50.0); + let r = 5.5; + // (half, how far it reaches sideways, how far up and down) + let extents = [(H::Linear, r, r), (H::Hold, r * 0.8, r * 0.8), (H::Bezier, r * 0.75, r), (H::Auto, r * 0.8, r * 0.8)]; + for (l, lx, ly) in extents { + for (rt, rx, ry) in extents { + let b = super::keyframe_bounds(c, 11.0, l, rt); + let want = egui::Rect::from_min_max(c - egui::vec2(lx, ly.max(ry)), c + egui::vec2(rx, ly.max(ry))).expand(0.25); + assert!(b.min.distance(want.min) < 1e-3 && b.max.distance(want.max) < 1e-3, "{l:?}/{rt:?}: {b:?}, want {want:?}"); + } + } + } + /// #284: keyframes have no dark border (overlapping keys looked messy): each half is filled /// and edged in the key's own colour. #[test] diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index e07aeb843..a34009658 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -13,7 +13,7 @@ use effectcraft_engine::project::{ }; use effectcraft_engine::render::EvalCtx; use effectcraft_engine::time::Tick; -use egui::{Align2, Color32, Pos2, Rect, Sense, Stroke, StrokeKind, pos2, vec2}; +use egui::{Align2, Color32, Pos2, Rect, Sense, Stroke, StrokeKind, Vec2, pos2, vec2}; use serde_json::json; use std::sync::Arc; @@ -4232,6 +4232,20 @@ fn key_drag_begin(app: &mut EffectcraftApp, g: &Rw, kh: &KeyHit<'_>) { } } +/// The size of a keyframe glyph and the radius of a roving key's dot. +const KEY_GLYPH_SIZE: f32 = 11.0; +const ROVING_KEY_RADIUS: f32 = 2.5; + +/// The rect a key's marker covers on screen (its hit rect is larger). +fn key_glyph_rect(k: &effectcraft_engine::keyframe::Keyframe, center: Pos2) -> Rect { + if k.roving { + Rect::from_center_size(center, Vec2::splat(ROVING_KEY_RADIUS * 2.0)) + } else { + let icon = k.icon(); + icons::keyframe_bounds(center, KEY_GLYPH_SIZE, icon.left, icon.right) + } +} + /// Paints one keyframe marker; returns its hit rect. fn key_paint(g: &Rw, rc: &RowCtx<'_>, k: &effectcraft_engine::keyframe::Keyframe, x: f32, ks: bool) -> Rect { let t = g.tl.t; @@ -4252,9 +4266,9 @@ fn key_paint(g: &Rw, rc: &RowCtx<'_>, k: &effectcraft_engine::keyframe::Keyframe let kcol = if k.muted { kcol.gamma_multiply(0.35) } else { kcol }; if k.roving { // Roving keys: a small dot (their time follows their neighbours). - gp.circle_filled(c, 2.5, kcol); + gp.circle_filled(c, ROVING_KEY_RADIUS, kcol); } else { - icons::keyframe(&gp, c, 11.0, icon.left, icon.right, kcol); + icons::keyframe(&gp, c, KEY_GLYPH_SIZE, icon.left, icon.right, kcol); } Rect::from_center_size(c, vec2(12.0, 14.0)) } @@ -4678,7 +4692,33 @@ fn key_drag_follow(app: &mut EffectcraftApp, g: &mut Rw) { } } -/// Releasing the key marquee selects the keys inside it. +/// The keys whose markers touch box `br`, in row order (the comp's). `top` is the y of the first +/// row, scrolled. +fn keys_in_box(comp: &Comp, rows: &[Row], tm: &TMap, top: f32, rh: f32, br: Rect) -> Vec { + let mut keys = vec![]; + let mut yy = top; + for row in rows { + let cy = yy + rh / 2.0; + yy += row_height(row, rh); + // No marker reaches a whole glyph from its row's center. + if let RowKind::Prop { uid } = row.kind + && cy >= br.min.y - KEY_GLYPH_SIZE + && cy <= br.max.y + KEY_GLYPH_SIZE + && let Some(l) = comp.layer(row.layer) + && let Some(pr) = l.props.find(uid) + { + for k in &pr.keys { + let x = tm.x(l.comp_time(k.time).seconds()); + if key_glyph_rect(k, pos2(x, cy)).intersects(br) { + keys.push(effectcraft_engine::KeyRef { layer: l.id, prop: uid, time: k.time }); + } + } + } + } + keys +} + +/// Releasing the key marquee selects the keys it touches. fn key_box_select(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw) { let tm = g.tl.tm; let rows_rect = g.tl.rows_rect; @@ -4692,25 +4732,8 @@ fn key_box_select(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw) { && let Some(br) = ctx.data(|d| d.get_temp::(egui::Id::unique("tl-box"))) { ctx.data_mut(|d| d.remove::(egui::Id::unique("tl-box"))); - let mut keys = vec![]; - let mut yy = rows_rect.min.y - app.ui.timeline.scroll_y; - for row in rows.iter() { - let cy = yy + rh / 2.0; - yy += row_height(row, rh); - if let RowKind::Prop { uid } = row.kind - && cy >= br.min.y - && cy <= br.max.y - && let Some(l) = comp.layer(row.layer) - && let Some(pr) = l.props.find(uid) - { - for k in &pr.keys { - let x = tm.x(l.comp_time(k.time).seconds()); - if x >= br.min.x && x <= br.max.x { - keys.push(json!({"layer": l.id.0, "prop": uid, "time": k.time.seconds()})); - } - } - } - } + let touched = keys_in_box(&comp, rows, &tm, rows_rect.min.y - app.ui.timeline.scroll_y, rh, br); + let keys: Vec = touched.iter().map(|k| json!({"layer": k.layer.0, "prop": k.prop, "time": k.time.seconds()})).collect(); let mods = ui.input(|i| i.modifiers); if keys.is_empty() && !(mods.shift || mods.command) { // A marquee that catches nothing leaves nothing selected (layers included). diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index ebcbfa8b9..5aa7cedc5 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -628,6 +628,52 @@ fn a_key_menu_edit_selects_the_clicked_key_before_applying_the_command() { assert_eq!(app.session.state.selected_keys[0].time, prop.keys[0].time); } +/// A key's marquee target is the marker it draws: a roving key's is its smaller dot. +#[test] +fn a_keys_glyph_rect_is_what_it_draws() { + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 64, "duration": 2})).unwrap(); + let id = s.execute("layer.newSolid", json!({"name": "Box", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": 0.5, "value": 0})).unwrap(); + let comp = s.active_comp().unwrap(); + let mut k = comp.layer(LayerId(id)).unwrap().props.prop("transform/opacity").unwrap().keys[0].clone(); + let c = pos2(100.0, 50.0); + k.roving = true; + assert_eq!(key_glyph_rect(&k, c), Rect::from_center_size(c, vec2(5.0, 5.0))); +} + +/// Keys come back in row order, and only the rows and keys the box touches. Rows taller than one +/// line (an expression editor) push the keys below them down, as they are drawn. +#[test] +fn keys_in_box_walks_the_rows_and_tests_each_glyph() { + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 64, "duration": 4, "frameRate": 30})).unwrap(); + let mut layers = vec![]; + for name in ["A", "B"] { + let id = s.execute("layer.newSolid", json!({"name": name, "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for t in [1.0, 2.0] { + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": t, "value": 50})).unwrap(); + } + layers.push(id); + } + let comp = s.active_comp().unwrap(); + let uid = |id: u64| comp.layer(LayerId(id)).unwrap().props.prop("transform/opacity").unwrap().uid; + let row = |id: u64, kind: RowKind| Row { layer: LayerId(id), depth: 1, kind }; + let rows = vec![ + row(layers[1], RowKind::Prop { uid: uid(layers[1]) }), + row(layers[1], RowKind::Expr { uid: uid(layers[1]), lines: 3 }), + row(layers[0], RowKind::Prop { uid: uid(layers[0]) }), + ]; + // 1 s = 100 pt. Rows are 20 pt: centers at y 10 and, past the 60 pt editor, 90. + let tm = TMap { x0: 0.0, start: 0.0, pps: 100.0 }; + let hit = |br: Rect| -> Vec<(u64, i64)> { keys_in_box(comp, &rows, &tm, 0.0, 20.0, br).iter().map(|k| (k.layer.0, k.time.0)).collect() }; + let (one, two) = (Tick::from_seconds_f64(1.0).0, Tick::from_seconds_f64(2.0).0); + let across = |x0: f32, x1: f32| Rect::from_min_max(pos2(x0, 0.0), pos2(x1, 200.0)); + assert_eq!(hit(across(0.0, 300.0)), [(layers[1], one), (layers[1], two), (layers[0], one), (layers[0], two)], "row order"); + assert_eq!(hit(Rect::from_min_max(pos2(0.0, 95.5), pos2(300.0, 200.0))), [(layers[0], one), (layers[0], two)], "the box reaches the glyph's bottom point"); + assert!(hit(Rect::from_min_max(pos2(0.0, 97.0), pos2(300.0, 200.0))).is_empty(), "the box is below the glyph"); +} + #[test] fn layer_drop_target_skips_moving_layers_beyond_the_viewport() { let mut rows: Vec = (1..=6).map(|id| Row { layer: LayerId(id), depth: 0, kind: RowKind::Layer }).collect(); diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 2de7ebee7..4398b4f9d 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -91,6 +91,7 @@ mod ui_time_zoom_controls; mod ui_timeline_audio_levels; mod ui_timeline_effect_label; mod ui_timeline_group_select; +mod ui_timeline_marquee; mod ui_timeline_pan; mod ui_timeline_shapes; mod ui_timeline_switches; diff --git a/crates/ui-egui/tests/it/ui_timeline_marquee.rs b/crates/ui-egui/tests/it/ui_timeline_marquee.rs new file mode 100644 index 000000000..10cfd1a5d --- /dev/null +++ b/crates/ui-egui/tests/it/ui_timeline_marquee.rs @@ -0,0 +1,76 @@ +//! The key marquee in the Timeline (#2857): a box selects a key as soon as it touches any part of +//! the key's glyph, not when it reaches the key's center. + +use effectcraft_engine::Session; +use effectcraft_engine::project::LayerId; +use effectcraft_engine::time::Tick; +use effectcraft_ui_egui::EffectcraftApp; +use egui::{Event, Modifiers, Pos2, Vec2, pos2, vec2}; +use egui_kittest::Harness; +use serde_json::json; + +/// How far a linear key's diamond reaches from its center, without its hairline (0.25 pt). +const REACH: f32 = 5.5; + +/// A comp with one solid, its Opacity keys at 1 s and 2 s (linear) revealed. +fn harness() -> (Harness<'static, EffectcraftApp>, u64) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Box", "width": 320, "height": 180, "frameRate": 30, "duration": 4})).unwrap(); + let id = s.execute("layer.newSolid", json!({"name": "L", "color": "#e04020", "width": 80, "height": 80})).unwrap()["layer"].as_u64().unwrap(); + for (t, v) in [(1.0, 0.0), (2.0, 100.0)] { + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": t, "value": v})).unwrap(); + } + s.execute("layer.select", json!({"layers": [id]})).unwrap(); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(3); + let ctx = h.ctx.clone(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "timeline.reveal.opacity", json!({})).unwrap(); + h.run_steps(4); + (h, id) +} + +/// The center of the key at `secs`: the middle of its hit rect, which is centered on the glyph. +fn key_center(h: &Harness<'_, EffectcraftApp>, id: u64, secs: f64) -> Pos2 { + let comp = h.state().session.active_comp().unwrap(); + let uid = comp.layer(LayerId(id)).unwrap().props.prop("transform/opacity").unwrap().uid; + let key = format!("timeline.keyAt.{uid}.{}", Tick::from_seconds_f64(secs).0); + let e = h.state().auto.find(&key).unwrap_or_else(|| panic!("no {key}")); + pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0) +} + +/// Draw a marquee from `from` to `to`, then report the selected keys' times in seconds. +fn marquee(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) -> Vec { + let modifiers = Modifiers::default(); + h.input_mut().events.push(Event::PointerMoved(from)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers }); + h.step(); + for i in 1..=24 { + h.input_mut().events.push(Event::PointerMoved(from + (to - from) * (i as f32 / 24.0))); + h.step(); + } + h.input_mut().events.push(Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers }); + h.run_steps(2); + h.state().session.state.selected_keys.iter().map(|k| k.time.seconds()).collect() +} + +#[test] +fn a_box_selects_a_key_it_grazes_from_any_side_and_not_one_it_stops_short_of() { + let (mut h, id) = harness(); + let c = key_center(&h, id, 1.0); + // Boxes end `gap` from the key's center, coming from the right, the left and below (the row + // above holds the layer bar, where a press would drag the layer). + let sides: [(&str, fn(f32) -> (Vec2, Vec2)); 3] = [ + ("right", |gap| (vec2(30.0, 5.0), vec2(gap, -5.0))), + ("left", |gap| (vec2(-30.0, 5.0), vec2(-gap, -5.0))), + ("below", |gap| (vec2(5.0, 40.0), vec2(-5.0, gap))), + ]; + for (side, gap_to_box) in sides { + let (from, to) = gap_to_box(REACH); + assert_eq!(marquee(&mut h, c + from, c + to), [1.0], "a box touching the glyph's {side} point selects the key"); + // Clear of the glyph and its hairline by 1 pt. + let (from, to) = gap_to_box(REACH + 0.25 + 1.0); + assert!(marquee(&mut h, c + from, c + to).is_empty(), "a box 1 pt short on the {side} misses it"); + assert!(h.state().session.state.selected_layers.is_empty(), "a box that catches nothing deselects (layers included)"); + } +}