Let the crop be held to a ratio while it is dragged
A photographer cropping for a print, a phone wallpaper or a 16:9 frame is not choosing four edges — they are choosing one edge and a known shape. Free-dragging every corner made them do that arithmetic by eye on every drag, and get it slightly wrong. The panel now offers Free, Original, 1:1, 3:2, 4:3 and 16:9, with a Portrait switch for the ones that have two orientations. Original follows the frame rather than naming a number, so it stays right on the next photograph from another body and after a quarter turn. **The ratio is of output pixels, and the rect is not.** `CropRect` is stored in fractions of a frame that is not itself square, so holding a shape needs the frame's size — `ratio * height / width` of the frame. Skipping that gives a "1:1" crop that is square only on a square photograph, which is the one case nobody would test on, so the conversion lives in `CropRect::with_aspect` where it is explained and pinned by a test that asserts the fractions are *not* equal. Two decisions worth recording: The reshaped rect **grows** onto the ratio rather than shrinking onto it, then scales down only as far as the frame's edge demands. Fitting inside instead makes a one-axis drag do nothing at all — the other axis clamps the first straight back, and the handle simply refuses to move. The overlay now reports **which corner the drag is holding**, because reshaping onto a ratio has to know which corner is nailed down and only the handle that took the press knows that. A move reports no corner and keeps its shape: reshaping about a centre would pull an over-moved rect smaller instead of sliding it along the edge. The lock lives with the window rather than the session. A `DevelopSession` is per image, and cropping a set of frames to one shape is exactly when the lock earns its place. It is not an edit and reaches no sidecar — what is saved is the rectangle it produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -583,6 +583,87 @@ fn bilinear_sample(mask: &[f32], w: usize, h: usize, x: f32, y: f32) -> f32 {
|
||||
top * (1.0 - fy) + bot * fy
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// A shape the crop rectangle is held to while it is dragged.
|
||||
///
|
||||
/// A photographer cropping for a print, a phone wallpaper or a 16:9 frame is
|
||||
/// not choosing four edges — they are choosing one edge and a known shape, and
|
||||
/// a free crop makes them do the arithmetic by eye on every drag. This is the
|
||||
/// lock that removes it.
|
||||
///
|
||||
/// **The ratio is of output pixels, not of the rect's own numbers.** The rect
|
||||
/// is stored in fractions of a frame that is not square, so `CropRect` needs
|
||||
/// the frame's size to hold a shape; see [`CropRect::with_aspect`], which is
|
||||
/// where that conversion is done and explained.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
|
||||
pub enum CropAspect {
|
||||
/// Any shape. The handles move independently, as they always have.
|
||||
#[default]
|
||||
Free,
|
||||
/// Whatever the frame already is, so a crop trims without reshaping.
|
||||
///
|
||||
/// Not the same as `Fixed(3, 2)` even on a 3:2 camera: it follows the
|
||||
/// frame, so it stays right on the next photograph from another body and
|
||||
/// after a quarter turn.
|
||||
Original,
|
||||
/// A named ratio of `w:h`, before the portrait switch is applied.
|
||||
Fixed(u32, u32),
|
||||
}
|
||||
|
||||
impl CropAspect {
|
||||
/// The ratios the panel offers, in the order it draws them.
|
||||
///
|
||||
/// Short on purpose. These sit as chips in a column narrow enough for a
|
||||
/// tablet, and every ratio a photographer reaches for repeatedly is here:
|
||||
/// the frame's own shape, the square, the two classic camera ratios, the
|
||||
/// large-format one that most print papers follow, and video's.
|
||||
pub const CHOICES: [Self; 6] = [
|
||||
Self::Free,
|
||||
Self::Original,
|
||||
Self::Fixed(1, 1),
|
||||
Self::Fixed(3, 2),
|
||||
Self::Fixed(4, 3),
|
||||
Self::Fixed(16, 9),
|
||||
];
|
||||
|
||||
/// The chip's text.
|
||||
pub fn label(self) -> String {
|
||||
match self {
|
||||
Self::Free => "Free".to_string(),
|
||||
Self::Original => "Original".to_string(),
|
||||
Self::Fixed(w, h) => format!("{w}:{h}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether this choice has a portrait form at all.
|
||||
///
|
||||
/// A square does not, and neither does `Free`. The switch is disabled
|
||||
/// rather than hidden for those, so the row does not change shape as the
|
||||
/// chips are tried.
|
||||
pub fn has_orientation(self) -> bool {
|
||||
!matches!(self, Self::Free | Self::Fixed(1, 1))
|
||||
}
|
||||
|
||||
/// Width over height in output pixels, or `None` where nothing is locked.
|
||||
///
|
||||
/// `frame` is the framed size the crop is measured against — the turned
|
||||
/// frame, not the sensor — which is what makes `Original` follow a quarter
|
||||
/// turn instead of becoming a portrait crop on a landscape photograph.
|
||||
pub fn ratio(self, frame: (u32, u32), portrait: bool) -> Option<f32> {
|
||||
let (fw, fh) = (frame.0.max(1) as f32, frame.1.max(1) as f32);
|
||||
let landscape = match self {
|
||||
Self::Free => return None,
|
||||
Self::Original => fw / fh,
|
||||
Self::Fixed(w, h) => w.max(1) as f32 / h.max(1) as f32,
|
||||
};
|
||||
Some(if portrait && self.has_orientation() {
|
||||
1.0 / landscape
|
||||
} else {
|
||||
landscape
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
pub struct DevelopSession {
|
||||
/// This session's name, for work that outlives the frame it started on.
|
||||
id: SessionId,
|
||||
@@ -3138,10 +3219,48 @@ impl DevelopSession {
|
||||
.record(&self.graph, Edit::Op(dr_pipeline::framing::ID));
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Set the crop rectangle, held to `aspect` about `anchor`.
|
||||
///
|
||||
/// The frame size the ratio needs is this session's own, so the caller
|
||||
/// passes a shape rather than a rectangle and never has to know what a
|
||||
/// quarter turn did to the frame's dimensions.
|
||||
///
|
||||
/// `anchor` is the point of the rect that must not move, in the rect's own
|
||||
/// `0..1` coordinates — the corner *opposite* the handle being dragged, so
|
||||
/// that shaping the rect onto the ratio pushes the held corner and leaves
|
||||
/// the far one where the user put it.
|
||||
pub fn set_crop_locked(
|
||||
&mut self,
|
||||
rect: CropRect,
|
||||
aspect: CropAspect,
|
||||
portrait: bool,
|
||||
anchor: (f32, f32),
|
||||
) {
|
||||
let frame = self.framed_size();
|
||||
let rect = match aspect.ratio(frame, portrait) {
|
||||
Some(r) => rect.with_aspect(frame.0, frame.1, r, anchor),
|
||||
None => rect,
|
||||
};
|
||||
self.set_crop(rect);
|
||||
}
|
||||
|
||||
pub fn crop(&self) -> CropRect {
|
||||
self.graph.crop()
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// The whole frame the crop is measured against, in output pixels.
|
||||
///
|
||||
/// The *framed* size, not the sensor's: quarter turns swap the axes, and a
|
||||
/// ratio resolved against the sensor would come out on its side the moment
|
||||
/// a portrait photograph was turned upright. The crop is excluded because
|
||||
/// this is the shape being selected *from*.
|
||||
pub fn framed_size(&self) -> (u32, u32) {
|
||||
let (sw, sh) = self.demosaiced.size();
|
||||
self.graph.framing().output_size_uncropped(sw, sh)
|
||||
}
|
||||
|
||||
/// Rotate by quarter turns, wrapping. The rotate-left/right buttons.
|
||||
///
|
||||
/// The crop travels with the frame rather than staying where it was on
|
||||
|
||||
+147
-7
@@ -359,6 +359,58 @@ fn sync_framing(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSession>>
|
||||
window.set_crop_h(crop.height);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Push the chosen crop ratio back to the panel.
|
||||
///
|
||||
/// The index is written from the choice actually held rather than from what
|
||||
/// was clicked, for the same reason [`sync_framing`] writes the applied crop:
|
||||
/// picking a ratio with no portrait form clears the orientation switch, and a
|
||||
/// panel showing the click rather than the result would light a switch that is
|
||||
/// doing nothing.
|
||||
fn sync_crop_aspect(
|
||||
window: &AppWindow,
|
||||
aspect: &Rc<Cell<develop::CropAspect>>,
|
||||
portrait: &Rc<Cell<bool>>,
|
||||
) {
|
||||
let chosen = aspect.get();
|
||||
let index = develop::CropAspect::CHOICES
|
||||
.iter()
|
||||
.position(|a| *a == chosen)
|
||||
.unwrap_or(0);
|
||||
window.set_crop_aspect(index as i32);
|
||||
window.set_crop_portrait(portrait.get());
|
||||
window.set_crop_aspect_turnable(chosen.has_orientation());
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Reshape the open image's crop onto the chosen ratio, about its centre.
|
||||
///
|
||||
/// Called when the ratio itself changes, never on opening a photograph: the
|
||||
/// lock outlives the session it is applied to, and reshaping a crop the user
|
||||
/// has not touched — on an image they have only just opened — would edit their
|
||||
/// work as a side effect of stepping through a shoot. A ratio takes effect
|
||||
/// when it is chosen and when a handle is dragged, both of which are things
|
||||
/// the user did on purpose.
|
||||
fn apply_crop_aspect(
|
||||
window: &AppWindow,
|
||||
session: &Rc<RefCell<Option<DevelopSession>>>,
|
||||
aspect: &Rc<Cell<develop::CropAspect>>,
|
||||
portrait: &Rc<Cell<bool>>,
|
||||
) {
|
||||
let Some(s) = session.borrow_mut().as_mut().map(|s| {
|
||||
s.set_crop_locked(s.crop(), aspect.get(), portrait.get(), (0.5, 0.5));
|
||||
(s.crop(), s.framing_edits_image())
|
||||
}) else {
|
||||
return;
|
||||
};
|
||||
let (crop, modified) = s;
|
||||
window.set_crop_x(crop.x);
|
||||
window.set_crop_y(crop.y);
|
||||
window.set_crop_w(crop.width);
|
||||
window.set_crop_h(crop.height);
|
||||
window.set_framing_modified(modified);
|
||||
}
|
||||
|
||||
/// TRACES: FR-EXP-6 | FR-EXP-9 | FR-EXP-7
|
||||
/// Render the open image at full resolution, ready to be handed to the worker.
|
||||
///
|
||||
@@ -2354,6 +2406,27 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
//
|
||||
// Zoom and pan are viewing state and touch no parameter, so unlike the
|
||||
// handlers above they do not `sync_rows`.
|
||||
|
||||
// TRACES: FR-DEV-3
|
||||
// **The ratio lock belongs to the tool, not to the photograph.** A
|
||||
// `DevelopSession` is built per image, so holding the lock there would
|
||||
// reset it at every step through a shoot — and cropping a set of frames to
|
||||
// one shape is precisely when the lock earns its place. It lives here, for
|
||||
// the life of the window, and is re-applied to whichever session is open.
|
||||
//
|
||||
// It is deliberately *not* an edit. Nothing about it reaches the sidecar:
|
||||
// what is saved is the rectangle it produced, which is the whole of what
|
||||
// the pipeline needs and the whole of what another application could read.
|
||||
let crop_aspect = Rc::new(Cell::new(develop::CropAspect::default()));
|
||||
let crop_portrait = Rc::new(Cell::new(false));
|
||||
window.set_crop_aspects(
|
||||
develop::CropAspect::CHOICES
|
||||
.iter()
|
||||
.map(|a| slint::SharedString::from(a.label()))
|
||||
.collect::<Vec<_>>()
|
||||
.as_slice()
|
||||
.into(),
|
||||
);
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
@@ -2459,22 +2532,42 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
});
|
||||
}
|
||||
{
|
||||
// The rect arrives raw from the drag; the session normalises it, and
|
||||
// the properties are written back from what it actually stored. That
|
||||
// round trip is what makes an over-drag slide along the edge rather
|
||||
// than letting the overlay and the pipeline disagree.
|
||||
// The rect arrives raw from the drag; the session normalises it, holds
|
||||
// it to whatever ratio is locked, and the properties are written back
|
||||
// from what it actually stored. That round trip is what makes an
|
||||
// over-drag slide along the edge rather than letting the overlay and
|
||||
// the pipeline disagree — and it is what lets the lock reshape a drag
|
||||
// without the overlay having to know a ratio exists.
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
window.on_crop_changed(move |x, y, width, height| {
|
||||
let crop_aspect = crop_aspect.clone();
|
||||
let crop_portrait = crop_portrait.clone();
|
||||
window.on_crop_changed(move |x, y, width, height, hx, hy| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
if let Some(s) = session.borrow_mut().as_mut() {
|
||||
s.set_crop(dr_pipeline::CropRect {
|
||||
let rect = dr_pipeline::CropRect {
|
||||
x,
|
||||
y,
|
||||
width,
|
||||
height,
|
||||
});
|
||||
};
|
||||
// The overlay reports the corner it is *holding*; the point
|
||||
// that must not move is the opposite one. A move reports no
|
||||
// corner at all, and keeps the shape it already has — there is
|
||||
// nothing to reshape, and reshaping about a centre would drag
|
||||
// an over-moved rect smaller instead of sliding it along the
|
||||
// edge.
|
||||
if hx < 0.0 {
|
||||
s.set_crop(rect);
|
||||
} else {
|
||||
s.set_crop_locked(
|
||||
rect,
|
||||
crop_aspect.get(),
|
||||
crop_portrait.get(),
|
||||
(1.0 - hx, 1.0 - hy),
|
||||
);
|
||||
}
|
||||
let c = s.crop();
|
||||
w.set_crop_x(c.x);
|
||||
w.set_crop_y(c.y);
|
||||
@@ -2485,6 +2578,53 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
redraw(&w);
|
||||
});
|
||||
}
|
||||
{
|
||||
// TRACES: FR-DEV-3
|
||||
// Choosing a ratio reshapes the crop there and then rather than
|
||||
// waiting for the next drag. A lock that only took effect on the
|
||||
// following gesture would leave the chip lit over a rect that is not
|
||||
// that shape, which is the panel lying about the image.
|
||||
//
|
||||
// Held about the rect's centre, so the composition stays where it is
|
||||
// and the shape changes around it.
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
let crop_aspect = crop_aspect.clone();
|
||||
let crop_portrait = crop_portrait.clone();
|
||||
window.on_crop_aspect_picked(move |index| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let Some(&aspect) = develop::CropAspect::CHOICES.get(index.max(0) as usize) else {
|
||||
return;
|
||||
};
|
||||
crop_aspect.set(aspect);
|
||||
// A ratio with no second orientation cannot stay stood on its
|
||||
// short edge, or the switch would be off and the crop upright.
|
||||
if !aspect.has_orientation() {
|
||||
crop_portrait.set(false);
|
||||
}
|
||||
sync_crop_aspect(&w, &crop_aspect, &crop_portrait);
|
||||
apply_crop_aspect(&w, &session, &crop_aspect, &crop_portrait);
|
||||
redraw(&w);
|
||||
});
|
||||
}
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
let crop_aspect = crop_aspect.clone();
|
||||
let crop_portrait = crop_portrait.clone();
|
||||
window.on_crop_portrait_toggled(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
if !crop_aspect.get().has_orientation() {
|
||||
return;
|
||||
}
|
||||
crop_portrait.set(!crop_portrait.get());
|
||||
sync_crop_aspect(&w, &crop_aspect, &crop_portrait);
|
||||
apply_crop_aspect(&w, &session, &crop_aspect, &crop_portrait);
|
||||
redraw(&w);
|
||||
});
|
||||
}
|
||||
|
||||
// ---- rotation, flips and straightening -------------------------------
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user