collective
Browse / github.com/storytold/filmcraft / patch
timeline: Track Select Forward/Backward selects and moves the captions with the clips (#3388)
Track Select Forward/Backward does not select or move captions ### FilmCraft version or commit 0.12.0. The same code is still on `main` at 8fda2f2. ### Operating system and architecture Ubuntu 24.04 x86_64 ### Installation type Portable archive or AppImage ### Steps to reproduce 1. Open a sequence that has clips on video and audio tracks and a subtitle caption track with captions along the whole timeline. The caption track is unlocked and has sync lock on. 2. Pick the Track Select Forward tool (A). 3. Click on a track somewhere in the middle of the sequence. Every clip after the pointer is selected, but none of the captions are. 4. Drag to move the
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- x-labb (imported from pull request #3407, open)
- Base
10d626ccc4888e0f020f9ca314c2bc9148ded4f4- Size
- +223 −6 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
ac0a56e37d8b0a882dac13a1
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/captions/tests.rs crates/engine/src/commands.rs crates/ui-egui/src/panels/timeline/interact.rs crates/ui-egui/tests/it/track_select_drag_ui.rs docs/control-protocol.md
@@ -164,6 +164,85 @@ fn caption_spans(s: &Session) -> Vec<(f64, f64, String)> { s.active_sequence().unwrap().caption_tracks[0].captions.iter().map(|c| (secs(c.start), secs(c.end()), c.text.clone())).collect() } +/// (clip id, start) of the clips on V1, and every caption's start.+fn v1_and_caption_starts(s: &Session) -> (Vec<(u64, Tick)>, Vec<Tick>) {+ let q = s.active_sequence().unwrap();+ (q.video_tracks[0].items.iter().map(|i| (i.id.0, i.start)).collect(), q.caption_tracks[0].captions.iter().map(|c| c.start).collect())+}++#[test]+fn timeline_move_takes_the_listed_captions_along_in_one_undo_step() {+ let mut s = demo();+ for (sec, text) in [(1.0, "early"), (9.0, "late one"), (12.0, "late two")] {+ s.execute("playhead.set", json!({"seconds": sec})).unwrap();+ s.execute("captions.add", json!({"text": text, "durationSeconds": 1.0})).unwrap();+ }+ let q = s.active_sequence().unwrap();+ let ids: Vec<u64> = q.caption_tracks[0].captions.iter().map(|c| c.id.0).collect();+ let (clip, start) = (q.video_tracks[0].items[2].id, q.video_tracks[0].items[2].start);+ let (clips_before, caps_before) = v1_and_caption_starts(&s);+ let undo_before = s.history.undo.len();+ let shift = 2 * TICKS_PER_SECOND;++ // the captions named move by the clip's own offset; `early` is not named and stays+ let r = s+ .execute(+ "timeline.move",+ json!({"moves": [{"clip": clip.0, "track": "V1", "time": (start + Tick(shift)).0}], "linked": false, "captions": [ids[1], ids[2], ids[1]]}),+ )+ .unwrap();+ assert_eq!(r["captions"], json!([ids[1], ids[2]]), "each caption once");+ let (clips_after, caps_after) = v1_and_caption_starts(&s);+ assert_eq!(clips_after.iter().find(|c| c.0 == clip.0).unwrap().1, start + Tick(shift));+ assert_eq!(caps_after, vec![caps_before[0], caps_before[1] + Tick(shift), caps_before[2] + Tick(shift)]);+ assert_eq!(s.history.undo.len(), undo_before + 1, "clips and captions are one undo step");+ s.active_sequence().unwrap().check().unwrap();++ s.execute("edit.undo", json!({})).unwrap();+ assert_eq!(v1_and_caption_starts(&s), (clips_before, caps_before), "undo restores both");+}++#[test]+fn timeline_move_with_captions_that_cannot_follow_changes_nothing() {+ let mut s = demo();+ for (sec, text) in [(2.0, "stays"), (5.0, "moves")] {+ s.execute("playhead.set", json!({"seconds": sec})).unwrap();+ s.execute("captions.add", json!({"text": text, "durationSeconds": 1.0})).unwrap();+ }+ let q = s.active_sequence().unwrap();+ let moving = q.caption_tracks[0].captions[1].id.0;+ let (clip, start) = (q.video_tracks[0].items[1].id, q.video_tracks[0].items[1].start);+ let before = v1_and_caption_starts(&s);+ let undo_before = s.history.undo.len();++ // 4 s earlier would put `moves` on top of `stays`: the clip does not move either+ let back = Tick(start.0 - 4 * TICKS_PER_SECOND).max(Tick::ZERO);+ let r = s.execute("timeline.move", json!({"moves": [{"clip": clip.0, "track": "V1", "time": back.0}], "linked": false, "captions": [moving]}));+ assert!(r.is_err(), "the captions cannot follow: {r:?}");+ assert_eq!(v1_and_caption_starts(&s), before);+ assert_eq!(s.history.undo.len(), undo_before, "a refused move leaves no undo step");++ // a copy leaves the originals where they are, so there is nothing for captions to follow+ let r = s.execute("timeline.move", json!({"moves": [{"clip": clip.0, "track": "V2", "time": start.0}], "copy": true, "captions": [moving]}));+ assert!(r.is_err());+ assert_eq!(v1_and_caption_starts(&s), before);++ // a caption that does not exist, or sits on a locked track, refuses the move as well+ let missing = s.execute(+ "timeline.move",+ json!({"moves": [{"clip": clip.0, "track": "V1", "time": (start + Tick(TICKS_PER_SECOND)).0}], "captions": [987_654_321u64]}),+ );+ assert!(missing.is_err());+ assert_eq!(v1_and_caption_starts(&s), before, "the clip did not move either");+ s.execute("captions.setTrack", json!({"track": "C1", "locked": true})).unwrap();+ let locked =+ s.execute("timeline.move", json!({"moves": [{"clip": clip.0, "track": "V1", "time": (start + Tick(TICKS_PER_SECOND)).0}], "captions": [moving]}));+ assert!(locked.is_err());+ assert_eq!(v1_and_caption_starts(&s), before);+ assert_eq!(s.history.undo.len(), undo_before + 1, "only the lock itself was recorded");+ s.active_sequence().unwrap().check().unwrap();+}+ #[test] fn captions_duplicate_copies_and_overwrites() { let mut s = demo();@@ -349,7 +349,9 @@ pub(crate) fn item_p(p: &Value, k: &str) -> Option<ItemId> { /// before and after (ticks) and the clips what is left of it now lives on as (none = removed; a /// clip a moved clip lands inside is split in two). With `copy: true` (Option-drag) the clips stay /// where they are and copies land at the destinations instead; the copies are selected and listed-/// as `copied`.+/// as `copied`. `captions` (caption ids) move with them by the same offset in the same undo step;+/// if they cannot go that far (they would overlap captions that stay, or sit on a locked track)+/// nothing moves. Not with `copy`. fn move_clips(s: &mut Session, p: &Value) -> Result<Value> { const CMD: &str = "timeline.move"; let mut listed: Vec<(ClipId, TrackId, Tick, bool)> = Vec::new();@@ -395,6 +397,22 @@ fn move_clips(s: &mut Session, p: &Value) -> Result<Value> { } } let late = plan.iter().map(|m| -m.2).max().unwrap_or(0).max(0);+ // `captions` ride along by the first listed clip's offset (shift above included), so subtitles+ // stay on the dialogue they belong to+ let mut caption_ids: Vec<ClipId> =+ p.get("captions").and_then(Value::as_array).map(|a| a.iter().filter_map(|v| v.as_u64().map(ClipId)).collect()).unwrap_or_default();+ let mut seen = std::collections::HashSet::new();+ caption_ids.retain(|c| seen.insert(*c));+ if !caption_ids.is_empty() && bool_p(p, "copy").unwrap_or(false) {+ return Err(bad(CMD, "`captions` cannot be combined with `copy`"));+ }+ let caption_delta = match plan.first() {+ Some(&(clip, _, start, ..)) if !caption_ids.is_empty() => {+ let from = q.find_item(clip).map(|(_, it)| i128::from(it.start.0)).ok_or(edit::EditError::NoItem(clip))?;+ Tick(i64::try_from(start + late - from).map_err(|_| bad(CMD, "`time` is out of range"))?)+ }+ _ => Tick::ZERO,+ }; let mut moves: Vec<(ClipId, TrackId, Tick, bool)> = Vec::with_capacity(plan.len()); for (clip, track, start, was, new_track) in plan { // a partner that ends up where it is does not move (its listed clip only changed track):@@ -454,7 +472,12 @@ fn move_clips(s: &mut Session, p: &Value) -> Result<Value> { } else { s.edit_sequence(if ins { "Move (Insert)" } else { "Move" }, |q, ctx, _| { let destinations = prepare(q, ctx)?;- Ok(edit::move_items(q, &destinations, ins, ctx)?)+ edit::move_items(q, &destinations, ins, ctx)?;+ // all or nothing: captions that cannot go as far as the clips would drift off their dialogue+ if caption_delta != Tick::ZERO && edit::captions::move_captions(q, &caption_ids, caption_delta)? != caption_delta {+ return Err(bad(CMD, "the captions cannot move as far as the clips (they would overlap other captions)"));+ }+ Ok(()) })?; Vec::new() };@@ -478,7 +501,11 @@ fn move_clips(s: &mut Session, p: &Value) -> Result<Value> { if copy { return Ok(json!({"copied": copied.iter().map(|c| c.0).collect::<Vec<_>>(), "overwritten": overwritten})); }- Ok(json!({"moved": moves.iter().map(|m| m.0.0).collect::<Vec<_>>(), "overwritten": overwritten}))+ let mut result = json!({"moved": moves.iter().map(|m| m.0.0).collect::<Vec<_>>(), "overwritten": overwritten});+ if !caption_ids.is_empty() {+ result["captions"] = json!(caption_ids.iter().map(|c| c.0).collect::<Vec<_>>());+ }+ Ok(result) } /// Slip clips together: the media delta of each clip is scaled down by the same factor so that the@@ -2784,7 +2811,7 @@ fn build() -> Vec<CommandSpec> { "Move Clips", [], None,- r#"{"moves":[{"clip":id,"track":id|"V2"|"new" (after the last of its kind),"time":ticks}],"insert":bool,"linked":bool?,"copy":bool?}"#,+ r#"{"moves":[{"clip":id,"track":id|"V2"|"new" (after the last of its kind),"time":ticks}],"insert":bool,"linked":bool?,"copy":bool?,"captions":[id]?}"#, has_seq, move_clips ),@@ -395,6 +395,19 @@ pub(super) fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence .map(|i| i.id.0) .collect(); let _ = app.session.execute("timeline.select", json!({"clips": ids}));+ // the captions on the same side go with the clips so the subtitles stay in sync;+ // Shift keeps to the clicked track, and a locked caption track stays put+ if !mods.shift {+ let captions: Vec<u64> = seq+ .caption_tracks+ .iter()+ .filter(|tr| !tr.locked)+ .flat_map(|tr| tr.captions.iter())+ .filter(|c| if fwd { c.end() > t } else { c.start < t })+ .map(|c| c.id.0)+ .collect();+ let _ = app.session.execute("captions.select", json!({"captions": captions, "add": true}));+ } // as in Premiere Pro, a drag with the tool moves what it just selected if resp.drag_started() { let clips = app.session.state.selection.clone();@@ -707,7 +720,14 @@ pub(super) fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence .filter_map(|c| seq.find_item(*c).map(|(tid, it)| json!({"clip": c.0, "track": to(tid), "time": (it.start + offset).max(Tick::ZERO).0}))) .collect(); // Option held while dropping copies (Premiere: Option-drag duplicates)- Some(app.session.execute("timeline.move", json!({"moves": moves, "insert": mods.command, "linked": false, "copy": copy || mods.alt})))+ let copying = copy || mods.alt;+ // the captions Track Select picked up move with the clips, in the same undo step+ let captions: Vec<u64> = if matches!(tool, Tool::TrackSelectForward | Tool::TrackSelectBackward) && !copying {+ app.session.state.caption_selection.iter().map(|c| c.0).collect()+ } else {+ Vec::new()+ };+ Some(app.session.execute("timeline.move", json!({"moves": moves, "insert": mods.command, "linked": false, "copy": copying, "captions": captions}))) } Drag::Trim { clip, edge, mode, delta, linked, .. } if delta != Tick::ZERO => Some(app.session.execute( "timeline.trim",@@ -99,6 +99,93 @@ fn a_drag_with_track_select_forward_moves_every_clip_after_the_pointer() { assert_moved_from(&before, &after, from); } +impl Driver {+ fn exec(&mut self, command: &str, params: Value) -> Value {+ self.ok("engine.execute", json!({"command": command, "params": params}))+ }+ fn add_caption(&mut self, seconds: f64, text: &str) -> u64 {+ self.exec("captions.add", json!({"seconds": seconds, "durationSeconds": 1.0, "text": text}))["caption"].as_u64().expect("caption id")+ }+ /// Every caption on C1 as (text, start).+ fn caption_starts(&mut self) -> Vec<(String, Tick)> {+ let r = self.exec("captions.list", json!({}));+ let caps = r["tracks"][0]["captions"].as_array().expect("captions");+ caps.iter().map(|c| (c["text"].as_str().expect("text").to_string(), Tick(c["start"].as_i64().expect("start")))).collect()+ }+}++/// #3388: the captions after the pointer are selected with the clips and move by the same amount,+/// so the subtitles stay on the dialogue they belong to; those before it stay; one undo restores both.+#[test]+fn a_drag_with_track_select_forward_moves_the_captions_after_the_pointer_with_the_clips() {+ let mut d = Driver::demo();+ let (x, y, from) = second_v1_clip(&mut d);+ let clip_end = d.harness.state().session.active_sequence().unwrap().video_tracks[0].items[1].end().seconds();+ assert!(from.seconds() > 0.75, "room for a caption before the second clip");+ d.add_caption(from.seconds() - 0.75, "before");+ d.add_caption(clip_end + 0.5, "after one");+ d.add_caption(clip_end + 2.5, "after two");+ let clips_before = d.spans();+ let caps_before = d.caption_starts();+ let undo_before = d.harness.state().session.history.undo.len();++ d.ok("ui.set", json!({"tool": "TrackSelectForward"}));+ d.ok("ui.drag", json!({"from": {"x": x, "y": y}, "to": {"x": x + 240.0, "y": y}, "steps": 12}));+ d.frames(3);++ let clips_after = d.spans();+ assert_moved_from(&clips_before, &clips_after, from);+ let offset = clips_after.iter().filter(|(id, _)| clips_before[id].0 >= from).map(|(id, a)| a.0 - clips_before[id].0).next().expect("a moved clip");+ let caps_after = d.caption_starts();+ assert_eq!(caps_after.len(), caps_before.len());+ for ((text, was), (text_after, now)) in caps_before.iter().zip(&caps_after) {+ assert_eq!(text, text_after);+ let expected = if text == "before" { *was } else { *was + offset };+ assert_eq!(*now, expected, "caption `{text}` moved with the clips");+ }+ assert_eq!(d.harness.state().session.history.undo.len(), undo_before + 1, "clips and captions move in one undo step");++ d.exec("edit.undo", json!({}));+ assert_eq!(d.spans(), clips_before, "undo puts the clips back");+ assert_eq!(d.caption_starts(), caps_before, "and the captions");+}++/// Backward selects the captions that start before the pointer, Forward those that end after it,+/// Shift keeps to the clicked track (no caption track), and a locked caption track is left out.+#[test]+fn track_select_picks_the_captions_on_its_side_of_the_pointer() {+ let mut d = Driver::demo();+ let (x, y, from) = second_v1_clip(&mut d);+ let clip_end = d.harness.state().session.active_sequence().unwrap().video_tracks[0].items[1].end().seconds();+ assert!(from.seconds() > 0.75, "room for a caption before the second clip");+ let before = d.add_caption(from.seconds() - 0.75, "before");+ let after = d.add_caption(clip_end + 0.5, "after");+ let picked = |d: &mut Driver| -> Vec<u64> {+ let mut ids: Vec<u64> = d.harness.state().session.state.caption_selection.iter().map(|c| c.0).collect();+ ids.sort_unstable();+ ids+ };++ d.ok("ui.set", json!({"tool": "TrackSelectBackward"}));+ d.ok("ui.click", json!({"x": x, "y": y}));+ d.frames(2);+ assert_eq!(picked(&mut d), vec![before], "Backward: the caption before the pointer");++ d.ok("ui.set", json!({"tool": "TrackSelectForward"}));+ d.ok("ui.click", json!({"x": x, "y": y}));+ d.frames(2);+ assert_eq!(picked(&mut d), vec![after], "Forward: the caption after the pointer");++ d.ok("ui.click", json!({"x": x, "y": y, "modifiers": {"shift": true}}));+ d.frames(2);+ assert!(picked(&mut d).is_empty(), "Shift: only the clicked track, which has no captions");++ d.exec("captions.setTrack", json!({"track": "C1", "locked": true}));+ d.ok("ui.click", json!({"x": x, "y": y}));+ d.frames(2);+ assert!(picked(&mut d).is_empty(), "a locked caption track is left alone");+}+ #[test] fn clips_selected_with_track_select_forward_move_with_the_selection_tool() { let mut d = Driver::demo();@@ -135,7 +135,7 @@ 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]}`.+**Moving clips**: `timeline.move {moves: [{clip, track, time}], insert?, linked?, captions?}` → `{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@@ -148,6 +148,10 @@ panel's drag passes `linked: false`, because it already lists exactly the clips `copy: true` (Option-drag in the timeline) the clips stay where they are and copies land at the destinations instead (linked partners are copied too, unless `linked: false`); the result lists the new clips as `copied`, they are selected, and copies of linked clips are linked to each other.+`captions: [caption id]` moves those captions by the same offset as the first listed clip, in the same undo+step (Track Select Forward / Backward drags use it so subtitles stay with the dialogue). If they cannot go+as far as the clips (they would overlap captions that stay, or one is on a locked track) the call fails and+nothing moves; it cannot be combined with `copy`, and the result lists them as `captions`. **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