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:
@@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user