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

fix(app): thumbnail PNG encoding, effect-card collapse echo, linked A/V drag

Browse Source 
- Thumbnails never appeared because the PNG was written to a .part
  file with format inferred from the extension (always failing); the
  writer now uses an explicit PNG encoder, and an e2e test proves the
  pipeline yields real files.
- Mock engine: the CardSelected -> SelectionChanged echo no longer
  re-expands a card the same click just collapsed.
- Dragging a clip moves its linked audio/video partners by the same
  frame offset in a single undo entry; the dragged clip may change
  tracks while partners keep theirs.
This commit is contained in:
Mike-Solar
2026-08-20 18:56:01 +08:00
parent cc0a15919f
commit 73bb686940
4 changed files with 437 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gpui
+1 -1
Submodule gpui updated: dea33c713f...8a0b7569a3
src/oakui/graphops.rs
+107 -26
View File
@@ -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<oakundo::undocommand::UndoCommand, String> {
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<Vec<oakundo::undocommand::UndoCommand>, 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<NodeId>,
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
src/oakui/mock.rs
+67 -4
View File
@@ -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| {
src/oakui/real.rs
+262 -12
View File
@@ -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<NodeId> = {
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<NodeId> = 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