#323: review fixes: no window renders during drags, current-look tiles only, margin by zoom

An adversarial review found that a slider drag at any zoom started a window
render (the 0.6-scale draft looked stretched), that a tile made for an earlier
look kept being drawn and that the tile table was cleared mid-drag, that the
fixed 256 px margin was far below the wide stages' kernels at depth, that spot
removal reads outside the window, and that the window texture stayed resident
after zooming out. Window renders now wait for the drag to end, a tile is drawn
only while photo, zoom and look match, the margin grows with the zoom (768 px
cap) and the window is capped at 6144 px, photos with spots keep the whole-frame
render, the texture and stages are freed when unneeded, and wasm never asks.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/ui-egui/src/panels/detail.rs

@@ -337,7 +337,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
         app.renderer.request(Slot::Before, job, 90);
     }
     // zoomed past what the whole-frame render holds: also render the part on screen at the zoom
-    // scale (not while comparing, hovering a look or showing an overlay: those draw one image)
+    // scale (not while comparing, hovering a look or showing an overlay: those draw one image; and
+    // not during a drag, whose draft is what shows until it ends)
     let plain_view = !split
         && !show_before
         && !split_view
@@ -345,10 +346,15 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
         && !app.ui.soft_proof
         && view_overlay(app, &d) == lightcraft_pipeline::Overlay::None;
     let drawn_long = target_rect.width().max(target_rect.height()) * ppp;
-    let region_wanted = plain_view && app.renderer.can_render_windows() && crate::region::needed(drawn_long, rw.max(rh));
-    if region_wanted {
+    let settled_edge = app.ui.settings.loupe_edge(drawn_long, native_long);
+    let region_possible =
+        plain_view && app.renderer.can_render_windows() && crate::region::windows_allowed(&d) && crate::region::needed(drawn_long, settled_edge);
+    let look = d.hash64();
+    let mut region_full = None;
+    if region_possible {
         let (fw, fh) = if aspect >= 1.0 { (drawn_long, drawn_long / aspect) } else { (drawn_long * aspect, drawn_long) };
         let (fw, fh) = (fw.round().max(1.0) as usize, fh.round().max(1.0) as usize);
+        region_full = Some((fw, fh));
         // what is on screen, in pixels of the zoomed frame
         let to_px = |v: f32| v * ppp;
         let visible = (
@@ -357,20 +363,21 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
             to_px(canvas.right() - target_rect.left()),
             to_px(canvas.bottom() - target_rect.top()),
         );
-        if let Some(win) = crate::region::window_for(fw, fh, visible)
+        if !interacting
+            && let Some(win) = crate::region::window_for(fw, fh, visible)
             && let Some(job) = app.session.region_job(id, fw, fh, win, !crop_tool)
         {
-            let job = if interacting { job.draft() } else { job };
-            let view = crate::region::RegionView { photo: id, key: job.key, full: (fw, fh), window: win };
-            if app.region_tiles.len() > 8 {
-                app.region_tiles.clear();
-            }
+            let view = crate::region::RegionView { photo: id, key: job.key, full: (fw, fh), window: win, settings: look };
+            // windows of other photos, looks and zooms can't be drawn any more
+            app.region_tiles.retain(|_, v| v.is_current(id, (fw, fh), look));
             app.region_tiles.insert(job.key, view);
             app.region_view = Some(view);
             app.renderer.request(Slot::Region, job, 99);
         }
     } else {
         app.region_view = None;
+        app.region_tiles.clear();
+        app.renderer.release(Slot::Region);
     }
     let p = ui.painter_at(canvas);
     // what a view slot shows: its own render of this photo, else the stand-ins (no blank frame
@@ -437,9 +444,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
         }
     }
     // the window render, drawn over the whole-frame one where it belongs in the frame
-    if region_wanted
+    if let Some(full) = region_full
         && let Some(tex) = app.renderer.textures.get(&Slot::Region).filter(|t| t.photo == id)
-        && let Some(v) = app.region_tiles.get(&tex.key).filter(|v| v.photo == id && Some(v.full) == app.region_view.map(|r| r.full))
+        && let Some(v) = app.region_tiles.get(&tex.key).filter(|v| v.is_current(id, full, look))
     {
         let (fw, fh) = (v.full.0 as f32, v.full.1 as f32);
         let at = img_rect.min + vec2(v.window.x as f32 / fw * img_rect.width(), v.window.y as f32 / fh * img_rect.height());

crates/ui-egui/src/region.rs

@@ -10,10 +10,10 @@ use lightcraft_engine::pipeline::PixelWindow;
 
 /// Windows start and end on multiples of this many pixels of the zoomed frame.
 pub const SNAP: usize = 256;
-/// Context rendered beyond the visible pixels (a multiple of [`SNAP`] after snapping).
-pub const MARGIN: usize = 256;
+/// The most context rendered beyond the visible pixels (a multiple of [`SNAP`]).
+pub const MAX_MARGIN: usize = 768;
 /// The most a window may span on either axis: a bound on its texture whatever the window size.
-pub const MAX_SPAN: usize = 8192;
+pub const MAX_SPAN: usize = 6144;
 
 /// A window render the loupe asked for: which photo, the size of the zoomed frame it is a window
 /// of, and the window. The texture that comes back is drawn at this place of the frame.
@@ -23,6 +23,30 @@ pub struct RegionView {
     pub key: u64,
     pub full: (usize, usize),
     pub window: PixelWindow,
+    /// Hash of the develop settings it was rendered with.
+    pub settings: u64,
+}
+
+impl RegionView {
+    /// Whether this window's pixels still belong in a loupe showing `photo` with `settings` in a
+    /// frame of `full` pixels (they are drawn until the next window arrives, if so).
+    pub fn is_current(&self, photo: lightcraft_catalog::PhotoId, full: (usize, usize), settings: u64) -> bool {
+        self.photo == photo && self.full == full && self.settings == settings
+    }
+}
+
+/// Whether windows can be rendered for these settings. Spot removal reads its source patch from
+/// outside the spot, and an automatic one is chosen from the pixels at hand: neither is the same
+/// in a window as in the whole frame, so those photos stay on the whole-frame render.
+pub fn windows_allowed(s: &lightcraft_develop::DevelopSettings) -> bool {
+    s.spots.is_empty()
+}
+
+/// Context to render around the visible pixels of a frame `full_long` pixels along: enough for the
+/// wide stages (base, clarity and dehaze planes have sigmas up to 0.02 of it) up to a cap.
+pub fn margin_for(full_long: usize) -> usize {
+    let want = (full_long as f64 * 0.045).min(MAX_MARGIN as f64) as usize;
+    want.div_ceil(SNAP).max(1) * SNAP
 }
 
 /// Whether a view drawn `drawn_long` pixels along its long edge needs a window render on top of
@@ -32,19 +56,20 @@ pub fn needed(drawn_long: f32, rendered_long: usize) -> bool {
 }
 
 /// The window of a `full_w × full_h` frame to render so that `visible` — `(x0, y0, x1, y1)` in the
-/// frame's pixels, as much of the frame as is on screen — is covered: expanded by [`MARGIN`],
-/// snapped outwards to the [`SNAP`] grid and clamped into the frame. `None`: nothing is visible.
+/// frame's pixels, as much of the frame as is on screen — is covered: expanded by a margin,
+/// snapped outwards to the [`SNAP`] grid (margin: see [`margin_for`]) and clamped into the frame. `None`: nothing is visible.
 pub fn window_for(full_w: usize, full_h: usize, visible: (f32, f32, f32, f32)) -> Option<PixelWindow> {
     if full_w == 0 || full_h == 0 || [visible.0, visible.1, visible.2, visible.3].iter().any(|v| !v.is_finite()) {
         return None;
     }
+    let margin = margin_for(full_w.max(full_h));
     let axis = |lo: f32, hi: f32, full: usize| -> Option<(usize, usize)> {
         let (lo, hi) = (lo.max(0.0), hi.min(full as f32));
         if hi <= lo {
             return None;
         }
-        let a = (lo as usize).saturating_sub(MARGIN) / SNAP * SNAP;
-        let b = ((hi.ceil() as usize).saturating_add(MARGIN)).div_ceil(SNAP).saturating_mul(SNAP).min(full);
+        let a = (lo as usize).saturating_sub(margin) / SNAP * SNAP;
+        let b = ((hi.ceil() as usize).saturating_add(margin)).div_ceil(SNAP).saturating_mul(SNAP).min(full);
         // a window larger than MAX_SPAN keeps the visible part (centred on it), snapped
         if b - a > MAX_SPAN {
             let mid = ((lo + hi) / 2.0) as usize;
@@ -76,8 +101,9 @@ mod tests {
     #[test]
     fn the_window_covers_the_visible_area_with_a_margin_on_the_grid() {
         let w = window_for(48_000, 32_000, (20_000.0, 10_000.0, 21_400.0, 10_900.0)).unwrap();
-        assert!(w.x <= 20_000 - MARGIN && w.x + w.w >= 21_400 + MARGIN, "{w:?}");
-        assert!(w.y <= 10_000 - MARGIN && w.y + w.h >= 10_900 + MARGIN, "{w:?}");
+        let m = margin_for(48_000);
+        assert!(w.x + m <= 20_000 && w.x + w.w >= 21_400 + m, "{w:?}");
+        assert!(w.y + m <= 10_000 && w.y + w.h >= 10_900 + m, "{w:?}");
         assert_eq!((w.x % SNAP, w.y % SNAP, w.w % SNAP, w.h % SNAP), (0, 0, 0, 0));
     }
 
@@ -101,6 +127,41 @@ mod tests {
         assert_eq!((w.x + w.w, w.y + w.h), (3000, 2000));
     }
 
+    // Given a deeper zoom, the margin grows with the wide stages' kernels (3σ of the base, clarity
+    // and dehaze planes are 0.045 of the frame), up to a cap that bounds the window
+    #[test]
+    fn the_margin_grows_with_the_zoom_up_to_a_cap() {
+        assert_eq!(margin_for(1000), SNAP, "never less than one grid step");
+        assert!(margin_for(8000) > margin_for(3000));
+        assert_eq!(margin_for(48_000), MAX_MARGIN);
+        assert_eq!(margin_for(0), SNAP);
+        assert!(margin_for(usize::MAX) <= MAX_MARGIN);
+        for l in [100, 3000, 8000, 48_000, 400_000] {
+            assert_eq!(margin_for(l) % SNAP, 0);
+        }
+    }
+
+    // Given a tile rendered for other settings, another photo or another zoom, it is not drawn
+    #[test]
+    fn a_tile_is_drawn_only_while_it_shows_what_the_loupe_shows() {
+        use lightcraft_catalog::PhotoId;
+        let v = RegionView { photo: PhotoId(1), key: 7, full: (6000, 4000), window: PixelWindow { x: 0, y: 0, w: 256, h: 256 }, settings: 11 };
+        assert!(v.is_current(PhotoId(1), (6000, 4000), 11));
+        assert!(!v.is_current(PhotoId(2), (6000, 4000), 11), "another photo");
+        assert!(!v.is_current(PhotoId(1), (6000, 4001), 11), "another zoom");
+        assert!(!v.is_current(PhotoId(1), (6000, 4000), 12), "another look (an edit, an undo)");
+    }
+
+    // Given a photo with spot removal, no window render: a spot reads pixels beyond the window
+    #[test]
+    fn photos_with_spots_stay_on_the_whole_frame_render() {
+        use lightcraft_develop::{DevelopSettings, Spot};
+        let mut s = DevelopSettings::default();
+        assert!(windows_allowed(&s));
+        s.spots.push(Spot::default());
+        assert!(!windows_allowed(&s));
+    }
+
     // Given nothing visible, or hostile numbers, there is no window (never a panic)
     #[test]
     fn nothing_visible_or_hostile_input_is_no_window() {

crates/ui-egui/src/render.rs

@@ -233,7 +233,13 @@ impl Renderer {
     /// Whether windows of the frame can be rendered here: not in the browser build, whose workers
     /// rebuild the request from a wire format that has no window yet.
     pub fn can_render_windows(&self) -> bool {
-        self.offload.is_none()
+        self.offload.is_none() && !cfg!(target_arch = "wasm32")
+    }
+
+    /// Free a view slot's texture and stage cache (the view is no longer shown).
+    pub fn release(&mut self, slot: Slot) {
+        self.textures.remove(&slot);
+        self.stages.remove(&slot);
     }
 
     /// Run jobs through `offload` from now on (instead of the job pool / inline).

crates/ui-egui/src/tests_preview_limit.rs

@@ -233,4 +233,47 @@ mod in_the_loupe {
         }
         assert!(sum / n < 6.0, "mean difference {:.2} / 255 between the window and the frame render", sum / n);
     }
+
+    // Given a Fit view, a slider drag (whose Main draft is at 0.6 scale) does not start window renders
+    #[test]
+    fn a_slider_drag_at_fit_makes_no_window_render() {
+        let (mut h, _) = detail();
+        h.app.session.begin_interaction("Exposure").unwrap();
+        for _ in 0..5 {
+            h.step();
+        }
+        assert_eq!(h.app.region_view, None);
+        assert_eq!(region_tile(&h), None);
+    }
+
+    // Given a zoomed view with a window render, when I zoom back to Fit, then the window's texture is freed
+    #[test]
+    fn the_window_texture_is_freed_when_it_is_not_needed() {
+        let (mut h, _) = detail();
+        h.app.ui.settings.preview_limit = 1600;
+        h.request("engine.execute", json!({"command": "view.zoom100"}), T);
+        h.settle(SETTLE);
+        assert!(region_tile(&h).is_some());
+        h.request("engine.execute", json!({"command": "view.zoomFit"}), T);
+        h.settle(SETTLE);
+        assert_eq!(region_tile(&h), None);
+        assert_eq!(h.app.region_view, None);
+    }
+
+    // Given a zoomed view, when the photo is edited, then the old window (made with the old look)
+    // is no longer current and is replaced by one for the new look
+    #[test]
+    fn an_edit_replaces_the_window() {
+        let (mut h, _) = detail();
+        h.app.ui.settings.preview_limit = 1600;
+        h.request("engine.execute", json!({"command": "view.zoom100"}), T);
+        h.settle(SETTLE);
+        let first = h.app.region_view.expect("a window");
+        h.request("engine.execute", json!({"command": "develop.set", "params": {"control": "light.exposure", "value": 1.0}}), T);
+        h.settle(SETTLE);
+        let second = h.app.region_view.expect("a window");
+        assert_ne!(first.settings, second.settings);
+        assert_ne!(first.key, second.key);
+        assert_eq!(h.app.renderer.textures.get(&Slot::Region).map(|t| t.key), Some(second.key), "the new window arrived");
+    }
 }