timeline: Track Select Forward/Backward selects and moves the captions with the clips (#3388)

crates/engine/src/captions/tests.rs

@@ -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();

crates/engine/src/commands.rs

@@ -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
         ),

crates/ui-egui/src/panels/timeline/interact.rs

@@ -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",

crates/ui-egui/tests/it/track_select_drag_ui.rs

@@ -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();

docs/control-protocol.md

@@ -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