Merge pull request #45 from punkhop/fix/move-linked-partners

engine: timeline.move takes linked partners along
This commit is contained in:
Brandon Thomas
2026-10-06 11:37:38 -04:00
committed by GitHub
6 changed files with 281 additions and 16 deletions
+60 -15
View File
@@ -245,6 +245,57 @@ pub(crate) fn item_p(p: &Value, k: &str) -> Option<ItemId> {
u64_p(p, k).map(ItemId)
}
/// `timeline.move`: move the listed clips to (track, time). With Linked Selection on (or
/// `linked: true`) the linked partners of a listed clip that are not listed themselves follow by
/// the same time offset on their own tracks, so picture and sound stay in sync; a partner on a
/// locked track, or one whose position would not change, stays as it is. Nothing moves before the sequence start: when a clip would,
/// every clip of the call lands later by the same amount, so the spacing asked for is kept.
fn move_clips(s: &mut Session, p: &Value) -> Result<Value> {
const CMD: &str = "timeline.move";
let mut listed: Vec<(ClipId, TrackId, Tick)> = Vec::new();
for m in p.get("moves").and_then(Value::as_array).into_iter().flatten() {
let track = track_p(s, m, "track", CMD)?;
if let (Some(clip), Some(track), Some(time)) = (clip_p(m, "clip"), track, m.get("time").and_then(Value::as_i64)) {
listed.push((clip, track, Tick(time)));
}
}
if listed.is_empty() {
return Err(bad(CMD, "need `moves`"));
}
let follow = bool_p(p, "linked").unwrap_or(s.state.linked_selection);
let q = s.active_sequence().ok_or(EngineError::NoSequence)?;
// (clip, destination track, new start, where a partner starts now). `time` is any i64: the
// sums are done in i128, where a few i64 values cannot overflow, and checked on the way back.
let mut plan: Vec<(ClipId, TrackId, i128, Option<i128>)> = Vec::new();
for &(clip, track, time) in &listed {
plan.push((clip, track, i128::from(time.0), None));
let Some((_, it)) = q.find_item(clip) else { continue }; // `edit::move_items` reports it
let Some(link) = it.link.filter(|_| follow) else { continue };
let offset = i128::from(time.0) - i128::from(it.start.0);
for t in q.all_tracks().filter(|t| !t.locked) {
for partner in t.items.iter().filter(|i| i.link == Some(link)) {
if !listed.iter().any(|m| m.0 == partner.id) && !plan.iter().any(|m| m.0 == partner.id) {
plan.push((partner.id, t.id, i128::from(partner.start.0) + offset, Some(i128::from(partner.start.0))));
}
}
}
}
let late = plan.iter().map(|m| -m.2).max().unwrap_or(0).max(0);
let mut moves: Vec<(ClipId, TrackId, Tick)> = Vec::with_capacity(plan.len());
for (clip, track, start, was) in plan {
// a partner that ends up where it is does not move (its listed clip only changed track):
// re-placing it would drop the transitions at its edges
if was == Some(start + late) {
continue;
}
let start = i64::try_from(start + late).ok().map(Tick).filter(|t| *t <= Tick::MAX).ok_or_else(|| bad(CMD, "`time` is out of range"))?;
moves.push((clip, track, start));
}
let ins = bool_p(p, "insert").unwrap_or(false);
s.edit_sequence(if ins { "Move (Insert)" } else { "Move" }, |q, ctx, _| Ok(edit::move_items(q, &moves, ins, ctx)?))?;
Ok(json!({"moved": moves.iter().map(|m| m.0.0).collect::<Vec<_>>()}))
}
/// Expand a clip selection with linked partners (when linked selection is on).
pub fn with_links(s: &Session, clips: &[ClipId]) -> Vec<ClipId> {
let Some(seq) = s.active_sequence() else { return clips.to_vec() };
@@ -1678,21 +1729,15 @@ fn build() -> Vec<CommandSpec> {
}
Ok(json!({"selection": s.state.selection.iter().map(|c| c.0).collect::<Vec<_>>()}))
}),
cmd!("timeline.move", "Move Clips", [], None, r#"{"moves":[{"clip":id,"track":id|"V2","time":ticks}],"insert":bool}"#, has_seq, |s, p| {
let mut moves: Vec<(ClipId, TrackId, Tick)> = Vec::new();
for m in p.get("moves").and_then(Value::as_array).into_iter().flatten() {
let track = track_p(s, m, "track", "timeline.move")?;
if let (Some(clip), Some(track), Some(time)) = (clip_p(m, "clip"), track, m.get("time").and_then(Value::as_i64)) {
moves.push((clip, track, Tick(time)));
}
}
if moves.is_empty() {
return Err(bad("timeline.move", "need `moves`"));
}
let ins = bool_p(p, "insert").unwrap_or(false);
s.edit_sequence(if ins { "Move (Insert)" } else { "Move" }, |q, ctx, _| Ok(edit::move_items(q, &moves, ins, ctx)?))?;
Ok(Value::Null)
}),
cmd!(
"timeline.move",
"Move Clips",
[],
None,
r#"{"moves":[{"clip":id,"track":id|"V2","time":ticks}],"insert":bool,"linked":bool?}"#,
has_seq,
move_clips
),
cmd!(
"timeline.trim",
"Trim Edit",
+2
View File
@@ -957,6 +957,8 @@ mod shortcuts_tests;
#[cfg(test)]
mod tests;
#[cfg(test)]
mod timeline_move_tests;
#[cfg(test)]
mod track_params_tests;
#[cfg(test)]
mod transcript_tests;
+204
View File
@@ -0,0 +1,204 @@
//! `timeline.move`: linked partners follow, locked partners stay, the group stops together at the
//! sequence start, and hostile times are refused.
use filmcraft_project::{ClipId, TrackId, TrackItem};
use filmcraft_time::Tick;
use serde_json::json;
use crate::{EngineError, Session};
fn demo() -> Session {
let mut s = Session::default();
s.execute("file.openDemoProject", json!({})).unwrap();
s
}
fn state(s: &Session, c: ClipId) -> (TrackId, Tick) {
s.active_sequence().unwrap().find_item(c).map(|(t, i)| (t, i.start)).unwrap()
}
/// The last picture clip of V1 and its linked sound: (V1, picture, sound's track, sound).
fn last_pair(s: &Session) -> (TrackId, TrackItem, TrackId, TrackItem) {
let q = s.active_sequence().unwrap();
let v = q.video_tracks[0].items.last().unwrap().clone();
let (a_track, a) = q.audio_tracks.iter().find_map(|t| t.items.iter().find(|i| i.link == v.link && v.link.is_some()).map(|i| (t.id, i.clone()))).unwrap();
(q.video_tracks[0].id, v, a_track, a)
}
/// An empty sequence with one linked picture + sound pair placed at `seconds`: (V1, picture, A1, sound).
fn pair_in_an_empty_sequence(s: &mut Session, seconds: f64) -> (TrackId, TrackItem, TrackId, TrackItem) {
s.execute("file.newSequence", json!({"name": "Moves", "video": 2, "audio": 2})).unwrap();
let item = s.project.items.values().find(|i| i.has_video() && i.has_audio() && i.as_media().is_some()).map(|i| i.id).unwrap();
s.execute("timeline.place", json!({"item": item.0, "seconds": seconds})).unwrap();
let q = s.active_sequence().unwrap();
let (v, a) = (q.video_tracks[0].items[0].clone(), q.audio_tracks[0].items[0].clone());
assert!(v.link.is_some() && v.link == a.link, "a placed clip's picture and sound are linked");
(q.video_tracks[0].id, v, q.audio_tracks[0].id, a)
}
/// `timeline.move` of one clip left its linked sound behind although Linked Selection was on.
#[test]
fn move_takes_linked_partners_along() {
let mut s = demo();
let rate = s.sequence_rate();
let (v_track, v, a_track, a) = last_pair(&s);
let end = s.active_sequence().unwrap().duration();
assert!(s.state.linked_selection, "linked selection is on by default");
let before = s.project.clone();
// only the picture is listed: the sound follows by the same offset, on its own track
let to = end + rate.tick_of(48);
let r = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": "V1", "time": to.0}]})).unwrap();
assert_eq!(state(&s, v.id), (v_track, to));
assert_eq!(state(&s, a.id), (a_track, a.start + (to - v.start)), "linked audio stays in sync");
assert_eq!(r["moved"], json!([v.id.0, a.id.0]));
s.active_sequence().unwrap().check().unwrap();
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(*s.project, *before, "one undo step restores both clips");
s.execute("edit.redo", json!({})).unwrap();
assert_eq!((state(&s, v.id).1, state(&s, a.id).1), (to, a.start + (to - v.start)));
s.execute("edit.undo", json!({})).unwrap();
// listing both moves each exactly once
let (tv, ta) = (to, a.start + (to - v.start));
let r = s
.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": "V1", "time": tv.0}, {"clip": a.id.0, "track": a_track.0, "time": ta.0}]}))
.unwrap();
assert_eq!((state(&s, v.id), state(&s, a.id)), ((v_track, tv), (a_track, ta)));
assert_eq!(r["moved"], json!([v.id.0, a.id.0]));
s.execute("edit.undo", json!({})).unwrap();
// `linked: false` moves just the listed clip, whatever the toggle says
let r = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": "V1", "time": to.0}], "linked": false})).unwrap();
assert_eq!((state(&s, v.id), state(&s, a.id)), ((v_track, to), (a_track, a.start)));
assert_eq!(r["moved"], json!([v.id.0]));
s.execute("edit.undo", json!({})).unwrap();
// linked selection off: unchanged behaviour, only the listed clip moves
s.execute("sequence.linkedSelection", json!({"on": false})).unwrap();
s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": "V1", "time": to.0}]})).unwrap();
assert_eq!((state(&s, v.id), state(&s, a.id)), ((v_track, to), (a_track, a.start)));
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(*s.project, *before);
// ...unless the command asks for it
s.execute("timeline.move", json!({"moves": [{"clip": a.id.0, "track": a_track.0, "time": ta.0}], "linked": true})).unwrap();
assert_eq!((state(&s, v.id), state(&s, a.id)), ((v_track, tv), (a_track, ta)), "moving the sound takes the picture along");
}
/// A partner on a locked track cannot move; that must not refuse the move of the listed clip.
#[test]
fn a_linked_partner_on_a_locked_track_stays_and_the_listed_clip_still_moves() {
let mut s = demo();
let (v_track, v, a_track, a) = last_pair(&s);
let to = s.active_sequence().unwrap().duration() + s.sequence_rate().tick_of(48);
s.execute("timeline.setTrack", json!({"track": a_track.0, "locked": true})).unwrap();
let r = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": "V1", "time": to.0}]})).unwrap();
assert_eq!(r["moved"], json!([v.id.0]), "the locked sound is not among the moved clips");
assert_eq!((state(&s, v.id), state(&s, a.id)), ((v_track, to), (a_track, a.start)));
s.active_sequence().unwrap().check().unwrap();
s.execute("edit.undo", json!({})).unwrap();
assert_eq!(state(&s, v.id), (v_track, v.start));
// a listed clip whose own destination is locked is still refused, and nothing moves
let before = s.project.clone();
assert!(s.execute("timeline.move", json!({"moves": [{"clip": a.id.0, "track": a_track.0, "time": to.0}]})).is_err());
assert_eq!(*s.project, *before);
}
/// A partner that would land where it already is does not move: re-placing it would drop the
/// transitions at its edges and list it in `moved` for nothing. Moving only the picture to
/// another track at the same time leaves the sound as it was.
#[test]
fn a_partner_whose_position_does_not_change_is_left_alone() {
let mut s = demo();
let q = s.active_sequence().unwrap();
let a = q.audio_tracks[0].items[1].clone();
let (v_track, v) = q.video_tracks.iter().find_map(|t| t.items.iter().find(|i| i.link == a.link && a.link.is_some()).map(|i| (t.id, i.clone()))).unwrap();
let (a_track, v2) = (q.audio_tracks[0].id, q.video_tracks[1].id);
assert_ne!(v_track, v2);
s.execute("sequence.applyAudioTransition", json!({"clip": a.id.0})).unwrap();
let sound = |s: &Session| s.active_sequence().unwrap().track(a_track).unwrap().clone();
let before = sound(&s);
assert_eq!(before.transitions.len(), 1, "the sound clip has a crossfade");
// picture to V2 at the same time, Linked Selection on
let r = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": v2.0, "time": v.start.0}]})).unwrap();
assert_eq!(state(&s, v.id), (v2, v.start));
assert_eq!(sound(&s), before, "the sound track, its clip and its crossfade are untouched");
assert_eq!(r["moved"], json!([v.id.0]), "the sound did not move, so it is not listed");
s.execute("edit.undo", json!({})).unwrap();
assert_eq!((state(&s, v.id), sound(&s)), ((v_track, v.start), before.clone()));
s.execute("edit.redo", json!({})).unwrap();
assert_eq!((state(&s, v.id), sound(&s)), ((v2, v.start), before.clone()));
s.execute("edit.undo", json!({})).unwrap();
// a real offset still takes the sound along
let later = v.start + s.sequence_rate().tick_of(2);
let r = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": v2.0, "time": later.0}]})).unwrap();
assert_eq!(r["moved"], json!([v.id.0, a.id.0]));
assert_eq!(state(&s, a.id).1 - a.start, later - v.start);
}
/// When a clip would start before the sequence start, every clip of the call lands later by the
/// same amount: a split edit keeps its offset and several listed clips keep their spacing.
#[test]
fn the_group_stops_together_at_the_sequence_start() {
let mut s = demo();
let (v_track, v, a_track, a) = pair_in_an_empty_sequence(&mut s, 60.0);
let rate = s.sequence_rate();
let f = |n: i64| rate.tick_of(n);
let (lead, gap) = (f(10), f(100));
// make a split edit: the sound starts 10 frames before the picture
s.execute("timeline.move", json!({"moves": [{"clip": a.id.0, "track": a_track.0, "time": (a.start - lead).0}], "linked": false})).unwrap();
assert_eq!(state(&s, a.id).1, v.start - lead);
// ask for the picture at 4 frames: the sound would start 6 frames before zero
let r = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": v_track.0, "time": f(4).0}]})).unwrap();
assert_eq!(r["moved"], json!([v.id.0, a.id.0]));
assert_eq!((state(&s, v.id).1, state(&s, a.id).1), (lead, Tick::ZERO), "the sound stops at zero and the picture keeps its 10 frames");
s.execute("edit.undo", json!({})).unwrap();
assert_eq!((state(&s, v.id).1, state(&s, a.id).1), (v.start, v.start - lead));
s.execute("edit.redo", json!({})).unwrap();
assert_eq!((state(&s, v.id).1, state(&s, a.id).1), (lead, Tick::ZERO));
s.execute("edit.undo", json!({})).unwrap();
// a second, unlinked clip 100 frames after the picture, on V2; both listed, both asked 20 frames too far left
let item = s.active_sequence().unwrap().video_tracks[0].items[0].item;
let v2 = s.active_sequence().unwrap().video_tracks[1].id;
s.execute("timeline.place", json!({"item": item.0, "time": (v.start + gap).0, "track": v2.0, "audioTrack": "A2"})).unwrap();
let w = s.active_sequence().unwrap().video_tracks[1].items[0].clone();
let back = v.start + f(20);
let moves = json!([{"clip": v.id.0, "track": v_track.0, "time": (v.start - back).0}, {"clip": w.id.0, "track": v2.0, "time": (w.start - back).0}]);
s.execute("timeline.move", json!({"moves": moves, "linked": false})).unwrap();
assert_eq!((state(&s, v.id).1, state(&s, w.id).1), (Tick::ZERO, gap), "the two clips are still 100 frames apart");
s.execute("edit.undo", json!({})).unwrap();
// with the links followed, the earliest clip of all (the split sound) is the one that stops at zero
s.execute("timeline.move", json!({"moves": moves})).unwrap();
assert_eq!((state(&s, a.id).1, state(&s, v.id).1, state(&s, w.id).1), (Tick::ZERO, lead, lead + gap));
s.active_sequence().unwrap().check().unwrap();
}
/// AGENTS.md §0.8: `time` is any i64. Out-of-range values are parameter errors, never a panic
/// (which `execute` would report as an internal error) and never a wrapped position.
#[test]
fn hostile_times_are_refused_or_clamped_never_a_panic() {
let mut s = demo();
let (v_track, v, a_track, a) = pair_in_an_empty_sequence(&mut s, 60.0);
let before = s.project.clone();
for linked in [true, false] {
for time in [i64::MAX, i64::MAX - 1, Tick::MAX.0 + 1] {
let e = s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": v_track.0, "time": time}], "linked": linked})).unwrap_err();
assert!(matches!(&e, EngineError::BadParams { cmd, .. } if cmd == "timeline.move"), "{time} linked={linked}: {e:?}");
assert_eq!(*s.project, *before, "{time}: nothing moved");
}
// far before the start: the clip lands at zero, as a small negative time does
for time in [i64::MIN, i64::MIN + 1, -1] {
s.execute("timeline.move", json!({"moves": [{"clip": v.id.0, "track": v_track.0, "time": time}], "linked": linked})).unwrap();
assert_eq!(state(&s, v.id).1, Tick::ZERO, "{time}");
assert_eq!(state(&s, a.id).1, if linked { Tick::ZERO } else { a.start }, "{time}");
s.execute("edit.undo", json!({})).unwrap();
}
}
// the sound asked far to the left and the picture far to the right in one call: out of range together
let moves = json!([{"clip": v.id.0, "track": v_track.0, "time": Tick::MAX.0}, {"clip": a.id.0, "track": a_track.0, "time": i64::MIN}]);
assert!(matches!(s.execute("timeline.move", json!({"moves": moves})), Err(EngineError::BadParams { .. })));
// junk entries are skipped as before; a call with nothing usable is a parameter error
for moves in
[json!([]), json!([{"clip": v.id.0, "track": v_track.0, "time": "soon"}]), json!([{"clip": v.id.0, "track": v_track.0, "time": 1e300}]), json!("all")]
{
assert!(matches!(s.execute("timeline.move", json!({"moves": moves})), Err(EngineError::BadParams { .. })), "{moves}");
}
assert!(s.execute("timeline.move", json!({"moves": [{"clip": 987_654_321u64, "track": v_track.0, "time": 0}]})).is_err(), "a clip that does not exist");
assert_eq!(*s.project, *before);
}
+1 -1
View File
@@ -1679,7 +1679,7 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: &
.iter()
.filter_map(|c| seq.find_item(*c).map(|(tid, it)| json!({"clip": c.0, "track": shift_track(seq, tid, track_delta).unwrap_or(tid).0, "time": (it.start + offset).max(Tick::ZERO).0})))
.collect();
Some(app.session.execute("timeline.move", json!({"moves": moves, "insert": mods.command})))
Some(app.session.execute("timeline.move", json!({"moves": moves, "insert": mods.command, "linked": false})))
}
Drag::Trim { clip, edge, mode, delta } if delta != Tick::ZERO => Some(app.session.execute(
"timeline.trim",
+3
View File
@@ -149,6 +149,9 @@ Notes:
nothing do not count. `command_list` / `engine.commands` keep reporting the selection's state.
- Element ids come from the previous frame. If an element is missing right after a layout change,
the app retries on later frames before giving up.
- `timeline.move` takes linked partners along while Linked Selection is on: moving a picture
clip moves its sound by the same offset (pass `"linked": false` to move only the listed clips).
It returns `{"moved": [ids]}`, every clip that moved.
- Without a window, `filmcraft-cli run script.jsonl` (lines of `{"id":"…","params":{…}}`) runs the
same commands headlessly. See §2b for the rest of the CLI.
+11
View File
@@ -80,6 +80,17 @@ management commands and dialog ids: offline media and relinking (`media.findMiss
proxies (`media.createProxies`, `media.attachProxies`, `media.toggleProxies`, `proxies.*`), ingest
(`project.ingestSettings`) and the Project Manager (`file.projectManager`, `pm.*`).
**Moving clips**: `timeline.move {moves: [{clip, track, time}], insert?, linked?}` → `{moved: [clip ids]}`.
With Linked Selection on (the default; `linked` overrides the toggle for one call) the linked
partners of a listed clip that are not listed themselves move by the same time offset and stay on
their own tracks, so picture and sound stay in sync. Like a listed clip, a partner overwrites
what is at its destination (or inserts, with `insert`). A partner on a locked track stays where it
is, and so does one whose position would not change (the listed clip only changed track). `moved`
lists every clip that moved, partners included. No clip is moved before the sequence
start: if one would be, all clips of the call land later by the same amount (their spacing is
kept). A `time` beyond the representable range is an "invalid parameters" error. The timeline
panel's drag passes `linked: false`, because it already lists exactly the clips it moves.
**Trim mode** (`crates/engine/src/trim.rs`): `trim.selectEditPoint` / `trim.selectNearest` enter trim
mode (the Program monitor becomes the Trim Monitor, ids `trimMonitor.*`); `trim.monitor` returns what
it shows. Dynamic trimming takes an explicit clock (seconds) so it is deterministic: `trim.shuttle