From c0f4f54e7fc869ccfcdc027fe7725c9d6145bd80 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Tue, 27 Oct 2026 21:00:50 -0700 Subject: [PATCH] LR-CROP-RECT: the crop's source size is counted in the turned photo (#2555) `source_edge_needed` bounds the crop in the photo as shown (turned by its orientation), but counted the rect's pixels with the unturned width and height. On a photo turned a quarter, a crop whose aspect differs from the photo's then asked for too small a source: a square crop of a 3:2 photo shown 2000 px long needs the full-size source, and got the 2560 px preview. It now counts them with the turned size. The two tests that pinned the old count use the turned size too, and one checks that square crop. Found by CodeRabbit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CvoftLURuf4uBmSatewB6V --- crates/engine/src/media/source.rs | 5 +++-- crates/engine/src/media/tests.rs | 12 ++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/media/source.rs b/crates/engine/src/media/source.rs index 308ca24c1..d49d146a7 100644 --- a/crates/engine/src/media/source.rs +++ b/crates/engine/src/media/source.rs @@ -60,12 +60,13 @@ impl SourceLevel { /// only part of the photo, so its pixels come from a source that many times larger (the source /// level is chosen from this, or a tight crop shown at its own pixels would be a stretched preview). /// The crop is bounded as the render bounds it (issue #2555), so one reaching far past the photo -/// doesn't pick a source too small for the frame it renders. +/// doesn't pick a source too small for the frame it renders. Its rect is in the photo as shown +/// (turned by its orientation), so it is measured with the turned size. pub(super) fn source_edge_needed(p: &Photo, settings: &DevelopSettings, apply_crop: bool, out_long: usize) -> usize { let (w, h) = (f64::from(p.width.max(1)), f64::from(p.height.max(1))); let (ow, oh) = if settings.orientation.swaps_axes() { (h, w) } else { (w, h) }; let r = settings.crop.geometry.bounded(ow, oh).rect; - let shown = if apply_crop { (r.width() * w).max(r.height() * h) } else { w.max(h) }; + let shown = if apply_crop { (r.width() * ow).max(r.height() * oh) } else { w.max(h) }; if !shown.is_finite() || shown < 1.0 { return out_long; } diff --git a/crates/engine/src/media/tests.rs b/crates/engine/src/media/tests.rs index 8eec0fd03..bf283fb04 100644 --- a/crates/engine/src/media/tests.rs +++ b/crates/engine/src/media/tests.rs @@ -389,7 +389,7 @@ fn a_crop_far_past_the_photo_reads_the_source_its_bounded_frame_needs() { let mut d = lightcraft_develop::DevelopSettings { orientation, ..Default::default() }; d.crop.geometry = wild; let r = wild.bounded(ow, oh).rect; - let expected = (1600.0 * w.max(h) / (r.width() * w).max(r.height() * h)) as usize; + let expected = (1600.0 * w.max(h) / (r.width() * ow).max(r.height() * oh)) as usize; assert_eq!(super::source::source_edge_needed(&p, &d, true, 1600), expected, "{orientation:?}"); } } @@ -408,12 +408,20 @@ fn crops_on_the_photo_keep_the_source_size_they_need() { for aspect in [None, Some(1.0), Some(3.0), Some(0.25)] { let mut d = lightcraft_develop::DevelopSettings { orientation, ..Default::default() }; d.crop.geometry = crop_fit_angle(ow, oh, angle, aspect); + // the rect is in the photo as shown, so its pixels are counted in the turned size let r = d.crop.geometry.rect; - let expected = (1600.0 * w.max(h) / (r.width() * w).max(r.height() * h)) as usize; + let expected = (1600.0 * w.max(h) / (r.width() * ow).max(r.height() * oh)) as usize; assert_eq!(super::source::source_edge_needed(&p, &d, true, 1600), expected, "{orientation:?} {angle} {aspect:?}"); } } } + // a square crop of the photo turned a quarter is the photo's short side across: shown 2000 px + // long it needs more than the 2560 px preview (counted in the unturned size, it asked for 2000) + let mut d = lightcraft_develop::DevelopSettings { orientation: Orientation::Rotate90, ..Default::default() }; + d.crop.geometry = crop_fit_angle(h, w, 0.0, Some(1.0)); + let needed = super::source::source_edge_needed(&p, &d, true, 2000); + assert_eq!(needed, (2000.0 * w.max(h) / w.min(h)) as usize); + assert_eq!(SourceLevel::for_size(needed), SourceLevel::Full, "{needed}"); } // a window whose spot reads from further away than a render can hold is refused