From 4c088c32846b989ce4b631d504dc135459533e89 Mon Sep 17 00:00:00 2001 From: Michael Anderson Date: Tue, 13 Oct 2026 07:15:32 -0500 Subject: [PATCH] Never crash: Give the loupe a finite aspect for a zero-size crop Count each side as at least one pixel. --- crates/pipeline/src/geometry.rs | 25 +++++++++++++++++++++++-- crates/ui-egui/src/tests_zoom.rs | 27 +++++++++++++++++++++++++++ docs/whats-new.md | 4 ++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/crates/pipeline/src/geometry.rs b/crates/pipeline/src/geometry.rs index bea792c19..5e35a0552 100644 --- a/crates/pipeline/src/geometry.rs +++ b/crates/pipeline/src/geometry.rs @@ -167,10 +167,12 @@ impl Frame { } } - /// Aspect ratio (w/h) of the output. + /// Aspect ratio (w/h) of the output. A crop narrower than a source pixel (or of no size, from + /// an agent or a sidecar) counts each side as one pixel, so the aspect stays finite: 0/0 made + /// it NaN, and the loupe laid the photo out at a NaN size. pub fn aspect(&self) -> f64 { let r = self.crop.rect_px(self.ow, self.oh); - r.width() / r.height() + r.width().max(1.0) / r.height().max(1.0) } /// Size of the (cropped) output at the source's own resolution, in pixels. @@ -383,6 +385,25 @@ mod tests { assert!((back.x - 50.0).abs() < 1e-6 && (back.y - 50.0).abs() < 1e-6); } + /// A crop of no size (or under a pixel wide) gave a NaN or infinite aspect; each side now + /// counts as at least one source pixel. + #[test] + fn degenerate_crops_have_a_finite_aspect() { + let mut s = DevelopSettings::default(); + for (rect, want) in [ + (Rect::new(0.5, 0.5, 0.5, 0.5), 1.0), + (Rect::new(0.0, 0.0, 1e-9, 1.0), 1.0 / 200.0), + (Rect::new(0.0, 0.5, 1.0, 0.5), 400.0), + (Rect::new(0.25, 0.0, 0.75, 1.0), 1.0), + ] { + s.crop.geometry.rect = rect; + let f = Frame::new(400, 200, &s, true); + assert!((f.aspect() - want).abs() < 1e-9, "{rect:?}: {}", f.aspect()); + let (w, h) = f.fit(1000, 1000); + assert!((1..=1000).contains(&w) && (1..=1000).contains(&h), "{rect:?}: {w}×{h}"); + } + } + #[test] fn flip_mirrors() { let mut s = DevelopSettings::default(); diff --git a/crates/ui-egui/src/tests_zoom.rs b/crates/ui-egui/src/tests_zoom.rs index ddea400d0..b0595d27d 100644 --- a/crates/ui-egui/src/tests_zoom.rs +++ b/crates/ui-egui/src/tests_zoom.rs @@ -108,3 +108,30 @@ fn zoomed_before_after_views_stay_in_their_panes() { } } } + +/// A crop of no size (or a hair wide), from an agent or a sidecar, gave the loupe a NaN aspect: +/// the photo was laid out at a NaN size, which egui's debug checks stop on (a panic in a debug +/// build) and nothing could be drawn or clicked. It now lays out at a finite size, fitted and at +/// 100 %. +#[test] +fn a_crop_of_no_size_lays_out_at_a_finite_size() { + let mut h = detail(); + for rect in [[0.5, 0.5, 0.5, 0.5], [0.0, 0.0, 1e-9, 1.0], [0.0, 0.3, 1.0, 0.3]] { + let r = h.request("engine.execute", json!({"command": "crop.set", "params": {"rect": rect}}), T); + assert_eq!(r["ok"], true, "{r}"); + for zoom in ["view.zoomFit", "view.zoom100"] { + let r = h.request("engine.execute", json!({"command": zoom}), T); + assert_eq!(r["ok"], true, "{zoom}: {r}"); + h.settle(SETTLE); + let img = h.app.image_rect.expect("the loupe laid the photo out"); + assert!(img.min.is_finite() && img.max.is_finite(), "{rect:?} at {zoom}: {img:?}"); + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": 0.5, "y": 0.5}, {"kind": "drag", "x": 0.6, "y": 0.4}, {"kind": "up", "x": 0.6, "y": 0.4}]}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + } + } +} diff --git a/docs/whats-new.md b/docs/whats-new.md index b5c2ac68c..e3661749c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,10 @@ ## 2026-10-13 +### Reliability +- A crop of no size (from an agent or a sidecar) no longer gives the Detail view a photo of no size to lay out + (its width over its height was 0 / 0, which stopped debug builds); it shows as a single pixel's colour. + ### Slider drags on large raws - Dragging a slider no longer re-renders the edited photo's filmstrip thumbnail on every frame; it follows once you let go. On a 64 MP raw the first of those renders decoded the whole file again, which held up the loupe's