collective
Browse / github.com/storytold/effectcraft / patch
Animation presets: anchor saved effect keyframes at the playhead
[Bug Report] [EffectCraft] Preset Function has faults ### Description I made a shake in effect craft. I made it in the adjustment layer and then i saved the preset to use the shake in my other projects. Now when I apply the preset in a new adjustment layer it does not work. All the effect comes into the layer but the keyframe work that i did in that effect it does not work in that ### Operating system and version Windows 11 pro <!-- ticket-relay:files --> ### Attachments  <!-- /ticket-relay:files --> ### Ticket details
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- tylerjenningsw (imported from pull request #2452, open)
- Base
dd12ed8f9c341a6081e08c8fd840aa3bb4325537- Size
- +132 −6 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
ac5edebbb10d0ad3b5b3f26a
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/animation.rs crates/engine/src/cmd/effect.rs crates/engine/src/tests/effects.rs
@@ -10,11 +10,16 @@ use effectcraft_project::{GroupKind, Layer, LayerId, LayerSource, Node, Project, use effectcraft_time::Tick; use serde_json::{Value, json}; +use super::effect::{effect_time_error, retime_effect}; use super::{CommandSpec, bad, frontend, has_comp, has_keys, has_layers, layer_mut, layers_p, match_path_of, resolve_effect_path, selected_leaf_props, str_p}; use crate::{EngineError, KeyRef, Result, Session, cmd}; /// Preset file format version tag (`*.ecpreset`, JSON). pub const PRESET_TAG: &str = "effectcraftAnimationPreset";+/// The preset format version that `anim.savePreset` writes.+pub const PRESET_VERSION: u64 = 2;+/// The first preset version that stores effect keys relative to the earliest key (#1971).+const RELATIVE_EFFECT_KEYS: u64 = 2; fn has_selected_props(s: &Session) -> std::result::Result<(), String> { has_comp(s)?;@@ -45,13 +50,26 @@ fn selected_effects(s: &Session) -> Vec<(LayerId, PropGroup)> { .collect() } +/// The earliest key anywhere in `groups`, nested controls included.+fn earliest_key<'a>(groups: impl IntoIterator<Item = &'a PropGroup>) -> Option<Tick> {+ let mut first: Option<Tick> = None;+ for g in groups {+ g.walk("", &mut |_, pr| {+ if let Some(t) = pr.keys.iter().map(|k| k.time).min() {+ first = Some(first.map_or(t, |f| f.min(t)));+ }+ });+ }+ first+}+ // ---------------------------------------------------------------- presets fn save_preset(s: &mut Session, p: &Value) -> Result<Value> { let path = str_p(p, "path").ok_or_else(|| bad("anim.savePreset", "missing `path`"))?.to_string(); let comp = s.active_comp().ok_or(EngineError::NoComp)?; let t = s.time();- let effects: Vec<(LayerId, PropGroup)> = selected_effects(s);+ let mut effects: Vec<(LayerId, PropGroup)> = selected_effects(s); let effect_uids: Vec<Uid> = effects.iter().map(|(_, g)| g.uid).collect(); let mut props = vec![]; for (lid, uid) in selected_leaf_props(s) {@@ -67,7 +85,11 @@ fn save_preset(s: &mut Session, p: &Value) -> Result<Value> { return Err(bad("anim.savePreset", "select properties or effects to save")); } // Key times are stored relative to the earliest key so the preset applies at the CTI.- let first = props.iter().flat_map(|(_, _, pr)| pr.keys.iter().map(|k| k.time)).min().unwrap_or(Tick::ZERO);+ let first =+ props.iter().flat_map(|(_, _, pr)| pr.keys.iter().map(|k| k.time)).chain(earliest_key(effects.iter().map(|(_, g)| g))).min().unwrap_or(Tick::ZERO);+ for (_, g) in &mut effects {+ retime_effect(g, 1.0, "anim.savePreset", |t| Ok(Tick(t.0.checked_sub(first.0).ok_or_else(|| effect_time_error("anim.savePreset"))?)))?;+ } let props_json: Vec<Value> = props .iter() .map(|(_, mp, pr)| {@@ -82,7 +104,7 @@ fn save_preset(s: &mut Session, p: &Value) -> Result<Value> { let name = str_p(p, "name") .map(str::to_string) .unwrap_or_else(|| std::path::Path::new(&path).file_stem().map(|s| s.to_string_lossy().to_string()).unwrap_or_default());- let doc = json!({PRESET_TAG: 1, "name": name, "properties": props_json, "effects": fx_json});+ let doc = json!({PRESET_TAG: PRESET_VERSION, "name": name, "properties": props_json, "effects": fx_json}); let text = serde_json::to_string_pretty(&doc).unwrap_or_default(); s.services.write_file(&path, text.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?; Ok(json!({"path": path, "properties": props.len(), "effects": effects.len()}))@@ -110,8 +132,16 @@ fn apply_preset(s: &mut Session, p: &Value) -> Result<Value> { s.save_prefs(); s.prefs_revision += 1; }- let effects: Vec<PropGroup> =+ let mut effects: Vec<PropGroup> = doc["effects"].as_array().map(|a| a.iter().filter_map(|v| serde_json::from_value(v.clone()).ok()).collect()).unwrap_or_default();+ // Older presets stored effect keys at the source layer's times.+ if doc[PRESET_TAG].as_u64().unwrap_or(1) < RELATIVE_EFFECT_KEYS+ && let Some(first) = earliest_key(&effects)+ {+ for g in &mut effects {+ retime_effect(g, 1.0, "anim.applyPreset", |t| Ok(Tick(t.0.checked_sub(first.0).ok_or_else(|| effect_time_error("anim.applyPreset"))?)))?;+ }+ } let props: Vec<(String, Property)> = doc["properties"] .as_array() .map(|a| {@@ -140,6 +170,7 @@ fn apply_preset(s: &mut Session, p: &Value) -> Result<Value> { if let Some(fx) = l.props.sub_mut("effects") { for g in &effects { let mut g = g.clone();+ retime_effect(&mut g, 1.0, "anim.applyPreset", |t| Ok(Tick(t.0.checked_add(lt.0).ok_or_else(|| effect_time_error("anim.applyPreset"))?)))?; g.reassign_uids(&mut next); next += 1; fx.children.push(Node::Group(g));@@ -415,7 +415,7 @@ fn effects_p(s: &Session, p: &Value, cmd: &str) -> Result<Vec<(LayerId, Uid)>> { Ok(sel) } -fn effect_time_error(cmd: &str) -> EngineError {+pub(crate) fn effect_time_error(cmd: &str) -> EngineError { bad(cmd, "an effect keyframe converts outside the supported time range") } @@ -451,7 +451,7 @@ fn effect_time(layer: &Layer, time: Tick, to_comp: bool, cmd: &str) -> Result<Ti /// Change the key time coordinate system throughout an effect, including nested controls. /// Ease speeds are per second in that coordinate system; reversed time swaps the key sides.-fn retime_effect(group: &mut PropGroup, scale: f64, cmd: &str, mut time: impl FnMut(Tick) -> Result<Tick>) -> Result<()> {+pub(crate) fn retime_effect(group: &mut PropGroup, scale: f64, cmd: &str, mut time: impl FnMut(Tick) -> Result<Tick>) -> Result<()> { let mut result = Ok(()); group.walk_mut(&mut |pr| { if result.is_err() {@@ -772,6 +772,101 @@ fn preset_round_trips_effect_properties_by_match_name() { } } +/// Save the selected effects and properties as a preset, then apply it to `target` at `at`.+fn save_and_apply_preset(s: &mut Session, name: &str, selected: Vec<(LayerId, u64)>, target: u64, at: f64) -> serde_json::Value {+ let path = std::env::temp_dir().join(format!("ec-1971-{name}-{}.ecpreset", std::process::id())).to_string_lossy().to_string();+ s.state.selected_props = selected;+ s.execute("anim.savePreset", json!({"path": path})).unwrap();+ s.execute("time.set", json!({"time": at})).unwrap();+ let r = s.execute("anim.applyPreset", json!({"path": path, "layers": [target]}));+ let _ = std::fs::remove_file(&path);+ r.unwrap()+}++/// The comp times of the keys at `path` on `lid`'s `n`th effect (0-based), or on the layer when+/// `n` is `None`.+fn key_times(s: &Session, lid: u64, n: Option<usize>, path: &str) -> Vec<f64> {+ let l = layer(s, lid);+ let group = match n {+ Some(n) => l.effects().unwrap().groups().nth(n).unwrap().clone(),+ None => l.props.clone(),+ };+ group.prop(path).unwrap().keys.iter().map(|k| l.comp_time(k.time).seconds()).collect()+}++/// #1971: a saved effect's keys start at the playhead of the layer it's applied to, as a saved+/// property's do, nested controls included, an effect without keys still applies, and the whole+/// apply is one undo step.+#[test]+fn preset_effect_keys_start_at_the_playhead() {+ let (mut s, a, b) = comp_with_two_solids();+ let blur = apply(&mut s, a, "Gaussian Blur");+ let fractal = apply(&mut s, a, "Fractal");+ let slider = apply(&mut s, a, "Slider Control");+ animate_effect(&mut s, a, blur, "blurriness", &[(5.0, 10.0), (5.5, 30.0), (6.0, 10.0)]);+ animate_effect(&mut s, a, fractal, "mandelbrot/mandelbrotX", &[(5.5, 1.25), (7.0, 2.5)]);+ let before = layer(&s, b);+ let undo = s.history.undo.len();+ let r = save_and_apply_preset(&mut s, "effects", vec![(LayerId(a), blur), (LayerId(a), fractal), (LayerId(a), slider)], b, 0.5);+ assert_eq!(r["applied"], 3, "{r}");+ assert_eq!(key_times(&s, b, Some(0), "blurriness"), vec![0.5, 1.0, 1.5]);+ assert_eq!(key_times(&s, b, Some(1), "mandelbrot/mandelbrotX"), vec![1.0, 2.5]);+ assert_eq!(s.history.undo.len(), undo + 1, "the whole apply is one undo step");+ s.execute("edit.undo", json!({})).unwrap();+ assert_eq!(layer(&s, b), before);+}++/// #1971: effects and properties saved together keep their relative timing: the earliest key of+/// either starts at the playhead.+#[test]+fn preset_effects_and_properties_share_one_anchor() {+ let (mut s, a, b) = comp_with_two_solids();+ let blur = apply(&mut s, a, "Gaussian Blur");+ animate_effect(&mut s, a, blur, "blurriness", &[(5.0, 10.0), (6.0, 30.0)]);+ for (time, value) in [(4.0, -12.0), (7.0, 12.0)] {+ s.execute("prop.addKey", json!({"layer": a, "path": "transform/rotation", "time": time, "timeBase": "comp", "value": value})).unwrap();+ }+ let rotation = layer(&s, a).props.prop("transform/rotation").unwrap().uid;+ save_and_apply_preset(&mut s, "mixed", vec![(LayerId(a), blur), (LayerId(a), rotation)], b, 1.0);+ assert_eq!(key_times(&s, b, None, "transform/rotation"), vec![1.0, 4.0]);+ assert_eq!(key_times(&s, b, Some(0), "blurriness"), vec![2.0, 3.0]);+}++/// #1971: on a stretched layer a saved effect's keys land where a saved property's keys do.+#[test]+fn preset_effect_keys_follow_a_stretched_layer_like_properties() {+ let (mut s, a, b) = comp_with_two_solids();+ let blur = apply(&mut s, a, "Gaussian Blur");+ animate_effect(&mut s, a, blur, "blurriness", &[(5.0, 10.0), (6.0, 30.0)]);+ for (time, value) in [(5.0, -12.0), (6.0, 12.0)] {+ s.execute("prop.addKey", json!({"layer": a, "path": "transform/rotation", "time": time, "timeBase": "comp", "value": value})).unwrap();+ }+ let rotation = layer(&s, a).props.prop("transform/rotation").unwrap().uid;+ set_layer_clock(&mut s, b, 1.0, 200.0);+ save_and_apply_preset(&mut s, "stretched", vec![(LayerId(a), blur), (LayerId(a), rotation)], b, 2.0);+ let property = key_times(&s, b, None, "transform/rotation");+ assert_eq!(property.first(), Some(&2.0), "the property's keys start at the playhead");+ assert_eq!(key_times(&s, b, Some(0), "blurriness"), property, "and the effect's keys land with them");+}++/// #1971: a preset saved before effect keys were stored relative to the earliest key (version 1)+/// has its effects' keys at the source layer's times; they also start at the playhead.+#[test]+fn old_preset_effect_keys_start_at_the_playhead() {+ let (mut s, a, b) = comp_with_two_solids();+ let blur = apply(&mut s, a, "Gaussian Blur");+ animate_effect(&mut s, a, blur, "blurriness", &[(5.0, 10.0), (6.0, 30.0)]);+ let old = json!({+ "effectcraftAnimationPreset": 1,+ "name": "shake",+ "properties": [],+ "effects": [serde_json::to_value(effect_group(&s, a, blur)).unwrap()],+ });+ s.execute("time.set", json!({"time": 0.5})).unwrap();+ s.execute("anim.applyPreset", json!({"preset": old, "layers": [b]})).unwrap();+ assert_eq!(key_times(&s, b, Some(0), "blurriness"), vec![0.5, 1.5]);+}+ /// #2064: `prop.addKey` accepts the `effects/<match name>[#n]/<property>` path that /// `essential.list` reports. #[test]