diff --git a/gpui b/gpui index dea33c713..8a0b7569a 160000 --- a/gpui +++ b/gpui @@ -1 +1 @@ -Subproject commit dea33c713f3eeaade8adf3a435d1aeae68cb394e +Subproject commit 8a0b7569a3c0b9b6e2a0fe75c9c91ea91fb8a911 diff --git a/src/oakui/graphops.rs b/src/oakui/graphops.rs index ca16a3376..4defad483 100644 --- a/src/oakui/graphops.rs +++ b/src/oakui/graphops.rs @@ -1504,13 +1504,14 @@ pub fn trim_clip(p: &ProjectRef, clip: NodeId, new_in_ts: i64, new_out_ts: i64) push_multi(children, "Trim Clip") } -/// Move `clip` within its track so its in point becomes `new_in_ts` -/// (undoable "Move Clip"; the module's `TrackMoveBlockCommand` — the old -/// spot becomes a gap, length and media-in are preserved). -pub fn move_clip(p: &ProjectRef, clip: NodeId, new_in_ts: i64) -> Result<(), String> { - if new_in_ts < 0 { - return Err("invalid move target".to_string()); - } +/// The undoable same-track move command for one clip (its in point becomes +/// `new_in_ts`; the module's `TrackMoveBlockCommand` — the old spot becomes +/// a gap, length and media-in are preserved). +fn move_clip_command( + p: &ProjectRef, + clip: NodeId, + new_in_ts: i64, +) -> Result { let (tb, list, track_index) = { let g = lock(p); let track = clip_track(&g.graph, clip).ok_or_else(|| "the clip is not on a track".to_string())?; @@ -1524,31 +1525,35 @@ pub fn move_clip(p: &ProjectRef, clip: NodeId, new_in_ts: i64) -> Result<(), Str .ok_or_else(|| "the sequence has no valid frame rate".to_string())?; (tb, list, track_index) }; - push( - oaktimeline::undopointer::TrackMoveBlockCommand::new( - node_ref(p, list), - track_index, - node_ref(p, clip), - ts_to_rational(new_in_ts, tb), - ) - .to_command(), - "Move Clip", + Ok(oaktimeline::undopointer::TrackMoveBlockCommand::new( + node_ref(p, list), + track_index, + node_ref(p, clip), + ts_to_rational(new_in_ts, tb), ) + .to_command()) } -/// Move `clip` to a different track at `new_in_ts` (undoable "Move Clip -/// to Track", one row): the source spot becomes a gap, the block's in -/// point is re-homed, and the clip is placed on the destination track -/// (the facade's `oakengine_sequence_move_clip_to_track` composition). -pub fn move_clip_to_track( +/// Move `clip` within its track so its in point becomes `new_in_ts` +/// (undoable "Move Clip"; the module's `TrackMoveBlockCommand` — the old +/// spot becomes a gap, length and media-in are preserved). +pub fn move_clip(p: &ProjectRef, clip: NodeId, new_in_ts: i64) -> Result<(), String> { + if new_in_ts < 0 { + return Err("invalid move target".to_string()); + } + push(move_clip_command(p, clip, new_in_ts)?, "Move Clip") +} + +/// The undoable cross-track move commands for one clip (gap on the source +/// track, re-homed in point, place on the destination track), WITHOUT +/// pushing — assembled by callers that move several clips in one undoable +/// entry. +fn move_clip_to_track_commands( p: &ProjectRef, clip: NodeId, dest_track: NodeId, new_in_ts: i64, -) -> Result<(), String> { - if new_in_ts < 0 { - return Err("invalid move target".to_string()); - } +) -> Result, String> { let (tb, list, dest_index, source_track) = { let g = lock(p); let source_track = @@ -1614,7 +1619,83 @@ pub fn move_clip_to_track( in_r, ) .to_command(); - push_multi(vec![gap, rehome, place], "Move Clip to Track") + Ok(vec![gap, rehome, place]) +} + +/// Move `clip` to a different track at `new_in_ts` (undoable "Move Clip +/// to Track", one row): the source spot becomes a gap, the block's in +/// point is re-homed, and the clip is placed on the destination track +/// (the facade's `oakengine_sequence_move_clip_to_track` composition). +pub fn move_clip_to_track( + p: &ProjectRef, + clip: NodeId, + dest_track: NodeId, + new_in_ts: i64, +) -> Result<(), String> { + if new_in_ts < 0 { + return Err("invalid move target".to_string()); + } + push_multi(move_clip_to_track_commands(p, clip, dest_track, new_in_ts)?, "Move Clip to Track") +} + +/// Move `clip` to `new_in_ts` (`dest_track` when the gesture crosses +/// tracks) while every clip in `linked` follows in lockstep: each linked +/// clip keeps its own track and moves by the same frame offset. The whole +/// group lands as ONE undoable "Move Clip" entry (C++ `block_links_` +/// semantics — grouped edits apply to the whole group). +pub fn move_clip_with_links( + p: &ProjectRef, + clip: NodeId, + dest_track: Option, + new_in_ts: i64, + linked: &[NodeId], +) -> Result<(), String> { + if new_in_ts < 0 { + return Err("invalid move target".to_string()); + } + // The dragged clip's old in point frames the shared frame delta. + let old_in_ts = { + let g = lock(p); + let tb = clip_track(&g.graph, clip) + .and_then(|t| track_behavior(&g.graph, t)) + .and_then(|t| t.track_list) + .and_then(|l| track_list_behavior(&g.graph, l)) + .and_then(|l| l.sequence) + .and_then(|s| sequence_time_base(&g.graph, s)) + .ok_or_else(|| "the clip's sequence has no valid frame rate".to_string())?; + let (in_r, _, _) = clip_range(&g.graph, clip) + .ok_or_else(|| "the node is not a clip".to_string())?; + rational_to_ts(in_r, tb) + }; + let delta = new_in_ts - old_in_ts; + + let mut commands = Vec::new(); + match dest_track { + Some(track) => commands.extend(move_clip_to_track_commands(p, clip, track, new_in_ts)?), + None => commands.push(move_clip_command(p, clip, new_in_ts)?), + } + for &other in linked { + if other == clip { + continue; + } + // Each linked clip stays on its own track; only its in point follows + // the shared frame delta. + let other_in_ts = { + let g = lock(p); + let tb = clip_track(&g.graph, other) + .and_then(|t| track_behavior(&g.graph, t)) + .and_then(|t| t.track_list) + .and_then(|l| track_list_behavior(&g.graph, l)) + .and_then(|l| l.sequence) + .and_then(|s| sequence_time_base(&g.graph, s)) + .ok_or_else(|| "a linked clip's sequence has no valid frame rate".to_string())?; + let (in_r, _, _) = clip_range(&g.graph, other) + .ok_or_else(|| "a linked node is not a clip".to_string())?; + rational_to_ts(in_r, tb) + }; + commands.push(move_clip_command(p, other, (other_in_ts + delta).max(0))?); + } + push_multi(commands, "Move Clip") } /// Delete `clip` leaving a gap (undoable "Delete Clips"; the facade's diff --git a/src/oakui/mock.rs b/src/oakui/mock.rs index acde65c35..7dfb96ce9 100644 --- a/src/oakui/mock.rs +++ b/src/oakui/mock.rs @@ -937,11 +937,14 @@ impl MockEngine { self.refresh_port_connectivity(); } NodeGraphEvent::SelectionChanged { nodes } => { + // Only auto-expand when the selection actually CHANGED. The + // card-header click selects the card's node (CardSelected → + // node_selection) and the panel mirrors that into the graph + // widget, whose SelectionChanged echo must not fight the + // expansion toggle the same click requested. + let changed = self.node_selection != *nodes; self.node_selection = nodes.clone(); - // The demo's node↔inspector link: a single selected node - // expands the matching effect card (the real engine mirrors - // this through `expanded_effects`). - if nodes.len() == 1 { + if changed && nodes.len() == 1 { let node = *nodes.iter().next().expect("a one-element set"); if let Some(effect) = self.effect_for_node(node) { if let Some(card) = self.effects.iter_mut().find(|e| e.id == effect) { @@ -2626,6 +2629,66 @@ mod tests { }); } + /// The card-header click round trip: expansion toggles collapse then + /// re-expand. The header click emits BOTH `ExpansionToggled` and + /// `CardSelected`; the card selection mirrors its node into the node + /// editor, whose `SelectionChanged` echo must not fight the toggle (the + /// demo's 变换 card maps to graph node 2). + #[gpui::test] + async fn effect_card_expansion_survives_the_card_selection_echo(cx: &mut TestAppContext) { + cx.update(|app| { + let engine = demo_engine(app); + let toggle = |app: &mut gpui::App, expanded: bool| { + engine.update(app, |engine, cx| { + engine.apply_effect_event( + &EffectStackEvent::ExpansionToggled { + effect: EffectId(1), + expanded, + }, + cx, + ); + }); + }; + let is_expanded = |app: &gpui::App| -> bool { + engine + .read(app) + .effects() + .iter() + .find(|e| e.id() == EffectId(1)) + .map(|e| e.is_expanded()) + .unwrap_or(false) + }; + + // The demo's 变换 card starts expanded; the collapse click also + // selects the card (CardSelected → node_selection = {2}), and the + // node-editor panel then mirrors that into the graph widget, whose + // SelectionChanged echo arrives through apply_node_graph_event. + assert!(is_expanded(app), "the demo card starts expanded"); + toggle(app, false); + engine.update(app, |engine, cx| { + engine.apply_effect_event(&EffectStackEvent::CardSelected { effect: EffectId(1) }, cx); + }); + engine.update(app, |engine, cx| { + engine.apply_node_graph_event( + &NodeGraphEvent::SelectionChanged { + nodes: BTreeSet::from([NodeId(2)]), + }, + cx, + ); + }); + assert!( + !is_expanded(app), + "the selection echo must not re-expand the collapsed card" + ); + + // Expand → collapse round trip stays clean. + toggle(app, true); + assert!(is_expanded(app)); + toggle(app, false); + assert!(!is_expanded(app)); + }); + } + #[gpui::test] async fn timeline_edits_are_applied_to_the_mock_model(cx: &mut TestAppContext) { cx.update(|app| { diff --git a/src/oakui/real.rs b/src/oakui/real.rs index 55ab54ab4..677d75e24 100644 --- a/src/oakui/real.rs +++ b/src/oakui/real.rs @@ -1935,9 +1935,17 @@ impl RealEngine { release_rendered_frame(&rendered); let image = image::RgbaImage::from_raw(width, height, bytes)?; std::fs::create_dir_all(path.parent()?).ok()?; - // Write aside then rename so readers never see a partial file. + // Write aside then rename so readers never see a partial file. The + // temp name ends in `.part` (not `.png`), so the encoder is given + // explicitly — `Image::save` infers the format from the extension + // and would reject it. let tmp = path.with_extension("part"); - image.save(&tmp).ok()?; + { + let file = std::fs::File::create(&tmp).ok()?; + image + .write_to(&mut std::io::BufWriter::new(file), image::ImageFormat::Png) + .ok()?; + } std::fs::rename(&tmp, &path).ok()?; Some(path) } @@ -3559,9 +3567,11 @@ impl AppEngine for RealEngine { new_track, new_start, } => { - // Cross-track moves go through the gap + re-home + place - // composition (one undoable entry); same-track moves use - // the plain move command. + // The dragged clip moves to `new_track`/`new_start`; every + // clip linked to it (the A/V pair dropped from one file) + // follows in lockstep, each staying on its own track and + // shifting by the same frame offset. All of it lands as ONE + // undoable entry. let Some(block) = self.clip_block(*clip) else { return; }; @@ -3571,19 +3581,45 @@ impl AppEngine for RealEngine { let Some(project) = self.project.clone() else { return; }; + // Linked clips on locked tracks are left in place. + let linked: Vec = { + let guard = graphops::lock(&project); + guard + .graph + .links_of(block) + .into_iter() + .filter(|&other| graphops::clip_behavior(&guard.graph, other).is_some()) + .collect() + }; + let linked: Vec = linked + .into_iter() + .filter(|&other| !self.clip_track_locked(other)) + .collect(); let current_track = self .tracks .iter() .position(|t| t.clips.iter().any(|c| c.block == block)); - let result = match (current_track, self.tracks.get(*new_track)) { - (Some(current), _) if current == *new_track => { - graphops::move_clip(&project, block, new_start.0) + // Cross-track moves go through the gap + re-home + place + // composition; same-track moves use the plain move command. + let dest_track = match (current_track, self.tracks.get(*new_track)) { + (Some(current), _) if current == *new_track => None, + (_, Some(dest)) => Some(dest.track), + _ => { + self.apply_edit( + Err("move clip: destination track out of range".to_string()), + "move clip", + cx, + ); + return; } - (_, Some(dest)) => { - graphops::move_clip_to_track(&project, block, dest.track, new_start.0) - } - _ => Err("move clip: destination track out of range".to_string()), }; + let result = graphops::move_clip_with_links( + &project, + block, + dest_track, + new_start.0, + &linked, + ); self.apply_edit(result, "move clip", cx); } TimelineEvent::TrackHeightChanged { track, height } => { @@ -5699,6 +5735,90 @@ mod tests { let _ = std::fs::remove_file(&media); } + /// The material-bin thumbnail pipeline end to end: importing a real media + /// file (tests/demo.mp4) lists a footage entry whose icon-view thumbnail + /// is rendered on a background worker, drained on the tick, and cached as + /// a PNG on disk keyed by the media filename — the entry eventually + /// carries a `thumbnail` path that resolves to an existing file (the + /// engine half of the icon-view img chain; the widget half is covered by + /// `icon_view_renders_thumbnail_img_or_placeholder`). + #[gpui::test] + async fn real_engine_thumbnail_pipeline_produces_png(cx: &mut gpui::TestAppContext) { + let _media = media_lock(); + let _worker = WorkerBinGuard::set(); + if !crate::oakui::renderops::ensure_render_manager() { + panic!("the render manager failed to start"); + } + let engine = cx.update(|cx| cx.new(|cx| RealEngine::create(cx))); + cx.update(|app| engine.update(app, |engine, cx| engine.new_project(cx))); + + let media = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/demo.mp4"); + let imported = cx + .update(|app| engine.update(app, |engine, cx| engine.import_footage(media.clone(), cx))); + assert!(imported.is_ok(), "import tests/demo.mp4: {imported:?}"); + let name = media.file_name().unwrap().to_string_lossy().into_owned(); + + // The core render step runs synchronously: the first frame decodes to + // a PNG in the shared thumbnail directory. + let entry_id = cx + .read(|app| { + engine + .read(app) + .roots() + .into_iter() + .find(|e| e.name.as_ref() == name) + .expect("the imported footage is listed") + .id + }); + let project = cx.read(|app| engine.read(app).project_ref().cloned().unwrap()); + let rendered = RealEngine::render_thumbnail(&project, entry_id); + let rendered = rendered + .expect("render_thumbnail produces a PNG path for the real media"); + assert!( + rendered.exists(), + "the rendered PNG exists: {}", + rendered.display() + ); + + // The async path installs it: a fresh engine (no cached done entries) + // spawns the worker on `roots()`, and the drain installs the completed + // path into the cache so the entry re-reads with the thumbnail. + let engine = cx.update(|cx| cx.new(|cx| RealEngine::create(cx))); + cx.update(|app| engine.update(app, |engine, cx| engine.new_project(cx))); + cx.update(|app| { + engine.update(app, |engine, cx| { + engine.import_footage(media.clone(), cx) + }) + }) + .expect("re-import the footage"); + let deadline = std::time::Instant::now() + Duration::from_secs(20); + loop { + let thumbnail = cx.read(|app| { + engine + .read(app) + .roots() + .into_iter() + .find(|e| e.name.as_ref() == name) + .and_then(|e| e.thumbnail.clone()) + }); + if let Some(thumbnail) = thumbnail { + assert!( + std::path::PathBuf::from(thumbnail.as_ref()).exists(), + "the attached thumbnail resolves to a file on disk" + ); + break; + } + assert!( + std::time::Instant::now() < deadline, + "the entry eventually carries a thumbnail path" + ); + cx.update(|app| engine.update(app, |engine, _cx| engine.drain_thumbnails())); + std::thread::sleep(Duration::from_millis(10)); + } + // Leave the shared cache clean for the next run. + let _ = std::fs::remove_file(&rendered); + } + /// Track header toggles through the app seam: a `TrackToggleRequested` /// event lands as ONE undoable engine command, the timeline snapshot /// reflects the new flag, and undo restores it. Visibility maps onto the @@ -6529,6 +6649,136 @@ mod tests { assert_eq!((video_clips, audio_clips), (1, 1), "one redo restores both clips"); } + /// Dragging a clip of a linked A/V pair drags its partner in lockstep: + /// the linked clip stays on its own track and shifts by the same frame + /// offset, both for a same-track drag and a cross-track drag, and ONE + /// undo restores the whole group. + #[gpui::test] + async fn moving_a_linked_clip_drags_its_partner(cx: &mut gpui::TestAppContext) { + let _media = media_lock(); + let engine = cx.update(|cx| cx.new(|cx| RealEngine::create(cx))); + cx.update(|app| engine.update(app, |engine, cx| engine.new_project(cx))); + + let media = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/demo.mp4"); + cx.update(|app| { + engine.update(app, |engine, cx| { + engine.import_footage(media.clone(), cx).expect("import") + }) + }); + let name = media.file_name().unwrap().to_string_lossy().into_owned(); + let entry = cx.read(|app| { + engine + .read(app) + .roots() + .into_iter() + .find(|e| e.name.as_ref() == name) + }) + .expect("imported footage is listed"); + cx.update(|app| { + engine.update(app, |engine, cx| { + engine.drop_footage(entry.id, TrackKind::Video, 0, Frame(40), cx) + }) + }); + + // The video clip id, the audio clip id, and their display-track + // indices. + let (video_id, audio_id, video_idx, audio_idx, other_video_idx) = cx.read(|app| { + let engine = engine.read(app); + let video_idx = engine + .tracks + .iter() + .position(|t| t.kind == TrackKind::Video && !t.clips.is_empty()) + .expect("a video track with the clip"); + let audio_idx = engine + .tracks + .iter() + .position(|t| t.kind == TrackKind::Audio && !t.clips.is_empty()) + .expect("an audio track with the clip"); + let other_video_idx = engine + .tracks + .iter() + .enumerate() + .find(|(i, t)| t.kind == TrackKind::Video && *i != video_idx) + .map(|(i, _)| i) + .expect("a second video track"); + ( + engine.tracks[video_idx].clips[0].id, + engine.tracks[audio_idx].clips[0].id, + video_idx, + audio_idx, + other_video_idx, + ) + }); + + let in_points = |cx: &mut gpui::TestAppContext, video_idx: usize, audio_idx: usize| { + cx.read(|app| { + let engine = engine.read(app); + let video = engine.tracks[video_idx] + .clips + .iter() + .find(|c| c.id == video_id) + .map(|c| c.range.start.0); + let audio = engine.tracks[audio_idx] + .clips + .iter() + .find(|c| c.id == audio_id) + .map(|c| c.range.start.0); + (video, audio) + }) + }; + + let move_video = |cx: &mut gpui::TestAppContext, track: usize, start: i64| { + cx.update(|app| { + engine.update(app, |engine, cx| { + engine.apply_timeline_event( + &TimelineEvent::ClipMoveRequested { + clip: video_id, + new_track: track, + new_start: Frame(start), + }, + cx, + ); + }) + }); + }; + + // Same-track drag: the video clip +40 frames; the audio clip follows + // on its own track. + move_video(cx, video_idx, 80); + assert_eq!(in_points(cx, video_idx, audio_idx), (Some(80), Some(80))); + + // Cross-track drag: the video clip moves to the other video track at + // 120; the audio clip stays on its audio track but shifts to 120 too. + move_video(cx, other_video_idx, 120); + assert_eq!( + in_points(cx, other_video_idx, audio_idx), + (Some(120), Some(120)), + "the audio clip follows the cross-track drag while keeping its track" + ); + assert!( + !cx.read(|app| engine.read(app).tracks[video_idx] + .clips + .iter() + .any(|c| c.id == video_id)), + "the video clip left its original track" + ); + + // ONE undo restores the whole group (the cross-track move was a + // single "Move Clip" entry). + cx.update(|app| engine.update(app, |engine, cx| engine.undo(cx))); + assert_eq!( + in_points(cx, video_idx, audio_idx), + (Some(80), Some(80)), + "one undo restores both clips to the same-track position" + ); + cx.update(|app| engine.update(app, |engine, cx| engine.undo(cx))); + assert_eq!( + in_points(cx, video_idx, audio_idx), + (Some(40), Some(40)), + "a second undo restores the pre-drag position" + ); + } + /// Playback pre-render window (M15 S2): during playback the playhead /// frame must come from the worker-rendered shm slot cache, NOT the /// synchronous render path (the main thread blocking in