#374: trim preview keeps the waveform in place instead of squeezing it

While an edge was dragged, the Timeline drew the clip at its new length
from the untrimmed item, so draw_waveform spread the clip's whole source
range over the shorter (or longer) box: the waveform squeezed and the
sound under the pointer moved, which made it hard to trim to where the
music starts (#374, item 13). Trim and roll previews now draw from the
item the edit will leave, so every timeline time still covered shows the
same media and the waveform is cut off at the moving edge. Video
thumbnails show the new first frame the same way. Rate Stretch, which
really fits the same media into a new length, still squeezes.

The item update is factored out of trim and ripple_trim_group into
filmcraft_edit::move_edge, which the preview uses too, so preview and
edit agree for speed-changed, reversed and frame-held clips.

Rendered mid-drag against the released result, the music clip's
waveform matched in 15% of pixel columns before and 100% after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
JordyUSA 2026-10-10 19:24:37 +07:00
parent 5a5684484f
commit 696f428faf
4 changed files with 188 additions and 38 deletions

View File

@ -569,6 +569,31 @@ pub fn clamp_trim(seq: &Sequence, clip: ClipId, edge: Edge, mode: TrimMode, delt
Ok(d)
}
/// What moving one edge by `d` (already clamped) does to the item itself: its length, the media
/// it starts at (a reversed clip consumes media from its other end; a frame hold has none to
/// move), and, for the In edge with `move_start`, where it starts. The trims and the timeline's
/// live trim preview share it, so the preview shows the media the edit will keep (#374).
pub fn move_edge(it: &mut TrackItem, edge: Edge, d: Tick, move_start: bool) {
let speed = it.speed.abs();
match edge {
Edge::In => {
if !it.reverse && it.frame_hold.is_none() {
it.source_in += src_of(d, speed);
}
it.duration -= d;
if move_start {
it.start += d;
}
}
Edge::Out => {
if it.reverse {
it.source_in -= src_of(d, speed);
}
it.duration += d;
}
}
}
/// Trim one edge of an item (Selection-tool edge drag = Regular; Ripple tool = Ripple).
/// Linked partners should be trimmed by the caller with the same delta.
pub fn trim(seq: &mut Sequence, clip: ClipId, edge: Edge, mode: TrimMode, delta: Tick, ctx: &mut EditCtx) -> Result<Tick> {
@ -580,27 +605,10 @@ pub fn trim(seq: &mut Sequence, clip: ClipId, edge: Edge, mode: TrimMode, delta:
let old_end = it.end();
let old_start = it.start;
let own_link = it.link;
let speed = it.speed.abs();
let mut work = seq.clone();
{
let (_, it) = work.find_item_mut(clip).ok_or(EditError::NoItem(clip))?;
match edge {
Edge::In => {
if !it.reverse && it.frame_hold.is_none() {
it.source_in += src_of(d, speed);
}
it.duration -= d;
if mode == TrimMode::Regular {
it.start += d;
}
}
Edge::Out => {
if it.reverse {
it.source_in -= src_of(d, speed);
}
it.duration += d;
}
}
move_edge(it, edge, d, mode == TrimMode::Regular);
}
if mode == TrimMode::Ripple {
let (at, shift) = match edge {
@ -658,25 +666,11 @@ pub fn ripple_trim_group(seq: &mut Sequence, clips: &[ClipId], edge: Edge, delta
let mut origins: Vec<(TrackId, Tick)> = Vec::new();
for c in clips {
let (tid, it) = work.find_item_mut(*c).ok_or(EditError::NoItem(*c))?;
let speed = it.speed.abs();
let from = match edge {
Edge::In => it.start + Tick(1),
Edge::Out => it.end(),
};
match edge {
Edge::In => {
if !it.reverse && it.frame_hold.is_none() {
it.source_in += src_of(d, speed);
}
it.duration -= d;
}
Edge::Out => {
if it.reverse {
it.source_in -= src_of(d, speed);
}
it.duration += d;
}
}
move_edge(it, edge, d, false);
if !origins.iter().any(|(t, _)| *t == tid) {
origins.push((tid, from));
}

View File

@ -749,3 +749,40 @@ fn roll_reversed_clips_keeps_media_mapping() {
let (_, r) = fx.seq.find_item(ib).unwrap();
assert_eq!((r.start, r.duration, r.source_in), (f(40), f(20), f(50)));
}
/// #374: moving an edge must not change which media sits under any timeline time the clip still
/// covers (the timeline's live trim preview draws its waveform from this item), and must leave the
/// item as the trim itself does.
#[test]
fn moving_an_edge_keeps_the_media_under_the_timeline() {
let fx = Fx::new();
let v1 = fx.v(0);
for (speed, reverse, hold) in [(1.0, false, None), (2.0, false, None), (0.5, false, None), (1.0, true, None), (2.0, true, None), (1.0, false, Some(f(50)))]
{
for (edge, d) in [(Edge::In, f(7)), (Edge::In, -f(4)), (Edge::Out, -f(9)), (Edge::Out, f(3))] {
let mut fresh = Fx::new();
let c = fresh.put(v1, 100, 40, 200);
{
let it = fresh.seq.find_item_mut(c).unwrap().1;
(it.speed, it.reverse, it.frame_hold) = (speed, reverse, hold);
}
let before = fresh.seq.find_item(c).unwrap().1.clone();
let mut moved = before.clone();
move_edge(&mut moved, edge, d, true);
let label = format!("speed {speed} reverse {reverse} hold {hold:?} {edge:?} {d:?}");
// every frame still covered shows the same media as before the move
let mut t = moved.start.max(before.start);
while t < moved.end().min(before.end()) {
let (a, b) = (before.source_time_at(t), moved.source_time_at(t));
assert!((a - b).0.abs() <= 1, "{label}: at {t:?} {a:?} became {b:?}");
t += f(1);
}
// and the item is what the real (unclamped) trim makes of it
let mut n = fresh.next;
let got = trim(&mut fresh.seq, c, edge, TrimMode::Regular, d, &mut Fx::ctx(&mut n)).unwrap();
assert_eq!(got, d, "{label}: not clamped");
let trimmed = fresh.seq.find_item(c).unwrap().1;
assert_eq!((trimmed.start, trimmed.duration, trimmed.source_in), (moved.start, moved.duration, moved.source_in), "{label}");
}
}
}

View File

@ -370,7 +370,10 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
let visible = (layout.tick_at(content.min.x - 2.0), layout.tick_at(content.max.x + 2.0));
let selection: Vec<ClipId> = app.session.state.selection.clone();
let mut previews: HashMap<ClipId, (Tick, Tick, Option<TrackId>)> = HashMap::new(); // live drag preview: (start, dur, track)
preview_drag(app, &seq, &layout, &mut previews);
// clips whose media changes in the preview (a trimmed or rolled edge): drawn from the edited
// item, so the waveform and thumbnail stay where the media is instead of squeezing (#374)
let mut edited: HashMap<ClipId, TrackItem> = HashMap::new();
preview_drag(app, &seq, &mut previews, &mut edited);
for r in &rows {
let Some(tr) = seq.track(r.track) else { continue };
let clip_rect = if r.kind == TrackKind::Video { vclip } else { aclip };
@ -381,6 +384,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
let p = painter.with_clip_rect(Rect::from_min_max(pos2(content.min.x, row.min.y), pos2(content.max.x, row.max.y)));
let volume_line = r.kind == TrackKind::Audio && !r.lane;
for it in &tr.items {
let it = edited.get(&it.id).unwrap_or(it);
let (start, dur, moved_track) = previews.get(&it.id).copied().unwrap_or((it.start, it.duration, None));
if moved_track.is_some_and(|m| m != r.track) {
continue;
@ -1518,7 +1522,14 @@ pub fn locate(app: &FilmcraftApp, clip: u64, edge: Option<&str>) -> Option<(f32,
Some((x, y))
}
fn preview_drag(app: &FilmcraftApp, seq: &Sequence, _layout: &Layout, out: &mut HashMap<ClipId, (Tick, Tick, Option<TrackId>)>) {
fn preview_drag(app: &FilmcraftApp, seq: &Sequence, out: &mut HashMap<ClipId, (Tick, Tick, Option<TrackId>)>, edited: &mut HashMap<ClipId, TrackItem>) {
// the item as the edge move leaves it, and its place in the preview
let mut moved = |it: &TrackItem, edge: filmcraft_edit::Edge, delta: Tick| {
let mut e = it.clone();
filmcraft_edit::move_edge(&mut e, edge, delta, true);
out.insert(e.id, (e.start, e.duration, None));
edited.insert(e.id, e);
};
let Some(d) = &app.tl.drag else { return };
match d {
Drag::Move { clips, offset, track_delta, .. } => {
@ -1534,7 +1545,15 @@ fn preview_drag(app: &FilmcraftApp, seq: &Sequence, _layout: &Layout, out: &mut
out.insert(*clip, (it.start, it.duration + *delta, None));
}
}
Drag::Trim { clip, edge, delta, .. } | Drag::Stretch { clip, edge, delta, .. } => {
Drag::Trim { clip, edge, delta, .. } => {
for c in filmcraft_engine::commands::with_links(&app.session, &[*clip]) {
if let Some((_, it)) = seq.find_item(c) {
moved(it, *edge, *delta);
}
}
}
// a rate stretch keeps the same media in a new length: that one does squeeze
Drag::Stretch { clip, edge, delta, .. } => {
let ids = filmcraft_engine::commands::with_links(&app.session, &[*clip]);
for c in ids {
if let Some((_, it)) = seq.find_item(c) {
@ -1548,10 +1567,10 @@ fn preview_drag(app: &FilmcraftApp, seq: &Sequence, _layout: &Layout, out: &mut
}
Drag::Roll { left, right, delta, .. } => {
if let Some((_, l)) = seq.find_item(*left) {
out.insert(*left, (l.start, l.duration + *delta, None));
moved(l, filmcraft_edit::Edge::Out, *delta);
}
if let Some((_, r)) = seq.find_item(*right) {
out.insert(*right, (r.start + *delta, r.duration - *delta, None));
moved(r, filmcraft_edit::Edge::In, *delta);
}
}
Drag::Slide { clip, delta } => {
@ -2464,6 +2483,72 @@ pub(crate) fn chain_starts(start: Tick, durations: &[Tick]) -> Vec<Tick> {
.collect()
}
/// #374: trimming an audio clip must cut its waveform off at the moving edge, not squeeze the whole
/// waveform into the shorter clip: the preview draws from the item the trim will leave, which shows
/// the same media at every timeline time it still covers.
#[cfg(test)]
mod trim_preview_tests {
use super::*;
fn demo() -> (FilmcraftApp, Sequence) {
let mut s = filmcraft_engine::Session::default();
s.execute("file.openDemoProject", serde_json::json!({})).unwrap();
let seq = s.active_sequence().unwrap().clone();
(FilmcraftApp::new(s), seq)
}
fn assert_same_media(before: &TrackItem, after: &TrackItem) {
let mut t = after.start.max(before.start);
while t < after.end().min(before.end()) {
assert!((before.source_time_at(t) - after.source_time_at(t)).0.abs() <= 1, "{}: media moved at {t:?}", before.name);
t += Tick(filmcraft_time::TICKS_PER_SECOND / 10);
}
}
#[test]
fn trimming_the_music_keeps_its_waveform_in_place() {
let (mut app, seq) = demo();
let music = seq.audio_tracks.iter().flat_map(|t| &t.items).find(|i| i.name.contains("Ambient")).unwrap().clone();
for (edge, delta) in [(filmcraft_edit::Edge::In, 5), (filmcraft_edit::Edge::Out, -5), (filmcraft_edit::Edge::In, -1)] {
let delta = Tick(delta * filmcraft_time::TICKS_PER_SECOND);
app.tl.drag = Some(Drag::Trim { clip: music.id, edge, mode: filmcraft_edit::TrimMode::Regular, delta, from: Tick::ZERO });
let (mut out, mut edited) = (HashMap::new(), HashMap::new());
preview_drag(&app, &seq, &mut out, &mut edited);
let shown = edited.get(&music.id).expect("the trimmed clip is drawn from its edited item");
// where the preview places it is unchanged
let want = match edge {
filmcraft_edit::Edge::In => (music.start + delta, music.duration - delta, None),
filmcraft_edit::Edge::Out => (music.start, music.duration + delta, None),
};
assert_eq!(out.get(&music.id).copied(), Some(want));
assert_eq!((shown.start, shown.duration), (want.0, want.1));
// the music under every remaining timeline time is the same: the waveform stays put
assert_same_media(&music, shown);
if edge == filmcraft_edit::Edge::In {
assert_eq!(shown.source_in, music.source_in + delta, "5 s into the music, not the music squeezed");
}
}
}
#[test]
fn rolling_an_edit_keeps_both_sides_in_place_but_a_stretch_still_squeezes() {
let (mut app, seq) = demo();
let (left, right) = seq.audio_tracks[0].items.windows(2).find(|w| w[0].end() == w[1].start).map(|w| (w[0].clone(), w[1].clone())).unwrap();
let delta = Tick(filmcraft_time::TICKS_PER_SECOND / 2);
app.tl.drag = Some(Drag::Roll { left: left.id, right: right.id, delta, from: Tick::ZERO });
let (mut out, mut edited) = (HashMap::new(), HashMap::new());
preview_drag(&app, &seq, &mut out, &mut edited);
assert_same_media(&left, &edited[&left.id]);
assert_same_media(&right, &edited[&right.id]);
assert_eq!(edited[&right.id].start, right.start + delta);
// Rate Stretch keeps the same media in a new length: drawn from the original item
app.tl.drag = Some(Drag::Stretch { clip: left.id, edge: filmcraft_edit::Edge::Out, delta, from: Tick::ZERO });
let (mut out, mut edited) = (HashMap::new(), HashMap::new());
preview_drag(&app, &seq, &mut out, &mut edited);
assert!(edited.is_empty() && out.contains_key(&left.id));
}
}
#[cfg(test)]
mod waveform_tests {
use super::waveform_display_gain;

View File

@ -298,3 +298,37 @@ fn dragging_a_caption_edge_inward_trims_it() {
assert_eq!(s1, s0, "the caption's start stayed: trimmed, not moved");
assert!(e1 < e0, "the caption's end moved in: {e0} → {e1}");
}
/// #374: renders the Ambient_Score music clip mid-way through a trim of its In edge, with the
/// mouse button still down, for visual review (needs a GPU): `cargo test -p filmcraft-ui-egui
/// --test timeline_trim_ui -- --ignored trim_waveform_screenshots`. The waveform stays where it was
/// and is cut off at the moving edge; it used to squeeze the whole clip's waveform into the shorter
/// clip, so the sound under the pointer moved while dragging.
#[test]
#[ignore]
fn trim_waveform_screenshots() {
let mut d = Driver::demo();
let seq = d.app().session.active_sequence().expect("sequence");
let music = seq.audio_tracks.iter().flat_map(|t| &t.items).find(|i| i.name.contains("Ambient")).expect("music clip").id.0;
let (x0, _, y) = d.edges(music);
// waveform peaks are read on a background thread
for _ in 0..40 {
std::thread::sleep(std::time::Duration::from_millis(50));
d.frames(1);
}
d.screenshot("trim-waveform-before");
let push = |d: &mut Driver, e: egui::Event| d.harness.input_mut().events.push(e);
let to = pos2(x0 + 160.0, y);
push(&mut d, egui::Event::PointerMoved(pos2(x0, y)));
d.frames(1);
push(&mut d, egui::Event::PointerButton { pos: pos2(x0, y), button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() });
d.frames(1);
for i in 1..=20 {
push(&mut d, egui::Event::PointerMoved(pos2(x0 + 160.0 * i as f32 / 20.0, y)));
d.frames(1);
}
d.screenshot("trim-waveform-dragging");
push(&mut d, egui::Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() });
d.frames(2);
d.screenshot("trim-waveform-released");
}