Merge pull request #663 from vangel76/fix-653-clamp-trim-drag

#653: a dragged trim edge stops at the end of the media
This commit is contained in:
Brandon Thomas 2026-10-10 15:52:03 -04:00 committed by GitHub
commit 8747305aa0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 79 additions and 8 deletions

View File

@ -407,6 +407,31 @@ pub fn with_links(s: &Session, clips: &[ClipId]) -> Vec<ClipId> {
out
}
/// The delta a trim of `clips` (a clip and its linked partners) can apply together: `delta`
/// clamped by each one's media and neighbours.
fn clamp_trim_linked(q: &filmcraft_project::Sequence, clips: &[ClipId], edge: Edge, mode: TrimMode, delta: Tick, ctx: &edit::EditCtx) -> Result<Tick> {
let mut d = delta;
for c in clips {
let x = edit::clamp_trim(q, *c, edge, mode, d, ctx)?;
if x.abs() < d.abs() {
d = x;
}
}
Ok(d)
}
/// The delta `timeline.trim` would apply for `delta`, without editing anything: the Timeline's
/// trim drag shows this, so the edge stops at the end of the media or the neighbouring clip
/// instead of following the pointer past it (#653).
pub fn trim_delta(s: &Session, clip: ClipId, edge: Edge, mode: TrimMode, delta: Tick) -> Result<Tick> {
let q = s.active_sequence().ok_or(EngineError::NoSequence)?;
let durations = |id: ItemId| crate::media_duration(&s.project, &s.media, id);
let starts = |id: ItemId| crate::media_start(&s.project, id);
let mut next = s.project.next_id;
let ctx = edit::EditCtx { next_id: &mut next, media_duration: &durations, media_start: &starts, min_duration: q.settings.frame_rate.frame_duration() };
clamp_trim_linked(q, &with_links(s, &[clip]), edge, mode, delta, &ctx)
}
/// Sequence settings matching a media clip (New Sequence From Clip, New Sequence from an item):
/// its frame size, rate and pixel aspect ratio (Interpret Footage's, else the file's), so a
/// 1440 x 1080 clip with 4:3 pixels makes a 1440 x 1080 sequence that displays 16:9.
@ -2248,13 +2273,7 @@ fn build() -> Vec<CommandSpec> {
let applied = s.edit_sequence(if mode == TrimMode::Ripple { "Ripple Trim" } else { "Trim" }, |q, ctx, st| {
let before = q.find_item(c).map(|(_, i)| i.range());
// clamp across all linked partners, then apply the common delta
let mut dd = d;
for c in &clips {
let x = edit::clamp_trim(q, *c, edge, mode, dd, ctx)?;
if x.abs() < dd.abs() {
dd = x;
}
}
let mut dd = clamp_trim_linked(q, &clips, edge, mode, d, ctx)?;
if mode == TrimMode::Ripple {
// the clip and its linked partners ripple as one edit
let mut group = vec![c];

View File

@ -236,3 +236,21 @@ fn extend_to_playhead_moves_each_unequal_out_point_to_the_playhead() {
assert_eq!(q.find_item(a).unwrap().1.end(), ph);
assert_eq!(q.find_item(b).unwrap().1.end(), ph);
}
/// #653: the delta the Timeline previews while an edge is dragged is the one `timeline.trim`
/// applies on release: dragging the last clip's Out edge far past the end of its media stops at
/// the media's end instead of following the pointer.
#[test]
fn trim_delta_stops_at_the_end_of_the_media() {
let mut s = demo();
let last = *v1(&s).last().unwrap();
let clip = ClipId(last.0);
let far = Tick(3600 * filmcraft_time::TICKS_PER_SECOND);
let shown = commands::trim_delta(&s, clip, filmcraft_edit::Edge::Out, filmcraft_edit::TrimMode::Regular, far).unwrap();
assert!(Tick::ZERO < shown && shown < far, "clamped to the media left after the clip: {shown:?}");
let r = s.execute("timeline.trim", json!({"clip": last.0, "edge": "out", "mode": "regular", "delta": far.0})).unwrap();
assert_eq!(r["delta"], json!(shown.0), "the preview and the trim agree");
let small = s.sequence_rate().frame_duration();
let back = commands::trim_delta(&s, clip, filmcraft_edit::Edge::Out, filmcraft_edit::TrimMode::Regular, -small).unwrap();
assert_eq!(back, -small, "a trim within the media is not clamped");
}

View File

@ -2119,7 +2119,9 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: &
Drag::Trim { clip, edge, mode, from, .. } => seq.find_item(clip).map(|(_, it)| {
let base = if edge == filmcraft_edit::Edge::In { it.start } else { it.end() };
let target = snap(app, seq, layout, rate.snap_nearest(base + (t_here - from)), &[clip]);
Drag::Trim { clip, edge, mode, delta: target - base, from }
// stop at the end of the media (or the neighbour), as the trim on release will (#653)
let delta = filmcraft_engine::commands::trim_delta(&app.session, clip, edge, mode, target - base).unwrap_or(target - base);
Drag::Trim { clip, edge, mode, delta, from }
}),
Drag::Remix { clip, from, .. } => seq.find_item(clip).map(|(_, it)| {
let target = rate.snap_nearest(it.end() + (t_here - from)).max(it.start + rate.frame_duration());

View File

@ -299,6 +299,38 @@ fn dragging_a_caption_edge_inward_trims_it() {
assert!(e1 < e0, "the caption's end moved in: {e0} → {e1}");
}
/// #653: an Out edge dragged far past the end of the clip's media stops there while the drag is
/// still in progress, at the same place the trim lands on release. It used to follow the pointer
/// past the media and then jump back when the button came up.
#[test]
fn an_edge_dragged_past_the_media_stops_at_its_end() {
use filmcraft_ui_egui::panels::timeline::Drag;
let mut d = Driver::demo();
let last = d.app().session.active_sequence().expect("sequence").video_tracks[0].items.last().expect("a clip on V1").id.0;
let (_, x1, y) = d.edges(last);
let (_, e0) = d.span(last);
let far = filmcraft_time::Tick(3600 * filmcraft_time::TICKS_PER_SECOND);
let limit = filmcraft_engine::commands::trim_delta(&d.app().session, ClipId(last), filmcraft_edit::Edge::Out, filmcraft_edit::TrimMode::Regular, far)
.expect("limit");
let (from, to) = (pos2(x1 - 3.0, y), pos2(x1 + 300.0, y));
let push = |d: &mut Driver, e: egui::Event| d.harness.input_mut().events.push(e);
push(&mut d, egui::Event::PointerMoved(from));
d.frames(1);
push(&mut d, egui::Event::PointerButton { pos: from, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() });
d.frames(1);
for i in 1..=60 {
push(&mut d, egui::Event::PointerMoved(from + (to - from) * (i as f32 / 60.0)));
d.frames(1);
}
match &d.app().tl.drag {
Some(Drag::Trim { delta, .. }) => assert_eq!(*delta, limit, "the dragged edge stops at the end of the media"),
other => panic!("not trimming: {other:?}"),
}
push(&mut d, egui::Event::PointerButton { pos: to, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() });
d.frames(2);
assert_eq!(d.span(last).1, e0 + limit.0, "released where it was shown");
}
/// Dragging a picture in the Trim Monitor must not deadlock: the shift bookkeeping used to call
/// `drag_delta()` inside `data_mut()`, and both take egui's non-reentrant context write-lock, so
/// the drag froze the whole app on its first recognised frame (the delta is read before locking