From 1162c2a01832cd424704f17aa9405fd48ffed445 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 04:09:25 +0200 Subject: [PATCH] =?UTF-8?q?crop.set=20refuses=20a=20crop=20smaller=20than?= =?UTF-8?q?=20the=20crop=20tool=20allows,=20instead=20of=20exporting=201?= =?UTF-8?q?=C3=971=20(part=20of=20#1952)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/engine/src/cmd/develop.rs | 13 +++++++++++-- crates/engine/src/tests.rs | 32 +++++++++++++++++++++++++++++--- crates/ui-egui/src/tests_zoom.rs | 6 +++--- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index e3d59c567..33e3c4a92 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -2,7 +2,7 @@ use lightcraft_catalog::{Op, PhotoId, Version}; use lightcraft_develop::{DevelopSettings, Preset, ProcessVersion, Section, SettingsGroup, Treatment, Upright, WbMode, controls}; -use lightcraft_geom::{CropGeometry, CropHandle, Point, Rect, crop_fit_angle, drag_crop}; +use lightcraft_geom::{CropGeometry, CropHandle, MIN_CROP_FRACTION, Point, Rect, crop_fit_angle, drag_crop}; use serde_json::{Value, json}; use super::{CommandSpec, always, bad, bool_or, cmd, f64_or, f64_req, has_active, has_clipboard, has_selection, index_req, ok, str_param}; @@ -530,7 +530,16 @@ pub fn specs() -> Vec { if v.len() != 4 { return Err(bad("crop.set", "rect needs 4 numbers")); } - d.crop.geometry.rect = Rect::new(v[0].min(v[2]), v[1].min(v[3]), v[0].max(v[2]), v[1].max(v[3])); + let r = Rect::new(v[0].min(v[2]), v[1].min(v[3]), v[0].max(v[2]), v[1].max(v[3])); + // the smallest crop a drag of the crop tool can make: below it an export comes out 1×1 (#1952) + let min_side = MIN_CROP_FRACTION * w.min(h) * (1.0 - 1e-9); + if r.width() * w < min_side || r.height() * h < min_side { + return Err(bad( + "crop.set", + format!("rect is smaller than the smallest crop ({} % of the shorter side)", MIN_CROP_FRACTION * 100.0), + )); + } + d.crop.geometry.rect = r; } let angle = p.get("angle").and_then(Value::as_f64).map(|a| a.clamp(-45.0, 45.0)); d.crop.geometry = match (p.get("rect"), angle) { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 400135ce0..f02032634 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -519,7 +519,7 @@ fn paste_selected_settings_pastes_only_chosen_copied_groups() { assert_eq!(s.develop_of(ids[1]).unwrap().color.vibrance, 30.0); } -/// A crop of no size (or a hair wide), as `crop.set` or a sidecar can set it, made the render's +/// A crop of no size (or a hair wide), as a sidecar can set it, made the render's /// samplers read at infinite coordinates and its blurs ask for sigmas of millions of pixels: with a /// healing spot and masks on, render workers panicked (an i32 square in the blur, a bilinear /// neighbour past isize::MAX). Such a photo renders, quickly. @@ -533,7 +533,7 @@ fn degenerate_crops_render_with_spots_and_masks() { s.execute("mask.add", &json!({"kind": "linear"})).unwrap(); s.execute("develop.set", &json!({"control": "effects.clarity", "value": 80})).unwrap(); s.execute("develop.set", &json!({"control": "effects.dehaze", "value": 50})).unwrap(); - s.execute("crop.set", &json!({"rect": rect, "angle": 30})).unwrap(); + set_crop_unchecked(&mut s, rect, 30.0); let t = std::time::Instant::now(); let r = s.render_now(id, 700, 450).unwrap(); assert!(r.image.width >= 1 && r.image.height >= 1, "{rect:?}"); @@ -541,13 +541,23 @@ fn degenerate_crops_render_with_spots_and_masks() { } } +/// Store a crop as a sidecar can, without `crop.set`'s minimum size, kept inside the turned photo. +fn set_crop_unchecked(s: &mut Session, [x0, y0, x1, y1]: [f64; 4], angle: f64) { + let id = s.active().unwrap(); + let p = s.catalog.photo(id).unwrap(); + let (w, h) = (p.width as f64, p.height as f64); + let mut d = active_dev(s); + d.crop.geometry = lightcraft_geom::CropGeometry { rect: lightcraft_geom::Rect::new(x0, y0, x1, y1), angle }.constrained(w, h); + s.set_develop(id, d, "Crop").unwrap(); +} + /// `crop.rotateAspect` on a crop of no size took its aspect from 0/0 = NaN and stored a NaN crop /// rectangle; it now falls back to the image's own aspect, a finite crop inside the image. #[test] fn rotate_aspect_of_degenerate_crop_stays_finite() { for rect in [[0.5, 0.5, 0.5, 0.5], [0.2, 0.2, 0.2, 0.8], [0.2, 0.2, 0.8, 0.2]] { let mut s = demo(); - s.execute("crop.set", &json!({"rect": rect})).unwrap(); + set_crop_unchecked(&mut s, rect, 0.0); s.execute("crop.rotateAspect", &json!({})).unwrap(); let r = active_dev(&s).crop.geometry.rect; assert!([r.x0, r.y0, r.x1, r.y1].iter().all(|v| v.is_finite()), "{rect:?}: {r:?}"); @@ -567,6 +577,22 @@ fn crop_set_huge_rect_stays_finite() { } } +/// `crop.set` took a crop of no size (or a sliver), and the next export wrote a 1×1 image (#1952). +/// It now refuses a crop smaller than a drag of the crop tool can make, and keeps the one it had. +#[test] +fn crop_set_refuses_a_crop_smaller_than_the_tool_allows() { + let mut s = demo(); + s.execute("crop.set", &json!({"rect": [0.1, 0.1, 0.9, 0.9]})).unwrap(); + let before = active_dev(&s).crop.geometry; + for rect in [[0.5, 0.5, 0.5, 0.5], [0.2, 0.2, 0.2, 0.8], [0.2, 0.4, 0.8, 0.4], [0.0, 0.0, 1e-9, 1.0]] { + let e = s.execute("crop.set", &json!({"rect": rect})).unwrap_err(); + assert!(e.to_string().contains("smaller"), "{rect:?}: {e}"); + assert_eq!(active_dev(&s).crop.geometry, before, "{rect:?}"); + } + s.execute("crop.set", &json!({"rect": [0.45, 0.45, 0.55, 0.55]})).unwrap(); + assert_eq!(active_dev(&s).crop.geometry.rect, lightcraft_geom::Rect::new(0.45, 0.45, 0.55, 0.55)); +} + /// `mask.move` with a delta near `i64::MAX` overflowed `index + delta` (a panic wherever overflow /// checks are on); extreme deltas and indices now move the mask to the bottom or the top. #[test] diff --git a/crates/ui-egui/src/tests_zoom.rs b/crates/ui-egui/src/tests_zoom.rs index b0595d27d..984a037cf 100644 --- a/crates/ui-egui/src/tests_zoom.rs +++ b/crates/ui-egui/src/tests_zoom.rs @@ -109,7 +109,7 @@ 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: +/// A crop of no size (or a hair wide), from 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 %. @@ -117,8 +117,8 @@ fn zoomed_before_after_views_stay_in_their_panes() { 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}"); + // as a sidecar can set it: `crop.set` refuses a crop this small + edit(&mut h, |d| d.crop.geometry.rect = lightcraft_geom::Rect::new(rect[0], rect[1], rect[2], rect[3])); for zoom in ["view.zoomFit", "view.zoom100"] { let r = h.request("engine.execute", json!({"command": zoom}), T); assert_eq!(r["ok"], true, "{zoom}: {r}");