diff --git a/Cargo.toml b/Cargo.toml index ee2e52812..786f9b397 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -140,7 +140,7 @@ image = "0.25" # --------------------------------------------------------------------------- [package.metadata.packager] name = "oak" -productName = "Oak" +productName = "Oak Video Editor" identifier = "org.oakvideoeditor.Oak" description = "Oak Video Editor" longDescription = "Oak Video Editor: a free, open-source non-linear video editor written in Rust." @@ -154,3 +154,9 @@ binaries = [ { path = "oak-cli", main = false }, { path = "oak-worker", main = false }, ] + +[profile.dev.package."*"] +opt-level = 3 + +[profile.dev] +opt-level = 1 diff --git a/crates/oaknode/src/graph.rs b/crates/oaknode/src/graph.rs index e662ba356..43a604a20 100644 --- a/crates/oaknode/src/graph.rs +++ b/crates/oaknode/src/graph.rs @@ -111,7 +111,12 @@ impl Graph { pub fn add_entry(&mut self, entry: NodeEntry, id: NodeId) -> NodeId { let index = id.index(); if (index as usize) < self.entries.len() && self.entries[index as usize].vacant { - // Original slot free: reuse (index, generation) unchanged. + // Original slot free: reuse (index, generation) unchanged. The + // slot was pushed to the free list by `take_node` — reclaim it, + // or `node_count` keeps undercounting and, worse, the next + // `add_node` hands the same slot out again and silently + // clobbers the restored node (the undo/redo divergence). + self.free_list.retain(|&i| i != index); let generation = entry.generation; self.entries[index as usize] = entry; return NodeId::new(index, generation); diff --git a/crates/oaknode/tests/graph_test.rs b/crates/oaknode/tests/graph_test.rs index de28a445c..0834b34a1 100644 --- a/crates/oaknode/tests/graph_test.rs +++ b/crates/oaknode/tests/graph_test.rs @@ -222,3 +222,35 @@ fn adjacency_queries() { assert_eq!(g.downstream(a), vec![b, c]); assert_eq!(g.downstream(d), Vec::::new()); } + +/// take_node + add_entry must restore the node count AND reclaim the +/// slot: before the fix, add_entry reused the slot without removing it +/// from the free list, so node_count kept undercounting and the next +/// add_node silently overwrote the restored node (the undo/redo +/// divergence the user hit: repeated undo/redo changed the result). +#[test] +fn add_entry_reclaims_the_free_slot() { + let (mut g, ids) = build(3); + let victim = ids[1]; + let count_before = g.node_count(); + + // Detach and re-attach: the count must round-trip. + let entry = g.take_node(victim).expect("take the node"); + assert_eq!(g.node_count(), count_before - 1, "detach drops the count"); + let readded = g.add_entry(entry, victim); + assert_eq!(readded, victim, "identity is preserved"); + assert_eq!(g.node_count(), count_before, "re-attach restores the count"); + + // A fresh add_node must NOT clobber the restored node (it must get a + // different slot). + let mut core = NodeCore::new(); + core.label = "fresh".to_string(); + let fresh = g.add_node(core, Box::new(TestNode { id: "fresh" })); + assert_ne!(fresh, victim, "the fresh node takes a different slot"); + assert!(g.is_valid(victim), "the restored node survives add_node"); + assert_eq!( + g.get(victim).map(|e| e.core.label.as_str()), + g.get(victim).map(|e| e.core.label.as_str()), + ); + assert_eq!(g.node_count(), count_before + 1); +} diff --git a/gpui b/gpui index a761d96c8..c4c1d34bd 160000 --- a/gpui +++ b/gpui @@ -1 +1 @@ -Subproject commit a761d96c826155211c21e6a13f420252757d0ee7 +Subproject commit c4c1d34bd785e2bf0453f18293718fb4a17e5e87 diff --git a/src/oakui/engine.rs b/src/oakui/engine.rs index 463655fd6..199b99bd3 100644 --- a/src/oakui/engine.rs +++ b/src/oakui/engine.rs @@ -548,6 +548,13 @@ pub trait AppEngine: /// `ripple` is set. fn delete_clip(&mut self, clip: ClipId, ripple: bool, cx: &mut Context); + /// Copy the selected clips to the engine clipboard (C++ Copy). + fn clipboard_copy(&mut self, _clips: Vec, _cx: &mut Context) {} + /// Copy then gap-delete the selected clips (C++ Cut). + fn clipboard_cut(&mut self, _clips: Vec, _cx: &mut Context) {} + /// Paste the clipboard at the playhead (C++ Paste), one undoable entry. + fn clipboard_paste(&mut self, _cx: &mut Context) {} + /// Whether the undo stack has an entry to undo. fn can_undo(&self) -> bool; diff --git a/src/oakui/graphops.rs b/src/oakui/graphops.rs index 4eeda674b..ca16a3376 100644 --- a/src/oakui/graphops.rs +++ b/src/oakui/graphops.rs @@ -1392,8 +1392,7 @@ pub fn place_footage_clips_linked( /// Split `clip` at `time_ts` (a frame timestamp strictly inside the /// clip's range), undoable "Split Clip" (the module's /// `BlockSplitCommand`). -pub fn split_clip(p: &ProjectRef, clip: NodeId, time_ts: i64) -> Result<(), String> { - let (tb, in_r, out_r) = { +pub fn split_clip(p: &ProjectRef, clip: NodeId, time_ts: i64) -> Result<(), String> { let (tb, in_r, out_r) = { let g = lock(p); let tb = clip_track(&g.graph, clip) .and_then(|t| track_behavior(&g.graph, t)) @@ -1730,6 +1729,212 @@ pub fn test_lock() -> std::sync::MutexGuard<'static, ()> { LOCK.lock().unwrap_or_else(|e| e.into_inner()) } +// --------------------------------------------------------------------------- +// Clipboard (Cut / Copy / Paste) +// --------------------------------------------------------------------------- + +/// One clip captured in the clipboard (Cut/Copy). Everything needed to +/// re-place it on a timeline is here; effects/multicam contexts are not +/// modeled yet (footage clips carry their block core only). +#[derive(Clone, Debug)] +pub struct ClipboardClip { + /// The footage node the clip decodes. + pub footage: NodeId, + /// Track type the clip was on. + pub kind: TrackType, + /// Per-type index of the track it was on (re-used on paste when the + /// track still exists). + pub track_index: usize, + /// Media in-point in frame timestamps. + pub media_in_ts: i64, + /// Timeline in-point in frame timestamps. + pub start_ts: i64, + /// Duration in frame timestamps. + pub length_ts: i64, + /// Playback speed. + pub speed: f64, +} + +/// Capture the selected clips into clipboard form (`Copy`; `Cut` copies +/// then deletes). Clips without a footage upstream are skipped. +pub fn copy_clips(p: &ProjectRef, clips: &[NodeId]) -> Vec { + let g = lock(p); + let mut out = Vec::new(); + for &clip in clips { + let Some(footage) = find_input_footage(&g.graph, clip) else { + continue; + }; + let Some((in_r, out_r, media_in)) = clip_range(&g.graph, clip) else { + continue; + }; + let Some(track) = clip_track(&g.graph, clip) else { + continue; + }; + let Some(t) = track_behavior(&g.graph, track) else { + continue; + }; + let Some(list) = t.track_list.and_then(|l| track_list_behavior(&g.graph, l)) else { + continue; + }; + let (tb_num, tb_den) = { + // Frame timestamps are stored rationally; the clipboard keeps + // the sequence timebase for exactness. + let seq = list.sequence.and_then(|s| sequence_time_base(&g.graph, s)); + seq.unwrap_or((1, 25)) + }; + let to_ts = |r: Rational| { + (r.numerator() * tb_den).checked_div(r.denominator() * tb_num).unwrap_or(0) + }; + let speed = clip_behavior(&g.graph, clip) + .map(|c| c.core.speed) + .unwrap_or(1.0); + out.push(ClipboardClip { + footage, + kind: list.kind, + track_index: t.index.max(0) as usize, + media_in_ts: to_ts(media_in), + start_ts: to_ts(in_r), + length_ts: to_ts(out_r - in_r), + speed, + }); + } + out.sort_by_key(|c| c.start_ts); + out +} + +/// Paste clipboard clips at `playhead_ts` (frame timestamps), undoable +/// "Paste". The first clip's in-point moves to the playhead; the others +/// keep their relative offsets. Tracks are reused by kind+index (falling +/// back to the last track of the kind). Clips that were linked stay +/// linked inside the pasted group. +pub fn paste_clips( + p: &ProjectRef, + seq: NodeId, + items: &[ClipboardClip], + playhead_ts: i64, +) -> Result, String> { + if items.is_empty() { + return Err("the clipboard is empty".to_string()); + } + let anchor = items[0].start_ts; + let mut clips = Vec::with_capacity(items.len()); + for item in items { + let start = playhead_ts + (item.start_ts - anchor); + let out = start + item.length_ts; + // Re-use the placement machinery one clip at a time, collecting + // the commands so everything lands in ONE undo entry. + clips.push((item, start, out)); + } + + let tb = { + let g = lock(p); + sequence_time_base(&g.graph, seq) + .ok_or_else(|| "sequence has no valid frame rate".to_string())? + }; + let track_count_of = |kind: TrackType| -> usize { + let g = lock(p); + track_list_of(&g.graph, seq, kind) + .and_then(|l| track_list_behavior(&g.graph, l)) + .map(|l| l.tracks.len()) + .unwrap_or(0) + }; + + let mut new_ids = Vec::with_capacity(items.len()); + let mut commands = Vec::new(); + for (item, start, out) in clips { + let list = { + let g = lock(p); + track_list_of(&g.graph, seq, item.kind) + .ok_or_else(|| "sequence has no track list for this type".to_string())? + }; + let count = track_count_of(item.kind); + if count == 0 { + return Err("no track of the clip's kind to paste onto".to_string()); + } + let track_index = item.track_index.min(count - 1); + let in_r = ts_to_rational(start, tb); + let media_r = ts_to_rational(item.media_in_ts, tb); + let length_r = ts_to_rational(out, tb) - in_r; + + let clip = { + let mut g = lock(p); + let (core, behavior) = oaknode::block::clip_create(); + let id = g.graph.add_node(core, behavior); + if let Some(c) = g + .graph + .get_mut(id) + .and_then(|e| e.behavior.as_any_mut()) + .and_then(|a| a.downcast_mut::()) + { + c.core.media_in = media_r; + c.core.speed = item.speed; + c.core.set_length_and_media_in(length_r); + } + id + }; + new_ids.push(clip); + commands.push( + oaktimeline::undopointer::TrackPlaceBlockCommand::new( + node_ref(p, list), + track_index as i32, + node_ref(p, clip), + in_r, + ) + .to_command(), + ); + commands.push(connect_command( + p, + item.footage, + clip, + oaknode::block::clip_input::TEXTURE_INPUT, + )?); + } + // Link the pasted group both ways (the C++ pastes linked selections as + // a linked group). + if new_ids.len() > 1 { + let first = new_ids[0]; + for &other in &new_ids[1..] { + let (p1, p2) = (p.clone(), p.clone()); + commands.push(oakundo::undocommand::UndoCommand::from_closures( + move || { + let mut g = lock(&p1); + for (a, b) in [(first, other), (other, first)] { + if let Some(entry) = g.graph.get_mut(a) { + let links = &mut entry + .behavior + .as_any_mut() + .and_then(|any| any.downcast_mut::()) + .expect("clip behavior") + .core + .links; + if !links.contains(&b) { + links.push(b); + } + } + } + }, + move || { + let mut g = lock(&p2); + for (a, b) in [(first, other), (other, first)] { + if let Some(entry) = g.graph.get_mut(a) { + let links = &mut entry + .behavior + .as_any_mut() + .and_then(|any| any.downcast_mut::()) + .expect("clip behavior") + .core + .links; + links.retain(|&l| l != b); + } + } + }, + )); + } + } + push_multi(commands, "Paste")?; + Ok(new_ids) +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -1790,6 +1995,64 @@ mod tests { oakundo::global::clear().unwrap(); } + /// Undo/redo stability: undo → redo → undo → redo must converge to the + /// SAME graph state every cycle (the user's "撤销再前进再撤销再前进， + /// 结果居然变了" regression). Snapshot the sequence's track blocks and + /// the graph node count around two full cycles. + #[test] + fn undo_redo_cycles_converge_to_the_same_state() { + let _g = test_lock(); + oakundo::global::clear().unwrap(); + let project = create_project(); + let seq = create_sequence(&project, "Undo Cycles"); + let media = std::env::temp_dir().join(format!("oak_undo_cycle_{}.mp4", std::process::id())); + oakcodec::testmedia::write_test_clip(&media, 64, 64, 10, 10).expect("generate test media"); + let footage = import_footage(&project, &media).expect("import"); + place_footage_clips_linked( + &project, + seq, + footage, + &[(TrackType::Video, 0), (TrackType::Audio, 0)], + 0, + 10, + 0, + ) + .expect("linked placement"); + + let snapshot = |p: &ProjectRef| -> (usize, Vec<(TrackType, Vec)>) { + let g = lock(p); + let node_count = g.graph.node_count(); + let mut tracks = Vec::new(); + for kind in [TrackType::Video, TrackType::Audio] { + let blocks: Vec = track_ids(&g.graph, seq, kind) + .iter() + .map(|t| track_behavior(&g.graph, *t).map(|t| t.blocks.len()).unwrap_or(0)) + .collect(); + tracks.push((kind, blocks)); + } + (node_count, tracks) + }; + + let before = snapshot(&project); + // Two full undo/redo cycles; the state must be identical after + // every redo and match the pre-undo state after every undo. + for cycle in 0..2 { + oakundo::global::undo().expect("undo"); + let g = lock(&project); + let video_blocks: usize = track_ids(&g.graph, seq, TrackType::Video) + .iter() + .map(|t| track_behavior(&g.graph, *t).map(|t| t.blocks.len()).unwrap_or(0)) + .sum(); + assert_eq!(video_blocks, 0, "cycle {cycle}: undo removes the clips"); + drop(g); + oakundo::global::redo().expect("redo"); + let state = snapshot(&project); + assert_eq!(state, before, "cycle {cycle}: redo must restore the exact state"); + } + oakundo::global::clear().unwrap(); + let _ = std::fs::remove_file(&media); + } + /// A stale (non-track) id is rejected, not silently ignored. #[test] fn track_flag_setters_reject_non_tracks() { @@ -1801,3 +2064,177 @@ mod tests { oakundo::global::clear().unwrap(); } } + +#[cfg(test)] +mod undo_cycle_track_tests { + use super::*; + + /// add_track undo/redo cycles must converge (the track count AND the + /// created track's identity stay stable; the C++ remove-last undo must + /// not eat a default track). + #[test] + fn add_track_undo_redo_cycles_converge() { + let _g = test_lock(); + oakundo::global::clear().unwrap(); + let project = create_project(); + let seq = create_sequence(&project, "Add Track Cycles"); + let count_of = |p: &ProjectRef, kind: TrackType| { + let g = lock(p); + track_ids(&g.graph, seq, kind).len() + }; + assert_eq!(count_of(&project, TrackType::Video), 2, "default 2 video tracks"); + + let index = add_track(&project, seq, TrackType::Video).expect("add a track"); + assert_eq!(index, 2, "the new track is the third video track"); + let track_id = { + let g = lock(&project); + track_ids(&g.graph, seq, TrackType::Video)[index] + }; + + for cycle in 0..3 { + oakundo::global::undo().expect("undo"); + assert_eq!( + count_of(&project, TrackType::Video), + 2, + "cycle {cycle}: undo removes only the added track" + ); + oakundo::global::redo().expect("redo"); + assert_eq!( + count_of(&project, TrackType::Video), + 3, + "cycle {cycle}: redo restores the added track" + ); + let g = lock(&project); + assert!( + g.graph.is_valid(track_id), + "cycle {cycle}: the same track node is back in the graph" + ); + assert_eq!( + track_ids(&g.graph, seq, TrackType::Video)[index], + track_id, + "cycle {cycle}: the track keeps its identity and position" + ); + drop(g); + } + oakundo::global::clear().unwrap(); + } +} + +#[cfg(test)] +mod undo_cycle_ops_tests { + use super::*; + + /// Full-graph snapshot for convergence checks: node count plus, for + /// every track, the ordered block ids and each block's range. + fn snapshot(p: &ProjectRef, seq: NodeId) -> (usize, Vec<(NodeId, Vec<(NodeId, i128, i128)>)>) { + let g = lock(p); + let mut tracks = Vec::new(); + for kind in [TrackType::Video, TrackType::Audio] { + for t in track_ids(&g.graph, seq, kind) { + let blocks: Vec<(NodeId, i128, i128)> = track_behavior(&g.graph, t) + .map(|t| { + t.blocks + .iter() + .map(|&b| { + let (in_r, out_r, _) = clip_range(&g.graph, b).unwrap_or_default(); + ( + b, + in_r.numerator() as i128 * 1_000_000 / in_r.denominator().max(1) as i128, + out_r.numerator() as i128 * 1_000_000 / out_r.denominator().max(1) as i128, + ) + }) + .collect() + }) + .unwrap_or_default(); + tracks.push((t, blocks)); + } + } + (g.graph.node_count(), tracks) + } + + fn cycle_assert(p: &ProjectRef, seq: NodeId, post: &(usize, Vec<(NodeId, Vec<(NodeId, i128, i128)>)>), what: &str) { + for cycle in 0..3 { + oakundo::global::undo().unwrap_or_else(|e| panic!("{what}: undo failed: {e:?}")); + oakundo::global::redo().unwrap_or_else(|e| panic!("{what}: redo failed: {e:?}")); + let state = snapshot(p, seq); + assert_eq!(&state, post, "{what}: cycle {cycle} diverged"); + } + } + + fn project_with_two_clips(media: &std::path::Path) -> (ProjectRef, NodeId, NodeId) { + let project = create_project(); + let seq = create_sequence(&project, "Cycle Ops"); + let footage = import_footage(&project, media).expect("import"); + place_footage_clips_linked( + &project, + seq, + footage, + &[(TrackType::Video, 0), (TrackType::Audio, 0)], + 0, + 10, + 0, + ) + .expect("linked placement") + .into_iter() + .next() + .map(|_| (project.clone(), seq, footage)) + .expect("one clip") + } + + /// Move / trim / delete / split undo-redo cycles must all converge. + #[test] + fn move_trim_delete_split_cycles_converge() { + let _g = test_lock(); + oakundo::global::clear().unwrap(); + let media = std::env::temp_dir().join(format!("oak_cycle_ops_{}.mp4", std::process::id())); + oakcodec::testmedia::write_test_clip(&media, 64, 64, 10, 10).expect("generate"); + + // --- move --- + let (project, seq, _footage) = project_with_two_clips(&media); + let clip = { + let g = lock(&project); + track_ids(&g.graph, seq, TrackType::Video) + .iter() + .find_map(|t| track_behavior(&g.graph, *t).and_then(|t| t.blocks.first().copied())) + .expect("a clip") + }; + move_clip(&project, clip, 20).expect("move"); + let post = snapshot(&project, seq); + cycle_assert(&project, seq, &post, "move"); + oakundo::global::clear().unwrap(); + + // --- trim --- + trim_clip(&project, clip, 22, 28).expect("trim"); + let post = snapshot(&project, seq); + cycle_assert(&project, seq, &post, "trim"); + oakundo::global::clear().unwrap(); + + // --- split --- + split_clip(&project, clip, 25).expect("split"); + let post = snapshot(&project, seq); + cycle_assert(&project, seq, &post, "split"); + oakundo::global::clear().unwrap(); + + // --- delete --- + delete_clip(&project, clip).expect("delete"); + let post = snapshot(&project, seq); + cycle_assert(&project, seq, &post, "delete"); + oakundo::global::clear().unwrap(); + + // --- ripple delete (on a fresh project, then check) --- + let (project, seq, _footage) = project_with_two_clips(&media); + let clip = { + let g = lock(&project); + track_ids(&g.graph, seq, TrackType::Video) + .iter() + .find_map(|t| track_behavior(&g.graph, *t).and_then(|t| t.blocks.first().copied())) + .expect("a clip") + }; + ripple_delete_clip(&project, clip).expect("ripple delete"); + let post = snapshot(&project, seq); + cycle_assert(&project, seq, &post, "ripple delete"); + oakundo::global::clear().unwrap(); + + let _ = std::fs::remove_file(&media); + } +} diff --git a/src/oakui/real.rs b/src/oakui/real.rs index 15cba5c5e..7f0418f60 100644 --- a/src/oakui/real.rs +++ b/src/oakui/real.rs @@ -959,6 +959,9 @@ pub struct RealEngine { /// (`None` for an empty or multi-clip selection, or before any /// selection event). selected_clip: Option, + /// The engine clipboard for Cut/Copy/Paste (graphops::ClipboardClip + /// entries in timeline order). + clipboard: Vec, /// Node identities whose effect cards are expanded (view state; kept /// here because `EffectData` is a pure read and expansion is not /// undoable). @@ -1138,6 +1141,7 @@ impl RealEngine { waveforms: Mutex::new(None), selected_item: None, selected_clip: None, + clipboard: Vec::new(), expanded_effects: BTreeSet::new(), program_playing: false, meter_phase: 0, @@ -3677,6 +3681,45 @@ impl AppEngine for RealEngine { ); } + /// Copy the selected clips to the engine clipboard (C++ Copy): the + /// clipboard holds footage/range/speed/track-kind per clip, in + /// timeline order. + fn clipboard_copy(&mut self, clips: Vec, _cx: &mut Context) { + let Some(project) = self.project.clone() else { + return; + }; + let blocks: Vec = clips + .iter() + .filter_map(|id| graphops::id_of(id.0)) + .collect(); + self.clipboard = graphops::copy_clips(&project, &blocks); + } + + /// Copy then gap-delete the selected clips (C++ Cut: copy + Delete, + /// leaving gaps — ripple deletion is the separate Ripple Delete + /// action). + fn clipboard_cut(&mut self, clips: Vec, cx: &mut Context) { + self.clipboard_copy(clips.clone(), cx); + for id in clips { + self.delete_clip(id, false, cx); + } + } + + /// Paste the clipboard at the program playhead (C++ Paste): one + /// undoable entry, clips keep their relative offsets and links. + fn clipboard_paste(&mut self, cx: &mut Context) { + let (Some(project), Some(seq)) = (self.project.clone(), self.sequence) else { + return; + }; + if self.clipboard.is_empty() { + return; + } + let playhead = self.clock_frame(Monitor::Program, cx).0.max(0); + let items = self.clipboard.clone(); + let result = graphops::paste_clips(&project, seq, &items, playhead).map(|_| ()); + self.apply_edit(result, "paste", cx); + } + fn can_undo(&self) -> bool { self.project.is_some() && oakundo::global::undoable() } diff --git a/src/panels/timeline.rs b/src/panels/timeline.rs index 0942a52d9..f5f2fe34e 100644 --- a/src/panels/timeline.rs +++ b/src/panels/timeline.rs @@ -219,7 +219,20 @@ impl TimelinePanel { cx: &mut Context, ) { let menu = match &hit { - TimelineHit::Clip(_) => { + TimelineHit::Clip(clip) => { + // C++ parity: right-clicking an unselected clip selects it + // first — the context menu acts on the clicked clip, never + // on a stale or empty selection (this is what made + // Cut/Delete appear to do nothing). + if !self.timeline.read(cx).selection().contains(clip) { + let clip = *clip; + self.timeline.update(cx, |view, cx| { + view.state.selection.clear(); + view.state.selection.insert(clip); + cx.emit(TimelineEvent::SelectionChanged); + cx.notify(); + }); + } let ids: Vec = self.timeline.read(cx).selection().iter().copied().collect(); let (sync, proxy, multicam) = { @@ -604,6 +617,20 @@ impl PanelCommandHandler for TimelinePanel { } // --- editing --- + fn cut_selected(&mut self, cx: &mut Context) -> bool { + let ids: Vec = self.timeline.read(cx).selection().iter().copied().collect(); + self.engine.update(cx, |engine, cx| engine.clipboard_cut(ids, cx)); + true + } + fn copy_selected(&mut self, cx: &mut Context) -> bool { + let ids: Vec = self.timeline.read(cx).selection().iter().copied().collect(); + self.engine.update(cx, |engine, cx| engine.clipboard_copy(ids, cx)); + true + } + fn paste(&mut self, cx: &mut Context) -> bool { + self.engine.update(cx, |engine, cx| engine.clipboard_paste(cx)); + true + } fn delete_selected(&mut self, cx: &mut Context) -> bool { self.delete_selection(false, cx); true