diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index fb4b525a2..50c261020 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -247,7 +247,8 @@ fn property_reply(p: &Value, layer: &Layer, pr: &Property, time: Tick, value: Va /// property (by match path, `transform/position`, `effects/#n/`) of every other /// selected, unlocked layer of the comp whose value is of the same kind, as (layer, uid). Empty /// when `selected` is not set or the edited layer isn't selected. After Effects applies a value -/// edit, the stopwatch and the keyframe button to all selected layers (#355, #491). +/// edit, the stopwatch and the keyframe button to all selected layers (#355, #491). A parameter +/// of a selected Puppet pin also reaches the same parameter of the other selected pins (#1751). fn selected_twins(s: &Session, p: &Value, cmd: &str) -> Result> { if b_p(p, "selected") != Some(true) { return Ok(vec![]); @@ -260,14 +261,31 @@ fn selected_twins(s: &Session, p: &Value, cmd: &str) -> Result = s + .state .selected_layers .iter() .filter(|o| **o != lid) .filter_map(|o| comp.layer(*o)) .filter(|l| !l.switches.locked) - .filter_map(|l| l.props.prop(&path).filter(|q| (q.value.kind_name(), q.value.components().len()) == kind).map(|q| (l.id, q.uid))) - .collect()) + .filter_map(|l| l.props.prop(&path).filter(same_kind).map(|q| (l.id, q.uid))) + .collect(); + // A parameter of a selected Puppet pin: the same parameter of every other selected pin, on + // this layer too, so several pins move or key together (#1751). + let pin = super::puppet::layer_pins(layer).find(|g| g.props().any(|q| q.uid == uid)).map(|g| g.uid); + if let Some(pin) = pin + && s.state.selected_props.contains(&(lid, pin)) + { + for (ol, ou) in &s.state.selected_props { + let Some(other) = comp.layer(*ol).filter(|l| !l.switches.locked) else { continue }; + let Some(q) = super::puppet::layer_pins(other).find(|g| g.uid == *ou).and_then(|g| g.get(&pr.match_id)).filter(same_kind) else { continue }; + if (*ol, q.uid) != (lid, uid) && !twins.contains(&(*ol, q.uid)) { + twins.push((*ol, q.uid)); + } + } + } + Ok(twins) } /// `p` aimed at property `uid` of layer `lid` instead. diff --git a/crates/engine/src/tests/paint.rs b/crates/engine/src/tests/paint.rs index db831ca96..3659ce0c4 100644 --- a/crates/engine/src/tests/paint.rs +++ b/crates/engine/src/tests/paint.rs @@ -308,6 +308,45 @@ fn puppet_select_all_takes_the_pin_kind_and_several_pins_record_together() { assert!(s.execute("puppet.recordPin", json!({"layer": id, "pin": a, "pins": [st], "samples": samples})).is_err()); } +/// Editing a parameter of one of several selected pins edits the same parameter of the others, +/// in one undo step; unselected pins keep theirs (#1751). +#[test] +fn editing_one_selected_pin_edits_the_other_selected_pins() { + let (mut s, id) = setup(); + let pin = |s: &mut Session, x: i64| s.execute("puppet.addPin", json!({"layer": id, "position": [x, 30]})).unwrap()["pin"].as_u64().unwrap(); + let (a, b, c) = (pin(&mut s, 10), pin(&mut s, 50), pin(&mut s, 90)); + let position = |s: &Session, pin: u64| layer(s, id).props.find_group(pin).unwrap().get("position").unwrap().clone(); + let (pa, pb) = (position(&s, a).uid, position(&s, b).uid); + let at = |s: &Session, pin: u64| position(s, pin).value_at(Tick::ZERO).components(); + s.execute("prop.select", json!({"layer": id, "prop": a, "selectKeys": false})).unwrap(); + s.execute("prop.select", json!({"layer": id, "prop": b, "selectKeys": false, "add": true})).unwrap(); + + // A typed value lands on both selected pins, and one Undo takes it back. + let steps = s.history.undo.len(); + s.execute("prop.set", json!({"layer": id, "prop": pa, "value": [20, 40], "selected": true})).unwrap(); + assert_eq!((at(&s, a), at(&s, b), at(&s, c)), (vec![20.0, 40.0], vec![20.0, 40.0], vec![90.0, 30.0])); + assert_eq!(s.history.undo.len(), steps + 1, "one undo step"); + s.undo(); + assert_eq!((at(&s, a), at(&s, b)), (vec![10.0, 30.0], vec![50.0, 30.0])); + + // A scrub moves each selected pin by the same change from its own value. + s.execute("prop.set", json!({"layer": id, "prop": pb, "value": [55, 25], "selected": true, "offset": true})).unwrap(); + assert_eq!((at(&s, a), at(&s, b), at(&s, c)), (vec![15.0, 25.0], vec![55.0, 25.0], vec![90.0, 30.0])); + + // The stopwatch follows on both (a new pin starts keyed, so the toggle turns it off). + let animated = |s: &Session| [a, b, c].map(|p| position(s, p).is_animated()); + assert_eq!(animated(&s), [true; 3]); + s.execute("prop.toggleAnimation", json!({"layer": id, "prop": pa, "selected": true})).unwrap(); + assert_eq!(animated(&s), [false, false, true]); + + // Without `selected`, or with only one pin selected, the edit stays on its own pin. + s.execute("prop.set", json!({"layer": id, "prop": pa, "value": [0, 0]})).unwrap(); + assert_eq!(at(&s, b), vec![55.0, 25.0]); + s.execute("prop.select", json!({"layer": id, "prop": a, "selectKeys": false})).unwrap(); + s.execute("prop.set", json!({"layer": id, "prop": pa, "value": [5, 5], "selected": true})).unwrap(); + assert_eq!(at(&s, b), vec![55.0, 25.0]); +} + fn expr_of(s: &Session, layer: u64, uid: u64) -> String { let l = layer_of(s, layer); l.props.find(uid).and_then(|p| p.expr.as_ref()).map(|e| e.text.clone()).unwrap_or_default() diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 7624aeac8..e07382434 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -78,6 +78,7 @@ mod ui_theme_favorites; mod ui_time_zoom_controls; mod ui_timeline_audio_levels; mod ui_timeline_effect_label; +mod ui_timeline_group_select; mod ui_timeline_pan; mod ui_timeline_shapes; mod ui_timeline_switches; diff --git a/crates/ui-egui/tests/it/ui_timeline_group_select.rs b/crates/ui-egui/tests/it/ui_timeline_group_select.rs new file mode 100644 index 000000000..0a553976b --- /dev/null +++ b/crates/ui-egui/tests/it/ui_timeline_group_select.rs @@ -0,0 +1,83 @@ +//! Shift-clicking a group row in the Timeline adds it to the selection, as on a property row, so +//! several Puppet pins can be selected and deleted together (#1751). + +use effectcraft_engine::Session; +use effectcraft_engine::project::{LayerId, PropGroup}; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::PanelKind; +use egui::{Modifiers, Pos2, vec2}; +use egui_kittest::Harness; +use serde_json::json; + +fn harness() -> (Harness<'static, EffectcraftApp>, u64, [u64; 3]) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 200, "height": 100, "frameRate": 30, "duration": 2})).unwrap(); + let layer = s.execute("layer.newSolid", json!({"width": 100, "height": 60, "color": [0, 0, 1]})).unwrap()["layer"].as_u64().unwrap(); + let pin = |s: &mut Session, x: i64| s.execute("puppet.addPin", json!({"layer": layer, "position": [x, 30]})).unwrap()["pin"].as_u64().unwrap(); + let pins = [pin(&mut s, 10), pin(&mut s, 50), pin(&mut s, 90)]; + let mut app = EffectcraftApp::new(s); + app.show_panel(PanelKind::Timeline); + app.ui.timeline.open_layers.insert(layer); + // Twirl open every group down to the pins. + fn all(g: &PropGroup, out: &mut Vec) { + for c in g.groups() { + out.push(c.uid); + all(c, out); + } + } + let mut open = vec![]; + if let Some(l) = app.session.active_comp().and_then(|c| c.layer(LayerId(layer))) { + all(&l.props, &mut open); + } + app.ui.timeline.open_groups.extend(open); + let mut h = Harness::builder().with_size(vec2(1600.0, 1100.0)).build_eframe(|_| app); + h.run_steps(4); + (h, layer, pins) +} + +fn row(h: &Harness<'_, EffectcraftApp>, uid: u64) -> Pos2 { + let id = format!("timeline.group.{uid}.name"); + let e = h.state().auto.find(&id).unwrap_or_else(|| panic!("no {id}")); + egui::pos2(e.rect[0] + e.rect[2].min(40.0) / 2.0, e.rect[1] + e.rect[3] / 2.0) +} + +fn click(h: &mut Harness<'_, EffectcraftApp>, uid: u64, modifiers: Modifiers) { + let at = row(h, uid); + h.input_mut().events.push(egui::Event::ModifiersChanged(modifiers)); + h.input_mut().events.push(egui::Event::PointerMoved(at)); + h.input_mut().events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers }); + h.input_mut().events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers }); + h.run_steps(3); + h.input_mut().events.push(egui::Event::ModifiersChanged(Modifiers::NONE)); + h.step(); +} + +fn selected(h: &Harness<'_, EffectcraftApp>, pins: [u64; 3]) -> Vec { + h.state().session.state.selected_props.iter().map(|(_, u)| *u).filter(|u| pins.contains(u)).collect() +} + +fn pin_count(h: &Harness<'_, EffectcraftApp>, layer: u64) -> usize { + let l = h.state().session.active_comp().unwrap().layer(LayerId(layer)).unwrap(); + l.effects().unwrap().groups().map(effectcraft_engine::effects::puppet::pin_count).sum() +} + +#[test] +fn shift_click_adds_puppet_pins_and_delete_removes_them_together() { + let (mut h, layer, pins) = harness(); + let [a, b, c] = pins; + click(&mut h, a, Modifiers::NONE); + assert_eq!(selected(&h, pins), [a]); + click(&mut h, b, Modifiers::SHIFT); + assert_eq!(selected(&h, pins), [a, b], "Shift-click keeps the first pin"); + click(&mut h, c, Modifiers::NONE); + assert_eq!(selected(&h, pins), [c], "a plain click selects only the clicked pin"); + + click(&mut h, a, Modifiers::NONE); + click(&mut h, b, Modifiers::SHIFT); + h.state_mut().session.execute("edit.clear", json!({})).unwrap(); + h.run_steps(3); + assert_eq!(pin_count(&h, layer), 1, "Delete removes both selected pins"); + assert!(h.state().session.active_comp().unwrap().layer(LayerId(layer)).is_some(), "the layer stays"); + h.state_mut().session.undo(); + assert_eq!(pin_count(&h, layer), 3, "one Undo restores both pins"); +}