oak-team/oak-editor
3
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(timeline): restore the C++ block length anchors and repair pointer edits

Browse Source 
The two BlockCore length setters swapped their anchors relative to the
C++ semantics they document, so the ported edit commands produced wrong
geometry on the live UI paths: roll edits kept the seam still, slides
left negative in-points, and trims wrote the timeline in-point into
media_in (playing the wrong media content).

Adopt three stored-range primitives in block.rs:

- set_length_and_media_out: in fixed, out moves, media untouched
  (resize, trim-out, gaps growing rightward).
- set_length_and_media_in: in fixed, out moves, media_in += old-new
  (resize-with-media-in, splice right half, ripple trim-in).
- set_length_keeping_out (new): out fixed, in moves, media_in +=
  old-new (trim-in body and out-neighbour, slide out-neighbour,
  ripple trim-in of the trailing block).

Point every command at the primitive matching its intent (undopointer,
undogeneral, undoripple, undosplit, graphops, cli, nodeops) and fix the
two real defects the swap hid:

- TrackReplaceBlockWithGapCommand grew a following gap rightward,
  swallowing whatever followed it: the "dragging one clip moves
  unrelated clips" regression. The gap now grows leftward over the
  removed block's span; regression test in domain_test.
- The ripple/splice trims now advance media_in instead of rewriting it,
  and BlockSplitCommand writes both halves' ranges and media
  explicitly (the second half continues from the split point).

Rewrite the KNOWN-SWAP expectations to the correct geometry (roll moves
the seam, slide has no negative in-point, insert-gaps grows rightward,
resize-with-media-in yields media_in = 20) and add the missing media
assertions. TrackSlideCommand documents that the caller positions the
sliding blocks (the stored model has no track layout).
This commit is contained in:
Mike-Solar
2026-09-24 12:41:29 +08:00
parent 415262a7b2
commit d88e2d6ec1
14 changed files with 374 additions and 252 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/oak-app/src/oakui/graphops.rs
+36 -33
View File
@@ -44,8 +44,8 @@ use oak_node::track::{TrackBehavior, TrackListBehavior, TrackType};
use oak_node::value::VideoParams;
use oak_timeline::handle::CHandle;
use oak_timeline::util::{
block_in, block_length, block_out, block_set_in, block_set_length_and_media_in,
block_set_length_and_media_out, clip_set_media_in, NodeRef,
block_in, block_length, block_out, block_set_in, block_set_length_and_media_out,
block_set_length_keeping_out, clip_set_media_in, NodeRef,
};
use oak_storage::backend::StorageBackend;
@@ -1565,7 +1565,8 @@ fn footage_display_name(g: &Graph, footage: NodeId) -> String {
/// Create a footage clip block in `g`: the shared span/state setup plus
/// the per-clip color and footage-name label, both fixed at creation
/// time (C++ `oaknode_clip_set_media_in` +
/// `oaknode_block_set_length_and_media_in`).
/// `oaknode_block_set_length_and_media_out`; the place command writes the
/// in point).
fn create_footage_clip(
g: &mut Graph,
footage: NodeId,
@@ -1584,7 +1585,7 @@ fn create_footage_clip(
.and_then(|a| a.downcast_mut::<ClipBlockBehavior>())
{
c.core.media_in = media_in;
c.core.set_length_and_media_in(length);
c.core.set_length_and_media_out(length);
}
}
id
@@ -2363,7 +2364,7 @@ pub fn place_generator_clip(
.as_any_mut()
.and_then(|a| a.downcast_mut::<ClipBlockBehavior>())
{
c.core.set_length_and_media_in(length);
c.core.set_length_and_media_out(length);
}
}
@@ -2545,11 +2546,6 @@ pub fn split_clips_preserving_links(
)
}
/// Trim `clip`'s timeline range to `[new_in_ts, new_out_ts)` (undoable
/// "Trim Clip"; the facade's `oakengine_clip_trim` semantics: one end at
/// a time, trim-in anchors the OUT, trim-out anchors the IN — the
/// module's own `BlockTrimCommand` applies its length setters with
/// inverted semantics, so the closures carry the correct mapping).
/// An undoable length change for `clip` (trim semantics: `out_anchored`
/// keeps the out point — trim-in — and shifts the in; otherwise the in
/// stays — trim-out). Shared by [`trim_clip`] and [`ripple_trim_clip`];
@@ -2567,9 +2563,9 @@ pub(crate) fn trim_command(
let mut guard = lock(&p1);
with_block_core_mut(&mut guard.graph, clip, |core| {
if out_anchored {
core.set_length_and_media_out(new);
core.set_length_keeping_out(new);
} else {
core.set_length_and_media_in(new);
core.set_length_and_media_out(new);
}
});
},
@@ -2577,9 +2573,9 @@ pub(crate) fn trim_command(
let mut guard = lock(&p2);
with_block_core_mut(&mut guard.graph, clip, |core| {
if out_anchored {
core.set_length_and_media_out(old);
core.set_length_keeping_out(old);
} else {
core.set_length_and_media_in(old);
core.set_length_and_media_out(old);
}
});
},
@@ -2588,9 +2584,9 @@ pub(crate) fn trim_command(
/// Trim `clip`'s timeline range to `[new_in_ts, new_out_ts)` (undoable
/// "Trim Clip"; the facade's `oakengine_clip_trim` semantics: one end at
/// a time, trim-in anchors the OUT, trim-out anchors the IN — the
/// module's own `BlockTrimCommand` applies its length setters with
/// inverted semantics, so the closures carry the correct mapping).
/// a time, trim-in anchors the OUT (its in moves and the media window
/// follows), trim-out anchors the IN (its out moves, media untouched) —
/// the same net anchors as the module's own `BlockTrimCommand`).
pub fn trim_clip(
p: &ProjectRef,
clip: NodeId,
@@ -2811,22 +2807,22 @@ pub fn slide_clip(p: &ProjectRef, clip: NodeId, new_start: i64) -> Result<(), St
if new > block_in(&l_ref) {
let (r1, r2) = (l_ref.clone(), l_ref);
children.push(oak_undo::undocommand::UndoCommand::from_closures(
move || block_set_length_and_media_in(&r1, l_len + delta),
move || block_set_length_and_media_in(&r2, l_len),
move || block_set_length_and_media_out(&r1, l_len + delta),
move || block_set_length_and_media_out(&r2, l_len),
));
}
}
// Right neighbor: its in follows the clip's new out (out anchored,
// media untouched); skipped when it would collapse to a negative
// length.
// Right neighbor: its in follows the clip's new out while its content
// end stays anchored (out anchored; the media in advances over the
// consumed head); skipped when it would collapse to a negative length.
if let Some(r) = right {
let r_ref = node_ref(p, r);
let r_len = block_length(&r_ref);
if new + clip_len < block_out(&r_ref) {
let (r1, r2) = (r_ref.clone(), r_ref);
children.push(oak_undo::undocommand::UndoCommand::from_closures(
move || block_set_length_and_media_out(&r1, r_len - delta),
move || block_set_length_and_media_out(&r2, r_len),
move || block_set_length_keeping_out(&r1, r_len - delta),
move || block_set_length_keeping_out(&r2, r_len),
));
}
}
@@ -6053,21 +6049,28 @@ mod gap_coverage_tests {
assert!(roll_edit(&project, track, a, b, 10).is_ok(), "unmoved boundary");
roll_edit(&project, track, a, b, 12).expect("roll");
{
// NOTE: the module's TrimOut roll mapping anchors the left clip's
// OUT (its IN moves) and the follower's IN (its OUT moves); the
// shared seam itself does not move. Assert the actual outcome so
// a future semantic fix has to update this expectation
// deliberately (review §3.1 of test-coverage-90-80-review.md).
// The roll moves the shared seam to 12: the left clip keeps its
// in point and grows at its out (media window untouched), the
// follower keeps its out point and its head is consumed (its
// media in advances by the rolled amount).
let g = lock(&project);
assert_eq!(
clip_range(&g.graph, a).unwrap(),
(
ts_to_rational(-2, tb),
ts_to_rational(10, tb),
ts_to_rational(-2, tb)
Rational::new(0, 1),
ts_to_rational(12, tb),
Rational::new(0, 1)
)
);
assert_eq!(clip_range(&g.graph, b).unwrap().1, ts_to_rational(18, tb));
assert_eq!(
clip_range(&g.graph, b).unwrap(),
(
ts_to_rational(12, tb),
ts_to_rational(20, tb),
ts_to_rational(2, tb)
),
"the follower's head loses the 2 rolled frames from its media"
);
}
oak_undo::global::undo().unwrap();
crates/oak-cli/src/engine.rs
+2 -2
View File
@@ -411,14 +411,14 @@ pub fn place_footage_clip(
};
// The clip block, positioned by media-in + length (the facade's
// `oaknode_clip_set_media_in` + `oaknode_block_set_length_and_media_in`).
// `oaknode_clip_set_media_in` + `oaknode_block_set_length_and_media_out`).
let clip_id = {
let mut guard = lock(project);
let (core, behavior) = block::clip_create();
let id = guard.graph.add_node(core, behavior);
if let Some(c) = clip_behavior_mut(&mut guard.graph, id) {
c.core.media_in = media_r;
c.core.set_length_and_media_in(length);
c.core.set_length_and_media_out(length);
}
id
};
crates/oak-node/src/block.rs
+24 -9
View File
@@ -90,20 +90,35 @@ impl BlockCore {
self.range = TimeRange::new(self.in_(), out);
}
/// Set the length, keeping the media out anchored (C++
/// `Block::set_length_and_media_out`): the timeline in-point shifts
/// so the out-point stays put, and the media in follows it.
/// Set the length, keeping the timeline in-point and the media in-point
/// anchored (C++ `Block::set_length_and_media_out`): the out-point
/// shifts (and the media out follows it). The stored-range model has no
/// track layout to derive the in-point from, so the caller arranges the
/// neighbouring blocks explicitly when a different anchor is wanted.
pub fn set_length_and_media_out(&mut self, length: Rational) {
let out = self.in_() + self.length();
self.range = TimeRange::new(out - length, out);
self.media_in = self.range.in_();
self.range = TimeRange::new(self.in_(), self.in_() + length);
}
/// Set the length, keeping the media in anchored (C++
/// `Block::set_length_and_media_in`): the in-point stays, the
/// out-point shifts.
/// Set the length, keeping the timeline in-point and the MEDIA OUT
/// anchored (C++ `Block::set_length_and_media_in`): the media in-point
/// moves by `old_length - length` so the media content end stays put
/// while the timeline out-point follows the length.
pub fn set_length_and_media_in(&mut self, length: Rational) {
let delta = self.length() - length;
self.range = TimeRange::new(self.in_(), self.in_() + length);
self.media_in = self.media_in + delta;
}
/// Set the length, keeping the timeline OUT-point and the media content
/// between the two ends anchored: the in-point shifts by the length
/// delta and the media in-point follows it, so the media out stays put
/// too (the stored-range model's net for a head trim, where the C++
/// derives the new in-point from the resized previous block).
pub fn set_length_keeping_out(&mut self, length: Rational) {
let delta = self.length() - length;
let out = self.out();
self.range = TimeRange::new(out - length, out);
self.media_in = self.media_in + delta;
}
/// Media out (in + length; C++ `Block::media_out`).
crates/oak-node/tests/phase2_units_test.rs
+16 -5
View File
@@ -46,16 +46,27 @@ fn block_core_ranges() {
core.set_out(Rational::new(9, 1));
assert_eq!(core.length(), Rational::new(4, 1));
// media_out anchored: in shifts so out stays.
// `set_length_and_media_out`: the in point (and the media in point)
// stays, the out point follows the length.
core.media_in = Rational::new(2, 1);
core.set_length_and_media_out(Rational::new(3, 1));
assert_eq!(core.out(), Rational::new(9, 1), "out stays put");
assert_eq!(core.length(), Rational::new(3, 1));
assert_eq!(core.in_(), Rational::new(5, 1), "in stays put");
assert_eq!(core.out(), Rational::new(8, 1), "out follows the length");
assert_eq!(core.media_in, Rational::new(2, 1), "media in untouched");
// media_in anchored: in stays, out shifts.
// `set_length_and_media_in`: the in point stays while the media in
// moves with the length (the media out is what stays anchored).
core.set_length_and_media_in(Rational::new(4, 1));
assert_eq!(core.in_(), Rational::new(6, 1), "in stays put");
assert_eq!(core.in_(), Rational::new(5, 1), "in stays put");
assert_eq!(core.length(), Rational::new(4, 1));
assert_eq!(core.media_in, Rational::new(1, 1), "media in follows the length");
// `set_length_keeping_out`: the out point (and the media content end)
// stays, the in point shifts.
core.set_length_keeping_out(Rational::new(2, 1));
assert_eq!(core.out(), Rational::new(9, 1), "out stays put");
assert_eq!(core.in_(), Rational::new(7, 1), "in shifts");
assert_eq!(core.media_in, Rational::new(3, 1), "media in follows the in point");
}
/// block.rs: behavior constructors + node inputs.
crates/oak-task/src/nodeops.rs
+2 -1
View File
@@ -774,7 +774,8 @@ pub fn block_media_in(project: &ProjectRef, block: NodeId) -> Rational {
.unwrap_or_else(|| Rational::new(0, 1))
}
/// Set the block's length keeping the media out anchored
/// Set the block's length keeping the in point and the media in point
/// anchored — the out point (and the media out) shift
/// (`oaknode_block_set_length_and_media_out`).
pub fn block_set_length_and_media_out(project: &ProjectRef, block: NodeId, n: i64, d: i64) {
let mut guard = lock_project(project);
crates/oak-timeline/src/undogeneral.rs
+23 -11
View File
@@ -37,8 +37,9 @@ use crate::util::{
block_add_to_graph, block_connect, block_connected_input, block_disconnect_input,
block_enabled, block_gap_create, block_in, block_kind, block_length, block_next, block_out,
block_previous, block_range, block_remove_from_graph, block_set_enabled, block_set_in,
block_set_length_and_media_in, block_set_length_and_media_out, block_set_range, block_track,
clip_media_in, clip_set_media_in, same_block, track_append_block, track_create,
block_set_length_and_media_in, block_set_length_and_media_out, block_set_length_keeping_out,
block_set_range, block_track, clip_media_in, clip_set_media_in, same_block, track_append_block,
track_create,
track_insert_block_after, track_insert_block_before, track_replace_block,
track_ripple_remove_block, tracklist_append, tracklist_remove, tracklist_track_at,
tracklist_track_count, tracklist_type, transition_offsets,
@@ -865,11 +866,17 @@ impl TrackReplaceBlockWithGapCommand {
if let Some(gap) = &self.existing_gap {
// Extend an existing gap. In the module world the block's in/out
// points are stored on the block (Olive derives them from the track
// order), so the extension must keep the gap's in point anchored —
// an out-anchored `set_length_and_media_out` would push the in point
// negative.
// order), so the extension must grow towards the block's span:
// a gap that PRECEDES the block grows rightward (in anchored),
// while a gap that merely FOLLOWS it must grow leftward to start
// at the block's in point (out anchored) — growing it at the out
// would swallow whatever follows the gap.
new_gap_length = new_gap_length + block_length(gap);
block_set_length_and_media_in(gap, new_gap_length);
if previous_is_a_gap {
block_set_length_and_media_out(gap, new_gap_length);
} else {
block_set_length_keeping_out(gap, new_gap_length);
}
track_ripple_remove_block(&self.track, &self.block);
self.existing_gap_precedes =
previous.as_ref().map(|p| same_block(gap, p)).unwrap_or(false);
@@ -882,7 +889,7 @@ impl TrackReplaceBlockWithGapCommand {
// (the module stores positions on the block; the gap
// fills the block's stored span).
block_set_in(gap, self.block_in_point);
block_set_length_and_media_in(gap, new_gap_length);
block_set_length_and_media_out(gap, new_gap_length);
}
}
if let Some(gap) = &self.our_gap {
@@ -935,9 +942,14 @@ impl TrackReplaceBlockWithGapCommand {
track_insert_block_before(&self.track, &self.block, gap);
}
// Restore the gap's original length (in-anchored: its in point was
// untouched by the redo extension).
block_set_length_and_media_in(gap, original_gap_length);
// Restore the gap's original length on the same anchor the redo
// grew it from (the in point for a preceding gap, the out point
// for a following one).
if self.existing_gap_precedes {
block_set_length_and_media_out(gap, original_gap_length);
} else {
block_set_length_keeping_out(gap, original_gap_length);
}
self.existing_gap = None;
}
} else {
@@ -1191,7 +1203,7 @@ impl TrackListInsertGaps {
// explicitly.
let gap_in = g.before.as_ref().map(block_out).unwrap_or(self.point);
block_set_in(&g.gap, gap_in);
block_set_length_and_media_in(&g.gap, self.length);
block_set_length_and_media_out(&g.gap, self.length);
block_add_to_graph(&g.gap, g.entry.take());
track_insert_block_after(&g.track, &g.gap, g.before.as_ref());
}
crates/oak-timeline/src/undopointer.rs
+36 -26
View File
@@ -34,7 +34,7 @@ use crate::undocommon::{
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_track,
block_set_length_and_media_out, block_set_length_keeping_out, block_track,
track_append_block, track_insert_block_after, track_insert_block_before, track_length,
track_ripple_remove_block, tracklist_track_at, tracklist_track_count, BlockKind, NodeRef,
};
@@ -159,18 +159,20 @@ impl BlockTrimCommand {
if self.we_created_adjacent_ {
// We shortened but don't have a viable adjacent to lengthen, so create
// one filling exactly the space the trim freed: for a trim-in it
// spans [in - diff, in), for a trim-out [out, out + diff). The
// module stores positions on the block, so both the in point and
// the length are written explicitly.
// one filling exactly the space the trim freed. The module stores
// positions on the block (Olive derives the inserted gap's position
// from the track order), so the in point is the freed space's start
// and the length closes the gap up to the block's new edge:
// trim-in frees [in, in + diff) (the gap is inserted before the
// block), trim-out frees [out - diff, out) (inserted after it).
self.adjacent_ = Some(block_gap_create(&self.track.project));
if let Some(gap) = &self.adjacent_ {
if self.mode == MovementMode::TrimIn {
block_set_in(gap, block_in(&self.block) - self.trim_diff_);
block_set_in(gap, block_in(&self.block));
} else {
block_set_in(gap, block_out(&self.block));
block_set_in(gap, block_out(&self.block) - self.trim_diff_);
}
block_set_length_and_media_in(gap, self.trim_diff_);
block_set_length_and_media_out(gap, self.trim_diff_);
}
} else if let Some(adjacent) = &self.adjacent_ {
// Determine if we're removing the adjacent
@@ -187,8 +189,10 @@ impl BlockTrimCommand {
}
if self.mode == MovementMode::TrimIn {
block_set_length_and_media_in(&self.block, self.new_length);
// Trim-in keeps the out fixed and moves the in (media follows).
block_set_length_keeping_out(&self.block, self.new_length);
} else {
// Trim-out keeps the in fixed and moves the out (media untouched).
block_set_length_and_media_out(&self.block, self.new_length);
}
@@ -221,9 +225,13 @@ impl BlockTrimCommand {
let adjacent_length = block_length(adjacent) + self.trim_diff_;
if self.mode == MovementMode::TrimIn {
// The previous neighbour grows/shrinks at its out edge
// to meet the trimmed in point.
block_set_length_and_media_out(adjacent, adjacent_length);
} else {
block_set_length_and_media_in(adjacent, adjacent_length);
// The next neighbour grows/shrinks at its in edge to
// meet the trimmed out point (its content end anchored).
block_set_length_keeping_out(adjacent, adjacent_length);
}
}
}
@@ -264,14 +272,14 @@ impl BlockTrimCommand {
if self.mode == MovementMode::TrimIn {
block_set_length_and_media_out(adjacent, adjacent_length);
} else {
block_set_length_and_media_in(adjacent, adjacent_length);
block_set_length_keeping_out(adjacent, adjacent_length);
}
}
}
}
if self.mode == MovementMode::TrimIn {
block_set_length_and_media_in(&self.block, self.old_length_);
block_set_length_keeping_out(&self.block, self.old_length_);
} else {
block_set_length_and_media_out(&self.block, self.old_length_);
}
@@ -365,7 +373,7 @@ impl TrackSlideCommand {
// of the first block. Positions are stored on the block in
// the module model, so both ends are written explicitly.
block_set_in(gap, block_in(&self.blocks[0]) + self.movement);
block_set_length_and_media_in(gap, Rational::new(0, 1) - self.movement);
block_set_length_and_media_out(gap, Rational::new(0, 1) - self.movement);
}
self.we_created_in_adjacent_ = true;
} else {
@@ -381,7 +389,7 @@ impl TrackSlideCommand {
// (movement > 0): the gap fills [last_out, last_out + movement)
// after the last block.
block_set_in(gap, block_out(self.blocks.last().expect("non-empty blocks")));
block_set_length_and_media_in(gap, self.movement);
block_set_length_and_media_out(gap, self.movement);
}
self.we_created_out_adjacent_ = true;
} else {
@@ -390,6 +398,12 @@ impl TrackSlideCommand {
}
/// `redo`: apply the slide.
///
/// The C++ derives the sliding blocks' new positions from the track
/// layout (the resized neighbours shift them implicitly); the
/// stored-range model has no layout, so the CALLER positions the
/// `blocks` by `movement` before running this command — it only owns
/// the adjacent resize/insert/remove half of the slide.
pub fn redo(&mut self) {
// We will always have an in adjacent if there was a valid slide
if self.we_created_in_adjacent_ {
@@ -452,16 +466,15 @@ impl TrackSlideCommand {
self.we_removed_out_adjacent_ = true;
} else {
// Simply resize the adjacent
block_set_length_and_media_in(
adjacent,
block_length(adjacent) - self.movement,
);
// Simply resize the adjacent: its in edge follows the last
// sliding block's out, so the content end stays anchored.
block_set_length_keeping_out(adjacent, block_length(adjacent) - self.movement);
}
}
}
/// `undo`: revert the slide.
/// `undo`: revert the slide (the caller restores the moved blocks
/// alongside this, mirroring how it positioned them for the redo).
pub fn undo(&mut self) {
if self.we_created_in_adjacent_ {
// We created this, so we can remove it now
@@ -508,11 +521,8 @@ impl TrackSlideCommand {
Some(self.blocks.last().expect("non-empty blocks")),
);
} else {
// Simply resize the adjacent
block_set_length_and_media_in(
adjacent,
block_length(adjacent) + self.movement,
);
// Simply resize the adjacent (mirror of the redo).
block_set_length_keeping_out(adjacent, block_length(adjacent) + self.movement);
}
}
}
@@ -625,7 +635,7 @@ impl TrackPlaceBlockCommand {
self.gap_ = Some(block_gap_create(&self.timeline.project));
if let Some(gap) = &self.gap_ {
block_set_in(gap, track_length(&track));
block_set_length_and_media_in(gap, in_ - track_length(&track));
block_set_length_and_media_out(gap, in_ - track_length(&track));
}
}
if let Some(gap) = &self.gap_ {
crates/oak-timeline/src/undoripple.rs
+22 -20
View File
@@ -41,8 +41,8 @@ use crate::undocommon::{
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_track,
sequence_all_tracks, sequence_track_list,
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,
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,
@@ -335,10 +335,11 @@ impl TrackRippleRemoveAreaCommand {
let cmd = self.splice_split_command_.as_mut().unwrap();
cmd.redo();
// Trim the in of the split (the second half produced by the
// split keeps the original out point; the in-end trim shifts
// its stored in point to the range out — the module equivalent
// of the C++ ripple shifting the remainder earlier).
// Trim the in of the split: the second half produced by the
// split keeps the original out point, so removing the first
// `range.length()` of it keeps the timeline in fixed at the
// range out and advances the media window past the removed
// content (in-anchored, media in moves with the length).
if let Some(split) = cmd.new_block() {
let new_len =
block_length(&split) - (self.range.out() - block_in(&split));
@@ -346,15 +347,15 @@ impl TrackRippleRemoveAreaCommand {
}
} else {
if let Some(t) = &self.trim_out_ {
// An out-end trim keeps the in point (in-anchored; the
// C++ setter name is kept, but the module's in/out are
// stored values, see the splice trim above).
block_set_length_and_media_in(&t.block, t.new_length);
// An out-end trim keeps the in point and the media window
// (in-anchored).
block_set_length_and_media_out(&t.block, t.new_length);
}
if let Some(t) = &self.trim_in_ {
// An in-end trim keeps the out point (out-anchored).
block_set_length_and_media_out(&t.block, t.new_length);
// An in-end trim keeps the out point and advances the media
// window past the removed head.
block_set_length_keeping_out(&t.block, t.new_length);
}
// Perform removals
@@ -389,12 +390,12 @@ impl TrackRippleRemoveAreaCommand {
} else {
if let Some(t) = &self.trim_out_ {
// In-anchored, matching the redo (see above).
block_set_length_and_media_in(&t.block, t.old_length);
block_set_length_and_media_out(&t.block, t.old_length);
}
if let Some(t) = &self.trim_in_ {
// Out-anchored, matching the redo (see above).
block_set_length_and_media_out(&t.block, t.old_length);
block_set_length_keeping_out(&t.block, t.old_length);
}
// Un-remove any blocks
@@ -699,14 +700,15 @@ impl TrackListRippleToolCommand {
if redo {
if wd.created_gap.is_none() {
let gap = block_gap_create(&track_copy.project);
// The gap takes the ripple movement's span ahead of
// `b`; positions are stored on the block in the
// The gap takes the ripple movement's span at `b`'s
// in point; positions are stored on the block in the
// module model, so both ends are written explicitly
// (approximation: the C++ ripple additionally shifts
// the successors, which the stored positions render
// as an overlap-free gap right before `b`).
// (approximation: the C++ layout additionally shifts
// `b` and its successors right by that span — the
// stored-range command leaves that shift to the
// caller).
block_set_in(&gap, block_in(&b));
block_set_length_and_media_in(
block_set_length_and_media_out(
&gap,
if self.ripple_movement < Rational::new(0, 1) {
rat_neg(self.ripple_movement)
crates/oak-timeline/src/undosplit.rs
+23 -19
View File
@@ -35,9 +35,9 @@ use oak_node::id::NodeId;
use oak_undo::undocommand::UndoCommand;
use crate::util::{
block_in, block_length, block_out, block_set_length_and_media_in,
block_set_length_and_media_out, block_track, track_insert_block_after,
track_ripple_remove_block, GraphBlockRange, NodeRef,
block_in, block_length, block_out, block_set_length_and_media_out, block_set_range, block_track,
clip_media_in, clip_set_media_in, track_insert_block_after, track_ripple_remove_block,
GraphBlockRange, NodeRef,
};
/// `BlockSplitCommand` — split one block at a point
@@ -177,10 +177,11 @@ impl BlockSplitCommand {
/// second half, and insert it after `block`.
///
/// The split keeps the ORIGINAL block anchored at its in-point (the
/// C++ `set_length_and_media_in` on the original) and anchors the
/// cloned `new_block` at its out-point (the C++
/// `set_length_and_media_out` on the copy — the copy carried the
/// original span, so the out point is preserved exactly).
/// C++ `set_length_and_media_out` on the original) and anchors the
/// cloned `new_block` between the split point and the original out
/// point (the C++ `set_length_and_media_in` on the copy — the copy
/// carried the original span, so the out point is preserved exactly),
/// with its media window continuing where the first half stops.
pub fn redo(&mut self) {
// Create the second half if redo is invoked without a preceding
// prepare() (the oakundo command path may call redo directly).
@@ -199,14 +200,19 @@ impl BlockSplitCommand {
let first_half_length = self.point - block_in;
let second_half_length = block_out - self.point;
// The second half continues the media exactly where the first half
// stops (C++ `ClipBlock::set_length_and_media_in` on the copy).
let media_in = clip_media_in(&self.block);
if let Some(new_block) = &self.new_block {
// In-anchored length for the first half keeps the original's in
// point (the C++ `set_length_and_media_in`); the out-anchored
// length for the second half keeps the copy's out point (the
// C++ `set_length_and_media_out`).
block_set_length_and_media_in(&self.block, first_half_length);
block_set_length_and_media_out(new_block, second_half_length);
// First half: the original's in point stays and its out lands on
// the split point (in-anchored, media unchanged).
block_set_length_and_media_out(&self.block, first_half_length);
// Second half: the split point up to the original out point. The
// stored-range model has no track layout to derive these from, so
// both ends are written explicitly along with the media window.
block_set_range(new_block, oak_core::TimeRange::new(self.point, self.point + second_half_length));
clip_set_media_in(new_block, media_in + first_half_length);
if let Some(track) = block_track(&self.block) {
track_insert_block_after(&track, new_block, Some(&self.block));
@@ -223,12 +229,10 @@ impl BlockSplitCommand {
/// and detach the whole copied subgraph from the project graph.
pub fn undo(&mut self) {
if let Some(track) = block_track(&self.block) {
// The redo shrank the original from its in point (it became the
// first half, in-anchored), so the restore grows it in-anchored
// too — the module stores the span on the block (the C++
// `set_length_and_media_out` derives positions from the track
// order and does not apply here).
block_set_length_and_media_in(&self.block, self.old_length);
// The redo shrank the original from its out point (it became the
// first half, in-anchored), so the restore grows it at the out
// again; the media window was never touched.
block_set_length_and_media_out(&self.block, self.old_length);
if let Some(new_block) = &self.new_block {
track_ripple_remove_block(&track, new_block);
}
crates/oak-timeline/src/util.rs
+17 -5
View File
@@ -311,8 +311,8 @@ pub fn block_set_enabled(b: &NodeRef, enabled: bool) {
}
}
/// `block_set_length_and_media_out`: keep the out point anchored (the
/// in point shifts).
/// `block_set_length_and_media_out`: keep the in point anchored (the out
/// point shifts; the media in point does not move).
pub fn block_set_length_and_media_out(b: &NodeRef, len: Rational) {
let mut p = b.lock();
if let Some(core) = block_core_of_mut(&mut p, b.id) {
@@ -320,8 +320,9 @@ pub fn block_set_length_and_media_out(b: &NodeRef, len: Rational) {
}
}
/// `block_set_length_and_media_in`: keep the in point anchored (the
/// out point shifts).
/// `block_set_length_and_media_in`: keep the in point anchored (the out
/// point shifts) while the media in point follows the length change, so
/// the media out stays anchored.
pub fn block_set_length_and_media_in(b: &NodeRef, len: Rational) {
let mut p = b.lock();
if let Some(core) = block_core_of_mut(&mut p, b.id) {
@@ -329,6 +330,16 @@ pub fn block_set_length_and_media_in(b: &NodeRef, len: Rational) {
}
}
/// Keep the OUT point anchored (the in point shifts) and move the media
/// in point with it: the net of a head trim in the stored-range model
/// (the C++ derives the shifted in-point from the resized previous block).
pub fn block_set_length_keeping_out(b: &NodeRef, len: Rational) {
let mut p = b.lock();
if let Some(core) = block_core_of_mut(&mut p, b.id) {
core.set_length_keeping_out(len);
}
}
/// `block_set_in`: set the in point, keeping the length (the out follows).
pub fn block_set_in(b: &NodeRef, in_: Rational) {
let mut p = b.lock();
@@ -364,7 +375,8 @@ pub fn block_range(b: &NodeRef) -> Option<oak_core::TimeRange> {
/// Set the block's stored timeline range (a no-op for a stale/non-block
/// node). The range is written whole, so the caller can move the in/out
/// pair together without a media-in side effect (unlike
/// [`block_set_length_and_media_out`] / [`block_set_length_and_media_in`]).
/// [`block_set_length_and_media_in`] / [`block_set_length_keeping_out`],
/// which adjust the media in point with the length).
pub fn block_set_range(b: &NodeRef, range: oak_core::TimeRange) {
let mut p = b.lock();
if let Some(core) = block_core_of_mut(&mut p, b.id) {
crates/oak-timeline/tests/domain_test.rs
+19 -8
View File
@@ -254,7 +254,10 @@ fn split_command_redo_undo() {
}
/// `TrackReplaceBlockWithGapCommand` merges a clip into a following gap on
/// redo and restores both blocks on undo.
/// redo and restores both blocks on undo. The gap grows LEFTWARD over the
/// clip's span (out anchored): growing it at the out instead would swallow
/// the clip that follows the gap — the "dragging one clip moved unrelated
/// clips" regression.
#[test]
fn replace_block_with_gap_round_trip() {
let project = make_project();
@@ -262,19 +265,23 @@ fn replace_block_with_gap_round_trip() {
let track = TimelineAddTrackCommand::run_immediately(list);
let clip = add_clip(&track, Rational::new(0, 1), Rational::new(50, 1));
let gap = add_gap(&track, Rational::new(50, 1), Rational::new(100, 1));
let tail = add_clip(&track, Rational::new(100, 1), Rational::new(150, 1));
let mut cmd = TrackReplaceBlockWithGapCommand::new(track.clone(), clip.clone(), false);
cmd.redo();
// The clip is gone; the gap absorbed its length (in-anchored at 50).
assert_eq!(track_block_count(&track), 1);
// The clip is gone; the FOLLOWING gap grew leftward over its span.
assert_eq!(track_block_count(&track), 2);
let remaining = track_block_at(&track, 0).unwrap();
assert_eq!(remaining.id, gap.id);
assert_eq!(block_in(&remaining), Rational::new(50, 1));
assert_eq!(block_in(&remaining), Rational::new(0, 1));
assert_eq!(block_length(&remaining), Rational::new(100, 1));
assert!(block_track(&clip).is_none());
assert_eq!(track_block_at(&track, 1).unwrap().id, tail.id);
assert_eq!(block_in(&tail), Rational::new(100, 1));
assert_eq!(block_out(&tail), Rational::new(150, 1), "the clip after the gap is untouched");
cmd.undo();
assert_eq!(track_block_count(&track), 2);
assert_eq!(track_block_count(&track), 3);
let first = track_block_at(&track, 0).unwrap();
assert_eq!(first.id, clip.id);
assert_eq!(block_in(&first), Rational::new(0, 1));
@@ -283,6 +290,9 @@ fn replace_block_with_gap_round_trip() {
assert_eq!(second.id, gap.id);
assert_eq!(block_in(&second), Rational::new(50, 1));
assert_eq!(block_length(&second), Rational::new(50, 1));
assert_eq!(track_block_at(&track, 2).unwrap().id, tail.id);
assert_eq!(block_in(&tail), Rational::new(100, 1));
assert_eq!(block_out(&tail), Rational::new(150, 1));
}
/// `TrackReplaceBlockWithGapCommand` creates its own gap when no
@@ -358,7 +368,7 @@ fn ripple_remove_area_round_trip() {
);
}
/// `BlockResizeCommand` changes a block's length out-anchored and restores
/// `BlockResizeCommand` changes a block's length in-anchored and restores
/// it on undo.
#[test]
fn block_resize_round_trip() {
@@ -370,8 +380,9 @@ fn block_resize_round_trip() {
let mut cmd = BlockResizeCommand::new(clip.clone(), Rational::new(30, 1));
cmd.redo();
assert_eq!(block_length(&clip), Rational::new(30, 1));
// Out-anchored: the in point shifts so the out stays at 50.
assert_eq!(block_out(&clip), Rational::new(50, 1));
// In-anchored: the in point stays so the out follows the length.
assert_eq!(block_in(&clip), Rational::new(0, 1));
assert_eq!(block_out(&clip), Rational::new(30, 1));
cmd.undo();
assert_eq!(block_length(&clip), Rational::new(50, 1));
assert_eq!(block_in(&clip), Rational::new(0, 1));
crates/oak-timeline/tests/undocommands_test.rs
+88 -103
View File
@@ -478,9 +478,12 @@ fn timeline_ripple_delete_gaps_resizes_longer_gap() {
assert!(cmd.has_commands());
cmd.redo();
// The gap keeps its out point and loses the 20-frame region length.
// The gap keeps its in point and loses the 20-frame region length. In
// the C++ the following clip also ripples left by the removed length
// (the layout derives it); the stored-range command trims the gap only,
// so callers that need the tail shifted do it explicitly.
assert_eq!(track_block_count(&track), 3);
assert_eq!(span_of(&gap), Some((Rational::new(70, 1), Rational::new(150, 1))));
assert_eq!(span_of(&gap), Some((Rational::new(50, 1), Rational::new(130, 1))));
cmd.undo();
assert_eq!(span_of(&gap), Some((Rational::new(50, 1), Rational::new(150, 1))));
@@ -556,14 +559,9 @@ fn track_list_ripple_tool_empty_info_is_noop() {
}
/// `TrackListRippleToolCommand` with real per-track info resizes the block
/// and restores it on undo.
///
/// KNOWN-SWAP: the expectations below pin the CURRENT (swapped)
/// `set_length_and_media_out` behavior documented in
/// `docs/zh/plans/test-coverage-90-80-review.md` §3.1/§9 — the trim ripples the
/// in-point and writes the timeline in into `media_in` instead of moving the
/// out-point. The semantic fix must rewrite these values deliberately
/// (C++ `BlockTrimCommand::redo` kTrimOut keeps the in-point and moves out).
/// and restores it on undo. Trim-out is in-anchored: the clip keeps its in
/// point and its out follows the movement, while the media window (which
/// the ripple does not consume here) stays put.
#[test]
fn track_list_ripple_tool_resizes_with_real_info() {
let project = make_project();
@@ -583,10 +581,10 @@ fn track_list_ripple_tool_resizes_with_real_info() {
cmd.redo();
assert_eq!(
span_of(&clip),
Some((Rational::new(-10, 1), Rational::new(100, 1))),
"current (swapped) behavior: the in-point shifts, the out-point stays"
Some((Rational::new(0, 1), Rational::new(110, 1))),
"the in-point stays and the out-point follows the movement"
);
assert_eq!(clip_media_in(&clip), Rational::new(-10, 1));
assert_eq!(clip_media_in(&clip), Rational::new(0, 1));
cmd.undo();
assert_eq!(span_of(&clip), Some((Rational::new(0, 1), Rational::new(100, 1))));
@@ -610,8 +608,9 @@ fn track_list_ripple_tool_append_gap_creates_a_gap() {
);
cmd.redo();
assert_eq!(track_block_count(&track), 2, "a gap was inserted");
// KNOWN-SWAP: the inserted gap overlaps the clip (its in is the clip's
// in); §3.1/§9's fix will move it to the clip's out.
// The gap takes the block's in point; the stored-range command leaves
// the block in place (the C++ layout would push it right by the gap
// length — the caller owns that shift, see the insert site's note).
let gap = track_block_at(&track, 0).expect("gap at the front");
assert_eq!(block_kind(&gap), BlockKind::Gap);
assert_eq!(span_of(&gap), Some((Rational::new(0, 1), Rational::new(10, 1))));
@@ -629,12 +628,9 @@ fn track_list_ripple_tool_append_gap_creates_a_gap() {
/// `BlockTrimCommand` shortens a clip and lengthens its adjacent gap so
/// the rest of the track keeps its position; undo restores both.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// the expectations below pin the current (swapped)
/// `set_length_and_media_out` behavior — the trim moves the clip's in-point
/// instead of its out-point. C++ `BlockTrimCommand` TrimOut is in-anchored
/// (`common.rs` `k_trim_out`: trim the out point); the semantic fix must
/// re-derive this geometry.
/// Trim-out is in-anchored (`common.rs` `k_trim_out`: trim the out point):
/// the clip keeps its in and the following gap grows leftward to meet the
/// new out, so everything after the gap stays put.
#[test]
fn block_trim_with_gap_adjacent_round_trip() {
let project = make_project();
@@ -651,27 +647,22 @@ fn block_trim_with_gap_adjacent_round_trip() {
);
cmd.prepare();
cmd.redo();
// Current (swapped) behavior: the clip's in shifts to 25 and the gap
// keeps its in at 50, growing its out by the 25 trimmed away. The
// in-anchored C++ TrimOut would keep the in and move the out instead
// (see the KNOWN-SWAP note).
assert_eq!(span_of(&clip), Some((Rational::new(25, 1), Rational::new(50, 1))));
assert_eq!(span_of(&gap), Some((Rational::new(50, 1), Rational::new(125, 1))));
assert_eq!(span_of(&clip), Some((Rational::new(0, 1), Rational::new(25, 1))));
assert_eq!(span_of(&gap), Some((Rational::new(25, 1), Rational::new(100, 1))));
assert_eq!(
clip_media_in(&clip),
Rational::new(0, 1),
"a trim-out does not move the media window"
);
cmd.undo();
assert_eq!(span_of(&clip), Some((Rational::new(0, 1), Rational::new(50, 1))));
assert_eq!(span_of(&gap), Some((Rational::new(50, 1), Rational::new(100, 1))));
assert_eq!(clip_media_in(&clip), Rational::new(0, 1));
}
/// `BlockTrimCommand` inserts a gap filling the space the trim freed when
/// the adjacent block is a clip and the trim is not a roll edit.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// the geometry below pins the swapped `set_length_and_media_out` behavior
/// — the swapped trim moved `first`'s in to 25 and inserted the
/// compensating gap at [50,75), sharing its in with `second` (the true
/// in-anchored TrimOut keeps `first` at [0,25); the semantic fix must
/// rewrite this geometry).
#[test]
fn block_trim_creates_gap_next_to_clip() {
let project = make_project();
@@ -692,11 +683,10 @@ fn block_trim_creates_gap_next_to_clip() {
assert_eq!(track_block_at(&track, 0).unwrap().id, first.id);
let created = track_block_at(&track, 1).unwrap();
assert_eq!(block_kind(&created), BlockKind::Gap);
// KNOWN-SWAP: the swapped trim shrinks `first` from the in side, so the
// "compensating" gap lands at [50,75) and overlaps `second` instead of
// filling the freed [25,50) space (§3.1/§9).
assert_eq!(span_of(&first), Some((Rational::new(25, 1), Rational::new(50, 1))));
assert_eq!(span_of(&created), Some((Rational::new(50, 1), Rational::new(75, 1))));
// The trim keeps `first`'s in fixed and moves its out to 25; the fresh
// gap fills the freed [25,50) space so `second` does not move.
assert_eq!(span_of(&first), Some((Rational::new(0, 1), Rational::new(25, 1))));
assert_eq!(span_of(&created), Some((Rational::new(25, 1), Rational::new(50, 1))));
assert_eq!(track_block_at(&track, 2).unwrap().id, second.id);
cmd.undo();
@@ -706,13 +696,8 @@ fn block_trim_creates_gap_next_to_clip() {
}
/// `BlockTrimCommand::set_trim_is_a_roll_edit` trims into the adjacent
/// clip instead of creating a compensating gap.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`): the
/// expectations below characterize the swapped `set_length_and_media_out`
/// behavior — the seam does NOT move; the left clip's IN shifts and the
/// follower's OUT grows. A true roll edit is first=[0,25) / second=[25,100)
/// with the seam at 25; the semantic fix must rewrite these values.
/// clip instead of creating a compensating gap: the shared seam moves, the
/// left clip keeps its in, the follower keeps its out.
#[test]
fn block_trim_roll_edit_resizes_adjacent_clip() {
let project = make_project();
@@ -731,23 +716,29 @@ fn block_trim_roll_edit_resizes_adjacent_clip() {
cmd.prepare();
cmd.redo();
assert_eq!(track_block_count(&track), 2);
assert_eq!(span_of(&first), Some((Rational::new(25, 1), Rational::new(50, 1))));
assert_eq!(span_of(&second), Some((Rational::new(50, 1), Rational::new(125, 1))));
// The seam stays at 50 under the current mapping (see the KNOWN-SWAP
// note); `media_in` pins the coordinate-space confusion: the left clip's
// timeline in (25) is written into `media_in`, and the follower's
// media-in is left untouched.
assert_eq!(span_of(&first), Some((Rational::new(0, 1), Rational::new(25, 1))));
assert_eq!(span_of(&second), Some((Rational::new(25, 1), Rational::new(100, 1))));
assert_eq!(
span_of(&first).map(|(_, out)| out),
span_of(&second).map(|(in_, _)| in_),
"the shared seam does not move under the current mapping"
"the two clips keep sharing the rolled seam"
);
assert_eq!(
clip_media_in(&first),
Rational::new(0, 1),
"the left clip's media window is untouched (trim-out)"
);
assert_eq!(
clip_media_in(&second),
Rational::new(-25, 1),
"the follower grows at its head, revealing earlier media"
);
assert_eq!(clip_media_in(&first), Rational::new(25, 1));
assert_eq!(clip_media_in(&second), Rational::new(0, 1));
cmd.undo();
assert_eq!(span_of(&first), Some((Rational::new(0, 1), Rational::new(50, 1))));
assert_eq!(span_of(&second), Some((Rational::new(50, 1), Rational::new(100, 1))));
assert_eq!(clip_media_in(&first), Rational::new(0, 1));
assert_eq!(clip_media_in(&second), Rational::new(0, 1));
}
/// `BlockTrimCommand` (default `set_remove_zero_length_from_graph`) removes
@@ -834,14 +825,12 @@ fn block_trim_same_length_is_noop() {
}
/// `TrackSlideCommand` resizes both adjacent blocks by the movement;
/// undo restores their original lengths.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// under the current setters the slid block itself does not move; the
/// previous block grows leftward into the slide's start (reaching a negative
/// timeline in-point) while the next block's out shrinks by the movement.
/// These expectations characterize that state; the semantic fix must rewrite
/// them to a true slide.
/// undo restores their original lengths. The previous neighbour grows at
/// its out edge (in anchored; no negative in-point) and the next shrinks at
/// its in edge (out anchored, media advancing over the consumed head). The
/// command does not position the sliding blocks itself — the stored-range
/// model has no track layout, so the caller places them by `movement` (see
/// `graphops::slide_clip`).
#[test]
fn track_slide_resizes_adjacent_blocks() {
let project = make_project();
@@ -860,22 +849,30 @@ fn track_slide_resizes_adjacent_blocks() {
);
cmd.prepare();
cmd.redo();
assert_eq!(span_of(&previous), Some((Rational::new(-20, 1), Rational::new(50, 1))));
assert_eq!(span_of(&previous), Some((Rational::new(0, 1), Rational::new(70, 1))));
assert_eq!(span_of(&slide), Some((Rational::new(50, 1), Rational::new(100, 1))));
assert_eq!(span_of(&next), Some((Rational::new(100, 1), Rational::new(130, 1))));
assert_eq!(span_of(&next), Some((Rational::new(120, 1), Rational::new(150, 1))));
assert_eq!(
clip_media_in(&previous),
Rational::new(0, 1),
"the in-anchored growth leaves the media window alone"
);
assert_eq!(
clip_media_in(&next),
Rational::new(20, 1),
"the next block's head is consumed, advancing its media in"
);
cmd.undo();
assert_eq!(span_of(&previous), Some((Rational::new(0, 1), Rational::new(50, 1))));
assert_eq!(span_of(&next), Some((Rational::new(100, 1), Rational::new(150, 1))));
assert_eq!(clip_media_in(&previous), Rational::new(0, 1));
assert_eq!(clip_media_in(&next), Rational::new(0, 1));
}
/// `TrackSlideCommand` removes the out adjacent when the movement exactly
/// consumes it; undo re-attaches it after the moving block.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// the swapped setters leave the slid clip in place and grow `in_gap`'s in
/// leftward (a negative timeline in-point) instead of resizing it on the
/// slide's trailing side; the semantic fix must rewrite this geometry.
/// consumes it; undo re-attaches it after the moving block. The in adjacent
/// grows at its out edge (in anchored) while the out adjacent disappears.
#[test]
fn track_slide_removes_out_adjacent_at_boundary() {
let project = make_project();
@@ -897,9 +894,7 @@ fn track_slide_removes_out_adjacent_at_boundary() {
assert_eq!(track_block_count(&track), 2);
assert!(block_track(&out_gap).is_none());
assert!(!node_in_graph(&project, &out_gap));
// KNOWN-SWAP: the gap grew leftward past zero because the slide did not
// move the clip (§3.1/§9).
assert_eq!(span_of(&in_gap), Some((Rational::new(-20, 1), Rational::new(20, 1))));
assert_eq!(span_of(&in_gap), Some((Rational::new(0, 1), Rational::new(40, 1))));
cmd.undo();
assert_eq!(track_block_count(&track), 3);
@@ -910,12 +905,8 @@ fn track_slide_removes_out_adjacent_at_boundary() {
}
/// `TrackSlideCommand` removes the in adjacent when a leftward movement
/// exactly consumes it; undo re-attaches it before the moving block.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// the swapped setters leave the slid clip in place and grow `out_gap`'s out
/// rightward instead of resizing it on the slide's leading side; the
/// semantic fix must rewrite this geometry.
/// exactly consumes it; undo re-attaches it before the moving block. The
/// out adjacent grows at its in edge (out anchored) as the clip slides left.
#[test]
fn track_slide_removes_in_adjacent_at_boundary() {
let project = make_project();
@@ -937,9 +928,7 @@ fn track_slide_removes_in_adjacent_at_boundary() {
assert_eq!(track_block_count(&track), 2);
assert!(block_track(&in_gap).is_none());
assert!(!node_in_graph(&project, &in_gap));
// KNOWN-SWAP: the gap grew rightward because the slide did not move the
// clip (§3.1/§9).
assert_eq!(span_of(&out_gap), Some((Rational::new(70, 1), Rational::new(110, 1))));
assert_eq!(span_of(&out_gap), Some((Rational::new(50, 1), Rational::new(90, 1))));
cmd.undo();
assert_eq!(track_block_count(&track), 3);
@@ -1230,15 +1219,9 @@ fn block_split_preserving_links_noop_time() {
// ---------------------------------------------------------------------------
/// `BlockResizeWithMediaInCommand` resizes keeping the timeline in-point
/// fixed (C++ `ClipBlock::set_length_and_media_in` adjusts `media_in` so the
/// media out stays put); undo restores the original length.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// today neither setter adjusts `media_in`, so the media
/// out is silently shortened with the block (media_in stays 0 here). The
/// assertion below pins that current state so the semantic fix must change
/// it deliberately (the fixed behavior yields `media_in = 20` for this
/// resize, keeping `media_out = 50`).
/// fixed while `media_in` follows the length change, so the media content
/// end stays anchored (C++ `ClipBlock::set_length_and_media_in`); undo
/// restores the original length and media window.
#[test]
fn block_resize_with_media_in_round_trip() {
let project = make_project();
@@ -1249,7 +1232,11 @@ fn block_resize_with_media_in_round_trip() {
let mut cmd = BlockResizeWithMediaInCommand::new(clip.clone(), Rational::new(30, 1));
cmd.redo();
assert_eq!(span_of(&clip), Some((Rational::new(0, 1), Rational::new(30, 1))));
assert_eq!(clip_media_in(&clip), Rational::new(0, 1));
assert_eq!(
clip_media_in(&clip),
Rational::new(20, 1),
"media_in advances by the trimmed 20 frames (media_out stays at 50)"
);
cmd.undo();
assert_eq!(span_of(&clip), Some((Rational::new(0, 1), Rational::new(50, 1))));
assert_eq!(clip_media_in(&clip), Rational::new(0, 1));
@@ -1302,12 +1289,10 @@ fn block_enable_disable_round_trip() {
}
/// `TrackListInsertGaps` extends an existing gap crossed by the insertion
/// point instead of adding a second one; undo restores its length.
///
/// KNOWN-SWAP (§3.1/§9 of `docs/zh/plans/test-coverage-90-80-review.md`):
/// the gap grows leftwards across the insertion point
/// and overlaps the preceding block; the fixed `_media_out`/`_media_in`
/// semantics must re-derive this geometry.
/// point instead of adding a second one; undo restores its length. The
/// extension is in-anchored (the insertion pushes the following content
/// rightward; the C++ derives that shift from the track layout, while this
/// caller-facing command leaves it to whoever places the followers).
#[test]
fn track_list_insert_gaps_extends_existing_gap() {
let project = make_project();
@@ -1321,9 +1306,9 @@ fn track_list_insert_gaps_extends_existing_gap() {
TrackListInsertGaps::new(list, Rational::new(75, 1), Rational::new(20, 1));
cmd.prepare();
cmd.redo();
// The gap keeps its out point and grows leftward by the gap length.
// The gap keeps its in point and grows rightward by the gap length.
assert_eq!(track_block_count(&track), 3);
assert_eq!(span_of(&gap), Some((Rational::new(30, 1), Rational::new(100, 1))));
assert_eq!(span_of(&gap), Some((Rational::new(50, 1), Rational::new(120, 1))));
cmd.undo();
assert_eq!(track_block_count(&track), 3);
@@ -1603,8 +1588,8 @@ fn boxed_prepared_commands_round_trip() {
cmd.prepare();
let mut boxed = cmd.to_command();
boxed.redo_now();
assert_eq!(span_of(&previous), Some((Rational::new(-20, 1), Rational::new(50, 1))));
assert_eq!(span_of(&next), Some((Rational::new(100, 1), Rational::new(130, 1))));
assert_eq!(span_of(&previous), Some((Rational::new(0, 1), Rational::new(70, 1))));
assert_eq!(span_of(&next), Some((Rational::new(120, 1), Rational::new(150, 1))));
boxed.undo_now();
assert_eq!(span_of(&previous), Some((Rational::new(0, 1), Rational::new(50, 1))));
assert_eq!(span_of(&next), Some((Rational::new(100, 1), Rational::new(150, 1))));
docs/zh/plans/test-coverage-90-80-review.md
+42 -10
View File
@@ -148,6 +148,10 @@ disturbing anything else"）。附带缺陷：`media_in := range.in_()` 把媒
统一裁决 block.rs:93-107 的语义互换，届时上述期望值按 graphops NOTE 的
意图 "deliberately" 改写。
**裁决（2026-09-24，已修复）**：准备项落地后按 §9 的三原语方案完成语义
修复，全部 KNOWN-SWAP 期望已按正确几何改写，并补上 `media_in` 与
"缺口后的块不受影响"回归断言（详见 §9 §3.1）。
### 3.2 🔴 `oak-task/tests/render_test.rs:932-951`：全批次唯一"永不失败"的测试 [实证]
`private_dispatcher_renders_frames_and_audio_through_workers` 对两次真实
@@ -328,10 +332,10 @@ M4 的平台/真机 job。
## 7. 建议行动（按优先级）
1. **裁决 `block.rs:93-107` setter 互换**（产品缺陷，§3.1）：修复时按
graphops NOTE 的意图 "deliberately" 改写约 12 个期望值；先给
`undocommands_test.rs` 的无标注固化处补偏差 NOTE、给全文件补
media_in 断言维度、给 `RippleInfo` 加公开构造器（M5）。
1. ✅ **裁决 `block.rs:93-107` setter 互换**（产品缺陷，§3.1）：2026-09-24
按 §9 的三原语方案修复，约 12 个期望值按 graphops NOTE 的意图
"deliberately" 改写；`RippleInfo` 公开构造器与 media_in 断言维度已先行
落地（M5）。
2. **消灭"双向接受"**（§3.2）：`render_test.rs:932` 的 Err 分支按错误
内容分流，产品性错误必须失败。
3. **修回放验收测试的跳过判别**（§3.3）：source-window 补槽位检查；
@@ -357,10 +361,10 @@ M4 的平台/真机 job。
## 9. 处理记录（2026-09-22）
本报告的高/中危与 §5 条目已按下列状态处理；仅 §3.1 的语义裁决按报告
建议延后（准备项已落地，修复清单已细化）。
本报告的高/中危与 §5 条目已按下列状态处理；§3.1 的语义裁决首先按报告
建议延后（准备项先行落地），随后于 2026-09-24 完成修复。
### §3.1（准备项已落地，语义裁决延后）
### §3.1 ✅（2026-09-24 裁决并修复）
**已用仓库内 C++ 上游源码核准语义**（此前报告 §8 的局限已解除）：
@@ -387,10 +391,38 @@ M4 的平台/真机 job。
| `BlockTrimCommand`(TrimOut) | in 固定、out 移动 | 无 |
另有 `TrackSlide`、`TrackListInsertGaps`、`TrackListRippleToolCommand`、multicam 等
调用点与 undosplit/undoripple/undogeneral 的补偿性选边需要一并裁决；应作为独立
PR 处理（先按 §7 第 7 条把 M5 与覆盖率测试分开提交）。
调用点与 undosplit/undoripple/undogeneral 的补偿性选边需要一并裁决（2026-09-24
已随修复落地，见下；`TrackSlide`/`TrackListInsertGaps` 的“调用方摆放/ripple”
契约同时记入模块文档）。
**本批已落地**：
**修复落地（2026-09-24）**。存储 range 模型定为三个原语
（`crates/oak-node/src/block.rs`），各调用点按上表逐点选择：
- `set_length_and_media_out`：**in 固定、out 移动，media 不动**
（Resize / TrimOut / 缺口向右生长）；
- `set_length_and_media_in`：**in 固定、out 移动，`media_in += old−new`**
（ResizeWithMediaIn、splice 的右半、ripple TrimIn）；
- 新增 `set_length_keeping_out`：**out 固定、in 移动，`media_in += old−new`**
（TrimIn 本体与 out 邻块、滑块右侧邻块、ripple trim_in）。
关键点：
- `TrackReplaceBlockWithGapCommand` 的“仅后随缺口”分支改为 out 锚定：
原实现把缺口向右生长，会吞掉缺口之后的块——这正是“拖动一个素材影响到
其他无关素材”的实证缺陷。回归断言已补（`domain_test`）。
- `undoripple` 的 trim_out 改为 in 锚定（不再误写 media_in），splice 与
trim_in 现在同步推进 `media_in`（内容损坏修复）。
- `undosplit` 显式写两半的 range 与 media：第二半从切点继续，不再依赖
`range.in_()` 的坐标空间巧合。
- `TrackSlideCommand` 保持“调用方摆放滑动块，命令只处理邻块”的存储模型
契约（文档已注明）；`TrackListInsertGaps`/`timeline_ripple_delete_gaps`
的后续块 ripple 仍由调用方负责（C++ 布局的显式替代，未在本次范围）。
- 测试：`domain_test`、`undocommands_test` 的全部 KNOWN-SWAP 期望按上表
“故意”改写（roll 真正移动接缝、slide 无负入点、insert-gaps 向右生长、
ResizeWithMediaIn `media_in=20`），`graphops` 内联 roll 期望同步改写；
`phase2_units_test` 按新语义修正并补 `set_length_keeping_out` 断言。
**本批已落地（准备项）**：
- `RippleInfo::new(block, append_gap)` + `block()`/`append_gap()` 访问器；
`undocommands_test.rs` 新增 2 个真实 info 测试（resize 往返、append_gap
docs/zh/plans/test-coverage-90-80.md
+24
View File
@@ -775,6 +775,30 @@ python3 tooling/coverage_report.py --json target/coverage.json
oak-task 89.5% / 62.8%；oak-storage 82.9% / 66.6%。
- 下一步仍为分支 80%（缺口 ~8.4pp）；热点见上。
### 第八批（§3.1 语义修复与两处 UI 回归，2026-09-24）
- **§3.1 修复**（详见 review §3.1/§9）：`block.rs` 的两个长度 setter 按其
C++ 契约重写为三个存储模型原语：
- `set_length_and_media_out` = in 固定、out 移动、media 不动；
- `set_length_and_media_in` = in 固定、out 移动、`media_in += old−new`；
- 新增 `set_length_keeping_out` = out 固定、in 移动、`media_in += old−new`。
undopointer/undogeneral/undoripple/undosplit/graphops/cli/nodeops 的调用点
逐点对齐，修掉两处实证缺陷：
1. `TrackReplaceBlockWithGapCommand` 在“块后紧跟缺口”时把缺口向右生长，
吞掉缺口之后的块——即用户报告的“拖动一个素材影响到其他无关素材”；
现在缺口向左覆盖被移除块的跨度，后续块不动（domain_test 回归断言）。
2. roll 不滚动、slide 出现负入点、trim-in/ripple splice 把时间线 in 写进
`media_in`（播放错误内容）；全部 KNOWN-SWAP 期望按正确几何改写，并补
`media_in` 维度断言（roll 接缝移动、follower 媒体推进、slide 无负坐标、
insert-gaps 向右生长、ResizeWithMediaIn `media_in=20`）。
- **i18n 刷新**：dock 的 `PanelHandle` 在注册时快照 `DockPanel::title`，
语言切换后页签停留在旧语言（截图的“英文界面 + 中文页签”）。gpui 侧新增
`DockArea::refresh_panel_titles`（经类型擦除 provider 重读标题 + 通知面板），
shell 在菜单与首选项两条切换路径调用；首选项对话框自身同步重绘。
（gpui 子模块提交见 `oak-gpui`。）
- **OCIO 参数面板**：Vec4 评分参数（对比度/偏移/曝光）建 4 个 SpinBox，
`base` 步长锚定范围（pivot 不再一拖就飞出 ±10000）。
## 8. 风险与对策
1. **oak-app UI 覆盖成本最高、最易 flaky**：只用 test-support 的确定性