From 59c55e8a267a11c23ccad8490e76887fb89dab44 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:14:52 +0700 Subject: [PATCH] Timeline: dragging over stopwatches sets them all (#284) As in After Effects, pressing a property's stopwatch toggles it, and dragging on over the stopwatches above or below gives every property the pointer passes the state the first one got (animated or not), whatever its own, in one undo step. Alt+click still adds an expression. The gesture is the layer switches' press-and-drag from #238, moved into `switch_gesture` and shared by both. `prop.toggleAnimation` takes `merge` so the drag folds into one undo step. Tests: dragging_over_stopwatches_sets_them_all (ui_timeline_switches), stopwatch_toggles_with_a_merge_key_are_one_undo_step (engine). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/prop.rs | 4 +- crates/engine/src/tests_timeline.rs | 13 +++++ crates/ui-egui/src/panels/timeline.rs | 57 +++++++++++--------- crates/ui-egui/tests/ui_timeline_switches.rs | 48 +++++++++++++++++ 4 files changed, 95 insertions(+), 27 deletions(-) diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index 7babf85d..d50c4353 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -144,7 +144,7 @@ fn get(s: &mut Session, p: &Value) -> Result { fn toggle_anim(s: &mut Session, p: &Value) -> Result { let (cid, lid, uid) = prop_ref(s, p, "prop.toggleAnimation")?; let on = b_p(p, "value"); - with_prop(s, "Toggle Animation", None, cid, lid, uid, |pr, lt| { + with_prop(s, "Toggle Animation", merge_p(p), cid, lid, uid, |pr, lt| { if pr.static_only { return Err(bad("prop.toggleAnimation", "this property can't be animated")); } @@ -769,7 +769,7 @@ pub fn specs() -> Vec { vec![ query!("prop.get", "Get Property", "{layer?, path|prop, time? (comp s)}", get), cmd!("prop.set", "Set Property Value", [], None, "{layer?, path|prop, value, time?, merge?}", has_layers, set), - cmd!("prop.toggleAnimation", "Toggle Stopwatch", [], None, "{layer?, path|prop, value?}", has_layers, toggle_anim), + cmd!("prop.toggleAnimation", "Toggle Stopwatch", [], None, "{layer?, path|prop, value?, merge?}", has_layers, toggle_anim), cmd!("prop.addKey", "Add Keyframe", [], None, "{layer?, path|prop, time?, value?}", has_layers, add_key), cmd!("prop.toggleKey", "Add or Remove Keyframe at Current Time", [], None, "{layer?, path|prop}", has_layers, toggle_key), cmd!( diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index d33a3c1d..03b91c28 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -428,6 +428,19 @@ fn parenting_keeps_the_layer_in_place() { assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(child)).unwrap().parent.is_some()); } +/// #284: toggling stopwatches with one `merge` key (dragging over them) is one undo step. +#[test] +fn stopwatch_toggles_with_a_merge_key_are_one_undo_step() { + let (mut s, l) = setup(); + let steps = s.history.undo.len(); + for path in ["transform/position", "transform/scale", "transform/opacity"] { + s.execute("prop.toggleAnimation", json!({"layer": l, "path": path, "value": true, "merge": "drag"})).unwrap(); + } + assert_eq!(s.history.undo.len(), steps + 1); + s.execute("edit.undo", json!({})).unwrap(); + assert!(["transform/position", "transform/scale", "transform/opacity"].iter().all(|p| !prop(&s, l, p).is_animated())); +} + #[test] fn pick_whip_generates_ae_reference_expressions() { let (mut s, l) = setup(); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 96a4b686..457a9396 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -980,8 +980,8 @@ fn key_drag_id() -> egui::Id { egui::Id::new("tl-key-drag") } -/// A layer switch pressed and dragged over other layers: the switch, the state the press gave -/// it, and the undo step the drag folds into. +/// A switch pressed and dragged over other rows: the switch, the state the press gave it, and +/// the undo step the drag folds into. #[derive(Clone, Debug)] struct SwitchDrag { switch: &'static str, @@ -993,9 +993,24 @@ fn switch_drag_id() -> egui::Id { egui::Id::new("tl-switch-drag") } -/// A layer's switch (A/V Features and Switches columns) in `row`. Pressing it sets it, and -/// dragging on gives the same switch of every layer the pointer passes the same new state, in -/// one undo step (After Effects). +/// The press-and-drag gesture of the Timeline's switches and stopwatches (`switch`, on `row`, +/// now `on`, pressed through `resp`): the press gives it the opposite state, and dragging on +/// gives every `switch` the pointer passes that same state, in one undo step, whatever its own +/// (After Effects). The state to set and the undo step to fold it into, when it changes now. +fn switch_gesture(ui: &egui::Ui, resp: &egui::Response, row: Rect, switch: &'static str, on: bool) -> Option<(bool, String)> { + if resp.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()) { + let merge = format!("tl-switch-{}", ui.input(|i| i.time)); + ui.data_mut(|d| d.insert_temp(switch_drag_id(), SwitchDrag { switch, value: !on, merge: merge.clone() })); + return Some((!on, merge)); + } + let d = ui.data(|d| d.get_temp::(switch_drag_id())).filter(|d| d.switch == switch && d.value != on)?; + // The heights the pointer moved over since the last frame, so a quick drag skips no row. + let (a, b) = ui.input(|i| i.pointer.interact_pos().map(|p| (p.y - i.pointer.delta().y, p.y)))?; + (a.min(b) < row.max.y && a.max(b) >= row.min.y).then_some((d.value, d.merge)) +} + +/// A layer's switch (A/V Features and Switches columns) in `row`, with the +/// [`switch_gesture`]. #[allow(clippy::too_many_arguments)] fn layer_switch( ui: &mut egui::Ui, @@ -1010,20 +1025,8 @@ fn layer_switch( actions: &mut Vec<(String, serde_json::Value)>, ) { let resp = widgets::icon_toggle(ui, br, icon, on, t, id, Sense::click_and_drag()); - let set = |actions: &mut Vec<(String, serde_json::Value)>, value: bool, merge: &str| { - actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name, "value": value, "merge": merge}))) - }; - if resp.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()) { - let merge = format!("tl-switch-{}", ui.input(|i| i.time)); - set(actions, !on, &merge); - ui.data_mut(|d| d.insert_temp(switch_drag_id(), SwitchDrag { switch: name, value: !on, merge })); - } else if let Some(d) = ui.data(|d| d.get_temp::(switch_drag_id())).filter(|d| d.switch == name && d.value != on) - // The heights the pointer moved over since the last frame, so a quick drag skips no row. - && let Some((a, b)) = ui.input(|i| i.pointer.interact_pos().map(|p| (p.y - i.pointer.delta().y, p.y))) - && a.min(b) < row.max.y - && a.max(b) >= row.min.y - { - set(actions, d.value, &d.merge); + if let Some((value, merge)) = switch_gesture(ui, &resp, row, name, on) { + actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name, "value": value, "merge": merge}))); } else if resp.clicked() && !resp.clicked_by(egui::PointerButton::Primary) { // Keyboard or accessibility activation (a pointer press was handled above). actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name}))); @@ -2161,10 +2164,11 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } app.auto.add(&format!("timeline.prop.{uid}.addKey"), mid, "Add or remove keyframe"); } - // Stopwatch. + // Stopwatch: Alt+click adds an expression; a press toggles the animation, and + // dragging on over other stopwatches gives them the same state (#284). if !prop.static_only { let swr = Rect::from_center_size(pos2(indent, cy), vec2(15.0, 15.0)); - let resp = ui.interact(swr, egui::Id::new(("stopwatch", uid)), Sense::click()); + let resp = ui.interact(swr, egui::Id::new(("stopwatch", uid)), Sense::click_and_drag()); let col = if prop.is_animated() { t.hot_text } else if resp.hovered() { @@ -2174,12 +2178,15 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { }; icons::paint(&lp, swr, Icon::Stopwatch, col); app.auto.add(&format!("timeline.prop.{uid}.stopwatch"), swr, &prop.name); - if resp.clicked() { - if ui.input(|i| i.modifiers.alt) { + if ui.input(|i| i.modifiers.alt) { + if resp.clicked() { actions.push(("prop.setExpression".into(), json!({"layer": layer.id.0, "prop": uid}))); - } else { - actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid}))); } + } else if let Some((value, merge)) = switch_gesture(ui, &resp, r, "stopwatch", prop.is_animated()) { + actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid, "value": value, "merge": merge}))); + } else if resp.clicked() && !resp.clicked_by(egui::PointerButton::Primary) { + // Keyboard or accessibility activation. + actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid}))); } } let name_x = indent + 12.0; diff --git a/crates/ui-egui/tests/ui_timeline_switches.rs b/crates/ui-egui/tests/ui_timeline_switches.rs index 501d2096..a37125b7 100644 --- a/crates/ui-egui/tests/ui_timeline_switches.rs +++ b/crates/ui-egui/tests/ui_timeline_switches.rs @@ -96,3 +96,51 @@ fn dragging_over_layer_switches_sets_them_all() { assert_eq!(states(&h, &ids, motion_blur), [false, true, true, true]); assert_eq!(states(&h, &ids, video), [false, true, false, true], "other switches untouched"); } + +/// #284: pressing a stopwatch and dragging over the stopwatches below gives every property the +/// state the first one got (animated or not), in one undo step; Alt+click still adds an +/// expression and a click toggles one property. +#[test] +fn dragging_over_stopwatches_sets_them_all() { + let (mut h, ids) = harness(); + // Layer A twirled open to its Transform properties. + let (layer, uids) = { + let comp = h.state().session.active_comp().unwrap(); + let l = comp.layer(LayerId(ids[0])).unwrap(); + let tr = l.props.sub("transform").unwrap(); + let uids: Vec = ["anchor", "position", "scale", "rotation", "opacity"].iter().map(|m| tr.get(m).unwrap().uid).collect(); + (l.id, (tr.uid, uids)) + }; + let (group, uids) = uids; + h.state_mut().ui.timeline.open_layers.insert(layer.0); + h.state_mut().ui.timeline.open_groups.insert(group); + h.run_steps(3); + let animated = |h: &Harness<'_, EffectcraftApp>| -> Vec { + let comp = h.state().session.active_comp().unwrap(); + let l = comp.layer(layer).unwrap(); + uids.iter().map(|u| l.props.find(*u).unwrap().is_animated()).collect() + }; + let sw = |h: &Harness<'_, EffectcraftApp>, i: usize| rect(h, &format!("timeline.prop.{}.stopwatch", uids[i])).center(); + let steps = h.state().session.history.undo.len(); + + // Scale animated first; from Anchor Point down to Opacity: all animated, one undo step. + h.state_mut().session.execute("prop.toggleAnimation", json!({"layer": layer.0, "prop": uids[2]})).unwrap(); + h.run_steps(2); + let steps = steps + 1; + let (a, o) = (sw(&h, 0), sw(&h, 4)); + drag(&mut h, a, o, 10); + assert_eq!(animated(&h), [true; 5]); + assert_eq!(h.state().session.history.undo.len(), steps + 1, "one undo step"); + // From Rotation (animated → not) up to Position, quickly: those three stop animating. + let (r, p) = (sw(&h, 3), sw(&h, 1)); + drag(&mut h, r, p, 2); + assert_eq!(animated(&h), [true, false, false, false, true]); + // A click toggles only that property. + let s = sw(&h, 2); + drag(&mut h, s, s, 1); + assert_eq!(animated(&h), [true, false, true, false, true]); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(animated(&h), [true; 5], "each drag was one step"); +}