From 47f2a5a3e68b80214658c293c081476ee5e678e9 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 5 Oct 2026 10:36:55 +0700 Subject: [PATCH] M5.15: J / K and Select All Keyframes use what the Timeline shows; Graph Editor Auto-Select Graph Type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - J / K (Go to Previous / Next Keyframe) and Ctrl+Alt+A (Select All Keyframes) act on what the Timeline shows, as in After Effects: the keys of its revealed properties, layer and comp markers and the work area (they used every property of the selected or all layers, revealed or not). The commands take `visible: [{layer, prop}]`, which the Timeline fills in; agents calling them without it keep the previous behaviour. - Graph Editor ▸ Auto-Select Graph Type (an "Auto" button, on by default as in After Effects): the speed graph when every property shown is spatial (Position, Anchor Point…), else the value graph; choosing Value or Speed turns it off. - Docs: parity, agents. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/anim.rs | 19 +++++++- crates/engine/src/commands/mod.rs | 7 +++ crates/engine/src/commands/time.rs | 68 ++++++++++++++++++++++----- crates/engine/src/tests_menu_cmds.rs | 24 ++++++++++ crates/ui-egui/src/menus.rs | 9 ++++ crates/ui-egui/src/panels/graph.rs | 38 +++++++++++++-- crates/ui-egui/src/panels/timeline.rs | 12 +++++ crates/ui-egui/src/state.rs | 11 +++-- crates/ui-egui/tests/ui_keyframes.rs | 28 +++++++++++ docs/agents.md | 2 + docs/parity.md | 8 +++- 11 files changed, 202 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/commands/anim.rs b/crates/engine/src/commands/anim.rs index 0747290e..7b830bf0 100644 --- a/crates/engine/src/commands/anim.rs +++ b/crates/engine/src/commands/anim.rs @@ -139,7 +139,14 @@ fn select_all(s: &mut Session, p: &Value) -> Result { let (cid, mut layers) = layers_p(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; let mut sel = vec![]; - if p.get("layers").is_none() && p.get("layer").is_none() && !s.state.selected_props.is_empty() { + if let Some(visible) = super::visible_p(p) { + // Ctrl+Alt+A in the Timeline: every key it shows (its revealed properties). + for (lid, uid) in visible { + if let Some(pr) = comp.layer(lid).and_then(|l| l.props.find(uid)) { + sel.extend(pr.keys.iter().map(|k| KeyRef { layer: lid, prop: uid, time: k.time })); + } + } + } else if p.get("layers").is_none() && p.get("layer").is_none() && !s.state.selected_props.is_empty() { for (lid, uid) in &s.state.selected_props { if let Some(pr) = comp.layer(*lid).and_then(|l| l.props.find(*uid)) { sel.extend(pr.keys.iter().map(|k| KeyRef { layer: *lid, prop: *uid, time: k.time })); @@ -422,7 +429,15 @@ pub fn specs() -> Vec { vec![ cmd!("keys.copy", "Copy Keyframes", [], None, "{}", has_keys, copy), cmd!("keys.paste", "Paste Keyframes", [], None, "{layers?, prop?|path?, time?}", has_key_clip, paste), - cmd!("keys.selectAll", "Select All Keyframes", [], Some("Cmd+Alt+A"), "{layers?}", keys_or_layers, select_all), + cmd!( + "keys.selectAll", + "Select All Keyframes", + [], + Some("Cmd+Alt+A"), + "{layers?, visible?: [{layer, prop}] (every key of these properties)}", + keys_or_layers, + select_all + ), cmd!("keys.nudge", "Nudge Keyframes", [], None, "{frames, merge?}", has_keys, nudge), cmd!("keys.nudgeForward", "Move Keyframes 1 Frame Later", [], Some("Alt+ArrowRight"), "{}", has_keys, |s, _| nudge(s, &json!({"frames": 1}))), cmd!("keys.nudgeBackward", "Move Keyframes 1 Frame Earlier", [], Some("Alt+ArrowLeft"), "{}", has_keys, |s, _| nudge(s, &json!({"frames": -1}))), diff --git a/crates/engine/src/commands/mod.rs b/crates/engine/src/commands/mod.rs index 52c90e3e..546eaa44 100644 --- a/crates/engine/src/commands/mod.rs +++ b/crates/engine/src/commands/mod.rs @@ -517,6 +517,13 @@ pub(crate) fn unlocked(s: &Session, cid: ItemId, ids: Vec, cmd: &str) - } } +/// `visible: [{layer, prop}]`: the properties the Timeline shows (J / K and Select All +/// Keyframes use only those, as in After Effects). `None` when not given. +pub(crate) fn visible_p(p: &Value) -> Option> { + let a = p.get("visible")?.as_array()?; + Some(a.iter().filter_map(|v| Some((LayerId(v.get("layer")?.as_u64()?), v.get("prop")?.as_u64()?))).collect()) +} + /// `time` (seconds) or `frame` param, else the CTI. pub(crate) fn time_p(s: &Session, p: &Value, comp: Option<&Comp>) -> Tick { if let Some(t) = f_p(p, "time") { diff --git a/crates/engine/src/commands/time.rs b/crates/engine/src/commands/time.rs index 1c4c1c64..9ed41f47 100644 --- a/crates/engine/src/commands/time.rs +++ b/crates/engine/src/commands/time.rs @@ -71,6 +71,7 @@ fn go(s: &mut Session, p: &Value) -> Result { let t = s.time(); let to = str_p(p, "to").unwrap_or("start"); let sel_layer = s.state.selected_layers.first().and_then(|id| comp.layer(*id)); + let visible = super::visible_p(p); let target = match to { "start" => Tick::ZERO, "end" => comp.duration - comp.frame_duration(), @@ -78,6 +79,23 @@ fn go(s: &mut Session, p: &Value) -> Result { "workEnd" => comp.work_area.1 - comp.frame_duration(), "layerIn" => sel_layer.map(|l| l.in_point).unwrap_or(t), "layerOut" => sel_layer.map(|l| l.out_point - comp.frame_duration()).unwrap_or(t), + "nextKey" | "prevKey" if visible.is_some() => { + // What the Timeline shows: keys of its revealed properties, layer and comp markers + // and the work area (After Effects' "visible items"). + let mut times: Vec = comp.markers.iter().map(|m| m.time).collect(); + times.extend([comp.work_area.0, comp.work_area.1 - comp.frame_duration()]); + for l in comp.layers.iter().filter(|l| !(comp.hide_shy && l.switches.shy)) { + times.extend(l.markers.iter().map(|m| l.comp_time(m.time))); + } + for (lid, uid) in visible.iter().flatten() { + if let Some(l) = comp.layer(*lid) + && let Some(pr) = l.props.find(*uid) + { + times.extend(pr.keys.iter().map(|k| l.comp_time(k.time))); + } + } + step_to(times, t, comp.frame_duration(), to == "nextKey") + } "nextKey" | "prevKey" => { // Keyframes (of visible/selected layers), markers and the work area, in comp time. let mut times: Vec = comp.markers.iter().map(|m| m.time).collect(); @@ -107,14 +125,7 @@ fn go(s: &mut Session, p: &Value) -> Result { times.extend(l.markers.iter().map(|m| l.comp_time(m.time))); } } - times.sort(); - times.dedup(); - let half = comp.frame_duration().0 / 2; - if to == "nextKey" { - times.into_iter().find(|x| x.0 > t.0 + half).unwrap_or(t) - } else { - times.into_iter().rev().find(|x| x.0 < t.0 - half).unwrap_or(t) - } + step_to(times, t, comp.frame_duration(), to == "nextKey") } _ => return Err(super::bad("time.go", "to: start|end|workStart|workEnd|layerIn|layerOut|nextKey|prevKey")), }; @@ -122,6 +133,22 @@ fn go(s: &mut Session, p: &Value) -> Result { Ok(report(s)) } +/// The nearest time in `times` after (or before) `t`, more than half a frame away; `t` itself +/// when there is none. +fn step_to(mut times: Vec, t: Tick, frame: Tick, next: bool) -> Tick { + times.sort(); + times.dedup(); + let half = frame.0 / 2; + if next { times.into_iter().find(|x| x.0 > t.0 + half).unwrap_or(t) } else { times.into_iter().rev().find(|x| x.0 < t.0 - half).unwrap_or(t) } +} + +/// `p` with `to` set (J / K pass their other parameters on). +fn with_to(p: &Value, to: &str) -> Value { + let mut p = if p.is_object() { p.clone() } else { json!({}) }; + p["to"] = json!(to); + p +} + pub fn specs() -> Vec { vec![ cmd!("time.set", "Go to Time...", ["View"], Some("Alt+Shift+J"), "{time? (s) | frame? | timecode?}", has_comp, set), @@ -134,8 +161,27 @@ pub fn specs() -> Vec { cmd!("time.end", "Go to End", [], Some("End"), "{}", has_comp, |s, _| go(s, &json!({"to": "end"}))), cmd!("time.layerIn", "Go to Layer In Point", [], Some("I"), "{}", has_comp, |s, _| go(s, &json!({"to": "layerIn"}))), cmd!("time.layerOut", "Go to Layer Out Point", [], Some("O"), "{}", has_comp, |s, _| go(s, &json!({"to": "layerOut"}))), - cmd!("time.nextKey", "Go to Next Keyframe or Marker", [], Some("K"), "{}", has_comp, |s, _| go(s, &json!({"to": "nextKey"}))), - cmd!("time.previousKey", "Go to Previous Keyframe or Marker", [], Some("J"), "{}", has_comp, |s, _| go(s, &json!({"to": "prevKey"}))), - cmd!("time.go", "Go To", [], None, "{to: start|end|workStart|workEnd|layerIn|layerOut|nextKey|prevKey, prop?: uid}", has_comp, go), + cmd!( + "time.nextKey", + "Go to Next Keyframe or Marker", + [], + Some("K"), + "{visible?: [{layer, prop}] (only these properties' keys, as the Timeline shows them)}", + has_comp, + |s, p| go(s, &with_to(p, "nextKey")) + ), + cmd!("time.previousKey", "Go to Previous Keyframe or Marker", [], Some("J"), "{visible?: [{layer, prop}]}", has_comp, |s, p| go( + s, + &with_to(p, "prevKey") + )), + cmd!( + "time.go", + "Go To", + [], + None, + "{to: start|end|workStart|workEnd|layerIn|layerOut|nextKey|prevKey, prop?: uid, visible?: [{layer, prop}]}", + has_comp, + go + ), ] } diff --git a/crates/engine/src/tests_menu_cmds.rs b/crates/engine/src/tests_menu_cmds.rs index 7f1fce5f..59c1403b 100644 --- a/crates/engine/src/tests_menu_cmds.rs +++ b/crates/engine/src/tests_menu_cmds.rs @@ -1030,3 +1030,27 @@ fn j_and_k_stop_at_keys_markers_and_the_work_area() { let last = (3.0 * 30.0 - 1.0) / 30.0; assert_eq!(stops, vec![1.0, 2.0, (last * 30.0f64).round() / 30.0, (last * 30.0f64).round() / 30.0]); } + +#[test] +fn j_k_and_select_all_use_only_the_properties_the_timeline_shows() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + opacity_keys(&mut s, a, &[1.0]); + s.execute("prop.addKey", json!({"layer": a, "path": "transform/rotation", "time": 2.0, "value": 45})).unwrap(); + let op = layer(&s, a).props.prop("transform/opacity").unwrap().uid; + // Only Opacity is revealed: K skips the hidden Rotation key at 2 s, stopping at 1 s and then + // at the work area's end. + let visible = json!([{"layer": a, "prop": op}]); + s.execute_checked("time.nextKey", json!({"visible": visible})).unwrap(); + assert_eq!(s.time().seconds(), 1.0); + s.execute_checked("time.nextKey", json!({"visible": visible})).unwrap(); + assert!(s.time().seconds() > 3.9, "work area end, not the hidden key: {}", s.time().seconds()); + // Without `visible` (agents), every key of the layers counts, as before. + s.execute("time.set", json!({"time": 1.0})).unwrap(); + s.execute("time.nextKey", json!({})).unwrap(); + assert_eq!(s.time().seconds(), 2.0); + // Select All Keyframes: the shown properties' keys only. + s.execute_checked("keys.selectAll", json!({"visible": visible})).unwrap(); + assert_eq!(s.state.selected_keys.len(), 1); + assert_eq!(s.state.selected_keys[0].prop, op); +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 68d62b58..77fa0016 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -196,6 +196,15 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V if crate::panels::unsaved::guard(app, id, ¶ms) { return Ok(json!({"dialog": "unsavedChanges"})); } + // J / K and Select All Keyframes act on what the Timeline shows (its revealed properties). + if matches!(id, "time.nextKey" | "time.previousKey" | "keys.selectAll") + && ["visible", "layers", "layer", "prop"].iter().all(|k| params.get(*k).is_none()) + && let Some(c) = app.session.active_comp_arc() + { + let mut p = if params.is_object() { params.clone() } else { json!({}) }; + p["visible"] = json!(crate::panels::timeline::visible_props(app, &c)); + return run_engine(app, ctx, id, p); + } // New Camera/Light and Camera/Light Settings without parameters open their dialogs. if crate::panels::dialogs_3d::route(app, id, ¶ms)? { return Ok(Value::Null); diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index d34bace3..0b03ef39 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -63,6 +63,22 @@ struct Curve<'a> { } /// Properties shown: the selected ones, or every animated property of the selected layers. +/// Whether the speed graph shows: Edit Speed Graph, or Auto-Select Graph Type with only spatial +/// properties shown (Position, Anchor Point… move along a path: their speed is what to edit). +fn speed_graph(app: &EffectcraftApp, comp: &Comp) -> bool { + match app.ui.timeline.graph_mode.as_str() { + "speed" => true, + "value" => false, + _ => { + let props = shown_props(app, comp); + !props.is_empty() + && props.iter().all(|(lid, uid)| { + comp.layer(*lid).and_then(|l| l.props.find(*uid)).is_some_and(|pr| pr.spatial && matches!(pr.value, Value::Vec2(_) | Value::Vec3(_))) + }) + } + } +} + fn shown_props(app: &EffectcraftApp, comp: &Comp) -> Vec<(LayerId, u64)> { let st = &app.session.state; let numeric = |pr: &Property| !pr.value.components().is_empty() && pr.value.interpolates(); @@ -179,7 +195,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let bg = ui.interact(plot, egui::Id::new("graph-bg"), Sense::click_and_drag()); p.rect_filled(area, 0.0, Color32::from_rgb(0x1d, 0x1d, 0x1d)); app.auto.add("timeline.graph", plot, &app.ui.timeline.graph_mode.clone()); - let speed = app.ui.timeline.graph_mode == "speed"; + let speed = speed_graph(app, comp); let cs = curves(app, comp, ectx, tm, plot, speed); // Vertical range: auto-zoom to the curves + handles, or the stored manual range. @@ -408,11 +424,25 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let y = bar.center().y; let mut x = bar.min.x + 8.0; let tl = &app.ui.timeline; - let (is_value, show_sel, auto_on) = (tl.graph_mode != "speed", tl.graph_show_selected, tl.graph_auto_zoom); - if text_button(app, ui, p, &mut x, y, "Value", is_value, "valueGraph", "Edit Value Graph") { + let (auto_type, show_sel, auto_on) = (tl.graph_mode == "auto", tl.graph_show_selected, tl.graph_auto_zoom); + if text_button( + app, + ui, + p, + &mut x, + y, + "Auto", + auto_type, + "autoSelectGraphType", + "Auto-Select Graph Type (speed graph for spatial properties, else value graph)", + ) { + app.ui.timeline.graph_mode = if auto_type { if speed { "speed" } else { "value" } } else { "auto" }.into(); + } + // Choosing a graph type turns Auto-Select off, as in After Effects. + if text_button(app, ui, p, &mut x, y, "Value", !speed, "valueGraph", "Edit Value Graph") { app.ui.timeline.graph_mode = "value".into(); } - if text_button(app, ui, p, &mut x, y, "Speed", !is_value, "speedGraph", "Edit Speed Graph") { + if text_button(app, ui, p, &mut x, y, "Speed", speed, "speedGraph", "Edit Speed Graph") { app.ui.timeline.graph_mode = "speed".into(); } x += 6.0; diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index d1a93618..f467b489 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -515,6 +515,18 @@ fn reveal_targets(kind: &str) -> Option<(&'static str, &'static [&'static str])> }) } +/// The properties the Timeline shows (revealed rows), as `{layer, prop}`: J / K and Select All +/// Keyframes act on these, as in After Effects. +pub(crate) fn visible_props(app: &EffectcraftApp, comp: &Comp) -> Vec { + build_rows(app, comp) + .iter() + .filter_map(|r| match r.kind { + RowKind::Prop { uid } => Some(json!({"layer": r.layer.0, "prop": uid})), + _ => None, + }) + .collect() +} + fn build_rows(app: &EffectcraftApp, comp: &Comp) -> Vec { let tl = &app.ui.timeline; let search = tl.search.trim().to_lowercase(); diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index aea71dfe..d0744141 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -363,8 +363,9 @@ pub struct TimelineState { /// Properties / groups (uids) shown by the `props` reveal (Animation ▸ Reveal Properties…). #[serde(default)] pub reveal_props: BTreeSet, - /// Graph Editor: `value` or `speed` graph. - #[serde(default = "value_graph")] + /// Graph Editor: `auto` (Auto-Select Graph Type, the default: the speed graph when every + /// property shown is spatial, else the value graph), `value` or `speed`. + #[serde(default = "auto_graph")] pub graph_mode: String, /// Show only the selected properties (else every animated property of the selected layers). #[serde(default = "yes")] @@ -401,8 +402,8 @@ pub fn default_tl_columns() -> BTreeSet { ["av", "label", "num", "switches", "parent"].map(String::from).into_iter().collect() } -fn value_graph() -> String { - "value".into() +fn auto_graph() -> String { + "auto".into() } fn yes() -> bool { true @@ -423,7 +424,7 @@ impl Default for TimelineState { open_groups: BTreeSet::new(), reveal: vec![], reveal_props: BTreeSet::new(), - graph_mode: value_graph(), + graph_mode: auto_graph(), graph_show_selected: true, graph_auto_zoom: true, graph_range: None, diff --git a/crates/ui-egui/tests/ui_keyframes.rs b/crates/ui-egui/tests/ui_keyframes.rs index dd269d9f..55b2ab75 100644 --- a/crates/ui-egui/tests/ui_keyframes.rs +++ b/crates/ui-egui/tests/ui_keyframes.rs @@ -208,3 +208,31 @@ fn double_click_a_key_to_edit_its_value() { let k = h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop("transform/opacity").unwrap().keys[1].clone(); assert_eq!((k.time.seconds(), k.value.as_f64()), (1.0, 100.0)); } + +#[test] +fn k_follows_the_revealed_properties_and_auto_select_picks_the_speed_graph_for_position() { + let (mut h, id, _) = harness(); + // A Rotation key at 2 s on a property that isn't revealed: K skips it. + h.state_mut().session.execute("prop.addKey", json!({"layer": id.0, "path": "transform/rotation", "time": 2.0, "value": 45})).unwrap(); + h.state_mut().session.set_time(effectcraft_engine::time::Tick::from_seconds_f64(1.0)); + h.run_steps(2); + h.input_mut().events.push(Event::Key { key: egui::Key::K, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }); + h.run_steps(2); + assert!(h.state().session.time().seconds() > 3.9, "past the hidden key to the work area end: {}", h.state().session.time().seconds()); + // Auto-Select Graph Type: Position shows its speed (one curve), Opacity its value. + let s = &mut h.state_mut().session; + for (t, v) in [(0.0, [40.0, 90.0]), (1.0, [280.0, 90.0])] { + s.execute("prop.addKey", json!({"layer": id.0, "path": "transform/position", "time": t, "value": v})).unwrap(); + } + let pos = s.active_comp().unwrap().layer(id).unwrap().props.prop("transform/position").unwrap().uid; + s.execute("prop.select", json!({"layer": id.0, "prop": pos})).unwrap(); + h.state_mut().ui.timeline.graph_editor = true; + h.run_steps(4); + assert_eq!(h.state().ui.timeline.graph_mode, "auto"); + assert!(h.state().auto.find(&format!("timeline.graph.key.{pos}.0.0")).is_some()); + assert!(h.state().auto.find(&format!("timeline.graph.key.{pos}.1.0")).is_none(), "speed graph: one curve"); + assert!(h.state().auto.find("timeline.graph.autoSelectGraphType").is_some()); + h.state_mut().ui.timeline.graph_mode = "value".into(); + h.run_steps(3); + assert!(h.state().auto.find(&format!("timeline.graph.key.{pos}.1.0")).is_some(), "value graph: X and Y"); +} diff --git a/docs/agents.md b/docs/agents.md index 396d8aaf..8cc26a9e 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -139,6 +139,8 @@ build real projects through these interfaces; they are worked examples of everyt `keys.transform {…, merge, fromStart: true}` (values are the whole transform since the drag started). Keys of locked layers can't be selected. Copy / paste: `keys.copy`, then `keys.paste` at the current time (keys copied from several layers go to as many selected layers in order). + `time.nextKey` / `time.previousKey` / `keys.selectAll` take `visible: [{layer, prop}]` to act + only on those properties (the Timeline passes its revealed ones, as J / K and Ctrl+Alt+A do). * **Easing with Keyframe Velocity**: select keys by path, `keys.select {"keys":[{"layer":"Ring","path":"contents/trim/end","time":0}]}`, then `keys.velocity {"outSpeed":0,"outInfluence":33.33}`. * **Gradients**: `set_property {"path":"contents/gfill/colors","value":["#0080ff","#ffff00"]}` or diff --git a/docs/parity.md b/docs/parity.md index 3c1d5294..6f20c92d 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -415,8 +415,12 @@ Browser in the web app (landed in M13.10: browser storage and File System Access - **J / K and Info** (M5.14): J / K also stop at the work area; the Info panel shows a selected key's property, time and value. -Not yet: J / K and Select All Keyframes use every property of the layers, not only the revealed -ones; Graph Editor keys are drawn as squares whatever their interpolation; Auto-Select Graph Type. +Since (M5.15): J / K and Select All Keyframes use what the Timeline shows — the revealed +properties' keys, layer and comp markers and the work area (`visible: [{layer, prop}]` on +`time.nextKey` / `previousKey` / `keys.selectAll`; without it, agents get every property of the +layers); the Graph Editor has Auto-Select Graph Type (the default: the speed graph when only +spatial properties are shown, else the value graph; choosing Value or Speed turns it off). Graph +Editor keys are small squares whatever their interpolation, as in After Effects. ## Update: M4.9–M4.11 nested comps and motion blur