Merge branch 'worktree-agent-a22a049c461818dbe' into integration
# Conflicts: # core/dr-pipeline/tests/mask_sidecar.rs
This commit is contained in:
+101
-34
@@ -24,6 +24,7 @@ mod collections_ui;
|
||||
mod derived_sync;
|
||||
mod develop;
|
||||
mod export;
|
||||
mod gradient;
|
||||
mod histogram;
|
||||
mod labels;
|
||||
mod library;
|
||||
@@ -257,11 +258,12 @@ fn is_supported(p: &Path) -> bool {
|
||||
|
||||
/// Return the view to its opening state for a newly loaded image.
|
||||
///
|
||||
/// Zoom and crop mode are properties of *looking at one photograph*, so
|
||||
/// Zoom and the view mode are properties of *looking at one photograph*, so
|
||||
/// carrying them to the next one would leave the second image cropped to a
|
||||
/// rect chosen for the first.
|
||||
/// rect chosen for the first — or, since local masking became a mode, would
|
||||
/// open the next photograph with a mask stack it does not have.
|
||||
fn reset_view_state(window: &AppWindow) {
|
||||
window.set_crop_mode(false);
|
||||
window.set_view_mode(ViewMode::Photo);
|
||||
window.set_zoom(1.0);
|
||||
window.set_zoomed(false);
|
||||
window.set_crop_x(0.0);
|
||||
@@ -1126,7 +1128,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// Crop mode shows the whole frame, or the area being cropped away
|
||||
// would not be on screen for the handles to drag across. The
|
||||
// overlay draws the rect on top of it.
|
||||
let rendered = if window.get_crop_mode() {
|
||||
let rendered = if window.get_view_mode() == ViewMode::Crop {
|
||||
s.render_uncropped(w, h).map(|(image, _, _)| image)
|
||||
} else {
|
||||
s.render(w, h)
|
||||
@@ -1882,25 +1884,52 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
});
|
||||
}
|
||||
{
|
||||
// Entering crop mode drops the zoom: the handles are placed against
|
||||
// the whole frame, and a zoomed view would put most of that frame off
|
||||
// screen where it cannot be dragged.
|
||||
// TRACES: FR-UI-5
|
||||
// **Entering a mode is a side effect, which is why Rust owns it** and
|
||||
// the strip does not simply write the property. Each of the three has
|
||||
// work to do that the interface cannot see:
|
||||
//
|
||||
// *Crop* drops the zoom. The handles are placed against the whole
|
||||
// frame, and a zoomed view would put most of that frame off screen
|
||||
// where it cannot be dragged.
|
||||
//
|
||||
// *Local* turns the region overlay on. It used to be a button in the
|
||||
// masking panel, so the mode could be open with the overlay off —
|
||||
// which is a mode you have entered that is doing nothing.
|
||||
//
|
||||
// *Leaving* clears the selection, and that is the fault this whole
|
||||
// pass exists for: a selected layer silently re-points thirty sliders
|
||||
// at that layer's chain, so a mode you have left must not leave one
|
||||
// behind. After this the controls are unambiguously global again,
|
||||
// which is what the panel's heading then says.
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
window.on_crop_mode_toggled(move |on| {
|
||||
let rows = rows.clone();
|
||||
window.on_mode_picked(move |mode| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
if let Some(s) = session.borrow_mut().as_mut() {
|
||||
if on {
|
||||
s.reset_zoom();
|
||||
let c = s.crop();
|
||||
w.set_crop_x(c.x);
|
||||
w.set_crop_y(c.y);
|
||||
w.set_crop_w(c.width);
|
||||
w.set_crop_h(c.height);
|
||||
match mode {
|
||||
ViewMode::Crop => {
|
||||
s.reset_zoom();
|
||||
let c = s.crop();
|
||||
w.set_crop_x(c.x);
|
||||
w.set_crop_y(c.y);
|
||||
w.set_crop_w(c.width);
|
||||
w.set_crop_h(c.height);
|
||||
}
|
||||
ViewMode::Local => s.set_overlay(true),
|
||||
ViewMode::Photo => {
|
||||
s.set_overlay(false);
|
||||
s.set_active_mask(None);
|
||||
}
|
||||
}
|
||||
}
|
||||
w.set_crop_mode(on);
|
||||
w.set_view_mode(mode);
|
||||
masks_ui::sync(&w, &session);
|
||||
// The scope may have just changed, so the panel below is now
|
||||
// describing a different chain.
|
||||
sync_rows(&w, &rows, &session);
|
||||
redraw(&w);
|
||||
});
|
||||
}
|
||||
@@ -2267,7 +2296,7 @@ fn back_one_step(w: &AppWindow) -> bool {
|
||||
launch: w.get_show_launch(),
|
||||
browsing: w.get_launch_browsing(),
|
||||
library: w.get_show_library(),
|
||||
crop: w.get_crop_mode(),
|
||||
mode: w.get_view_mode(),
|
||||
zoomed: w.get_zoomed(),
|
||||
// Files named on the command line have no grid behind them — the same
|
||||
// condition the status strip uses to decide whether to offer the way
|
||||
@@ -2283,7 +2312,7 @@ fn back_one_step(w: &AppWindow) -> bool {
|
||||
match step {
|
||||
BackStep::CloseSettings => w.invoke_settings_close(),
|
||||
BackStep::CancelBrowse => w.invoke_launch_browse_cancel(),
|
||||
BackStep::LeaveCrop => w.invoke_crop_mode_toggled(false),
|
||||
BackStep::LeaveMode => w.invoke_mode_picked(ViewMode::Photo),
|
||||
BackStep::ResetZoom => w.invoke_zoom_reset(),
|
||||
BackStep::ToLibrary => w.invoke_back_to_library(),
|
||||
BackStep::ClearScope => w.invoke_collection_select(0),
|
||||
@@ -2297,13 +2326,21 @@ fn back_one_step(w: &AppWindow) -> bool {
|
||||
/// stated and tested without a Slint backend: which of two states is left first
|
||||
/// is the whole of this feature, and it is the part that is easy to get subtly
|
||||
/// wrong when it is spelled out in nested `if`s over live properties.
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
|
||||
// No `Eq`: `ViewMode` is generated by Slint and derives only `PartialEq`,
|
||||
// which is all the comparisons below need. `Default` still derives, and it
|
||||
// gives `mode` the enum's own first variant — `photo`, which is what "no mode"
|
||||
// means and what the tests below want as their baseline.
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq)]
|
||||
struct NavState {
|
||||
settings: bool,
|
||||
launch: bool,
|
||||
browsing: bool,
|
||||
library: bool,
|
||||
crop: bool,
|
||||
/// Which develop mode is on, if any. One field rather than one flag per
|
||||
/// mode, so "leave the innermost" cannot be asked of two at once — the
|
||||
/// ordering below would have had to invent an answer for a state the
|
||||
/// interface can no longer be in.
|
||||
mode: ViewMode,
|
||||
zoomed: bool,
|
||||
has_grid: bool,
|
||||
scoped: bool,
|
||||
@@ -2314,7 +2351,10 @@ struct NavState {
|
||||
enum BackStep {
|
||||
CloseSettings,
|
||||
CancelBrowse,
|
||||
LeaveCrop,
|
||||
/// Leave whichever develop mode is on — crop or local — and return to the
|
||||
/// whole photograph. One step for both, because there is one mode at a
|
||||
/// time and "back" means the same thing from either.
|
||||
LeaveMode,
|
||||
ResetZoom,
|
||||
ToLibrary,
|
||||
ClearScope,
|
||||
@@ -2334,10 +2374,15 @@ fn back_step(s: NavState) -> Option<BackStep> {
|
||||
}
|
||||
|
||||
if !s.library {
|
||||
// Develop. Crop is a mode and zoom is a view state; both are left
|
||||
// before the image is.
|
||||
if s.crop {
|
||||
return Some(BackStep::LeaveCrop);
|
||||
// Develop. Crop and local are modes and zoom is a view state; all are
|
||||
// left before the image is.
|
||||
//
|
||||
// The mode goes before the zoom because that is the order they were
|
||||
// entered in — a photographer zooms to place a mask, not the other way
|
||||
// about — and because leaving local mode drops the mask selection,
|
||||
// which is a bigger step back than returning to fit.
|
||||
if s.mode != ViewMode::Photo {
|
||||
return Some(BackStep::LeaveMode);
|
||||
}
|
||||
if s.zoomed {
|
||||
return Some(BackStep::ResetZoom);
|
||||
@@ -2381,7 +2426,7 @@ mod tests {
|
||||
|
||||
let from_develop = NavState {
|
||||
settings: true,
|
||||
crop: true,
|
||||
mode: ViewMode::Crop,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(from_develop), Some(BackStep::CloseSettings));
|
||||
@@ -2389,14 +2434,20 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn back_leaves_a_mode_before_it_leaves_the_image() {
|
||||
// Crop then zoom then the view: innermost first, because that is the
|
||||
// order they were entered in.
|
||||
let cropping = NavState {
|
||||
crop: true,
|
||||
zoomed: true,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(cropping), Some(BackStep::LeaveCrop));
|
||||
// A mode, then zoom, then the view: innermost first, because that is
|
||||
// the order they were entered in.
|
||||
for mode in [ViewMode::Crop, ViewMode::Local] {
|
||||
let in_mode = NavState {
|
||||
mode,
|
||||
zoomed: true,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(
|
||||
back_step(in_mode),
|
||||
Some(BackStep::LeaveMode),
|
||||
"{mode:?} must be left before the zoom is reset"
|
||||
);
|
||||
}
|
||||
|
||||
let zoomed = NavState {
|
||||
zoomed: true,
|
||||
@@ -2407,6 +2458,22 @@ mod tests {
|
||||
assert_eq!(back_step(developing()), Some(BackStep::ToLibrary));
|
||||
}
|
||||
|
||||
/// TRACES: FR-UI-5
|
||||
/// Local masking joins the existing order rather than inventing an exit.
|
||||
///
|
||||
/// It is the point of making it a mode: before this, back and Escape did
|
||||
/// nothing about a masking session, so the only way out of it was to find
|
||||
/// the two toggles that had armed it and press them again — and neither
|
||||
/// was anywhere near the photograph the user was looking at.
|
||||
#[test]
|
||||
fn local_masking_is_left_by_the_same_step_crop_is() {
|
||||
let masking = NavState {
|
||||
mode: ViewMode::Local,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(masking), Some(BackStep::LeaveMode));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_from_an_image_with_no_grid_behind_it_is_the_top_of_the_stack() {
|
||||
// Files named on the command line: there is no library to return to,
|
||||
|
||||
Reference in New Issue
Block a user