mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-25 18:47:07 -06:00
fix(engine): copy only the selected keyframes across properties (#1552)
Clicking keyframes on Position, Rotation and Scale highlights those properties, and Copy then added every key of a highlighted property that had none selected, so deselected keys came back on paste. The keyframe copy branch also left a stale value clipboard that Paste preferred over the new keys. Copy now takes whole animations only when no key is selected or the Transform group is selected, and every copy branch clears the clipboards. Also collapses a nested `if` in the STL importer into a match guard, which Clippy 1.95 flags under -D warnings (no behaviour change). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
2c08f675c2
commit
c4c079d4f4
@@ -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 }));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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 == ¬e)));
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user