shifathosn
/
filmcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ui-egui: never crash — panels without expect/unreachable 

- Timeline, monitors, Reference, Effect Controls and Export mode: a missing active sequence,
  track or clip (stale ids from a damaged project, or a clip deleted mid-drag) shows the empty
  state or skips the row instead of `expect`.
- Frame queue pops without `expect`; Graphics text editing, clip dialogs, Lumetri panel and
  Project rename without `expect`/`unreachable!`.
- The deliberate fault-injection panics (UI pass and frame jobs, used to test the guard) go
  through one documented `crash::injected_fault`.
- `#![deny(clippy::unwrap_used, …)]` outside tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas 2026-10-04 16:07:12 -04:00
parent 7b96f22684
commit d40a371724
11 changed files with 63 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
crates/ui-egui/src/crash.rs
View File

@ -34,6 +34,13 @@ pub fn install(log_dir: Option<std::path::PathBuf>) {
});
}
/// Deliberately panic: the fault-injection hooks (`ui.injectPanic` and the frame-job equivalent)
/// use this to prove the guard above keeps the session alive.
#[allow(clippy::panic)]
pub fn injected_fault(what: &str) -> ! {
panic!("{what}")
}
/// Write a non-panic fatal error (e.g. the window could not be created) to the crash log.
pub fn record(msg: &str) {
write_log(msg);

11
crates/ui-egui/src/frames.rs
View File

@ -636,7 +636,10 @@ impl FrameServer {
break;
}
let best = q.iter().enumerate().min_by_key(|(i, j)| (j.prio, *i)).map(|(i, _)| i).unwrap_or(0);
q.remove(best).expect("index valid")
match q.remove(best) {
Some(job) => job,
None => break,
}
};
if run_job(&self.shared, &job, &self.pool, &self.services, &self.previews) {
done += 1;
@ -744,7 +747,9 @@ fn worker(
if !q.is_empty() {
// highest priority (lowest number) first; FIFO among equals
let best = q.iter().enumerate().min_by_key(|(i, j)| (j.prio, *i)).map(|(i, _)| i).unwrap_or(0);
break q.remove(best).expect("index valid");
if let Some(job) = q.remove(best) {
break job;
}
}
q = sh.cv.wait(q).unwrap_or_else(|e| e.into_inner());
}
@ -777,7 +782,7 @@ static INJECT_PANICS: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU3
fn run_job(sh: &Shared, job: &Job, pool: &Arc<MediaPool>, services: &Arc<dyn Services>, previews: &PreviewStore) -> bool {
sh.in_flight.lock().unwrap_or_else(|e| e.into_inner()).push((job.key, job.cancel.clone(), job.prefetch));
if INJECT_PANICS.load(Ordering::Relaxed) > 0 && INJECT_PANICS.fetch_sub(1, Ordering::Relaxed) > 0 {
panic!("injected frame-job fault");
crate::crash::injected_fault("injected frame-job fault");
}
let profiling = sh.profiling.load(Ordering::Relaxed);
let (started, cpu0) = (Instant::now(), if profiling { thread_cpu_time().unwrap_or_default() } else { Duration::ZERO });

4
crates/ui-egui/src/lib.rs
View File

@ -4,6 +4,8 @@
//! owns only presentation state ([`state::UiState`]), GPU textures, the playback clock and the
//! control-channel handlers. Swap it for another toolkit without touching the engine.
#![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))]
pub mod automation;
pub mod brand;
pub mod control;
@ -1003,7 +1005,7 @@ impl FilmcraftApp {
fn frame(&mut self, ui: &mut egui::Ui) {
if std::mem::take(&mut self.panic_next_frame) {
panic!("injected UI fault");
crash::injected_fault("injected UI fault");
}
let ctx = ui.ctx().clone();
self.auto.begin_frame();

5
crates/ui-egui/src/panels/clip_dialogs.rs
View File

@ -156,10 +156,9 @@ fn defaults(app: &FilmcraftApp, id: &str) -> (Value, Value) {
}
}
"clip.modifyTimecode" => {
let it = s.state.project_selection.iter().find_map(|i| s.project.item(*i).filter(|it| it.as_media().is_some()));
let it = s.state.project_selection.iter().find_map(|i| s.project.item(*i).and_then(|it| Some((it, it.as_media()?))));
let (tc, tape) = it
.map(|it| {
let m = it.as_media().expect("media");
.map(|(it, m)| {
let f = m.info.start_timecode.unwrap_or(0);
(filmcraft_time::format_timecode_frames(f, m.frame_rate(), false), it.metadata.get("Tape Name").cloned().unwrap_or_default())
})

7
crates/ui-egui/src/panels/effect_controls.rs
View File

@ -34,7 +34,10 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
crate::dock::placeholder(ui, rect, &t, "(no clip selected)");
return;
};
let seq = app.session.active_sequence().expect("seq").clone();
let Some(seq) = app.session.active_sequence().cloned() else {
crate::dock::placeholder(ui, rect, &t, "(no sequences)");
return;
};
let split = rect.min.x + (rect.width() * 0.58).max(260.0).min(rect.width() - 60.0);
let head = Rect::from_min_size(rect.min + vec2(8.0, 4.0), vec2(split - rect.min.x - 12.0, 24.0));
// Premiere: two pill tabs — "Source · clip" and "Sequence · clip" (active)
@ -469,7 +472,7 @@ pub fn lumetri_panel(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
return;
};
let e = it.effects[idx].clone();
let def = e.def().expect("lumetri def");
let Some(def) = e.def() else { return };
let ph = app.session.playhead();
let mt = it.source_time_at(ph.clamp(it.start, it.end() - Tick(1)));
let mut actions = Vec::new();

2
crates/ui-egui/src/panels/export_mode.rs
View File

@ -828,7 +828,7 @@ const RANGES: [(&str, &str); 4] = [("entire", "Entire Source"), ("inOut", "Sourc
fn preview_column(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, seq_id: filmcraft_engine::project::ItemId) {
let t = app.tokens;
let mut reg = Reg::default();
let q = app.session.active_sequence().expect("seq").clone();
let Some(q) = app.session.active_sequence().cloned() else { return };
let rate = q.settings.frame_rate;
// preview (aspect of the output frame)
let r = app.ui.export.settings.resolve(q.settings.width, q.settings.height, rate, q.settings.sample_rate);

12
crates/ui-egui/src/panels/graphics.rs
View File

@ -256,8 +256,10 @@ pub fn monitor_overlay(app: &mut FilmcraftApp, ui: &mut egui::Ui, pic: Rect, fra
let in_edit = edit_view.is_some_and(|v| v.hit(p));
drag = None;
if in_edit {
if let (Some(v), Some(mut ed)) = (edit_view, editing.clone()) {
let l = text_layout(text_of(&v.spec).expect("text"));
if let (Some(v), Some(mut ed)) = (edit_view, editing.clone())
&& let Some(tp) = text_of(&v.spec)
{
let l = text_layout(tp);
if let Some((x, y)) = v.to_local(p) {
ed.caret = l.hit(x, y);
if !shift {
@ -338,8 +340,10 @@ pub fn monitor_overlay(app: &mut FilmcraftApp, ui: &mut egui::Ui, pic: Rect, fra
}
}
DragKind::TextSelect => {
if let (Some(v), Some(mut ed)) = (v, app.ui.gfx_edit.clone()) {
let l = text_layout(text_of(&v.spec).expect("text"));
if let (Some(v), Some(mut ed)) = (v, app.ui.gfx_edit.clone())
&& let Some(tp) = text_of(&v.spec)
{
let l = text_layout(tp);
if let Some((x, y)) = v.to_local(cur) {
ed.caret = l.hit(x, y);
app.ui.gfx_edit = Some(ed);

6
crates/ui-egui/src/panels/monitor.rs
View File

@ -36,7 +36,11 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, which: Which)
crate::dock::placeholder(ui, rect, &t, "(no sequences)");
return;
};
let q = app.session.active_sequence().expect("active");
// A damaged project can name an active sequence that no longer exists.
let Some(q) = app.session.active_sequence() else {
crate::dock::placeholder(ui, rect, &t, "(no sequences)");
return;
};
(
Target::Sequence(seq_id),
(q.settings.width, q.settings.height),

2
crates/ui-egui/src/panels/project.rs
View File

@ -928,7 +928,7 @@ fn start_rename(app: &mut FilmcraftApp, params: &Value) -> Result<Value, String>
let text = match (item, bin) {
(Some(i), _) => app.session.project.item(ItemId(i)).map(|x| x.name.clone()).ok_or("no such item")?,
(_, Some(b)) => app.session.project.root.find_bin(BinId(b)).map(|x| x.name.clone()).ok_or("no such bin")?,
_ => unreachable!(),
_ => return Err("rename needs an item or a bin".to_string()),
};
let panel = view_of(app, shown_inst(app)).prefix;
app.ui.project_panel.rename = Some(Rename { item, bin, text, panel });

2
crates/ui-egui/src/panels/reference.rs
View File

@ -35,7 +35,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
return;
};
let Some(frame) = frame(app) else { return };
let q = app.session.active_sequence().expect("active").clone();
let Some(q) = app.session.active_sequence().cloned() else { return };
let rate = q.settings.frame_rate;
let bar_h = 34.0;
let area = Rect::from_min_max(rect.min + vec2(4.0, 4.0), pos2(rect.max.x - 4.0, rect.max.y - bar_h));

44
crates/ui-egui/src/panels/timeline.rs
View File

@ -159,7 +159,11 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
empty_state(app, ui, rect);
return;
};
let seq = app.session.active_sequence().expect("active").clone();
// A damaged project can name an active sequence that no longer exists.
let Some(seq) = app.session.active_sequence().cloned() else {
empty_state(app, ui, rect);
return;
};
let rate = seq.settings.frame_rate;
let dt = ctx.input(|i| i.stable_dt).min(0.05) as f64;
let header_w = app.ui.timeline.header_w;
@ -289,7 +293,7 @@ pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
let mut previews: HashMap<ClipId, (Tick, Tick, Option<TrackId>)> = HashMap::new(); // live drag preview: (start, dur, track)
preview_drag(app, &seq, &layout, &mut previews);
for r in &rows {
let tr = seq.track(r.track).expect("track");
let Some(tr) = seq.track(r.track) else { continue };
let clip_rect = if r.kind == TrackKind::Video { vclip } else { aclip };
let row = r.rect.intersect(clip_rect);
if row.height() <= 0.0 {
@ -779,7 +783,7 @@ fn draw_headers(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, rows:
let mut actions: Vec<(String, Value)> = Vec::new();
let mut vo_action = None;
for r in rows {
let tr = seq.track(r.track).expect("track");
let Some(tr) = seq.track(r.track) else { continue };
let clip_rect = if r.kind == TrackKind::Video { vclip } else { aclip };
let hrect = Rect::from_min_max(pos2(rect.min.x, r.rect.min.y), pos2(rect.min.x + hw, r.rect.max.y));
let visible = hrect.intersect(clip_rect);
@ -1268,7 +1272,7 @@ pub fn hit(seq: &Sequence, layout: &Layout, pos: Pos2) -> Hit {
return Hit::None;
}
let Some(row) = layout.row_at(pos.y) else { return Hit::None };
let tr = seq.track(row.track).expect("track");
let Some(tr) = seq.track(row.track) else { return Hit::None };
for trn in &tr.transitions {
let x0 = layout.x_of(trn.start);
let x1 = layout.x_of(trn.end());
@ -1504,7 +1508,7 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: &
None
}
(Tool::Selection | Tool::Ripple | Tool::Rolling | Tool::RateStretch, Hit::Clip { clip, edge: Some(edge), track }) => {
let tr = seq.track(track).expect("track");
let Some(tr) = seq.track(track) else { return };
let sel_kind = if tool == Tool::Selection {
let (dist, neighbour) = edge_geometry(seq, layout, track, clip, edge, p.x);
selection_trim_kind(app.session.prefs.trim.selection_tool_roll_ripple, mods.command, mods.shift, dist, neighbour)
@ -1514,7 +1518,7 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: &
let mode = if tool == Tool::Ripple || sel_kind == "ripple" { filmcraft_edit::TrimMode::Ripple } else { filmcraft_edit::TrimMode::Regular };
if tool == Tool::Rolling || sel_kind == "roll" {
// roll the cut between this and its neighbour
let it = tr.item(clip).expect("clip");
let Some(it) = tr.item(clip) else { return };
let (l, r) = match edge {
filmcraft_edit::Edge::Out => (Some(clip), tr.items.iter().find(|x| x.start == it.end()).map(|x| x.id)),
filmcraft_edit::Edge::In => (tr.items.iter().find(|x| x.end() == it.start).map(|x| x.id), Some(clip)),
@ -1615,29 +1619,25 @@ fn interact(app: &mut FilmcraftApp, ui: &mut egui::Ui, seq: &Sequence, layout: &
};
Some(Drag::Move { clips, grab_tick, start_track, offset, track_delta })
}
Drag::Trim { clip, edge, mode, .. } => {
let (_, it) = seq.find_item(clip).expect("clip");
Drag::Trim { clip, edge, mode, .. } => seq.find_item(clip).map(|(_, it)| {
let base = if edge == filmcraft_edit::Edge::In { it.start } else { it.end() };
let target = snap(app, seq, layout, rate.snap_nearest(t_here), &[clip]);
let delta = target - base;
Some(Drag::Trim { clip, edge, mode, delta })
}
Drag::Remix { clip, .. } => {
let (_, it) = seq.find_item(clip).expect("clip");
Drag::Trim { clip, edge, mode, delta }
}),
Drag::Remix { clip, .. } => seq.find_item(clip).map(|(_, it)| {
let target = rate.snap_nearest(t_here).max(it.start + rate.frame_duration());
Some(Drag::Remix { clip, delta: target - it.end() })
}
Drag::Stretch { clip, edge, .. } => {
let (_, it) = seq.find_item(clip).expect("clip");
Drag::Remix { clip, delta: target - it.end() }
}),
Drag::Stretch { clip, edge, .. } => seq.find_item(clip).map(|(_, it)| {
let base = if edge == filmcraft_edit::Edge::In { it.start } else { it.end() };
let target = snap(app, seq, layout, rate.snap_nearest(t_here), &[clip]);
Some(Drag::Stretch { clip, edge, delta: target - base })
}
Drag::Roll { left, right, .. } => {
let (_, l) = seq.find_item(left).expect("clip");
Drag::Stretch { clip, edge, delta: target - base }
}),
Drag::Roll { left, right, .. } => seq.find_item(left).map(|(_, l)| {
let target = snap(app, seq, layout, rate.snap_nearest(t_here), &[left, right]);
Some(Drag::Roll { left, right, delta: target - l.end() })
}
Drag::Roll { left, right, delta: target - l.end() }
}),
Drag::Slip { clip, .. } => {
let start = resp.interact_pointer_pos().map(|_| ()).and(ctx.input(|i| i.pointer.press_origin()));
let origin = start.map(|o| layout.tick_at(o.x)).unwrap_or(t_here);