Merge pull request #1584 from arnav-hiwarkar/fix/1552-keyframe-copy-selection

Copy only the selected keyframes across Position, Rotation and Scale (#1552)

crates/engine/src/commands/edit.rs

@@ -228,9 +228,7 @@ fn copy(s: &mut Session, p: &Value) -> Result<Value> {
     }
     // Keyframes selected → copy keys (pasted at the CTI).
     if !s.state.selected_keys.is_empty() && p.get("layers").is_none() {
-        s.state.link_clipboard = None;
-        s.state.effect_clipboard.clear();
-        s.state.contents_clipboard.clear();
+        clear_clipboards(s);
         return s.execute("keys.copy", json!({}));
     }
     // Effects selected (Effect Controls / timeline) → copy the effects.
@@ -256,12 +254,14 @@ fn copy(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 /// Edit ▸ Copy with properties selected (Position, or the Transform group), as in After
-/// Effects: an animated property copies its keyframes (the selected ones, else all of them) and
-/// an unanimated one its value, for Paste onto the same properties of other layers (#493).
+/// Effects: an animated property copies its keyframes and an unanimated one its value, for Paste
+/// onto the same properties of other layers (#493). With keyframes selected only those are copied,
+/// even across several highlighted properties (#1552); a selected Transform group copies them all.
 /// `None` when no property (or Transform group) is selected.
 fn copy_properties(s: &mut Session) -> Result<Option<Value>> {
     let Some(comp) = s.active_comp() else { return Ok(None) };
     let mut leaves = vec![];
+    let mut copy_all_keys = s.state.selected_keys.is_empty();
     for (lid, uid) in &s.state.selected_props {
         let Some(l) = comp.layer(*lid) else { continue };
         if l.props.find(*uid).is_some() {
@@ -269,6 +269,7 @@ fn copy_properties(s: &mut Session) -> Result<Option<Value>> {
         } else if let Some(g) = l.props.find_group(*uid)
             && l.props.match_path_of(*uid).as_deref() == Some("transform")
         {
+            copy_all_keys = true;
             g.walk("", &mut |_, pr| leaves.push((*lid, pr.uid)));
         }
     }
@@ -286,7 +287,7 @@ fn copy_properties(s: &mut Session) -> Result<Option<Value>> {
             {
                 values.push((path, pr.value.clone()));
             }
-        } else if !keys.iter().any(|k| k.layer == *lid && k.prop == *uid) {
+        } else if copy_all_keys && !keys.iter().any(|k| k.layer == *lid && k.prop == *uid) {
             keys.extend(pr.keys.iter().map(|k| KeyRef { layer: *lid, prop: *uid, time: k.time }));
         }
     }

crates/engine/src/tests_menu_cmds.rs

@@ -433,8 +433,10 @@ fn copy_a_selected_property_pastes_onto_the_same_property() {
     s.execute("prop.toggleAnimation", json!({"layer": a, "path": "transform/opacity"})).unwrap();
     s.execute("time.set", json!({"time": 1.0})).unwrap();
     s.execute("prop.set", json!({"layer": a, "path": "transform/opacity", "value": 0})).unwrap();
+    s.execute("prop.select", json!({"layer": a, "path": "transform/opacity"})).unwrap();
+    assert_eq!(s.state.selected_keys.len(), 2);
     let transform = layer(&s, a).props.sub("transform").unwrap().uid;
-    s.execute("prop.select", json!({"layer": a, "prop": transform})).unwrap();
+    s.execute("prop.select", json!({"layer": a, "prop": transform, "selectKeys": false})).unwrap();
     s.execute("edit.copy", json!({})).unwrap();
     s.execute("layer.select", json!({"layers": [b]})).unwrap();
     s.execute("time.set", json!({"time": 2.0})).unwrap();

crates/engine/src/tests_timeline.rs

@@ -108,6 +108,85 @@ fn keyframe_copy_paste_keeps_relative_timing_and_crosses_properties() {
     assert!(e.is_err() || prop(&s, l2, "transform/position").keys.is_empty());
 }
 
+#[test]
+fn copy_selected_keys_excludes_deselected_properties_and_stale_values() {
+    let (mut s, source) = setup();
+    s.execute("prop.set", json!({"layer": source, "path": "transform/opacity", "value": 25})).unwrap();
+    select_prop_keys(&mut s, source, "transform/opacity");
+    s.execute("edit.copy", json!({})).unwrap();
+    assert_eq!(s.state.value_clipboard.len(), 1);
+    animate(&mut s, source, "transform/position", &[(0.0, json!([10, 20, 0])), (1.0, json!([30, 40, 0]))]);
+    animate(&mut s, source, "transform/rotation", &[(0.0, json!(15)), (1.0, json!(90))]);
+    animate(&mut s, source, "transform/scale", &[(0.0, json!([50, 60, 100])), (1.0, json!([150, 160, 100]))]);
+    select_prop_keys(&mut s, source, "transform/scale");
+    s.execute(
+        "keys.select",
+        json!({"keys": [{"layer": source, "path": "transform/position", "time": 0}, {"layer": source, "path": "transform/rotation", "time": 1}], "add": true, "selectProperties": true}),
+    )
+    .unwrap();
+    s.execute(
+        "keys.select",
+        json!({"keys": [{"layer": source, "path": "transform/scale", "time": 0}, {"layer": source, "path": "transform/scale", "time": 1}], "toggle": true, "selectProperties": true}),
+    )
+    .unwrap();
+    s.execute("keys.easyEase", json!({})).unwrap();
+    assert_eq!(s.state.selected_keys.len(), 2);
+    assert_eq!(s.state.selected_props.len(), 3);
+    assert_eq!(s.execute("edit.copy", json!({})).unwrap(), json!(2));
+    assert!(s.state.value_clipboard.is_empty());
+    let target = s.execute("layer.newSolid", json!({"color": "#ff0000"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("time.set", json!({"time": 2})).unwrap();
+    s.execute("layer.select", json!({"layers": [target]})).unwrap();
+    s.execute("edit.paste", json!({})).unwrap();
+    undo_redo_roundtrip(
+        &mut s,
+        |s| {
+            for (path, time, value) in [("transform/position", 2.0, KV::Vec3([10.0, 20.0, 0.0])), ("transform/rotation", 3.0, KV::Scalar(90.0))] {
+                let p = prop(s, target, path);
+                assert_eq!(p.keys.len(), 1);
+                let k = &p.keys[0];
+                assert_eq!((k.time.seconds(), &k.value), (time, &value));
+                assert_eq!((k.in_interp, k.out_interp), (Interp::Bezier, Interp::Bezier));
+                assert_eq!(k.in_ease[0], effectcraft_keyframe::Ease::EASY);
+                assert_eq!(k.out_ease[0], effectcraft_keyframe::Ease::EASY);
+            }
+            assert_eq!(s.state.selected_keys.len(), 2);
+            assert_eq!(prop(s, target, "transform/scale").value, KV::Vec3([100.0; 3]));
+            assert!(prop(s, target, "transform/scale").keys.is_empty());
+            assert_eq!(prop(s, target, "transform/opacity").value, KV::Scalar(100.0));
+        },
+        |s| {
+            assert!(prop(s, target, "transform/position").keys.is_empty());
+            assert!(prop(s, target, "transform/rotation").keys.is_empty());
+        },
+    );
+}
+
+#[test]
+fn copy_mixed_animated_and_static_properties_keeps_the_static_value() {
+    let (mut s, source) = setup();
+    animate(&mut s, source, "transform/position", &[(0.0, json!([10, 20, 0])), (1.0, json!([30, 40, 0]))]);
+    s.execute("prop.set", json!({"layer": source, "path": "transform/rotation", "value": 45})).unwrap();
+    select_prop_keys(&mut s, source, "transform/position");
+    s.execute("prop.select", json!({"layer": source, "path": "transform/rotation", "add": true})).unwrap();
+    assert_eq!(s.execute("edit.copy", json!({})).unwrap(), json!({"keys": 2, "values": 1}));
+    let target = s.execute("layer.newSolid", json!({"color": "#ff0000"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("time.set", json!({"time": 2})).unwrap();
+    s.execute("layer.select", json!({"layers": [target]})).unwrap();
+    let undo = s.history.undo.len();
+    s.execute("edit.paste", json!({})).unwrap();
+    assert_eq!(s.history.undo.len(), undo + 1);
+    assert_eq!(prop(&s, target, "transform/rotation").value, KV::Scalar(45.0));
+    let position = prop(&s, target, "transform/position");
+    assert_eq!(
+        position.keys.iter().map(|k| (k.time.seconds(), k.value.clone())).collect::<Vec<_>>(),
+        vec![(2.0, KV::Vec3([10.0, 20.0, 0.0])), (3.0, KV::Vec3([30.0, 40.0, 0.0]))]
+    );
+    s.execute("edit.undo", json!({})).unwrap();
+    assert!(prop(&s, target, "transform/position").keys.is_empty());
+    assert_eq!(prop(&s, target, "transform/rotation").value, KV::Scalar(0.0));
+}
+
 #[test]
 fn cut_removes_keys_and_paste_restores_them() {
     let (mut s, l) = setup();

crates/model/src/stl.rs

@@ -175,10 +175,8 @@ fn ascii(bytes: &[u8]) -> Result<Model> {
         let Some(key) = it.next() else { continue };
         let bad = || perr(format!("STL line {}: expected three numbers", ln + 1));
         match key.to_ascii_lowercase().as_str() {
-            "solid" => {
-                if name.is_none() {
-                    name = Some(line.get(5..).unwrap_or("").trim().to_string());
-                }
+            "solid" if name.is_none() => {
+                name = Some(line.get(5..).unwrap_or("").trim().to_string());
             }
             "facet" => {
                 // `facet normal nx ny nz`

crates/ui-egui/tests/ui_keyframes.rs

@@ -126,6 +126,102 @@ fn click_with(h: &mut Harness<'_, EffectcraftApp>, p: Pos2, modifiers: egui::Mod
     h.run_steps(2);
 }
 
+#[test]
+fn copy_and_paste_key_subsets_across_position_rotation_and_scale() {
+    for graph in [false, true] {
+        let (mut h, id, _) = harness();
+        let paths = ["transform/position", "transform/rotation", "transform/scale"];
+        for (path, times, values) in [
+            (paths[0], [0.5, 1.0], [json!([100, 200, 0]), json!([300, 400, 0])]),
+            (paths[1], [0.5, 1.5], [json!(500), json!(900)]),
+            (paths[2], [0.0, 1.0], [json!([1500, 1600, 100]), json!([1700, 1800, 100])]),
+        ] {
+            for (time, value) in times.into_iter().zip(values) {
+                h.state_mut().session.execute("prop.addKey", json!({"layer": id.0, "path": path, "time": time, "value": value})).unwrap();
+            }
+        }
+        let uids: Vec<u64> = paths.iter().map(|path| h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop(path).unwrap().uid).collect();
+        let ctx = h.ctx.clone();
+        for command in ["timeline.reveal.position", "timeline.revealAdd.rotation", "timeline.revealAdd.scale"] {
+            effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, command, json!({})).unwrap();
+        }
+        if graph {
+            h.state_mut().session.execute("edit.deselectAll", json!({})).unwrap();
+            for uid in &uids {
+                h.state_mut().session.execute("prop.select", json!({"layer": id.0, "prop": uid, "add": true, "selectKeys": false})).unwrap();
+            }
+            h.state_mut().ui.timeline.graph_editor = true;
+            h.state_mut().ui.timeline.graph_mode = "value".into();
+        }
+        h.run_steps(4);
+        let key_positions = |h: &Harness<'_, EffectcraftApp>, uid: u64| {
+            if !graph {
+                return keys(h, uid);
+            }
+            let mut positions: Vec<Pos2> = h
+                .state()
+                .auto
+                .query(&format!("timeline.graph.key.{uid}.0."))
+                .iter()
+                .map(|e| pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0))
+                .collect();
+            positions.sort_by(|a, b| a.x.total_cmp(&b.x));
+            positions
+        };
+        let shift = egui::Modifiers { shift: true, ..Default::default() };
+        for include_scale in [true, false] {
+            h.state_mut().session.execute("keys.select", json!({"keys": []})).unwrap();
+            h.run_steps(2);
+            let original: Vec<_> =
+                paths.iter().map(|path| h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop(path).unwrap().keys.clone()).collect();
+            let positions = key_positions(&h, uids[0]);
+            let rotations = key_positions(&h, uids[1]);
+            let scales = key_positions(&h, uids[2]);
+            click_with(&mut h, scales[0], Default::default());
+            click_with(&mut h, positions[0], shift);
+            click_with(&mut h, rotations[1], shift);
+            if !include_scale {
+                click_with(&mut h, scales[1], shift);
+                click_with(&mut h, scales[0], shift);
+                click_with(&mut h, scales[1], shift);
+            }
+            let count = if include_scale { 3 } else { 2 };
+            assert_eq!(h.state().session.state.selected_keys.len(), count);
+            assert_eq!(h.state().session.state.selected_props.len(), 3);
+            h.input_mut().events.push(Event::Copy);
+            h.step();
+            let note = format!("EffectCraft: {count} keyframes");
+            assert!(h.output().platform_output.commands.iter().any(|command| matches!(command, egui::OutputCommand::CopyText(text) if text == &note)));
+            h.state_mut().session.set_time(effectcraft_engine::time::Tick::from_seconds_f64(2.0));
+            let undo_steps = h.state().session.history.undo.len();
+            h.input_mut().events.push(Event::Paste(note));
+            h.run_steps(2);
+            assert_eq!(h.state().session.history.undo.len(), undo_steps + 1);
+            for (path, time, value) in
+                [(paths[0], if include_scale { 2.5 } else { 2.0 }, json!([100.0, 200.0, 0.0])), (paths[1], if include_scale { 3.5 } else { 3.0 }, json!(900.0))]
+            {
+                let p = h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop(path).unwrap();
+                assert_eq!(p.keys.len(), 3);
+                assert_eq!(p.keys[2].time.seconds(), time);
+                assert_eq!(p.keys[2].value.to_json(), value);
+            }
+            let scale = h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop(paths[2]).unwrap();
+            if include_scale {
+                assert_eq!(scale.keys.len(), 3);
+                assert_eq!(scale.keys[2].time.seconds(), 2.0);
+                assert_eq!(scale.keys[2].value.to_json(), json!([1500.0, 1600.0, 100.0]));
+            } else {
+                assert_eq!(scale.keys, original[2]);
+            }
+            h.state_mut().session.execute("edit.undo", json!({})).unwrap();
+            for (path, expected) in paths.iter().zip(&original) {
+                assert_eq!(&h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop(path).unwrap().keys, expected);
+            }
+            h.run_steps(2);
+        }
+    }
+}
+
 #[test]
 fn shift_click_toggles_and_ctrl_click_switches_interpolation() {
     let (mut h, id, uid) = harness();