From aa6f0d1b73205dbaa833c2e25c033ea596346d0e Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Thu, 12 Nov 2026 22:30:25 +0530 Subject: [PATCH] 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 | 16 +- crates/ui-egui/src/canvas/tool_input.rs | 16 +- crates/ui-egui/src/marquee_tests.rs | 2 + crates/ui-egui/src/tools.rs | 3 + .../src/tools/object_selection_mode_tests.rs | 230 ++++++++++++++++++ crates/ui-egui/src/tools/retouch_ui.rs | 26 +- 6 files changed, 264 insertions(+), 29 deletions(-) create mode 100644 crates/ui-egui/src/tools/object_selection_mode_tests.rs diff --git a/crates/ui-egui/src/canvas/tool_feedback.rs b/crates/ui-egui/src/canvas/tool_feedback.rs index 88aec042e5..ba7bf35cf6 100644 --- a/crates/ui-egui/src/canvas/tool_feedback.rs +++ b/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); diff --git a/crates/ui-egui/src/canvas/tool_input.rs b/crates/ui-egui/src/canvas/tool_input.rs index b3b4c42544..f84867326a 100644 --- a/crates/ui-egui/src/canvas/tool_input.rs +++ b/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; } diff --git a/crates/ui-egui/src/marquee_tests.rs b/crates/ui-egui/src/marquee_tests.rs index 5a42aab0fc..d0087e55e3 100644 --- a/crates/ui-egui/src/marquee_tests.rs +++ b/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); diff --git a/crates/ui-egui/src/tools.rs b/crates/ui-egui/src/tools.rs index 684c8dbc32..41ef241233 100644 --- a/crates/ui-egui/src/tools.rs +++ b/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)] diff --git a/crates/ui-egui/src/tools/object_selection_mode_tests.rs b/crates/ui-egui/src/tools/object_selection_mode_tests.rs new file mode 100644 index 0000000000..53be1b163f --- /dev/null +++ b/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 { + 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 = 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(); + } +} diff --git a/crates/ui-egui/src/tools/retouch_ui.rs b/crates/ui-egui/src/tools/retouch_ui.rs index 68abe3dfae..06cca4f683 100644 --- a/crates/ui-egui/src/tools/retouch_ui.rs +++ b/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"));