Say so when a crop leaves a mask outside the frame

Cropping tighter past a mask layer made it invisible without a word:
the layer stayed in the panel and the sidecar, and its adjustment went
on landing on pixels nobody would see again.

When a crop is let go, develop now measures what the gesture did to the
mask stack (dr_pipeline::orphan) and, if any layer is now entirely or
mostly outside the frame, shows a notice over the photograph: how many
layers, their names, "Undo crop" and "Keep crop". The crop is already
applied and nothing waits on the answer.

The crop overlay gains a release callback carrying the rect the press
began from, so the measurement runs once per gesture and never on the
drag's per-frame changes. Choosing a ratio is measured the same way,
being a crop committed in one click.

"Undo crop" is the ordinary undo, and the notice is tied to the history
revision it was raised at: the redraw that follows any history move
clears it, so the crop and its warning go back as one step. A second
drag folded into the same step is measured from where that step began.
A crop that strands nothing shows nothing.
This commit is contained in:
2026-09-24 21:52:03 -04:00
parent fc0ea8824d
commit c3d1f83b96
9 changed files with 432 additions and 46 deletions
File diff suppressed because one or more lines are too long
+44
View File
@@ -53,7 +53,11 @@ A model's mask stops inside a shoulder and leaks into the hair, and no single ed
A whole drag is one step, so undo takes back a decision rather than a frame of a gesture. The list is there because arriving six steps back costs what arriving from one does.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2123`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2116`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Do it again after taking it back
@@ -61,7 +65,11 @@ A whole drag is one step, so undo takes back a decision rather than a frame of a
- **Pointer** — Click it, or press Redo in the History header
- **Keyboard** — Ctrl+Shift+Z
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2136`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2129`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Copy the settings from this photograph
@@ -71,7 +79,11 @@ A whole drag is one step, so undo takes back a decision rather than a frame of a
The button is the copy that has to work: a tablet has no modifier key to hold and no menu bar to hang the action from. The shortcut is an accelerator for a control that is on screen either way.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2169`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2162`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Paste the settings onto this photograph
@@ -81,7 +93,11 @@ The button is the copy that has to work: a tablet has no modifier key to hold an
The button names what would be pasted — "3 adjustments", and whether the crop is coming with it — which the shortcut cannot say. Both paste the same scope.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2181`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2174`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Choose which kinds of edit a copy carries
@@ -91,7 +107,11 @@ The button names what would be pasted — "3 adjustments", and whether the crop
Lightroom's Copy Settings. Pasting a look across a shoot usually means leaving each frame's crop and rotation alone, and that is a choice to make at the moment of copying.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2198`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2191`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Export this photograph as the last one was
@@ -101,7 +121,11 @@ Lightroom's Copy Settings. Pasting a look across a shoot usually means leaving e
Every export runs on the defaults in Settings, so "as the last one was" is what the button already does. The chord is Lightroom's and darktable's, kept so hands that learned it there need not learn it again.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2227`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2220`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Choose how to export, then export
@@ -111,7 +135,11 @@ Every export runs on the defaults in Settings, so "as the last one was" is what
The export sheet is the export defaults alone with an Export button. What is chosen there is kept, so it is also what the next Ctrl+Shift+E uses.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2239`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2232`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Change which group of adjustments is on screen
@@ -121,7 +149,11 @@ The export sheet is the export defaults alone with an Export button. What is cho
The groups are whatever the operation set declares itself to be about, so there are as many as the pipeline has and no key can be assigned to one of them by name. Stepping is the binding that survives a node being added.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2264`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2257`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Look at the photograph at 1:1
@@ -131,7 +163,11 @@ The groups are whatever the operation set declares itself to be about, so there
Noise reduction and capture sharpening are judgements about single pixels, and a fitted view averages several of the file's into each one on screen — so the frame looks softer than it is and the correction goes too far. The point and the magnification survive opening the next photograph, which is what makes checking the same eye across forty portraits forty keystrokes rather than forty pans. From 1:1 on the photograph is drawn as its own pixels, each a hard-edged square, rather than smoothed into a blur.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2299`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2292`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Move to the next or previous photograph
@@ -141,7 +177,11 @@ Noise reduction and capture sharpening are judgements about single pixels, and a
The edit on screen is saved on the way out, so stepping through a folder is as much a departure as going back to the grid and loses nothing. A and D as well as the arrows, so the left hand steps along the roll while the right stays on the mouse. Unmodified only: Ctrl+D and Ctrl+A are not this.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2354`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2344`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### See the photograph before you edited it
@@ -151,7 +191,11 @@ The edit on screen is saved on the way out, so stepping through a folder is as m
Held rather than toggled, and no split screen: a split halves the working image on the tablet the column was sized for, and the comparison photographers describe making is a flick back and forth. It takes no history step, so checking whether a frame is overcooked costs nothing to undo afterwards.
<<<<<<< HEAD
<sub>`ui/dr-ui/ui/app.slint:2487`</sub>
=======
<sub>`ui/dr-ui/ui/app.slint:2477`</sub>
>>>>>>> 5b5164f (Say so when a crop leaves a mask outside the frame)
### Put one control back to its default
+168
View File
@@ -130,7 +130,122 @@ fn rotate_crop(rect: CropRect, turns: i32) -> CropRect {
r.normalised()
}
/// TRACES: FR-DEV-17
/// The mask layers a committed crop took out of the frame.
///
/// **Tied to a position in the history, not to a timer or a click.** The
/// notice speaks about the step on top of the stack, and it is only true
/// while that step is on top: an undo takes the crop back, a later edit puts
/// something else there, and either way the notice has nothing left to say.
/// So it carries the revision it was raised at, and
/// [`DevelopSession::crop_notice`] stops returning it the moment the history
/// moves — which is what makes taking the crop back and taking the warning
/// away one step rather than two.
#[derive(Debug, Clone)]
pub struct CropNotice {
/// [`DevelopSession::history_revision`] when the crop was let go.
revision: u64,
/// The framing before the crop, kept so a second drag folded into the
/// same step is measured from where the step began rather than from the
/// already-hidden state the first drag left.
from: dr_pipeline::Framing,
/// Each hidden layer's name, in stack order.
names: Vec<String>,
}
impl CropNotice {
pub fn names(&self) -> &[String] {
&self.names
}
}
impl DevelopSession {
/// The framing as it stands — what a gesture that is about to change the
/// crop hands back to [`Self::notice_hidden_masks`] once it is let go.
pub fn framing(&self) -> dr_pipeline::Framing {
*self.graph.framing()
}
/// TRACES: FR-DEV-17
/// Measure the crop just committed against `before`, and raise a notice
/// for the mask layers it took out of the frame. Returns the notice, or
/// `None` when nothing was stranded — the common case, which says nothing.
///
/// Called when a crop is *let go*, never per frame of the drag: a handle
/// passing over a mask on its way somewhere else is not an event, and the
/// measurement samples every layer, which is work for a commit rather than
/// a pointer move.
///
/// A model's selection is measured from the raster the live segmentation
/// holds when the layer carries none of its own — the session keeps its
/// model output outside the graph until a save folds it in.
pub fn notice_hidden_masks(&mut self, before: &dr_pipeline::Framing) -> Option<&CropNotice> {
use dr_pipeline::mask::{MaskPart, MaskSource};
use dr_pipeline::orphan::{hidden_by_crop, Raster};
use std::borrow::Cow;
let revision = self.history.revision();
// Still standing on the step a notice was raised for: a second drag
// folded into that step is measured from where the step began.
let from = match &self.crop_notice {
Some(n) if n.revision == revision => n.from,
_ => *before,
};
let source = self.demosaiced.size();
let seg = self.segmentation.as_ref();
let model = |part: &MaskPart| -> Option<Raster<'_>> {
let seg = seg?;
if part.is_stale(seg.signature()) {
return None;
}
let values = match &part.source {
MaskSource::Subject { index, .. } => {
Cow::Borrowed(seg.instance_mask(*index as usize)?)
}
MaskSource::Category { name, .. } => seg.category_mask_at(name, part.refine)?,
_ => return None,
};
let (width, height) = seg.proxy_size();
Some(Raster {
values,
width,
height,
})
};
let names: Vec<String> = hidden_by_crop(
self.graph.masks(),
&from,
self.graph.framing(),
source,
&model,
)
.into_iter()
.map(|layer| layer.display_name().to_string())
.collect();
self.crop_notice = (!names.is_empty()).then_some(CropNotice {
revision,
from,
names,
});
self.crop_notice.as_ref()
}
/// TRACES: FR-DEV-17
/// The notice for the crop on top of the history, if it is still on top.
pub fn crop_notice(&self) -> Option<&CropNotice> {
self.crop_notice
.as_ref()
.filter(|n| n.revision == self.history.revision())
}
/// TRACES: FR-DEV-17
/// Keep the crop: the photographer has read the notice and meant it.
pub fn dismiss_crop_notice(&mut self) {
self.crop_notice = None;
}
/// The sensor's own dimensions, before framing.
///
/// What a crop overlay needs: its handles are placed against the full
@@ -990,4 +1105,57 @@ mod tests {
assert!(rotate_crop(CropRect::default(), 1).is_full());
assert!(rotate_crop(CropRect::default(), -3).is_full());
}
/// TRACES: FR-DEV-17
/// The acceptance, end to end through the session: a crop that strands a
/// layer raises a notice naming it, a crop that does not says nothing,
/// and one undo takes back the crop and the notice together.
#[test]
fn a_crop_that_strands_a_mask_is_noticed_and_undone_as_one_step() {
let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else {
log::warn!("no GPU adapter; skipping");
return;
};
let rgba = vec![128u8; 64 * 64 * 4];
let mut session =
DevelopSession::open_rgb(&ctx, &rgba, 64, 64, dr_types::Orientation::NORMAL)
.expect("session");
// A radial in the middle of the frame.
session.add_gradient_mask(true).expect("a radial layer");
// Trimmed, not stranded: no notice on the common path.
let before = session.framing();
session.set_crop(CropRect {
x: 0.1,
y: 0.1,
width: 0.8,
height: 0.8,
});
assert!(session.notice_hidden_masks(&before).is_none());
assert!(session.crop_notice().is_none());
// Into a corner the radial does not reach.
let before = session.framing();
std::thread::sleep(dr_pipeline::history::COALESCE_WINDOW);
session.set_crop(CropRect {
x: 0.0,
y: 0.0,
width: 0.12,
height: 0.12,
});
let names = session
.notice_hidden_masks(&before)
.map(|n| n.names().to_vec())
.expect("the radial was stranded");
assert_eq!(names, vec!["radial".to_string()]);
assert!(session.crop_notice().is_some());
// The crop was applied, not refused.
assert!(session.crop().width < 0.2);
// One undo: the crop goes back and the notice goes with it.
assert!(session.undo());
assert!((session.crop().width - 0.8).abs() < 1e-6);
assert!(session.crop_notice().is_none());
}
}
+5
View File
@@ -107,6 +107,10 @@ pub struct DevelopSession {
/// comparison is held. Viewing state: nothing about the edit changes,
/// and it goes down with the session.
pub(super) compared_snapshot: Option<String>,
/// TRACES: FR-DEV-17
/// The layers the last committed crop took out of the frame, while the
/// history still stands on that crop. See [`super::framing::CropNotice`].
pub(super) crop_notice: Option<super::framing::CropNotice>,
pub(super) demosaiced: Arc<DemosaicedImage>,
pub(super) adjust: AdjustPass,
/// TRACES: FR-DSP-7
@@ -373,6 +377,7 @@ impl DevelopSession {
snapshots: Vec::new(),
removed_snapshots: Vec::new(),
compared_snapshot: None,
crop_notice: None,
demosaiced: Arc::new(demosaiced),
adjust: AdjustPass::new(ctx),
histogram: HistogramPass::new(ctx)
+62
View File
@@ -968,6 +968,47 @@ fn wire_zoom_pan_crop(
redraw(&w);
});
}
{
// TRACES: FR-DEV-17
// The end of a crop gesture, carrying the rect it was pressed from.
// Measured here and never in `on_crop_changed`: the drag reports every
// frame, and a handle passing over a mask on its way somewhere else
// is not an event.
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window
.global::<Framing>()
.on_crop_released(move |x, y, width, height| {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
let mut before = s.framing();
before.set_crop(dr_pipeline::CropRect {
x,
y,
width,
height,
});
s.notice_hidden_masks(&before);
}
redraw(&w);
});
}
{
// TRACES: FR-DEV-17
// Keep the crop. The only thing to do is stop saying so: the crop
// was applied when it was let go and has been in the edit since.
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.global::<Framing>().on_hidden_masks_kept(move || {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.dismiss_crop_notice();
}
redraw(&w);
});
}
{
// TRACES: FR-DEV-3
// Choosing a ratio reshapes the crop there and then rather than
@@ -1017,6 +1058,27 @@ fn wire_zoom_pan_crop(
}
}
/// TRACES: FR-DEV-17
/// Mirror the session's crop notice into the window: how many mask layers the
/// crop on top of the history left outside the frame, and which.
///
/// Called on every redraw, which is every path that can move the history —
/// so the notice goes the frame an undo takes its crop back. Written only
/// when it differs, since a drag redraws sixty times a second.
pub(crate) fn sync_crop_notice(window: &AppWindow, s: &DevelopSession) {
let framing = window.global::<Framing>();
let (count, names) = match s.crop_notice() {
Some(n) => (n.names().len() as i32, n.names().join(", ")),
None => (0, String::new()),
};
if framing.get_hidden_masks() != count {
framing.set_hidden_masks(count);
}
if framing.get_hidden_mask_names().as_str() != names {
framing.set_hidden_mask_names(names.into());
}
}
/// ---- rotation, flips and straightening -------------------------------
///
/// Framing edits, so unlike zoom and pan they mark the image modified — but
+10
View File
@@ -607,7 +607,12 @@ fn apply_crop_aspect(
portrait: &Rc<Cell<bool>>,
) {
let Some(s) = session.borrow_mut().as_mut().map(|s| {
let before = s.framing();
s.set_crop_locked(s.crop(), aspect.get(), portrait.get(), (0.5, 0.5));
// TRACES: FR-DEV-17
// A ratio chosen is a crop committed in one click — there is no drag
// to wait for the end of — so it is measured here, as a release is.
s.notice_hidden_masks(&before);
(s.crop(), s.framing_edits_image())
}) else {
return;
@@ -2198,6 +2203,11 @@ fn build_render_now(
steps.set_rows(slint::ModelRc::new(slint::VecModel::from(s.history_rows())));
steps.set_undo_label(s.undo_label().into());
}
// TRACES: FR-DEV-17
// The notice about a crop that stranded a mask, on the path every
// history move takes, so an undo that takes the crop back takes
// the notice with it in the same frame.
develop_ui::sync_crop_notice(window, s);
// TRACES: FR-DEV-5
// The snapshots, on the same path. Rebuilt only when the list
// would read differently, for the reason the steps are: a held
+16
View File
@@ -679,6 +679,22 @@ export global Framing {
callback portrait-toggled();
/// Crop, angle, rotation and flips back to neutral, leaving colour alone.
callback reset();
/// TRACES: FR-DEV-17
/// A crop gesture was let go; the rect it was pressed from. See
/// `CropOverlay.crop-released`.
callback crop-released(float, float, float, float);
/// TRACES: FR-DEV-17
/// How many mask layers the crop just let go took out of the frame, and
/// their names, already joined. Zero, and the notice is not drawn — the
/// common case says nothing.
///
/// Rust clears it when the history moves off that crop, so an undo takes
/// the notice away with the crop it was about.
in property <int> hidden-masks: 0;
in property <string> hidden-mask-names: "";
/// Keep the crop anyway: the notice has been read.
callback hidden-masks-kept();
}
// Crop, rotation, flips and straightening — the framing controls.
+61
View File
@@ -2024,6 +2024,9 @@ in property <bool> panel-visible: true;
crop-changed(x, y, w, h, hx, hy) => {
root.crop-changed(x, y, w, h, hx, hy);
}
crop-released(x, y, w, h) => {
Framing.crop-released(x, y, w, h);
}
}
// --- gradient handles ---------------------------------------
@@ -2545,6 +2548,64 @@ in property <bool> panel-visible: true;
pick(i) => { Library.library-roll-pick(i); }
}
// --- a crop that stranded a mask (FR-DEV-17) ----------------
//
// TRACES: FR-DEV-17
// A notice, not a dialog. The crop is already applied and
// nothing waits on an answer: the photograph can go on
// being edited with this up, and the notice goes the
// moment the history moves off the crop it is about —
// Rust clears it — so ignoring it is keeping the crop.
//
// "Undo crop" is the ordinary undo, not a second route
// back. The crop is one step, so taking it back is that
// step, and the notice leaves with it rather than needing
// a step of its own.
//
// At the top of the canvas and last in it, so it draws
// over the photograph and clear of the roll's band at the
// foot, which takes every press that lands in it.
if Framing.hidden-masks > 0 && root.total > 0: Rectangle {
x: (parent.width - self.width) / 2;
y: Theme.gap;
width: min(parent.width - 2 * Theme.gap, notice-row.preferred-width);
height: notice-row.preferred-height;
background: Theme.surface;
border-radius: 6px;
border-width: 1px;
border-color: Theme.rule;
// Swallow presses on the notice's own background, so
// a click that misses a button does not pan or paint
// the photograph underneath.
TouchArea { }
notice-row := HorizontalLayout {
padding: Theme.gap;
spacing: Theme.gap;
Caption {
text: (Framing.hidden-masks == 1
? "This crop leaves 1 mask outside the frame: "
: "This crop leaves " + Framing.hidden-masks
+ " masks outside the frame: ")
+ Framing.hidden-mask-names;
warn: true;
vertical-alignment: center;
overflow: elide;
horizontal-stretch: 1;
}
Button {
text: "Undo crop";
clicked => { Steps.undo(); }
}
Button {
text: "Keep crop";
primary: true;
clicked => { Framing.hidden-masks-kept(); }
}
}
}
// Report size changes so the render target can be resized to
// match. Width and height are tracked separately because
// Slint has no single "geometry changed" hook.
+20
View File
@@ -48,6 +48,14 @@ export component CropOverlay inherits Rectangle {
/// written here — Rust takes the opposite corner.
callback crop-changed(float, float, float, float, float, float);
/// TRACES: FR-DEV-17
/// A drag of the rect or a handle was let go. Carries the rect as it was
/// when the press began, so Rust can measure what the whole gesture did.
///
/// The one boundary a crop gesture reports. `crop-changed` fires on
/// every pointer move and a handle passing over a mask on its way
/// somewhere else is not an event; only the release is a decision.
callback crop-released(float, float, float, float);
property <length> rx: root.crop-x * self.width;
property <length> ry: root.crop-y * self.height;
@@ -135,6 +143,8 @@ export component CropOverlay inherits Rectangle {
property <float> start-x;
property <float> start-y;
property <float> start-w;
property <float> start-h;
property <float> from-x;
property <float> from-y;
@@ -149,9 +159,16 @@ export component CropOverlay inherits Rectangle {
if (ev.kind == PointerEventKind.down) {
self.start-x = root.crop-x;
self.start-y = root.crop-y;
self.start-w = root.crop-w;
self.start-h = root.crop-h;
self.from-x = self.fraction-x(self.pressed-x);
self.from-y = self.fraction-y(self.pressed-y);
}
if (ev.kind == PointerEventKind.up || ev.kind == PointerEventKind.cancel) {
// The extent too, not only the origin: Rust clamps a rect
// moved past an edge, and that can narrow it.
root.crop-released(self.start-x, self.start-y, self.start-w, self.start-h);
}
}
moved => {
@@ -224,6 +241,9 @@ export component CropOverlay inherits Rectangle {
self.from-x = self.fraction-x(self.pressed-x);
self.from-y = self.fraction-y(self.pressed-y);
}
if (ev.kind == PointerEventKind.up || ev.kind == PointerEventKind.cancel) {
root.crop-released(self.ox, self.oy, self.ow, self.oh);
}
}
// Movement as a fraction of the frame, live while