crop.set checks the minimum on the crop it stores, so an oversized rect can't come out a sliver (part of #1952)

This commit is contained in:
Marco Arnulfo
2026-10-18 13:28:27 +02:00
parent 1162c2a018
commit be34283943
2 changed files with 22 additions and 11 deletions
+16 -8
View File
@@ -524,20 +524,22 @@ pub fn specs() -> Vec<CommandSpec> {
cmd!("crop.set", "Set Crop", [], None, "{rect?: [x0,y0,x1,y1] normalized in the straightened frame, angle?: degrees}", has_active, |s, p| {
let id = active(s, "crop.set")?;
let (w, h) = crop_dims(s, id);
// 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);
let too_small = |r: Rect| {
(r.width() * w < min_side || r.height() * h < min_side)
.then(|| bad("crop.set", format!("rect is smaller than the smallest crop ({} % of the shorter side)", MIN_CROP_FRACTION * 100.0)))
};
edit(s, "crop.set", "Crop", |d| {
if let Some(a) = p.get("rect").and_then(Value::as_array) {
let explicit = p.get("rect").and_then(Value::as_array);
if let Some(a) = explicit {
let v: Vec<f64> = a.iter().filter_map(Value::as_f64).collect();
if v.len() != 4 {
return Err(bad("crop.set", "rect needs 4 numbers"));
}
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),
));
if let Some(e) = too_small(r) {
return Err(e);
}
d.crop.geometry.rect = r;
}
@@ -547,6 +549,12 @@ pub fn specs() -> Vec<CommandSpec> {
(None, Some(a)) => d.crop.geometry.with_angle(a, w, h),
(_, a) => CropGeometry { angle: a.unwrap_or(d.crop.geometry.angle), ..d.crop.geometry }.constrained(w, h),
};
// a rect far larger than the photo is shrunk to fit it, and can come out a sliver
if explicit.is_some()
&& let Some(e) = too_small(d.crop.geometry.rect)
{
return Err(e);
}
Ok(())
})
}),
+6 -3
View File
@@ -566,12 +566,13 @@ fn rotate_aspect_of_degenerate_crop_stays_finite() {
}
}
/// `crop.set` with a huge rect shrank it by a scale of 0 (`inf * 0`) and stored a NaN crop.
/// `crop.set` with a huge rect shrank it by a scale of 0 (`inf * 0`) and stored a NaN crop. Their
/// maths overflow, so they fall back to a full-size crop and pass the minimum size.
#[test]
fn crop_set_huge_rect_stays_finite() {
for rect in [[0.0, 0.0, 1e308, 1e308], [-1e308, -1e308, 1e308, 1e308]] {
let mut s = demo();
let _ = s.execute("crop.set", &json!({"rect": rect}));
s.execute("crop.set", &json!({"rect": rect})).unwrap();
let r = active_dev(&s).crop.geometry.rect;
assert!([r.x0, r.y0, r.x1, r.y1].iter().all(|v| v.is_finite() && (-1e-6..=1.0 + 1e-6).contains(v)), "{rect:?}: {r:?}");
}
@@ -579,12 +580,14 @@ 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.
/// A rect far wider than the photo is a sliver too once it is shrunk to fit inside it.
#[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]] {
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], [0.0, 0.0, 1e6, 1.0], [0.0, 0.0, 1.0, 1e6]]
{
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:?}");