diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 6f6f0359..6c917b23 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -803,7 +803,31 @@ impl RenderJob { self } + /// Render. A panic inside (a decoder or pipeline bug) is this job's failure: the view waiting + /// for it is told, instead of waiting forever on a worker that died (the pool doesn't replace + /// it, so a few such panics used to stop every render). pub fn run(self) -> RenderResult { + let failed = self.failed(String::new()); + crate::guard::catch("the render", || self.run_unguarded()).unwrap_or_else(|e| RenderResult { rendered: Err(e), ..failed }) + } + + /// This job's result when it fails with `e`. + fn failed(&self, e: String) -> RenderResult { + RenderResult { + request_id: self.request_id, + source_key: self.source_key, + photo: self.photo, + level: self.level, + key: self.key, + rendered: Err(e), + gpu_fallback: None, + loaded: None, + quick: None, + display: None, + } + } + + fn run_unguarded(self) -> RenderResult { if let Some((cache, key)) = &self.cache && let Some(img) = cache.get_at(self.cache_generation, *key) { @@ -877,18 +901,7 @@ impl RenderJob { display, } } - Err(e) => RenderResult { - request_id: self.request_id, - source_key: self.source_key, - photo: self.photo, - level: self.level, - key: self.key, - rendered: Err(e), - gpu_fallback: None, - loaded: None, - quick: None, - display: None, - }, + Err(e) => self.failed(e), } } } @@ -976,7 +989,25 @@ pub struct QuickJob { } impl QuickJob { + /// The stand-in, from the first source that has one. A panic inside is this job's failure + /// (see [`RenderJob::run`]). pub fn run(self) -> RenderResult { + let (request_id, photo, key) = (self.request_id, self.photo, self.key); + crate::guard::catch("the render", || self.run_unguarded()).unwrap_or_else(|e| RenderResult { + request_id, + source_key: None, + photo, + level: SourceLevel::Thumb, + key, + rendered: Err(e), + gpu_fallback: None, + loaded: None, + quick: None, + display: None, + }) + } + + fn run_unguarded(self) -> RenderResult { let (photo, key) = (self.photo, self.key); let request_id = self.request_id; let done = |rendered: Result, quick| RenderResult { @@ -1588,6 +1619,31 @@ mod thumbnail_hash_tests { s.accept(&old); assert_eq!(s.media.source_usage().0, 0); } + /// A panic while rendering (here in the decoder) used to end the pool's worker thread without + /// an answer, so the view that asked showed "Rendering…" forever. It is the job's failure now, + /// for renders and for stand-ins, answering the request it came from. + #[test] + fn a_render_that_panics_fails_instead_of_never_answering() { + let mut s = session(); + s.media.file_loader = Some(Arc::new(|_, _| panic!("decoder bug"))); + let mut job = s.thumb_job(PhotoId(1), 256).unwrap(); + job.request_id = 42; + let (key, level, source_key) = (job.key, job.level, job.source_key); + let r = job.run(); + assert_eq!((r.request_id, r.photo, r.key, r.level, r.source_key), (42, PhotoId(1), key, level, source_key)); + assert_eq!(r.rendered.err().as_deref(), Some("the render failed unexpectedly: decoder bug")); + let quick = QuickJob { + request_id: 7, + photo: PhotoId(1), + key: 9, + cached: Vec::new(), + embedded: Some(("x.arw".into(), Arc::new(|_: &str, _: usize| -> Option { panic!("preview reader bug") }), 256)), + small: None, + }; + let r = quick.run(); + assert_eq!((r.request_id, r.key), (7, 9)); + assert_eq!(r.rendered.err().as_deref(), Some("the render failed unexpectedly: preview reader bug")); + } } #[cfg(test)] diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 72528fca..d94bc619 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -98,8 +98,8 @@ fn watermark_preview( let (out, repaint) = (base.clone(), ctx.clone()); let work = move || { for job in jobs { - let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| job.run().rendered.map(|r| r.image))) - .unwrap_or_else(|_| Err("the preview failed unexpectedly".into())); + // (a panic in the render is its failure: `RenderJob::run`) + let r = job.run().rendered.map(|r| r.image); let mut got = out.lock().unwrap_or_else(std::sync::PoisonError::into_inner); // a failed sharp render keeps the thumbnail if r.is_ok() || !matches!(got.1, Some(Ok(_))) { diff --git a/crates/ui-egui/src/render.rs b/crates/ui-egui/src/render.rs index 66bce01f..03ac1894 100644 --- a/crates/ui-egui/src/render.rs +++ b/crates/ui-egui/src/render.rs @@ -1057,6 +1057,39 @@ mod thumbnail_tests { let _ = std::fs::remove_dir_all(&lib); } + /// A render that panics (a decoder bug) is a failure like any other: the slot stops waiting, + /// the error is kept (not retried every frame), and the pool's workers live on. Before, the + /// worker died without an answer and the slot stayed pending forever. + #[test] + fn a_panicking_render_is_a_failure_not_a_dead_worker() { + let (mut app, _, ctx) = fixture_with(Session::new(), (*photo()).clone()); + app.session.media.file_loader = Some(Arc::new(|_, _| panic!("decoder bug"))); + app.renderer.offload = None; // the real job pool + let slot = Slot::Thumb(PhotoId(1)); + let ask = |app: &mut crate::LightcraftApp| { + if let Some(job) = app.session.thumb_job(PhotoId(1), 256) { + app.renderer.request(slot, job, 30); + } + app.renderer.poll(&ctx, &mut app.session); + }; + ask(&mut app); + let t0 = std::time::Instant::now(); + while app.renderer.is_pending(slot) { + assert!(t0.elapsed() < std::time::Duration::from_secs(20), "the render never answered"); + std::thread::sleep(std::time::Duration::from_millis(5)); + app.renderer.poll(&ctx, &mut app.session); + } + assert_eq!(app.renderer.failure(slot), Some("the render failed unexpectedly: decoder bug")); + let workers = app.renderer.live_workers(); + assert!(workers > 0, "the worker that panicked is still there"); + let done = app.renderer.completed; + for _ in 0..30 { + ask(&mut app); + } + assert!(!app.renderer.is_pending(slot) && app.renderer.completed == done, "no retry loop"); + assert_eq!(app.renderer.live_workers(), workers); + } + #[test] fn raw_quick_path_frames_build_no_jobs() { // an unedited raw: the grid shows its embedded preview (held here until released) and stops diff --git a/docs/whats-new.md b/docs/whats-new.md index fb8b285a..60c470d7 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -45,6 +45,10 @@ - Upgrading from a Windows installer older than 0.6 (such as 0.5.0) keeps LightCraft in the folder it was installed in, instead of moving it back to Program Files (issue #399). If that folder has been deleted since, the upgrade installs to Program Files. +- A photo that runs into a bug in LightCraft while it renders (in a raw decoder or the editing pipeline) now fails + like a photo that can't be opened, with the reason, instead of rendering forever; its thumbnail stops waiting too. + Each such bug used to take one of the render threads with it, so after a few of them no photo rendered until + LightCraft was restarted. The bug is still written to the log (Help ▸ Open Log Folder) for a report. ## 2026-10-10