Crop away the corners a straighten exposed, once the slider is let go

Turning a rectangle inside its own bounds exposes its corners: there is no
source pixel out there, and the shader renders it black. Nothing in the
render prevents that, deliberately — a free angle does not change the
output size, which is what leaves the frame where the user put it while
the slider moves. Correct during the drag; four black wedges on the
finished photograph.

Letting go of the slider now pulls the crop inside the area the angle
leaves defined. `Framing::max_inscribed_crop` already computed that
bound and had no caller; this is the caller its doc comment described.

**Once, at the end of the gesture.** Applied per frame it would shrink
the crop on every step of the slider and never grow it back, so a user
who overshot to 20° and came back to 3° would be left with a crop
ratcheted down by the excursion rather than by the angle they settled on.
Per gesture it is bounded by the angles actually rested at, and undo steps
back through them.

**The crop is fitted into the bound, not replaced by it.** A crop placed
deliberately off-centre is a decision, and an automatic correction that
recentred it would undo the user's work to fix a problem they did not
have. `CropRect::fitted_into` scales only as far as the bound demands and
then slides the rect the shortest distance needed to be inside — so a
ratio locked in the crop panel survives the straighten too, since the
shape is never touched.

It returns the rect unchanged, bit for bit, when nothing needed to move.
That matters more than it looks: this runs on every release of the
slider, including releases at zero, and a rect that drifted by a rounding
error each time would be an edit recorded for no reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-29 13:49:19 +02:00
co-authored by Claude Opus 5
parent e6ad906bc1
commit d9eb8faffd
6 changed files with 233 additions and 32 deletions
+126
View File
@@ -231,6 +231,56 @@ impl CropRect {
} }
.normalised() .normalised()
} }
/// TRACES: FR-DEV-3
/// The largest copy of this rect, at its own shape, sitting inside
/// `bound`.
///
/// Scaled down only as far as `bound` demands and then slid inside it,
/// rather than replaced by `bound` or centred within it. Both of those
/// throw away a composition: a crop placed deliberately off-centre is a
/// decision, and an automatic correction that recentres it has undone the
/// user's work to fix a problem they did not have.
pub fn fitted_into(self, bound: Self) -> Self {
let rect = self.normalised();
let bound = bound.normalised();
let shrink = (bound.width / rect.width)
.min(bound.height / rect.height)
.min(1.0);
let shrink = if shrink.is_finite() && shrink > 0.0 {
shrink
} else {
1.0
};
let (w, h) = (rect.width * shrink, rect.height * shrink);
// Shrunk about its own centre, so what was framed stays framed — but
// only when it actually shrank. Routing the untouched case through
// the same arithmetic moves the origin by a rounding error, and a
// rect that is already inside its bound must come back *identical*:
// this is called on every straighten, and a rect that drifts a
// millionth each time is an edit recorded for no reason.
let (x, y) = if shrink < 1.0 {
(
rect.x + (rect.width - w) * 0.5,
rect.y + (rect.height - h) * 0.5,
)
} else {
(rect.x, rect.y)
};
Self {
// `max` on the upper limit for the same reason `normalised`
// orders its bounds that way: rounding can put the far edge a
// hair *below* the near one, and `clamp` panics on an inverted
// range rather than resolving it.
x: x.clamp(bound.x, (bound.x + bound.width - w).max(bound.x)),
y: y.clamp(bound.y, (bound.y + bound.height - h).max(bound.y)),
width: w,
height: h,
}
.normalised()
}
} }
/// How far a rect anchored at `p` with the anchor `a` fractions along it may /// How far a rect anchored at `p` with the anchor `a` fractions along it may
@@ -1806,6 +1856,82 @@ mod tests {
assert_eq!(start.with_aspect(0, 0, 1.0, (0.5, 0.5)), start); assert_eq!(start.with_aspect(0, 0, 1.0, (0.5, 0.5)), start);
} }
#[test]
fn a_rect_already_inside_its_bound_is_left_where_it_is() {
let bound = CropRect {
x: 0.1,
y: 0.1,
width: 0.8,
height: 0.8,
};
let rect = CropRect {
x: 0.2,
y: 0.2,
width: 0.3,
height: 0.3,
};
assert_eq!(rect.fitted_into(bound), rect);
}
#[test]
fn fitting_into_a_bound_keeps_the_shape_and_the_side_it_was_on() {
// A crop placed deliberately off-centre is a decision. Recentring it
// to solve a problem the user did not have would undo their work.
let bound = CropRect {
x: 0.25,
y: 0.25,
width: 0.5,
height: 0.5,
};
let rect = CropRect {
x: 0.0,
y: 0.0,
width: 0.8,
height: 0.4,
};
let fitted = rect.fitted_into(bound);
assert!(
(fitted.width / fitted.height - rect.width / rect.height).abs() < 1e-4,
"shape changed: {fitted:?}"
);
assert!(
fitted.x >= bound.x - 1e-5
&& fitted.y >= bound.y - 1e-5
&& fitted.x + fitted.width <= bound.x + bound.width + 1e-5
&& fitted.y + fitted.height <= bound.y + bound.height + 1e-5,
"{fitted:?} is not inside {bound:?}"
);
// It came from the top-left, so it should still be against those
// edges rather than centred in the bound.
assert!((fitted.x - bound.x).abs() < 1e-5, "{fitted:?}");
assert!((fitted.y - bound.y).abs() < 1e-5, "{fitted:?}");
}
#[test]
fn a_locked_crop_still_fits_inside_the_straightened_safe_area() {
// The two new pieces meet here: an angle shrinks the safe area, and a
// ratio the user locked has to survive being fitted into it.
let mut f = Framing::new();
f.set_param(ANGLE, 7.0);
let bound = f.max_inscribed_crop(6000, 4000);
let locked = CropRect::default().with_aspect(6000, 4000, 1.0, (0.5, 0.5));
let fitted = locked.fitted_into(bound);
assert!(
(pixel_ratio(fitted, 6000, 4000) - 1.0).abs() < 1e-3,
"the lock did not survive the fit: {fitted:?}"
);
assert!(
fitted.x >= bound.x - 1e-5
&& fitted.y >= bound.y - 1e-5
&& fitted.x + fitted.width <= bound.x + bound.width + 1e-5
&& fitted.y + fitted.height <= bound.y + bound.height + 1e-5,
"{fitted:?} is not inside {bound:?}"
);
}
#[test] #[test]
fn parameters_round_trip() { fn parameters_round_trip() {
let mut f = Framing::new(); let mut f = Framing::new();
+32 -32
View File
File diff suppressed because one or more lines are too long
+34
View File
@@ -3315,6 +3315,40 @@ impl DevelopSession {
.record(&self.graph, Edit::Action(labels::step::FLIP_V)); .record(&self.graph, Edit::Action(labels::step::FLIP_V));
} }
/// TRACES: FR-DEV-3
/// Pull the crop inside the area a straightening angle leaves defined.
///
/// Turning a rectangle inside its own bounds exposes the corners: there is
/// no source pixel out there and the shader renders it black. Nothing in
/// the render prevents it — a free angle deliberately does *not* change the
/// output size, so that straightening a horizon leaves the frame where the
/// user put it — which is correct for the drag and leaves black wedges in
/// the corners of the finished photograph.
///
/// This is the correction, and it runs when the gesture **finishes**.
/// Applied continuously it would shrink the crop on every frame of the
/// slider and never grow it back, so a user who overshot to 20° and came
/// back to 3° would be left with a crop ratcheted down by the excursion
/// rather than by the angle they settled on. Once per gesture, that is
/// bounded: the crop is only ever as small as the angles actually rested
/// at, and undo steps back through them.
///
/// The crop keeps its own shape — so a locked ratio survives — and keeps
/// the side of the frame it was on; see [`CropRect::fitted_into`] for why
/// it is not simply replaced by the inscribed rectangle.
pub fn auto_crop_to_angle(&mut self) {
let (sw, sh) = self.demosaiced.size();
let bound = self.graph.framing().max_inscribed_crop(sw, sh);
// Upright, so every corner is already defined and there is nothing to
// pull in from. Returning early rather than fitting into a full rect
// matters: it keeps letting go of a slider at zero from touching the
// crop at all.
if bound.is_full() {
return;
}
self.set_crop(self.graph.crop().fitted_into(bound));
}
/// Set the straightening angle, in degrees. /// Set the straightening angle, in degrees.
pub fn set_angle(&mut self, degrees: f32) { pub fn set_angle(&mut self, degrees: f32) {
self.graph.set_param( self.graph.set_param(
+26
View File
@@ -2684,6 +2684,32 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
redraw(&w); redraw(&w);
}); });
} }
{
// TRACES: FR-DEV-3
// **The end of a straightening gesture crops away the corners it
// exposed.** A free angle does not change the output size, by design —
// that is what leaves the frame where the user put it while the slider
// moves — so the corners of the finished photograph would otherwise be
// black wedges of undefined area.
//
// Only on release, and only on release. Doing it per frame ratchets
// the crop down through every angle the slider passed through rather
// than the one it stopped at; see `auto_crop_to_angle`.
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.on_straighten_drag_changed(move |dragging| {
if dragging {
return;
}
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.auto_crop_to_angle();
}
sync_framing(&w, &session);
redraw(&w);
});
}
{ {
// The geometry section's reset: crop, angle, rotation and flips back // The geometry section's reset: crop, angle, rotation and flips back
// to neutral, leaving every colour adjustment where it is. The panel's // to neutral, leaving every colour adjustment where it is. The panel's
+5
View File
@@ -512,6 +512,10 @@ export component GeometryPanel inherits Rectangle {
callback flip-h-toggled(); callback flip-h-toggled();
callback flip-v-toggled(); callback flip-v-toggled();
callback angle-changed(float); callback angle-changed(float);
/// The straightening gesture began or ended. What the auto-crop hangs
/// off — see `DevelopSession::auto_crop_to_angle` for why it must be the
/// end of the drag and not every frame of it.
callback angle-drag-changed(bool);
callback angle-reset(); callback angle-reset();
callback aspect-picked(int); callback aspect-picked(int);
callback portrait-toggled(); callback portrait-toggled();
@@ -604,6 +608,7 @@ export component GeometryPanel inherits Rectangle {
maximum: root.max-straighten; maximum: root.max-straighten;
unit: "°"; unit: "°";
changed(v) => { root.angle-changed(v); } changed(v) => { root.angle-changed(v); }
drag-changed(on) => { root.angle-drag-changed(on); }
reset => { root.angle-reset(); } reset => { root.angle-reset(); }
} }
+10
View File
@@ -161,6 +161,13 @@ export component AppWindow inherits Window {
callback flip-h-toggled(); callback flip-h-toggled();
callback flip-v-toggled(); callback flip-v-toggled();
callback straighten-changed(float); callback straighten-changed(float);
/// TRACES: FR-DEV-3
/// The straightening gesture began (`true`) or ended (`false`).
///
/// Reported separately from the value because the auto-crop that follows a
/// straighten has to happen once, at the end — see
/// `DevelopSession::auto_crop_to_angle`.
callback straighten-drag-changed(bool);
/// Crop, angle, rotation and flips back to neutral, leaving colour alone. /// Crop, angle, rotation and flips back to neutral, leaving colour alone.
callback framing-reset(); callback framing-reset();
@@ -2158,6 +2165,9 @@ in property <bool> panel-visible: true;
flip-h-toggled => { root.flip-h-toggled(); } flip-h-toggled => { root.flip-h-toggled(); }
flip-v-toggled => { root.flip-v-toggled(); } flip-v-toggled => { root.flip-v-toggled(); }
angle-changed(v) => { root.straighten-changed(v); } angle-changed(v) => { root.straighten-changed(v); }
angle-drag-changed(on) => {
root.straighten-drag-changed(on);
}
angle-reset => { root.straighten-changed(0); } angle-reset => { root.straighten-changed(0); }
reset => { root.framing-reset(); } reset => { root.framing-reset(); }
} }