From 7b4b300d7540f92f522857d9b49fae4af751a814 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 17:26:39 +0500 Subject: [PATCH] Crop: the spiral guide is complete at every aspect ratio The squares were carved off the crop rectangle itself, so the guide depended on the crop's shape. A square crop lost everything after the first arc -- the first subtraction leaves no rectangle to carve, and the loop stops -- and other ratios kept different amounts. Counting the quarter-circle arcs: crop before after 1:1 1 10 phi:1 10 10 3:2 3 10 16:9 4 10 4:5 3 10 Only a golden-ratio rectangle subdivides into squares forever, so that is the one shape that yields a complete spiral. The construction now uses a PHI x 1 rectangle whatever the crop is, and the normalisation already at the end of the function stretches it to fit. That distorts the curve on a crop of another ratio, which is the price of the guide staying complete, and is the approach the report proposes. The portrait transpose and the orientation flips are unchanged, so a portrait crop still gets a portrait spiral and all four orientations still mirror it. Fixes #2393. --- crates/ui-egui/src/panels/crop_overlay.rs | 35 +++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/crop_overlay.rs b/crates/ui-egui/src/panels/crop_overlay.rs index 37fe76a5e..763c4b9ab 100644 --- a/crates/ui-egui/src/panels/crop_overlay.rs +++ b/crates/ui-egui/src/panels/crop_overlay.rs @@ -45,12 +45,22 @@ pub fn paths(kind: CropOverlay, orient: u8, w: f32, h: f32) -> Vec Vec> { + // A `PHI × 1` rectangle less a `1 × 1` square is `1 × 1/PHI`, the same shape again. + const PHI: f32 = std::f32::consts::GOLDEN_RATIO; // work landscape; transpose back for portrait crops let portrait = h > w; - let (rw, rh) = if portrait { (h, w) } else { (w, h) }; + let (rw, rh) = (PHI, 1.0); let (mut x, mut y, mut cw, mut ch) = (0.0f32, 0.0f32, rw, rh); let mut curve = Vec::new(); let mut squares = Vec::new(); @@ -128,6 +138,27 @@ mod tests { assert_eq!(paths(CropOverlay::Triangle, 1, w, h)[0], vec![(1.0, 0.0), (0.0, 1.0)]); } + /// The spiral is the same complete guide at every crop shape. Carving the squares off the + /// crop itself made it depend on the crop: a square kept one arc of ten, 3:2 kept three and + /// 16:9 four, so changing the crop truncated the guide (#2393). + #[test] + fn the_spiral_is_complete_at_every_aspect_ratio() { + let shapes = [(1.0f32, 1.0f32), (1.618, 1.0), (1.0, 1.618), (3.0, 2.0), (2.0, 3.0), (16.0, 9.0), (9.0, 16.0), (4.0, 5.0)]; + let (squares, points) = { + let p = paths(CropOverlay::Spiral, 0, 1.618, 1.0); + (p.len() - 1, p.last().unwrap().len()) + }; + assert!(squares >= 8, "the golden rectangle itself gives a long spiral: {squares}"); + for (w, h) in shapes { + for o in 0..4 { + let p = paths(CropOverlay::Spiral, o, w, h); + assert_eq!(p.len() - 1, squares, "{w}x{h} orient {o}: {} squares, not {squares}", p.len() - 1); + assert_eq!(p.last().unwrap().len(), points, "{w}x{h} orient {o}: a shorter curve"); + assert!(in_unit(&p), "{w}x{h} orient {o}"); + } + } + } + #[test] fn spiral_is_one_continuous_curve() { for (w, h) in [(1.618, 1.0), (1.0, 1.618), (3.0, 2.0)] {