Object Selection: New, Add, Subtract and Intersect mode buttons (#12853) (#13195)

* Object Selection: New, Add, Subtract and Intersect mode buttons (#12853)

The options bar shows the four mode buttons before Object Finder. Clicks and
rectangle drags combine in the bar's mode, with Shift adding, Alt subtracting
and Shift+Alt intersecting (rectangle drags used to subtract with Shift+Alt).
In New mode a click inside the selection deselects and a drag inside moves
the outline, as the marquees do.

* Object Selection: Cmd stays the Move tool inside the selection; update tests for New-mode Deselect

* Object Selection mode tests: a Cmd-drag floats the pixels and lands as Move on the drop

* Object Selection: keep Object Finder off's Quick Selection dab and rectangle

A press inside the selection drags the outline only with Object Finder on;
with it off the click is still a Quick Selection dab and the drag a
rectangle. The dab subtracts with Alt again, including Shift+Alt.

* Object Selection: keep the Indexed/Bitmap alert for a press inside the selection

The New-mode outline gesture skipped finish_object_selection, so a click
deselected and a drag moved the outline without the tool's alert (#12867).

crates/ui-egui/src/canvas/tool_feedback.rs

@@ -64,16 +64,8 @@ pub fn selection_mode(tool: Tool, bar: u8, m: egui::Modifiers) -> &'static str {
                 "add"
             }
         }
-        // Object Selection: ⌥ subtracts, ⇧ adds (see `retouch_ui::finish_object_selection`).
-        Tool::ObjectSelection => {
-            if m.alt {
-                "subtract"
-            } else if m.shift {
-                "add"
-            } else {
-                "replace"
-            }
-        }
+        // Every other selection tool, Object Selection included: ⇧⌥ intersects, ⇧ adds, ⌥
+        // subtracts, otherwise the options-bar mode (`retouch_ui::finish_object_selection`).
         _ => {
             if m.shift && m.alt {
                 "intersect"
@@ -290,8 +282,12 @@ mod tests {
         // Smart selection tools.
         assert_eq!(badge(&app, Tool::QuickSelection, none), Some(Badge::Add));
         assert_eq!(badge(&app, Tool::QuickSelection, alt), Some(Badge::Subtract));
+        // Object Selection follows the options bar like the other selection tools (#12853).
+        assert_eq!(badge(&app, Tool::ObjectSelection, none), Some(Badge::Intersect));
+        app.ui.selection_mode = 0;
         assert_eq!(badge(&app, Tool::ObjectSelection, none), None);
         assert_eq!(badge(&app, Tool::ObjectSelection, shift), Some(Badge::Add));
+        assert_eq!(badge(&app, Tool::ObjectSelection, both), Some(Badge::Intersect));
         // Other tools never show one.
         for t in [Tool::Brush, Tool::Move, Tool::Zoom, Tool::Crop] {
             assert_eq!(badge(&app, t, shift), None);

crates/ui-egui/src/canvas/tool_input.rs

@@ -891,7 +891,21 @@ pub fn selection_drag_kind(app: &PhotocraftApp, tool: Tool, p: [f64; 2], mods: e
     // Click-driven selection tools (a press places a point or samples): only ⌘ drags the
     // selection, and not while a polygon is being drawn.
     let clicky = matches!(tool, Tool::PolygonLasso | Tool::MagicWand);
-    let marquee = matches!(tool, Tool::RectMarquee | Tool::EllipseMarquee | Tool::SingleRowMarquee | Tool::SingleColumnMarquee);
+    // Object Selection with Object Finder on drags the outline like a marquee in New mode (#12853).
+    // With Object Finder off a press stays a Quick Selection dab or a rectangle, as before (the
+    // issue measured only Finder on). Unlike the marquees, ⌘ with it is the Move tool
+    // (`hold_keys::cmd_moves`), which moves the pixels itself: a ⌘ press must not start a cut here
+    // first.
+    let marquee = matches!(tool, Tool::RectMarquee | Tool::EllipseMarquee | Tool::SingleRowMarquee | Tool::SingleColumnMarquee)
+        || (tool == Tool::ObjectSelection && app.ui.tool_options.object_finder);
+    if tool == Tool::ObjectSelection && mods.command {
+        return None;
+    }
+    // In Indexed Color and Bitmap the press must reach `finish_object_selection` for the tool's
+    // alert (#12867) rather than deselect or move the outline.
+    if tool == Tool::ObjectSelection && app.session.active().is_some_and(|st| photocraft_engine::smartselect_cmds::object_mode_refusal(st.doc.mode).is_some()) {
+        return None;
+    }
     if !(clicky || marquee || tool == Tool::Lasso) || (clicky && !app.ui.polygon.is_empty()) {
         return None;
     }

crates/ui-egui/src/marquee_tests.rs

@@ -188,6 +188,8 @@ fn object_selection_keeps_the_mode_from_the_mouse_press() {
                 let sel = app.session.active().unwrap().doc.selection.as_ref().expect("the old selection survives outside the object");
                 assert!(sel.sample_channel(20, 20, 0) > 0.99, "the gesture must preserve selection outside the object");
                 assert_eq!(sel.sample_channel(220, 150, 0) > 0.5, !at_press.alt, "the disc follows the requested mode");
+                // From no selection: a New-mode click inside the selection deselects (#12853).
+                h.state_mut().run("select.deselect", json!({})).unwrap();
                 mods(&mut h, Modifiers::NONE);
                 move_to(&mut h, 220.0, 150.0);
                 let next = screen(&h, 220.0, 150.0);

crates/ui-egui/src/tools.rs

@@ -63,6 +63,9 @@ pub mod zoom_tool;
 #[cfg(test)]
 mod gradient_mode_greyed_tests;
 
+#[cfg(test)]
+mod object_selection_mode_tests;
+
 #[cfg(test)]
 mod count_show_extras_tests;
 #[cfg(test)]

crates/ui-egui/src/tools/object_selection_mode_tests.rs

@@ -0,0 +1,230 @@
+//! Object Selection's New / Add / Subtract / Intersect buttons (#12853), as measured in the
+//! reference application: the bar's mode (or ⇧ / ⌥ / ⇧⌥) combines each Object Finder click and
+//! rectangle drag with the selection; in New mode a press inside the selection deselects (click)
+//! or moves the outline (drag); ⌘ moves the pixels.
+
+use egui::Modifiers;
+use serde_json::json;
+
+use crate::PhotocraftApp;
+use crate::canvas::{ToolEvent, tool_event};
+use crate::state::Tool;
+use crate::tool_feedback::{Badge, badge};
+
+const RED: [f64; 2] = [200.0, 180.0];
+const BLUE: [f64; 2] = [560.0, 300.0];
+
+/// An 800 x 600 document with an opaque red block (80,80–320,280) and a blue disc (centre
+/// 560,300, radius 120) on one layer, nothing selected, Object Selection with Object Finder on.
+fn app() -> PhotocraftApp {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({"width": 800, "height": 600})).unwrap();
+    app.sync_views();
+    app.run("layer.new.layer", json!({})).unwrap();
+    app.run("select.rect", json!({"x": 80, "y": 80, "width": 240, "height": 200})).unwrap();
+    app.run("edit.fill", json!({"contents": "color", "color": "#ff0000"})).unwrap();
+    app.run("select.rect", json!({"x": 440, "y": 180, "width": 240, "height": 240, "ellipse": true})).unwrap();
+    app.run("edit.fill", json!({"contents": "color", "color": "#0000ff"})).unwrap();
+    app.run("select.deselect", json!({})).unwrap();
+    app.ui.tool = Tool::ObjectSelection;
+    app.ui.tool_options.object_finder = true;
+    app
+}
+
+fn click(app: &mut PhotocraftApp, p: [f64; 2], mods: Modifiers) {
+    tool_event(app, ToolEvent::Down { x: p[0], y: p[1], pressure: 1.0 }, mods);
+    tool_event(app, ToolEvent::Up { x: p[0], y: p[1] }, mods);
+}
+
+fn drag(app: &mut PhotocraftApp, from: [f64; 2], to: [f64; 2], mods: Modifiers) {
+    tool_event(app, ToolEvent::Down { x: from[0], y: from[1], pressure: 1.0 }, mods);
+    tool_event(app, ToolEvent::Move { x: (from[0] + to[0]) / 2.0, y: (from[1] + to[1]) / 2.0, pressure: 1.0 }, mods);
+    tool_event(app, ToolEvent::Move { x: to[0], y: to[1], pressure: 1.0 }, mods);
+    tool_event(app, ToolEvent::Up { x: to[0], y: to[1] }, mods);
+}
+
+fn selected(app: &PhotocraftApp, p: [f64; 2]) -> bool {
+    app.session.active().and_then(|st| st.doc.selection.as_ref()).is_some_and(|s| s.sample_channel(p[0] as i32, p[1] as i32, 0) >= 0.5)
+}
+
+fn any_selection(app: &PhotocraftApp) -> bool {
+    app.session.active().and_then(|st| st.doc.selection.as_ref()).is_some_and(|s| !s.content_bounds().is_empty())
+}
+
+fn last_step(app: &PhotocraftApp) -> Option<String> {
+    app.session.active()?.history.entries().last().map(ToString::to_string)
+}
+
+fn alpha_at(app: &PhotocraftApp, x: i32, y: i32) -> f32 {
+    let st = app.session.active().unwrap();
+    st.doc.layer(st.active_layer.unwrap()).unwrap().surface().unwrap().pixel(x, y)[3]
+}
+
+#[test]
+fn add_mode_clicks_accumulate() {
+    let mut app = app();
+    app.ui.selection_mode = 1;
+    click(&mut app, RED, Modifiers::NONE);
+    click(&mut app, BLUE, Modifiers::NONE);
+    assert!(selected(&app, RED) && selected(&app, BLUE));
+}
+
+#[test]
+fn subtract_mode_removes_the_clicked_object() {
+    let mut app = app();
+    app.ui.selection_mode = 1;
+    click(&mut app, RED, Modifiers::NONE);
+    click(&mut app, BLUE, Modifiers::NONE);
+    app.ui.selection_mode = 2;
+    click(&mut app, RED, Modifiers::NONE);
+    assert!(!selected(&app, RED), "red is removed");
+    assert!(selected(&app, BLUE), "blue stays");
+}
+
+#[test]
+fn intersect_mode_keeps_only_the_overlap() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    app.ui.selection_mode = 3;
+    click(&mut app, RED, Modifiers::NONE);
+    assert!(selected(&app, RED), "red with red is red");
+    click(&mut app, BLUE, Modifiers::NONE);
+    assert!(!any_selection(&app), "red with blue is empty");
+    assert!(!app.ui.status_error && app.ui.dialogs.is_empty(), "an empty intersection is not an error");
+}
+
+#[test]
+fn new_mode_press_inside_the_selection_deselects_or_moves_the_outline() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    assert!(selected(&app, RED));
+    click(&mut app, RED, Modifiers::NONE);
+    assert!(!any_selection(&app));
+    assert_eq!(last_step(&app).as_deref(), Some("Deselect"));
+
+    click(&mut app, RED, Modifiers::NONE);
+    drag(&mut app, RED, [250.0, 220.0], Modifiers::NONE);
+    assert_eq!(last_step(&app).as_deref(), Some("Move Selection"));
+    assert!(selected(&app, [340.0, 300.0]) && !selected(&app, [100.0, 100.0]), "the outline moved by +50,+40");
+    assert_eq!(alpha_at(&app, 100, 100), 1.0, "the pixels stay");
+    assert_eq!(alpha_at(&app, 340, 300), 0.0);
+
+    // Outside the selection a click replaces it.
+    click(&mut app, BLUE, Modifiers::NONE);
+    assert!(selected(&app, BLUE) && !selected(&app, [340.0, 300.0]));
+}
+
+#[test]
+fn the_mode_stays_new_after_a_selection() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    assert_eq!(app.ui.selection_mode, 0);
+}
+
+#[test]
+fn modifiers_override_the_bar() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    click(&mut app, BLUE, Modifiers::SHIFT);
+    assert!(selected(&app, RED) && selected(&app, BLUE), "Shift adds");
+    click(&mut app, RED, Modifiers::ALT);
+    assert!(!selected(&app, RED) && selected(&app, BLUE), "Alt subtracts");
+
+    // Shift inside the selection in New mode adds rather than deselecting.
+    click(&mut app, BLUE, Modifiers::SHIFT);
+    assert!(selected(&app, BLUE));
+
+    // Shift+Alt rectangle drag intersects: both selected, a rectangle around red leaves red.
+    click(&mut app, RED, Modifiers::SHIFT);
+    assert!(selected(&app, RED) && selected(&app, BLUE));
+    let both = Modifiers { shift: true, alt: true, ..Default::default() };
+    drag(&mut app, [60.0, 60.0], [340.0, 300.0], both);
+    assert!(selected(&app, RED), "red is in both");
+    assert!(!selected(&app, BLUE), "blue is outside the rectangle");
+}
+
+#[test]
+fn the_badge_follows_the_bar_and_modifiers() {
+    let mut app = app();
+    let both = Modifiers { shift: true, alt: true, ..Default::default() };
+    for (mode, want) in [(0, None), (1, Some(Badge::Add)), (2, Some(Badge::Subtract)), (3, Some(Badge::Intersect))] {
+        app.ui.selection_mode = mode;
+        assert_eq!(badge(&app, Tool::ObjectSelection, Modifiers::NONE), want, "bar mode {mode}");
+    }
+    app.ui.selection_mode = 0;
+    assert_eq!(badge(&app, Tool::ObjectSelection, both), Some(Badge::Intersect));
+}
+
+#[test]
+fn command_drag_inside_the_selection_moves_the_pixels() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    drag(&mut app, RED, [250.0, 220.0], Modifiers::COMMAND);
+    // The pixels float at the pointer (as with a marquee's ⌘-drag) and land as "Move" on the drop.
+    let offset = photocraft_engine::float_cmds::floating(app.session.active().unwrap()).map(|f| f.offset);
+    assert_eq!(offset, Some((50, 40)), "the selected pixels follow the drag");
+    crate::menus::invoke(&mut app, &egui::Context::default(), "select.deselect", json!({})).unwrap();
+    let labels: Vec<String> = app.session.active().unwrap().history.entries().into_iter().map(|e| e.to_string()).collect();
+    assert!(labels.iter().any(|l| l == "Move"), "{labels:?}");
+    assert_eq!(alpha_at(&app, 100, 100), 0.0, "the pixels moved away");
+    assert_eq!(alpha_at(&app, 340, 300), 1.0);
+}
+
+#[test]
+fn reset_tool_sets_the_mode_back_to_new() {
+    let ctx = egui::Context::default();
+    let mut app = app();
+    app.ui.selection_mode = 3;
+    crate::menus::invoke(&mut app, &ctx, "tool.reset", json!({})).unwrap();
+    assert_eq!(app.ui.selection_mode, 0);
+}
+
+#[test]
+fn object_finder_off_keeps_quick_selection_and_the_rectangle_inside_the_selection() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    app.ui.tool_options.object_finder = false;
+    // A New-mode click inside the selection is still a Quick Selection dab, not Deselect.
+    click(&mut app, RED, Modifiers::NONE);
+    assert_eq!(app.session.journal.last().map(|(id, _)| id.as_str()), Some("select.quick"));
+    assert!(selected(&app, RED));
+    // A drag inside it is still a rectangle Object Selection, not Move Selection.
+    drag(&mut app, [100.0, 100.0], [300.0, 260.0], Modifiers::NONE);
+    assert_eq!(app.session.journal.last().map(|(id, _)| id.as_str()), Some("select.object"));
+    assert_ne!(last_step(&app).as_deref(), Some("Move Selection"));
+}
+
+#[test]
+fn object_finder_off_dab_subtracts_with_alt_and_in_subtract_mode() {
+    let mut app = app();
+    app.ui.tool_options.object_finder = false;
+    let shift_alt = Modifiers { shift: true, alt: true, ..Default::default() };
+    for (bar, mods) in [(0, Modifiers::ALT), (0, shift_alt), (2, Modifiers::NONE)] {
+        app.run("select.all", json!({})).unwrap();
+        app.ui.selection_mode = bar;
+        click(&mut app, RED, mods);
+        assert!(!selected(&app, RED), "bar mode {bar}, {mods:?} subtracts");
+        assert!(selected(&app, [20.0, 20.0]), "bar mode {bar}, {mods:?} keeps the rest");
+    }
+}
+
+#[test]
+fn indexed_color_alerts_instead_of_deselecting_or_moving_the_outline() {
+    let mut app = app();
+    click(&mut app, RED, Modifiers::NONE);
+    app.run("image.mode.indexedColor", json!({"palette": "exact", "forced": "none", "dither": "none"})).unwrap();
+    assert!(selected(&app, RED));
+    let steps = app.session.active().unwrap().history.past_len();
+    let errors = |app: &PhotocraftApp| app.ui.dialogs.iter().filter(|d| d.kind == crate::state::DialogKind::Error).count();
+    for gesture in 0..2 {
+        if gesture == 0 {
+            click(&mut app, RED, Modifiers::NONE);
+        } else {
+            drag(&mut app, RED, [250.0, 220.0], Modifiers::NONE);
+        }
+        assert_eq!(errors(&app), 1, "gesture {gesture}: the tool's alert");
+        assert!(selected(&app, RED) && !selected(&app, [340.0, 300.0]), "gesture {gesture}: the selection stays");
+        assert_eq!(app.session.active().unwrap().history.past_len(), steps, "gesture {gesture}: no history step");
+        app.ui.dialogs.clear();
+    }
+}

crates/ui-egui/src/tools/retouch_ui.rs

@@ -619,10 +619,10 @@ pub fn content_aware_move_params(app: &PhotocraftApp, offset: [i64; 2]) -> Value
 
 /// Object Selection: the dragged rectangle. A click (or a sub-2 px nudge) cannot be a
 /// rectangle: with Object Finder on it selects the object under the pointer, the one hovering
-/// highlighted (#4087; ⇧ adds, ⌥ subtracts, ⇧⌥ intersects, otherwise replaces). With it off it
-/// falls back to a Quick Selection dab at
-/// the brush size instead of looking dead (⌥ subtracts, otherwise it adds). Larger drags
-/// run the rectangle GrabCut (⌥ subtracts, ⇧ adds, otherwise replaces).
+/// highlighted (#4087). With it off it falls back to a Quick Selection dab at the brush size
+/// instead of looking dead (⌥ or Subtract mode subtracts, otherwise it adds). Larger drags
+/// run the rectangle GrabCut. Both combine as the options bar's New / Add / Subtract /
+/// Intersect buttons say, with ⇧ adding, ⌥ subtracting and ⇧⌥ intersecting (#12853).
 pub fn finish_object_selection(app: &mut PhotocraftApp, start: [f64; 2], end: [f64; 2], mods: egui::Modifiers) {
     // Indexed Color and Bitmap: the reference application's alert with OK (#12867), and nothing runs.
     if let Some(why) = app.session.active().and_then(|st| photocraft_engine::smartselect_cmds::object_mode_refusal(st.doc.mode)) {
@@ -633,14 +633,9 @@ pub fn finish_object_selection(app: &mut PhotocraftApp, start: [f64; 2], end: [f
     let (x, y) = (start[0].min(end[0]), start[1].min(end[1]));
     let (w, h) = ((end[0] - start[0]).abs(), (end[1] - start[1]).abs());
     let sample_all = app.ui.tool_options.sample_all.object_selection;
+    let mode = crate::tool_feedback::selection_mode(Tool::ObjectSelection, app.ui.selection_mode, mods);
     if (w < 2.0 || h < 2.0) && app.ui.tool_options.object_finder {
         // Object Finder (#4087): the object under the click, as hovering highlighted it.
-        let mode = match (mods.shift, mods.alt) {
-            (true, true) => "intersect",
-            (true, false) => "add",
-            (false, true) => "subtract",
-            (false, false) => "replace",
-        };
         let r = app.run("select.object", json!({"point": [end[0], end[1]], "mode": mode, "sampleAllLayers": sample_all}));
         crate::object_finder::clear(app);
         report_smart_result(app, r, tl!("No object found there — drag a rectangle around it"));
@@ -648,18 +643,11 @@ pub fn finish_object_selection(app: &mut PhotocraftApp, start: [f64; 2], end: [f
     }
     if w < 2.0 || h < 2.0 {
         let size = app.session.tools.brush.size;
-        let mode = if mods.alt { "subtract" } else { "add" };
+        let mode = if mods.alt || mode == "subtract" { "subtract" } else { "add" };
         let r = app.run("select.quick", json!({"points": [[end[0], end[1]]], "size": size, "mode": mode, "sampleAllLayers": sample_all}));
         report_smart_result(app, r, tl!("Quick Selection found nothing there"));
         return;
     }
-    let mode = if mods.alt {
-        "subtract"
-    } else if mods.shift {
-        "add"
-    } else {
-        "replace"
-    };
     let r = app.run("select.object", json!({"rect": [x.round(), y.round(), w.round(), h.round()], "mode": mode, "sampleAllLayers": sample_all}));
     report_smart_result(app, r, tl!("Object Selection found nothing there — try a tighter rectangle"));
 }
@@ -1154,6 +1142,8 @@ pub fn options_bar(app: &mut PhotocraftApp, ui: &mut egui::Ui, tool: Tool) -> bo
             }
         }
         Tool::ObjectSelection => {
+            crate::panels::selection_mode_buttons(&mut app.ui.selection_mode, ui);
+            crate::widgets::vline(ui, 22.0);
             crate::widgets::checkbox(ui, &mut o.object_finder, tl!("Object Finder"));
             crate::widgets::checkbox(ui, &mut o.sample_all.object_selection, tl!("Sample All Layers"));
             opt(ui, tl!("Drag a rectangle around the object"));