fix(timeline): place a clip from empty space on top of what it covers
Dropping a new clip whose in-point landed in empty track space (the stored-range model allows holes between blocks; the C++ layout is contiguous) left the overlapped clip untouched AND inserted the new clip before it in track order, so it slid UNDER the clip it covered. Starting on a clip already overwrote correctly, so the behavior depended on where the in-point happened to fall. TrackRippleRemoveAreaCommand::prepare now handles the hole case (no block spans the range start): nothing is trimmed on the left, the insertion anchor is the last block ending at/before the range, and the shared trailing scan removes/head-trims the blocks the range covers. Regression tests: domain_test (command level) and graphops (the app's place_footage_clip path).
This commit is contained in:
@@ -6660,4 +6660,61 @@ mod gap_coverage_tests {
|
||||
let _ = std::fs::remove_file(&media);
|
||||
oak_undo::global::clear().unwrap();
|
||||
}
|
||||
|
||||
/// Dropping a new clip whose in-point lands in EMPTY track space still
|
||||
/// overwrites the clip its tail covers (the start-on-a-clip behavior):
|
||||
/// the covered head is removed and the placed clip lands before the
|
||||
/// overlapped clip in track order. The stored model allows holes between
|
||||
/// blocks; the place command used to leave the overlapped clip untouched
|
||||
/// and insert the new one before it, sliding it UNDER the clip it
|
||||
/// covered.
|
||||
#[test]
|
||||
fn place_clip_from_empty_space_overwrites_the_overlapped_clip() {
|
||||
let _g = test_lock();
|
||||
oak_undo::global::clear().unwrap();
|
||||
let (project, seq, footage, media) = project_with_footage("place_from_empty");
|
||||
let track = video_track_of(&project, seq);
|
||||
let head = place_clip(&project, seq, footage, 0, 40);
|
||||
let tail = place_clip(&project, seq, footage, 100, 200);
|
||||
|
||||
// Drop a 100-frame clip at 50: its tail covers the tail clip's head.
|
||||
let placed = place_clip(&project, seq, footage, 50, 150);
|
||||
|
||||
let tb = {
|
||||
let g = lock(&project);
|
||||
sequence_time_base(&g.graph, seq).expect("time base")
|
||||
};
|
||||
let span = |id: NodeId| {
|
||||
let g = lock(&project);
|
||||
clip_range(&g.graph, id).map(|r| (r.0, r.1))
|
||||
};
|
||||
assert_eq!(
|
||||
clip_ids(&lock(&project).graph, track),
|
||||
vec![head, placed, tail],
|
||||
"the placed clip lands before the clip it covers"
|
||||
);
|
||||
assert_eq!(
|
||||
span(head),
|
||||
Some((Rational::new(0, 1), ts_to_rational(40, tb)))
|
||||
);
|
||||
assert_eq!(
|
||||
span(placed),
|
||||
Some((ts_to_rational(50, tb), ts_to_rational(150, tb)))
|
||||
);
|
||||
assert_eq!(
|
||||
span(tail),
|
||||
Some((ts_to_rational(150, tb), ts_to_rational(200, tb))),
|
||||
"the covered head is removed"
|
||||
);
|
||||
|
||||
oak_undo::global::undo().unwrap();
|
||||
assert_eq!(
|
||||
span(tail),
|
||||
Some((ts_to_rational(100, tb), ts_to_rational(200, tb)))
|
||||
);
|
||||
assert_eq!(clip_ids(&lock(&project).graph, track), vec![head, tail]);
|
||||
|
||||
let _ = std::fs::remove_file(&media);
|
||||
oak_undo::global::clear().unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,7 +42,7 @@ use crate::util::{
|
||||
block_add_to_graph, block_gap_create, block_in, block_kind, block_length, block_next,
|
||||
block_out, block_previous, block_remove_from_graph, block_set_in,
|
||||
block_set_length_and_media_in, block_set_length_and_media_out, block_set_length_keeping_out,
|
||||
block_track, sequence_all_tracks, sequence_track_list,
|
||||
block_track, sequence_all_tracks, sequence_track_list, track_block_at,
|
||||
track_insert_block_after, track_locked, track_nearest_block_after_or_at,
|
||||
track_nearest_block_before_or_at, track_prepend_block, track_ripple_remove_block,
|
||||
tracklist_track_at, tracklist_track_count, BlockKind, NodeRef,
|
||||
@@ -229,11 +229,35 @@ impl TrackRippleRemoveAreaCommand {
|
||||
let in_ = self.range.in_();
|
||||
let out = self.range.out();
|
||||
|
||||
// CPP-PARITY timelineundoripple.cpp:57-64
|
||||
let Some(first_block) = nearest_block_before_or_at(&track, in_) else {
|
||||
return;
|
||||
// C++'s track layout is contiguous (gaps fill the holes), so the
|
||||
// nearest block before-or-at `in_` always spans it. The stored-range
|
||||
// model allows genuine holes BETWEEN blocks: the helper can instead
|
||||
// return the last block that merely ENDS at/before `in_` (or
|
||||
// nothing at all), with blocks after the hole still overlapping the
|
||||
// range. There is nothing to trim on the left in that case: the
|
||||
// replacement goes after that block and the trailing scan starts at
|
||||
// its successor. Missing this branch left the overlapped clips
|
||||
// untouched AND inserted the replacement before them, so a drop
|
||||
// whose in-point landed in empty space slid UNDER the clip it
|
||||
// covered instead of overwriting it.
|
||||
let first = nearest_block_before_or_at(&track, in_);
|
||||
let starts_in_a_hole = match &first {
|
||||
Some(fb) => block_out(fb) <= in_,
|
||||
None => true,
|
||||
};
|
||||
let fb = first_block;
|
||||
if starts_in_a_hole {
|
||||
self.insert_previous_ = first.clone();
|
||||
let start = match &first {
|
||||
Some(fb) => block_next(fb),
|
||||
None => track_block_at(&track, 0),
|
||||
};
|
||||
self.scan_successors(start, out);
|
||||
self.prepared = true;
|
||||
return;
|
||||
}
|
||||
|
||||
// CPP-PARITY timelineundoripple.cpp:57-64
|
||||
let fb = first.expect("the hole case returned early");
|
||||
|
||||
// Determine if this first block is getting trimmed or removed
|
||||
let first_block_is_out_trimmed = block_in(&fb) < in_;
|
||||
@@ -285,43 +309,47 @@ impl TrackRippleRemoveAreaCommand {
|
||||
// If the first block is getting in trimmed, we're already at the
|
||||
// end of our range
|
||||
if !first_block_is_in_trimmed {
|
||||
let mut next = block_next(&fb);
|
||||
while let Some(nx) = next {
|
||||
// The module world allows gaps BETWEEN blocks (their
|
||||
// in/out points are stored, unlike Olive's contiguous
|
||||
// track ordering), so a successor starting at/after the
|
||||
// region does not overlap it and must not be touched.
|
||||
if block_in(&nx) >= out {
|
||||
break;
|
||||
}
|
||||
let trimming = block_out(&nx) > out;
|
||||
|
||||
if trimming {
|
||||
self.trim_in_ = Some(TrimOperation {
|
||||
block: nx.clone(),
|
||||
old_length: block_length(&nx),
|
||||
new_length: block_length(&nx) - (out - block_in(&nx)),
|
||||
});
|
||||
break;
|
||||
} else {
|
||||
self.removals_.push(RemoveOperation {
|
||||
block: nx.clone(),
|
||||
before: block_previous(&nx),
|
||||
});
|
||||
|
||||
if block_out(&nx) == out {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
next = block_next(&nx);
|
||||
}
|
||||
self.scan_successors(block_next(&fb), out);
|
||||
}
|
||||
}
|
||||
|
||||
self.prepared = true;
|
||||
}
|
||||
|
||||
/// Remove the blocks fully inside `[_, out)` and head-trim the first one
|
||||
/// reaching past it — the shared trailing scan of [`Self::prepare`].
|
||||
fn scan_successors(&mut self, mut next: Option<NodeRef>, out: Rational) {
|
||||
while let Some(nx) = next {
|
||||
// The module world allows gaps BETWEEN blocks (their in/out
|
||||
// points are stored, unlike Olive's contiguous track ordering),
|
||||
// so a successor starting at/after the region does not overlap
|
||||
// it and must not be touched.
|
||||
if block_in(&nx) >= out {
|
||||
break;
|
||||
}
|
||||
let trimming = block_out(&nx) > out;
|
||||
|
||||
if trimming {
|
||||
self.trim_in_ = Some(TrimOperation {
|
||||
block: nx.clone(),
|
||||
old_length: block_length(&nx),
|
||||
new_length: block_length(&nx) - (out - block_in(&nx)),
|
||||
});
|
||||
break;
|
||||
}
|
||||
self.removals_.push(RemoveOperation {
|
||||
block: nx.clone(),
|
||||
before: block_previous(&nx),
|
||||
});
|
||||
|
||||
if block_out(&nx) == out {
|
||||
break;
|
||||
}
|
||||
|
||||
next = block_next(&nx);
|
||||
}
|
||||
}
|
||||
|
||||
/// `redo`: apply the ripple removal.
|
||||
pub fn redo(&mut self) {
|
||||
// The oakundo command path never invokes `prepare()` (the oakundo
|
||||
|
||||
@@ -483,6 +483,61 @@ fn place_block_splice_round_trip() {
|
||||
);
|
||||
}
|
||||
|
||||
/// `TrackPlaceBlockCommand` starting in EMPTY SPACE between blocks (the
|
||||
/// stored model allows holes; the C++ layout is contiguous) must still
|
||||
/// overwrite what it covers: the following clip is head-trimmed at the
|
||||
/// placed block's out point and the placed block lands BEFORE it in track
|
||||
/// order. Missing this used to leave the overlapped clip untouched and
|
||||
/// insert the placed block before it, so the drop slid UNDER the clip it
|
||||
/// covered instead of overwriting it.
|
||||
#[test]
|
||||
fn place_block_from_empty_space_overwrites_the_overlapped_clip() {
|
||||
let project = make_project();
|
||||
let (_seq, list) = sequence_and_list(&project);
|
||||
let track = TimelineAddTrackCommand::run_immediately(list.clone());
|
||||
let head = add_clip(&track, Rational::new(0, 1), Rational::new(40, 1));
|
||||
let tail = add_clip(&track, Rational::new(100, 1), Rational::new(200, 1));
|
||||
let placed = add_clip(&track, Rational::new(0, 1), Rational::new(100, 1));
|
||||
track_ripple_remove_block(&track, &placed);
|
||||
|
||||
let mut cmd = oak_timeline::undopointer::TrackPlaceBlockCommand::new(
|
||||
list.clone(),
|
||||
0,
|
||||
placed.clone(),
|
||||
Rational::new(50, 1),
|
||||
);
|
||||
cmd.redo();
|
||||
assert_eq!(track_block_count(&track), 3);
|
||||
assert_eq!(track_block_at(&track, 0).unwrap().id, head.id);
|
||||
assert_eq!(
|
||||
track_block_at(&track, 1).unwrap().id,
|
||||
placed.id,
|
||||
"the placed block lands before the clip it covers"
|
||||
);
|
||||
assert_eq!(
|
||||
span_of(&placed),
|
||||
Some((Rational::new(50, 1), Rational::new(150, 1)))
|
||||
);
|
||||
assert_eq!(
|
||||
span_of(&tail),
|
||||
Some((Rational::new(150, 1), Rational::new(200, 1))),
|
||||
"the covered head is removed"
|
||||
);
|
||||
assert_eq!(
|
||||
span_of(&head),
|
||||
Some((Rational::new(0, 1), Rational::new(40, 1))),
|
||||
"the block before the hole is untouched"
|
||||
);
|
||||
|
||||
cmd.undo();
|
||||
assert_eq!(track_block_count(&track), 2);
|
||||
assert_eq!(span_of(&head), Some((Rational::new(0, 1), Rational::new(40, 1))));
|
||||
assert_eq!(
|
||||
span_of(&tail),
|
||||
Some((Rational::new(100, 1), Rational::new(200, 1)))
|
||||
);
|
||||
}
|
||||
|
||||
/// `TrackListInsertGaps` splits a block at the insertion point and
|
||||
/// inserts a gap of the requested length after it; undo removes the gap
|
||||
/// and re-joins the block.
|
||||
|
||||
@@ -798,6 +798,11 @@ python3 tooling/coverage_report.py --json target/coverage.json
|
||||
(gpui 子模块提交见 `oak-gpui`。)
|
||||
- **OCIO 参数面板**:Vec4 评分参数(对比度/偏移/曝光)建 4 个 SpinBox,
|
||||
`base` 步长锚定范围(pivot 不再一拖就飞出 ±10000)。
|
||||
- **拖放覆盖修复**:`TrackRippleRemoveAreaCommand::prepare` 补"范围起点落在
|
||||
块间空洞"分支(C++ 布局连续、无此情形)——此前该情形下既不裁剪后面的
|
||||
重叠块、又把新块插到它前面,导致"起点在空白、终点压在别的 clip 上"时
|
||||
新 clip 被压在下层而不是覆盖。现在统一与起点在 clip 上时一致:重叠部分
|
||||
被移除、新块在前。`domain_test` 与 `graphops` 各补一条回归断言。
|
||||
|
||||
## 8. 风险与对策
|
||||
|
||||
|
||||
Reference in New Issue
Block a user