From 057a8d308e20cdb2a1cc7a5ba628fad9e5cd34f4 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 8 Oct 2026 18:38:39 -0400 Subject: [PATCH] #323: the Before window waits for the original the After is decoding At first open of Before/After the two window jobs were asked for together, each decoded its own 24 MP copy of the original (a review saw two decodes and three uploads for one photo) and the shared device copy was swapped between them. The Before's window now waits until the After's job has handed the original to the session (region::defer_before_window, Session media has_source); later windows of both share the one copy. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/media.rs | 23 +++++++++++++++++++++++ crates/ui-egui/src/panels/detail.rs | 5 +++++ crates/ui-egui/src/region.rs | 17 +++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 6a28a5fe3..67367e0ea 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -403,6 +403,15 @@ impl MediaCache { (self.thumbs.len(), self.thumbs.cost()) } + /// Whether the decoded source of `id` at `level` is held. + pub fn has_source(&self, id: PhotoId, level: SourceLevel) -> bool { + match level { + SourceLevel::Thumb => self.thumbs.contains(&id), + SourceLevel::Preview => self.previews.iter().any(|e| e.0 == id), + SourceLevel::Full => self.full.as_ref().is_some_and(|e| e.0 == id), + } + } + /// Decoded sources held: (thumbnail level, preview level, full size). pub fn usage(&self) -> (crate::memory::Usage, crate::memory::Usage, crate::memory::Usage) { use crate::memory::Usage; @@ -1389,6 +1398,20 @@ mod tests { assert!(mean(&a) > mean(&b) + 10.0, "two stops brighter after: {} vs {}", mean(&a), mean(&b)); } + #[test] + fn has_source_follows_what_the_session_accepted() { + let mut s = crate::Session::with_demo(); + let id = s.active().unwrap(); + let p = s.catalog.photo(id).unwrap().clone(); + let long = p.width.max(p.height) as usize; + assert!(!s.media.has_source(id, SourceLevel::Full)); + let r = s.render_job(id, long, long, false, true).unwrap().run(); + s.accept(&r); + assert!(s.media.has_source(id, SourceLevel::Full)); + assert!(!s.media.has_source(id, SourceLevel::Preview)); + assert!(!s.media.has_source(PhotoId(id.0 + 1), SourceLevel::Full)); + } + #[test] fn variant_jobs_have_their_own_keys_and_hit_the_cache() { let mut s = crate::Session::with_demo(); diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 32bf919b6..c9fb643d4 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -266,6 +266,11 @@ fn request_window(app: &mut LightcraftApp, c: &WindowCtx, v: &WindowView) -> Opt to_px * (v.visible.bottom() - v.target.top()), ); let win = crate::region::window_for(fw, fh, visible, crate::region::max_span(c.texture_side))?; + // the Before side waits for the original the After is decoding (see `defer_before_window`) + let original_held = app.session.media.has_source(c.id, lightcraft_engine::SourceLevel::for_size(fw.max(fh))); + if crate::region::defer_before_window(v.before, app.renderer.is_pending(Slot::Region), original_held) { + return app.region_before_view.filter(|w| w.photo == c.id); + } let job = if v.before { app.session.region_job_before(c.id, fw, fh, win, !c.crop_tool) } else { diff --git a/crates/ui-egui/src/region.rs b/crates/ui-egui/src/region.rs index 031db8ebe..db170da4b 100644 --- a/crates/ui-egui/src/region.rs +++ b/crates/ui-egui/src/region.rs @@ -170,6 +170,13 @@ pub fn window_clip(mode: crate::state::BeforeAfter, before_side: bool, pane: egu clip.intersect(canvas) } +/// Whether the Before window waits for the After's. Both are cut from the same decoded original; +/// asked for together while it isn't held yet, each worker decodes its own copy (two full-size +/// decodes, two uploads, one thrown away). The Before follows once the original is in. +pub fn defer_before_window(before_side: bool, after_pending: bool, original_held: bool) -> bool { + before_side && after_pending && !original_held +} + /// Whether the picture on screen has the frame's aspect (to the one pixel a texture's whole-pixel /// size is off by): a window is a part of the frame, so it can only be placed over a picture that /// is the frame (an unsupported raw's embedded JPEG may be cropped differently). @@ -403,6 +410,16 @@ mod tests { assert_eq!(window_clip(Original, true, canvas, canvas, img), canvas); } + // Given Before and After windows asked for at once at first open, only the After's goes: the + // Before's waits for the decoded original instead of decoding another copy + #[test] + fn the_before_window_waits_for_the_original_the_after_is_decoding() { + assert!(defer_before_window(true, true, false)); + assert!(!defer_before_window(true, true, true), "the original is in"); + assert!(!defer_before_window(true, false, false), "nothing is decoding it"); + assert!(!defer_before_window(false, true, false), "the After never waits"); + } + // Hostile numbers give a plan, never a panic #[test] fn hostile_sizes_give_a_usable_plan() {