Merge pull request #522 from sanyamsingh001/fix/property-key-time-bases

This commit is contained in:
Claude committed 2026-10-10 12:49:17 +00:00
commit 21a9486d8a
8 files changed
+661 -37

No files matched your search

+23 -11
View File
@@ -6,7 +6,7 @@ use effectcraft_project::{LayerId, Property, Uid};
use effectcraft_time::Tick;
use serde_json::{Value, json};
use super::prop::prop_ref;
use super::prop::{KeyTimeBase, key_time, key_timing, prop_ref};
use super::{CommandSpec, bad, comp_id, f_p, has_comp, has_keys, layers_p, merge_p, str_p, time_p};
use crate::{EngineError, KeyClip, KeyRef, Result, Session, cmd, query};
@@ -206,11 +206,13 @@ fn nudge(s: &mut Session, p: &Value) -> Result<Value> {
/// Edit one key directly: move it in time and/or set its value (Graph Editor drags).
fn set_key(s: &mut Session, p: &Value) -> Result<Value> {
let (cid, lid, uid) = prop_ref(s, p, "keys.set")?;
let t = Tick::from_seconds_f64(f_p(p, "time").ok_or_else(|| bad("keys.set", "missing `time` (layer time of the key)"))?);
let new_t = f_p(p, "newTime").map(Tick::from_seconds_f64);
let base = KeyTimeBase::from_params(p, "keys.set")?;
let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?;
let layer = comp.layer(lid).ok_or(EngineError::NoComp)?;
let t = key_time(layer, comp.frame_rate, p, "time", base, "keys.set", false)?.ok_or_else(|| bad("keys.set", "missing `time` (layer time by default)"))?;
let new_t = key_time(layer, comp.frame_rate, p, "newTime", base, "keys.set", true)?;
let value = p.get("value").cloned();
let fr = s.project.comp(cid).map(|c| c.frame_rate);
s.edit("Edit Keyframe", merge_p(p), |proj, st| {
let nt = s.edit("Edit Keyframe", merge_p(p), |proj, st| {
let l = super::layer_mut(proj, cid, lid)?;
let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.set", "no property"))?;
let i = nearest_key(&pr.keys, t).ok_or_else(|| bad("keys.set", "the property has no keyframes"))?;
@@ -221,10 +223,10 @@ fn set_key(s: &mut Session, p: &Value) -> Result<Value> {
}
let mut nt = old_t;
if let Some(n) = new_t {
nt = fr.map(|r| r.snap_nearest(n)).unwrap_or(n);
nt = n;
// Keys can't pass their neighbours or share a time.
let lo = if i > 0 { pr.keys[i - 1].time.0 + 1 } else { i64::MIN };
let hi = if i + 1 < pr.keys.len() { pr.keys[i + 1].time.0 - 1 } else { i64::MAX };
let lo = if i > 0 { pr.keys[i - 1].time.0.saturating_add(1) } else { i64::MIN };
let hi = if i + 1 < pr.keys.len() { pr.keys[i + 1].time.0.saturating_sub(1) } else { i64::MAX };
nt = Tick(nt.0.clamp(lo, hi));
pr.keys[i].time = nt;
}
@@ -233,8 +235,10 @@ fn set_key(s: &mut Session, p: &Value) -> Result<Value> {
for k in st.selected_keys.iter_mut().filter(|k| k.layer == lid && k.prop == uid && k.time == old_t) {
k.time = nt;
}
Ok(json!({"time": nt.seconds()}))
})
Ok(nt)
})?;
let layer = s.project.comp(cid).and_then(|c| c.layer(lid)).ok_or(EngineError::NoComp)?;
Ok(key_timing(layer, nt))
}
fn nearest_key(keys: &[Keyframe], t: Tick) -> Option<usize> {
@@ -449,7 +453,15 @@ pub fn specs() -> Vec<CommandSpec> {
s,
&json!({"frames": -10})
)),
cmd!("keys.set", "Edit Keyframe", [], None, "{layer?, path|prop, time (layer s), newTime?, value?, merge?}", has_comp, set_key),
cmd!(
"keys.set",
"Edit Keyframe",
[],
None,
"{layer?, path|prop, time (layer s by default), newTime? (same base), timeBase?: layer|comp, value?, merge?}",
has_comp,
set_key
),
cmd!("keys.selectEqual", "Select Equal Keyframes", [], None, "{}", has_keys, |s, _| select_related(s, "equal")),
cmd!("keys.selectPrevious", "Select Previous Keyframes", [], None, "{}", has_keys, |s, _| select_related(s, "previous")),
cmd!("keys.selectFollowing", "Select Following Keyframes", [], None, "{}", has_keys, |s, _| select_related(s, "following")),
+164 -21
View File
@@ -2,7 +2,7 @@
use effectcraft_keyframe::{Ease, Interp, Keyframe, easy_ease, key_at, set_key};
use effectcraft_project::{GroupKind, ItemId, Layer, LayerId, Property, Uid};
use effectcraft_time::Tick;
use effectcraft_time::{FrameRate, Tick};
use serde_json::{Value, json};
use super::{CommandSpec, b_p, bad, f_p, has_comp, has_keys, has_layers, layer_mut, layer_p, layers_p, merge_p, str_p};
@@ -29,7 +29,7 @@ fn with_prop<T>(
uid: Uid,
f: impl FnOnce(&mut Property, Tick) -> Result<T>,
) -> Result<T> {
let t = s.time();
let t = s.time_of(cid);
let linear = s.prefs.general.default_spatial_linear;
s.edit(label, merge, |proj, _| {
let l = layer_mut(proj, cid, lid)?;
@@ -50,12 +50,106 @@ fn with_prop<T>(
})
}
/// An explicit key time (layer seconds) moved onto the nearest comp frame.
fn snapped_key_time(s: &Session, cid: ItemId, lid: LayerId, p: &Value) -> Option<Tick> {
let t = Tick::from_seconds_f64(f_p(p, "time")?);
let comp = s.project.comp(cid)?;
let l = comp.layer(lid)?;
Some(l.layer_time(comp.frame_rate.snap_nearest(l.comp_time(t))))
/// Explicit command key times default to layer time; readers and the CTI use comp time.
#[derive(Clone, Copy)]
pub(crate) enum KeyTimeBase {
Layer,
Comp,
}
impl KeyTimeBase {
pub(crate) fn from_params(p: &Value, cmd: &str) -> Result<Self> {
match p.get("timeBase") {
None => Ok(Self::Layer),
Some(Value::String(b)) if b == "layer" => Ok(Self::Layer),
Some(Value::String(b)) if b == "comp" => Ok(Self::Comp),
Some(_) => Err(bad(cmd, "`timeBase` must be layer or comp")),
}
}
}
/// Convert a supplied time to stored layer ticks, snapping writes in the comp's frame grid.
pub(crate) fn key_time(layer: &Layer, frame_rate: FrameRate, p: &Value, param: &str, base: KeyTimeBase, cmd: &str, snap: bool) -> Result<Option<Tick>> {
let Some(value) = p.get(param) else { return Ok(None) };
let seconds = value
.as_f64()
.filter(|t| t.is_finite() && *t >= Tick::MIN.seconds() && *t <= Tick::MAX.seconds())
.ok_or_else(|| bad(cmd, format!("`{param}` must be finite seconds within the supported time range")))?;
if !layer.stretch.is_finite() || layer.stretch == 0.0 {
return Err(bad(cmd, "the layer has an invalid time stretch"));
}
let t = Tick::from_seconds_f64(seconds);
let conversion_error = || bad(cmd, format!("`{param}` converts outside the supported time range"));
let in_range = |ticks: i64| (Tick::MIN.0..=Tick::MAX.0).contains(&ticks);
// Validate before Layer's saturating conversion: otherwise a huge stretch can silently
// clamp an intended key to a different comp time while still producing a valid Tick.
let scaled_tick = |ticks: f64| -> Result<Tick> {
if !ticks.is_finite() || ticks < Tick::MIN.0 as f64 || ticks > Tick::MAX.0 as f64 {
return Err(conversion_error());
}
let ticks = ticks as i64;
if !in_range(ticks) {
return Err(conversion_error());
}
Ok(Tick(ticks))
};
let normal = (layer.stretch - 100.0).abs() < 1e-9;
let ct = match base {
KeyTimeBase::Layer => {
let offset = if normal { t } else { scaled_tick(t.0 as f64 * layer.stretch / 100.0)? };
Tick(layer.start_time.0.checked_add(offset.0).filter(|t| in_range(*t)).ok_or_else(conversion_error)?)
}
KeyTimeBase::Comp => t,
};
let ct = if snap { frame_rate.snap_nearest(ct) } else { ct };
if !in_range(ct.0) {
return Err(conversion_error());
}
let offset = ct.0.checked_sub(layer.start_time.0).filter(|t| in_range(*t)).ok_or_else(conversion_error)?;
let lt = if normal { Tick(offset) } else { scaled_tick(offset as f64 * 100.0 / layer.stretch)? };
// Unsnapped legacy layer times remain exact, rather than making a float round-trip.
Ok(Some(if !snap && matches!(base, KeyTimeBase::Layer) { t } else { lt }))
}
pub(crate) fn key_timing(layer: &Layer, t: Tick) -> Value {
let ct = layer.comp_time(t);
let mut out = json!({"time": t.seconds(), "layerTime": t.seconds(), "compTime": ct.seconds()});
if !layer.is_active_at(ct) {
out["warning"] = json!(format!(
"Keyframe at comp time {} s is outside the layer's active range [{} s, {} s).",
ct.seconds(),
layer.in_point.seconds(),
layer.out_point.seconds()
));
}
out
}
/// Omitted bases keep the original JSON reply; explicit bases opt in to timing metadata.
fn key_reply(p: &Value, mut timing: Value, field: &str, result: Value) -> Value {
if p.get("timeBase").is_some() {
timing[field] = result;
timing
} else {
result
}
}
fn snapped_key_time(s: &Session, cid: ItemId, lid: LayerId, p: &Value, cmd: &str) -> Result<Option<Tick>> {
let base = KeyTimeBase::from_params(p, cmd)?;
let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?;
let layer = comp.layer(lid).ok_or(EngineError::NoComp)?;
key_time(layer, comp.frame_rate, p, "time", base, cmd, true)
}
fn property_reply(p: &Value, layer: &Layer, pr: &Property, time: Tick, value: Value) -> Value {
if p.get("timeBase").is_some() {
let mut timing = key_timing(layer, time);
timing["animated"] = json!(pr.is_animated());
key_reply(p, timing, "value", value)
} else {
value
}
}
fn set(s: &mut Session, p: &Value) -> Result<Value> {
@@ -87,7 +181,7 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
return Ok(json!(out));
}
let v = p.get("value").ok_or_else(|| bad("prop.set", "missing `value`"))?.clone();
let at = snapped_key_time(s, cid, lid, p);
let at = snapped_key_time(s, cid, lid, p, "prop.set")?;
let out = with_prop(s, "Change Property", merge_p(p), cid, lid, uid, |pr, lt| {
let cur = pr.value_at(lt);
let mut nv = cur.coerce_json(&v).ok_or_else(|| bad("prop.set", format!("can't use {v} for a {} property", cur.kind_name())))?;
@@ -100,7 +194,12 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
pr.set_value_at(at.unwrap_or(lt), nv.clone());
Ok(nv.to_json())
})?;
Ok(out)
if p.get("timeBase").is_none() {
return Ok(out);
}
let layer = s.project.comp(cid).and_then(|c| c.layer(lid)).ok_or(EngineError::NoComp)?;
let pr = layer.props.find(uid).ok_or_else(|| bad("prop.set", "property vanished"))?;
Ok(property_reply(p, layer, pr, at.unwrap_or_else(|| layer.layer_time(s.time_of(cid))), out))
}
/// Read one property: value at `time` (comp seconds, default the CTI), keyframes and expression.
@@ -108,9 +207,18 @@ fn get(s: &mut Session, p: &Value) -> Result<Value> {
let (cid, lid, uid) = prop_ref(s, p, "prop.get")?;
let l = s.project.comp(cid).and_then(|c| c.layer(lid)).ok_or(EngineError::NoComp)?;
let pr = l.props.find(uid).ok_or_else(|| bad("prop.get", "property vanished"))?;
let t = f_p(p, "time").map(Tick::from_seconds_f64).unwrap_or_else(|| s.time());
let keys: Vec<Value> =
pr.keys.iter().map(|k| json!({"time": k.time.seconds(), "value": k.value.to_json(), "in": k.in_interp.label(), "out": k.out_interp.label()})).collect();
let t = f_p(p, "time").map(Tick::from_seconds_f64).unwrap_or_else(|| s.time_of(cid));
let keys: Vec<Value> = pr
.keys
.iter()
.map(|k| {
let mut info = key_timing(l, k.time);
info["value"] = k.value.to_json();
info["in"] = json!(k.in_interp.label());
info["out"] = json!(k.out_interp.label());
info
})
.collect();
let raw = pr.value_at(l.layer_time(t));
let mut out = json!({
"layer": lid.0, "uid": pr.uid, "match": pr.match_id, "name": pr.name, "type": pr.value.kind_name(),
@@ -156,9 +264,9 @@ fn toggle_anim(s: &mut Session, p: &Value) -> Result<Value> {
fn add_key(s: &mut Session, p: &Value) -> Result<Value> {
let (cid, lid, uid) = prop_ref(s, p, "prop.addKey")?;
let at = snapped_key_time(s, cid, lid, p);
let at = snapped_key_time(s, cid, lid, p, "prop.addKey")?;
let value = p.get("value").cloned();
with_prop(s, "Add Keyframe", None, cid, lid, uid, |pr, lt| {
let count = with_prop(s, "Add Keyframe", None, cid, lid, uid, |pr, lt| {
let t = at.unwrap_or(lt);
let cur = pr.value_at(t);
let v = match &value {
@@ -171,7 +279,12 @@ fn add_key(s: &mut Session, p: &Value) -> Result<Value> {
}
set_key(&mut pr.keys, k);
Ok(json!(pr.keys.len()))
})
})?;
if p.get("timeBase").is_none() {
return Ok(count);
}
let layer = s.project.comp(cid).and_then(|c| c.layer(lid)).ok_or(EngineError::NoComp)?;
Ok(key_reply(p, key_timing(layer, at.unwrap_or_else(|| layer.layer_time(s.time_of(cid)))), "count", count))
}
fn toggle_key(s: &mut Session, p: &Value) -> Result<Value> {
@@ -366,6 +479,7 @@ fn path_points(l: &Layer, uid: Uid, lt: Tick) -> Option<(Uid, usize)> {
// ---------------------------------------------------------------- keyframes
fn select_keys(s: &mut Session, p: &Value) -> Result<Value> {
let base = KeyTimeBase::from_params(p, "keys.select")?;
let comp = s.active_comp().ok_or(EngineError::NoComp)?;
let mut sel = vec![];
if let Some(Value::Array(keys)) = p.get("keys") {
@@ -382,7 +496,7 @@ fn select_keys(s: &mut Session, p: &Value) -> Result<Value> {
_ => return Err(bad("keys.select", format!("{k}: give `prop` (uid) or `path`"))),
}
.ok_or_else(|| bad("keys.select", format!("{k}: no such property on layer {lv}")))?;
let t = k.get("time").and_then(Value::as_f64).map(Tick::from_seconds_f64).ok_or_else(|| bad("keys.select", format!("{k}: missing `time`")))?;
let t = key_time(ly, comp.frame_rate, k, "time", base, "keys.select", false)?.ok_or_else(|| bad("keys.select", format!("{k}: missing `time`")))?;
// Snap to the stored key time.
let kt = pr
.keys
@@ -432,7 +546,28 @@ fn select_keys(s: &mut Session, p: &Value) -> Result<Value> {
}
}
}
Ok(json!(s.state.selected_keys.len()))
if p.get("timeBase").is_none() {
return Ok(json!(s.state.selected_keys.len()));
}
let comp = s.active_comp().ok_or(EngineError::NoComp)?;
let keys: Vec<Value> = s
.state
.selected_keys
.iter()
.filter_map(|k| {
let layer = comp.layer(k.layer)?;
let mut timing = key_timing(layer, k.time);
timing["layer"] = json!(k.layer.0);
timing["prop"] = json!(k.prop);
Some(timing)
})
.collect();
let warnings: Vec<&str> = keys.iter().filter_map(|k| k.get("warning").and_then(Value::as_str)).collect();
let mut out = json!({"count": s.state.selected_keys.len(), "keys": keys});
if !warnings.is_empty() {
out["warning"] = json!(warnings.join(" "));
}
Ok(out)
}
/// Apply `f` to every selected key; keeps the selection pointing at moved keys.
@@ -800,9 +935,17 @@ fn convert_expr_to_keys(s: &mut Session, p: &Value) -> Result<Value> {
pub fn specs() -> Vec<CommandSpec> {
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.set",
"Set Property Value",
[],
None,
"{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?}",
has_layers,
set
),
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.addKey", "Add Keyframe", [], None, "{layer?, path|prop, time? (layer s by default), timeBase?: layer|comp, value?}", has_layers, add_key),
cmd!("prop.toggleKey", "Add or Remove Keyframe at Current Time", [], None, "{layer?, path|prop}", has_layers, toggle_key),
cmd!(
"keys.toggleTransform",
@@ -838,7 +981,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Select Keyframes",
[],
None,
"{keys: [{layer, prop: uid | path (or `path`), time (layer s)}], add?, toggle?: bool (Shift+click: in or out of the selection), selectProperties?: bool (their properties and layers too, as a Timeline click does)}",
"{keys: [{layer, prop: uid | path (or `path`), time (layer s by default)}], timeBase?: layer|comp, add?, toggle?: bool (Shift+click: in or out of the selection), selectProperties?: bool (their properties and layers too, as a Timeline click does)}",
has_comp,
select_keys
),
+3
View File
@@ -1167,3 +1167,6 @@ mod tests_paint;
mod tests_panels;
#[cfg(test)]
mod tests_roto;
#[cfg(test)]
mod tests_key_time;
+149
View File
@@ -0,0 +1,149 @@
//! Agent keyframe times on offset, stretched and reversed layers (#257).
use effectcraft_project::LayerId;
use effectcraft_time::Tick;
use serde_json::{Value, json};
use crate::{Session, tests_timeline::prop};
fn offset_layer(start: f64, stretch: f64) -> (Session, u64) {
let mut s = Session::default();
s.execute("comp.new", json!({"name": "Offset", "duration": 20, "frameRate": 30})).unwrap();
let layer = s.execute("layer.newSolid", json!({"name": "A", "color": "#ffffff"})).unwrap()["layer"].as_u64().unwrap();
s.execute("layer.timing", json!({"layer": layer, "start": start, "in": 10.5, "out": 20})).unwrap();
let cid = s.active_comp_id().unwrap();
std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(layer)).unwrap().stretch = stretch;
(s, layer)
}
fn assert_seconds(value: &Value, seconds: f64) {
assert!((value.as_f64().unwrap() - seconds).abs() < 1e-8, "{value} != {seconds}");
}
#[test]
fn comp_key_times_on_an_offset_layer_read_at_the_intended_time_and_undo() {
let (mut s, layer) = offset_layer(10.5, 100.0);
for (time, value) in [(15.3, 0), (16.0, 100)] {
let reply = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": time, "timeBase": "comp", "value": value})).unwrap();
assert_seconds(&reply["compTime"], time);
assert_seconds(&reply["layerTime"], time - 10.5);
assert!(reply.get("warning").is_none());
}
let read = s.execute("prop.get", json!({"layer": layer, "path": "transform/opacity", "time": 15.65})).unwrap();
assert_seconds(&read["value"], 50.0);
assert_seconds(&read["keys"][0]["compTime"], 15.3);
let reply = s.execute("prop.set", json!({"layer": layer, "path": "transform/opacity", "time": 15.5, "timeBase": "comp", "value": 45})).unwrap();
assert_seconds(&reply["compTime"], 15.5);
assert_eq!(prop(&s, layer, "transform/opacity").keys.len(), 3);
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(prop(&s, layer, "transform/opacity").keys.len(), 2);
s.execute("edit.redo", json!({})).unwrap();
assert_eq!(prop(&s, layer, "transform/opacity").keys.len(), 3);
}
#[test]
fn default_layer_times_are_preserved_and_outside_keys_warn() {
let (mut s, layer) = offset_layer(10.5, 100.0);
let legacy = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": 4.8, "value": 0})).unwrap();
assert_eq!(legacy, json!(1), "ordinary legacy replies stay scalar");
let outside = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": 15.3, "value": 100})).unwrap();
assert_eq!(outside, json!(2), "legacy replies keep their raw count outside the active range");
let read = s.execute("prop.get", json!({"layer": layer, "path": "transform/opacity"})).unwrap();
assert_seconds(&read["keys"][1]["time"], 15.3);
assert_seconds(&read["keys"][1]["compTime"], 25.8);
assert!(read["keys"][1].get("warning").is_some());
let selected = s.execute("keys.select", json!({"keys": [{"layer": layer, "path": "transform/opacity", "time": 25.8}], "timeBase": "comp"})).unwrap();
assert_seconds(&selected["keys"][0]["compTime"], 25.8);
assert!(selected.get("warning").is_some());
let moved = s.execute("keys.set", json!({"layer": layer, "path": "transform/opacity", "time": 15.3, "newTime": 9.5, "timeBase": "layer"})).unwrap();
assert_seconds(&moved["compTime"], 20.0);
assert!(moved.get("warning").is_some(), "out point is exclusive");
}
#[test]
fn stretched_and_reversed_keys_use_the_comp_frame_grid_for_both_bases() {
for (start, stretch) in [(10.5, 200.0), (18.0, -200.0)] {
let (mut s, layer) = offset_layer(start, stretch);
for time in [15.3, 16.0] {
let layer_time = (time - start) * 100.0 / stretch;
let comp_reply =
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": time, "timeBase": "comp", "value": 20})).unwrap();
assert_seconds(&comp_reply["layerTime"], layer_time);
let layer_reply =
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/rotation", "time": layer_time, "timeBase": "layer", "value": 20})).unwrap();
assert_seconds(&layer_reply["compTime"], time);
}
let selected = s.execute("keys.select", json!({"keys": [{"layer": layer, "path": "transform/opacity", "time": 15.3}], "timeBase": "comp"})).unwrap();
assert_seconds(&selected["keys"][0]["compTime"], 15.3);
let moved = s
.execute("keys.set", json!({"layer": layer, "path": "transform/opacity", "time": 15.3, "newTime": 15.41, "timeBase": "comp", "value": 60}))
.unwrap();
assert_seconds(&moved["compTime"], 15.4);
let ly = s.active_comp().unwrap().layer(LayerId(layer)).unwrap();
assert_eq!(ly.comp_time(s.state.selected_keys[0].time), Tick::from_seconds_f64(15.4));
let layer_reply = s.execute("keys.set", json!({"layer": layer, "path": "transform/rotation", "time": (15.3 - start) * 100.0 / stretch, "newTime": (15.41 - start) * 100.0 / stretch, "timeBase": "layer"})).unwrap();
assert_seconds(&layer_reply["compTime"], 15.4);
s.execute("edit.undo", json!({})).unwrap();
s.execute("edit.undo", json!({})).unwrap();
let ly = s.active_comp().unwrap().layer(LayerId(layer)).unwrap();
let restored = prop(&s, layer, "transform/opacity");
assert!(restored.keys.iter().any(|k| ly.comp_time(k.time) == Tick::from_seconds_f64(15.3)));
}
}
#[test]
fn invalid_key_time_arguments_leave_properties_and_history_unchanged() {
let (mut s, layer) = offset_layer(10.5, 100.0);
let before = prop(&s, layer, "transform/opacity");
let history_len = s.history.undo.len();
for cmd in ["prop.addKey", "prop.set", "keys.set"] {
for extra in [json!({"time": "oops"}), json!({"time": 1e300}), json!({"timeBase": "seconds", "time": 0})] {
let mut p = json!({"layer": layer, "path": "transform/opacity", "value": 10});
p.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone());
assert!(s.execute(cmd, p).is_err(), "{cmd}: {extra}");
assert_eq!(prop(&s, layer, "transform/opacity"), before);
assert_eq!(s.history.undo.len(), history_len);
}
}
assert!(s.execute("keys.select", json!({"keys": [], "timeBase": null})).is_err());
}
#[test]
fn omitted_key_times_and_reads_use_the_target_comps_cti() {
let (mut s, layer) = offset_layer(10.5, 100.0);
let target = s.active_comp_id().unwrap().0;
s.execute("time.set", json!({"time": 15.3})).unwrap();
s.execute("comp.new", json!({"name": "Other", "duration": 20})).unwrap();
s.execute("time.set", json!({"time": 1})).unwrap();
let reply = s.execute("prop.addKey", json!({"comp": target, "layer": layer, "path": "transform/opacity", "timeBase": "comp", "value": 25})).unwrap();
assert_seconds(&reply["compTime"], 15.3);
let read = s.execute("prop.get", json!({"comp": target, "layer": layer, "path": "transform/opacity"})).unwrap();
assert_seconds(&read["time"], 15.3);
assert_seconds(&read["value"], 25.0);
}
#[test]
fn excessive_stretch_or_start_conversions_fail_without_clamping_keys() {
for (start, stretch, base, time) in [(10.5, 1e300, "layer", 1.0), (10.5, 1e-300, "comp", 15.3), (Tick::MAX.seconds(), 100.0, "layer", 1.0)] {
let (mut s, layer) = offset_layer(10.5, 100.0);
let cid = s.active_comp_id().unwrap();
let ly = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(layer)).unwrap();
ly.start_time = Tick::from_seconds_f64(start);
ly.stretch = stretch;
let before = s.history.undo.len();
let result = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": time, "timeBase": base, "value": 10}));
assert!(result.is_err(), "start {start}, stretch {stretch}: {result:?}");
assert_eq!(s.history.undo.len(), before);
assert!(prop(&s, layer, "transform/opacity").keys.is_empty());
}
}
#[test]
fn explicit_base_on_static_property_reports_timing_without_enabling_animation() {
let (mut s, layer) = offset_layer(10.5, 100.0);
let reply = s.execute("prop.set", json!({"layer": layer, "path": "transform/opacity", "time": 15.3, "timeBase": "comp", "value": 25})).unwrap();
assert_seconds(&reply["compTime"], 15.3);
assert_eq!(reply["animated"], false);
assert_eq!(reply["value"], 25.0);
assert!(prop(&s, layer, "transform/opacity").keys.is_empty());
}
@@ -0,0 +1,77 @@
//! A checked comp time must remain supported after nearest-frame rounding.
use effectcraft_engine::Session;
use effectcraft_project::LayerId;
use effectcraft_time::{TICKS_PER_SECOND, Tick};
use serde_json::{Value, json};
fn project(s: &Session) -> Value {
serde_json::to_value(s.project.as_ref()).unwrap()
}
fn stored_comp_time(s: &Session, layer: u64, value: f64) -> Tick {
let owner = s.active_comp().unwrap().layer(LayerId(layer)).unwrap();
let property = owner.props.prop("transform/opacity").unwrap();
let key = property.keys.iter().find(|key| key.value.to_json().as_f64() == Some(value)).unwrap();
owner.comp_time(key.time)
}
fn check_bound(upper: bool) {
let mut s = Session::default();
s.execute("comp.new", json!({"name": "Post-snap bound", "width": 80, "height": 60, "duration": 20, "frameRate": 25})).unwrap();
let layer = s.execute("layer.newSolid", json!({"name": "A", "color": "#ffffff"})).unwrap()["layer"].as_u64().unwrap();
let start = if upper { 10.0 } else { -10.0 };
s.execute("layer.timing", json!({"layer": layer, "start": start})).unwrap();
let owner = s.active_comp().unwrap().layer(LayerId(layer)).unwrap();
assert_eq!(owner.start_time, Tick::from_seconds_f64(start), "ordinary frame-aligned start");
assert_eq!(owner.stretch, 100.0);
let bound = if upper { Tick::MAX } else { Tick::MIN };
let nearby = if upper { bound - Tick(TICKS_PER_SECOND) } else { bound + Tick(TICKS_PER_SECOND) };
let control_history = s.history.undo.len();
let control =
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": nearby.seconds(), "timeBase": "comp", "value": 25})).unwrap();
assert_eq!(control["count"].as_u64(), Some(1), "nearby in-range key succeeds");
let actual_control = stored_comp_time(&s, layer, 25.0);
assert!((Tick::MIN..=Tick::MAX).contains(&actual_control), "actual nearby comp time remains supported");
assert!(
actual_control.0.abs_diff(nearby.0) < (TICKS_PER_SECOND / 25) as u64,
"nearby control remains within one ordinary 25fps frame of its requested time"
);
assert_eq!(s.history.undo.len(), control_history + 1, "control records one edit");
eprintln!("BOUND_NEARBY_CONTROL upper={upper} requested={} actual={} reply={control}", nearby.0, actual_control.0);
let before = project(&s);
let history_before = s.history.undo.len();
let redo_before = s.history.redo.len();
let outcome = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": bound.seconds(), "timeBase": "comp", "value": 75}));
if let Ok(reply) = &outcome {
assert_eq!(reply["count"].as_u64(), Some(2), "the accepted boundary call added its key");
let actual = stored_comp_time(&s, layer, 75.0);
let exceeds = if upper { actual > Tick::MAX } else { actual < Tick::MIN };
assert!(exceeds, "an accepted boundary call must prove the predicted outward comp-time snap");
assert_eq!(s.history.undo.len(), history_before + 1, "the accepted boundary call is an actual edit");
eprintln!(
"BOUND_OUTWARD_ACCEPTED upper={upper} requested={} actual={} history_before={} history_after={} reply={reply}",
bound.0,
actual.0,
history_before,
s.history.undo.len()
);
}
assert!(outcome.is_err(), "nearest-frame rounding beyond the supported comp-time bound must reject before an edit: upper={upper}, outcome={outcome:?}");
let error = outcome.unwrap_err().to_string();
assert!(error.contains("converts outside the supported time range"), "contextual conversion error: {error}");
assert_eq!(project(&s), before, "rejected post-snap time leaves the full project unchanged");
assert_eq!(s.history.undo.len(), history_before, "rejected post-snap time records no Undo step");
assert_eq!(s.history.redo.len(), redo_before, "rejected post-snap time leaves Redo unchanged");
}
#[test]
fn upper_supported_comp_time_is_rejected_after_outward_frame_snap() {
check_bound(true);
}
#[test]
fn lower_supported_comp_time_is_rejected_after_outward_frame_snap() {
check_bound(false);
}
@@ -0,0 +1,201 @@
//! Omitted time bases retain existing result shapes even for valid keys outside [in, out).
use effectcraft_engine::Session;
use effectcraft_project::LayerId;
use effectcraft_time::Tick;
use serde_json::{Value, json};
const LAYER_TIME: f64 = 15.3;
const COMP_TIME: f64 = 25.8;
fn setup(text: bool) -> (Session, u64) {
let mut s = Session::default();
s.execute("comp.new", json!({"name": "Reply contract", "width": 80, "height": 60, "duration": 20, "frameRate": 30})).unwrap();
let created = if text {
s.execute("layer.newText", json!({"name": "Document", "text": "Before"})).unwrap()
} else {
s.execute("layer.newSolid", json!({"name": "A", "color": "#ffffff"})).unwrap()
};
let layer = created["layer"].as_u64().unwrap();
s.execute("layer.timing", json!({"layer": layer, "start": 10.5, "in": 10.5, "out": 20})).unwrap();
(s, layer)
}
fn project(s: &Session) -> Value {
serde_json::to_value(s.project.as_ref()).unwrap()
}
fn read(s: &mut Session, layer: u64, path: &str, time: f64) -> Value {
s.execute("prop.get", json!({"layer": layer, "path": path, "time": time})).unwrap()
}
fn outside_key(s: &Session, layer: u64, path: &str, layer_time: f64, comp_time: f64) {
let comp = s.active_comp().unwrap();
let owner = comp.layer(LayerId(layer)).unwrap();
let property = owner.props.prop(path).unwrap();
let at = Tick::from_seconds_f64(layer_time);
assert!(property.keys.iter().any(|key| key.time == at), "actual stored layer-time key");
assert_eq!(owner.comp_time(at), Tick::from_seconds_f64(comp_time), "actual comp-time mapping");
assert!(!owner.is_active_at(owner.comp_time(at)), "authored key is outside the active layer range");
}
fn undo_redo(s: &mut Session, before: &Value, after: &Value, history_before: usize) {
assert_ne!(before, after, "the public edit changed the project");
assert_eq!(s.history.undo.len(), history_before + 1, "one public edit or batch Undo step");
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(project(s), *before, "complete project Undo");
s.execute("edit.redo", json!({})).unwrap();
assert_eq!(project(s), *after, "complete project Redo");
}
fn seconds(actual: &Value, expected: f64) {
assert!((actual.as_f64().unwrap() - expected).abs() < 1e-8, "{actual} != {expected}");
}
#[test]
fn legacy_outside_add_key_keeps_its_numeric_count_and_undo() {
let (mut s, layer) = setup(false);
let positive = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": 4.8, "value": 20})).unwrap();
assert_eq!(positive.as_u64(), Some(1), "ordinary in-range numeric reply positive");
let before = project(&s);
let history_before = s.history.undo.len();
let reply = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "value": 60})).unwrap();
outside_key(&s, layer, "transform/opacity", LAYER_TIME, COMP_TIME);
let after = project(&s);
undo_redo(&mut s, &before, &after, history_before);
eprintln!("LEGACY_REPLY addKey outside: {reply}");
assert_eq!(reply.as_u64(), Some(2), "omitted timeBase must preserve the numeric addKey count outside the active range");
}
#[test]
fn legacy_outside_scalar_set_keeps_its_value_and_undo() {
let (mut s, layer) = setup(false);
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": 4.8, "value": 20})).unwrap();
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "value": 60})).unwrap();
let before = project(&s);
let history_before = s.history.undo.len();
let reply = s.execute("prop.set", json!({"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "value": 40})).unwrap();
outside_key(&s, layer, "transform/opacity", LAYER_TIME, COMP_TIME);
seconds(&read(&mut s, layer, "transform/opacity", COMP_TIME)["value"], 40.0);
let after = project(&s);
undo_redo(&mut s, &before, &after, history_before);
eprintln!("LEGACY_REPLY scalar set outside: {reply}");
assert_eq!(reply.as_f64(), Some(40.0), "omitted timeBase must preserve the scalar set value outside the active range");
}
#[test]
fn legacy_outside_selection_keeps_its_numeric_count_without_an_edit() {
let (mut s, layer) = setup(false);
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "value": 60})).unwrap();
outside_key(&s, layer, "transform/opacity", LAYER_TIME, COMP_TIME);
let before = project(&s);
let history_before = s.history.undo.len();
let reply = s.execute("keys.select", json!({"keys": [{"layer": layer, "path": "transform/opacity", "time": LAYER_TIME}]})).unwrap();
assert_eq!(s.state.selected_keys.len(), 1, "actual owner selected");
assert_eq!(s.state.selected_keys[0].layer, LayerId(layer));
assert_eq!(s.state.selected_keys[0].time, Tick::from_seconds_f64(LAYER_TIME));
assert_eq!(project(&s), before);
assert_eq!(s.history.undo.len(), history_before);
eprintln!("LEGACY_REPLY select outside: {reply}");
assert_eq!(reply.as_u64(), Some(1), "omitted timeBase must preserve the numeric selection count outside the active range");
}
#[test]
fn legacy_outside_position_set_keeps_its_array_value_and_undo() {
let (mut s, layer) = setup(false);
s.execute("prop.addKey", json!({"layer": layer, "path": "transform/position", "time": LAYER_TIME, "value": [10, 20, 0]})).unwrap();
let before = project(&s);
let history_before = s.history.undo.len();
let reply = s.execute("prop.set", json!({"layer": layer, "path": "transform/position", "time": LAYER_TIME, "value": [12, 34, 0]})).unwrap();
outside_key(&s, layer, "transform/position", LAYER_TIME, COMP_TIME);
let actual = read(&mut s, layer, "transform/position", COMP_TIME)["value"].clone();
let values = actual.as_array().unwrap();
assert_eq!(values.len(), 3, "ordinary Position value remains a vector");
seconds(&values[0], 12.0);
seconds(&values[1], 34.0);
seconds(&values[2], 0.0);
let after = project(&s);
undo_redo(&mut s, &before, &after, history_before);
eprintln!("LEGACY_REPLY array set outside: {reply}");
assert_eq!(reply, actual, "omitted timeBase must return the raw array value rather than a timing wrapper");
}
#[test]
fn legacy_outside_text_set_keeps_its_string_value_and_undo() {
let (mut s, layer) = setup(true);
let source = read(&mut s, layer, "text/sourceText", 10.5)["value"].clone();
assert_eq!(source.as_str(), Some("Before"), "public Source Text returns its text string");
s.execute("prop.addKey", json!({"layer": layer, "path": "text/sourceText", "time": LAYER_TIME, "value": source})).unwrap();
let before = project(&s);
let history_before = s.history.undo.len();
let reply = s.execute("prop.set", json!({"layer": layer, "path": "text/sourceText", "time": LAYER_TIME, "value": "Outside final"})).unwrap();
outside_key(&s, layer, "text/sourceText", LAYER_TIME, COMP_TIME);
let actual = read(&mut s, layer, "text/sourceText", COMP_TIME)["value"].clone();
assert_eq!(actual.as_str(), Some("Outside final"), "actual Source Text edit");
let after = project(&s);
undo_redo(&mut s, &before, &after, history_before);
eprintln!("LEGACY_REPLY text set outside: {reply}");
assert_eq!(reply, actual, "omitted timeBase must return the raw Source Text string unchanged");
}
#[test]
fn legacy_outside_count_remains_usable_by_the_documented_batch_reference() {
let (mut s, layer) = setup(false);
let before = project(&s);
let history_before = s.history.undo.len();
let outcome = s.execute(
"engine.batch",
json!({"label": "Use key count", "steps": [
{"command": "prop.addKey", "params": {"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "value": 60}},
{"command": "prop.set", "params": {"layer": layer, "path": "transform/rotation", "value": "$1"}}
]}),
);
eprintln!("LEGACY_REPLY batch count substitution: {outcome:?}");
assert!(outcome.is_ok(), "a valid outside addKey count must remain usable as the documented $1 numeric value: {outcome:?}");
let reply = outcome.unwrap();
assert_eq!(reply["steps"].as_u64(), Some(2));
assert_eq!(reply["results"][0].as_u64(), Some(1));
assert_eq!(reply["results"][1].as_f64(), Some(1.0));
seconds(&read(&mut s, layer, "transform/rotation", 10.5)["value"], 1.0);
outside_key(&s, layer, "transform/opacity", LAYER_TIME, COMP_TIME);
let after = project(&s);
undo_redo(&mut s, &before, &after, history_before);
}
#[test]
fn explicit_time_bases_keep_timing_and_existing_object_warnings() {
let (mut s, layer) = setup(false);
let outside = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "timeBase": "layer", "value": 60})).unwrap();
assert_eq!(outside["count"].as_u64(), Some(1));
seconds(&outside["layerTime"], LAYER_TIME);
seconds(&outside["compTime"], COMP_TIME);
assert!(outside["warning"].as_str().unwrap().contains("outside"));
let inside = s.execute("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": 15.3, "timeBase": "comp", "value": 20})).unwrap();
seconds(&inside["layerTime"], 4.8);
seconds(&inside["compTime"], 15.3);
assert_eq!(inside["count"].as_u64(), Some(2));
assert!(inside.get("warning").is_none());
let selected = s.execute("keys.select", json!({"keys": [{"layer": layer, "path": "transform/opacity", "time": COMP_TIME}], "timeBase": "comp"})).unwrap();
assert_eq!(selected["count"].as_u64(), Some(1));
seconds(&selected["keys"][0]["compTime"], COMP_TIME);
assert!(selected["warning"].as_str().unwrap().contains("outside"));
let changed = s.execute("prop.set", json!({"layer": layer, "path": "transform/opacity", "time": COMP_TIME, "timeBase": "comp", "value": 40})).unwrap();
seconds(&changed["value"], 40.0);
seconds(&changed["compTime"], COMP_TIME);
assert!(changed["warning"].as_str().unwrap().contains("outside"));
let info = read(&mut s, layer, "transform/opacity", COMP_TIME);
let keys = info["keys"].as_array().unwrap();
let key = keys.iter().find(|key| (key["time"].as_f64().unwrap() - LAYER_TIME).abs() < 1e-8).unwrap();
seconds(&key["layerTime"], LAYER_TIME);
seconds(&key["compTime"], COMP_TIME);
assert!(key["warning"].as_str().unwrap().contains("outside"), "prop.get already has an object response");
let before = project(&s);
let history_before = s.history.undo.len();
let moved = s.execute("keys.set", json!({"layer": layer, "path": "transform/opacity", "time": LAYER_TIME, "newTime": 15.4})).unwrap();
assert!(moved.is_object(), "keys.set already returned an object without timeBase");
seconds(&moved["layerTime"], 15.4);
seconds(&moved["compTime"], 25.9);
assert!(moved["warning"].as_str().unwrap().contains("outside"));
outside_key(&s, layer, "transform/opacity", 15.4, 25.9);
let after = project(&s);
undo_redo(&mut s, &before, &after, history_before);
}
+27 -5
View File
@@ -111,6 +111,28 @@ example `transform/position`, `transform/opacity`, `effects/#1/blurriness` or `@
are in seconds. Keyframe times are layer time, which equals comp time unless the layer is offset or
stretched.
Use MCP `execute_command` for engine commands `prop.set`, `prop.addKey`, `keys.set` and
`keys.select` with `timeBase: "layer" | "comp"`; omitted means `layer`. The convenience tools
`set_property` and `add_keyframe` retain their layer-time interface. The input base applies to
both `keys.set.time` and `newTime`, and to every `keys.select.keys[].time`.
Omitted `prop.set` or `prop.addKey` times use the target comp's CTI. Supplied write times snap
to its nearest comp frame before conversion to stored layer time, including stretched or
reversed layers; existing `keys.set` neighbor constraints and roving adjustment still apply.
Explicit bases add timing metadata to mutation replies; `keys.select` reports timing in
`keys[]` for the resulting selection. Returned key `time` and `layerTime` are layer seconds;
`compTime` is comp seconds. Omitted-base `prop.set`, `prop.addKey` and `keys.select` retain
their existing JSON value/count replies even outside the active range. Timing metadata
carries outside-range warnings; the existing objects from `keys.set` and `prop.get.keys[]`
also report both bases and warnings. `prop.get.time` is comp seconds. Separated Position
`prop.set` retains per-axis replies; for these commands, `path` or `prop` identifies a property, not a group.
Set offsets with `layer.timing {layer, start: 10.5}`; `in`, `out` and `delta` also use comp seconds.
For an unstretched layer starting at 10.5 s in a 30 fps comp,
`prop.addKey {layer: "A", path: "transform/opacity", time: 15.3,
timeBase: "comp", value: 0}` writes a key at comp 15.3 s (layer 4.8 s), matching
`prop.get {layer: "A", path: "transform/opacity", time: 15.3}`.
Command params are checked against each command's `params` doc. An unknown key, such as
`layer.select {"index": 2}`, returns an error that lists the accepted keys (here `layers, add, toggle`)
instead of being ignored.
@@ -149,11 +171,11 @@ the Progress panel, `jobs.list`); headless sessions run `footage.check` when the
Things that tripped up a long agent-driven session (a multi-scene 3D piece built entirely over MCP).
- **Layer time vs comp time.** `add_keyframe`, `set_property` with `time`, `prop.addKey`,
`keys.select` and `keys.set` take layer time; `get_property` and `render_frame` take comp time;
`run_script`'s `setValueAtTime` and `keyTime` use comp time, as in After Effects. On a layer that
doesn't start at 0, such as a nested comp placed later, subtract the layer's start time from the
comp time before keying (unstretched layers), or set the keys from `run_script`
- **Layer time vs comp time.** `add_keyframe` and `set_property` with `time` take layer time;
`get_property` and `render_frame` take comp time. Engine keyframe commands default to layer
time; use `execute_command` with `timeBase: "comp"` to supply comp seconds on offset or
stretched layers. `run_script`'s `setValueAtTime` and `keyTime` also use comp time. Without an
explicit engine time base, subtract the layer's start time before keying an unstretched layer
([#257](https://github.com/storytold/effectcraft/issues/257)).
- **Moving a layer in time.** `execute_command layer.timing {"layers":["Scene 2"],"start":5.1}` sets
the start time in comp seconds and moves the in and out points with it.
+17
View File
@@ -124,3 +124,20 @@ Editor transform box, timeline Alt-drag scaling).
The MCP server's bridge mode (`effectcraft-cli mcp --bridge 9877`) is a thin client of this protocol;
see [agents.md](agents.md).
Keyframe command times (`prop.set`, `prop.addKey`, `keys.set`, `keys.select`) default to layer
seconds. Set `timeBase: "comp"` for comp seconds; `keys.set.newTime` and every
`keys.select.keys[].time` use the same input base. Omitted `prop.set` or `prop.addKey` times use
the target comp's CTI. Supplied write times snap to its nearest comp frame before conversion
to stored layer time, including on stretched or reversed layers. Existing `keys.set` neighbor
constraints and roving adjustment can change the final stored time.
Explicit bases add `layerTime` and `compTime` to unsplit `prop.set` and `prop.addKey` replies,
alongside `value` or `count`. Separated Position `prop.set` keeps an array of per-axis replies.
Explicit `keys.select` replies contain `count` and `keys[]`, with timing on each key in the
resulting selection. Key metadata's `time` remains layer seconds regardless of the input base.
Without `timeBase`, these three commands retain their existing JSON value/count replies,
including outside the layer's `[in, out)` range. Timing metadata carries outside-range warnings;
`keys.set` and `prop.get.keys[]` report both bases and warnings without an explicit base too.
`prop.get.time` is comp seconds. For these commands, `path` or `prop` identifies a property, not a group.
Set a layer's offset with `layer.timing {layer, start}` (comp seconds).