Move tool: Auto-Select on by default; a click picks and drags the layer under the pointer (#296) (#330)

crates/engine/src/pick_cmds.rs

@@ -15,6 +15,10 @@ fn has_doc(s: &Session) -> std::result::Result<(), String> {
 
 /// Layers with visible pixels at (x, y), topmost first (hidden layers and hidden groups skipped).
 pub fn layers_at(doc: &Document, x: i32, y: i32) -> Vec<LayerId> {
+    // A 1×1 read at i32::MAX would be an empty rect (huge `x` params saturate there).
+    if x.checked_add(1).is_none() || y.checked_add(1).is_none() {
+        return Vec::new();
+    }
     let rows = doc.walk();
     let mut out = Vec::new();
     for (path, _, l) in rows.iter().rev() {
@@ -67,7 +71,12 @@ fn pick(s: &mut Session, p: &Value) -> Result<Value> {
     let target = if p.get("target").and_then(Value::as_str) == Some("group") { top_group(&doc, hit) } else { hit };
     if p.get("select").and_then(Value::as_bool).unwrap_or(true) {
         let mode = p.get("mode").and_then(Value::as_str).unwrap_or("replace");
-        s.execute("layer.select", json!({"layer": target.0, "mode": mode}))?;
+        // Like Photoshop, a plain click on one of several selected layers keeps them all
+        // selected, so the drag that follows moves the whole selection.
+        let keeps = mode == "replace" && s.active().is_some_and(|st| st.is_layer_selected(target) && st.selected_layers().len() > 1);
+        if !keeps {
+            s.execute("layer.select", json!({"layer": target.0, "mode": mode}))?;
+        }
     }
     Ok(json!({ "layer": target.0 }))
 }
@@ -110,4 +119,69 @@ mod tests {
         s.execute("layer.setProps", json!({"layer": a.0, "visible": false})).unwrap();
         assert_eq!(s.execute("layer.pickAt", json!({"x": 5, "y": 5, "select": false})).unwrap()["layer"], bg.0);
     }
+
+    /// Two filled squares: A at (0..10), B at (20..30), both in their own layers.
+    fn two_squares() -> (Session, LayerId, LayerId) {
+        let mut s = Session::new();
+        s.execute("file.new", json!({"width": 40, "height": 40})).unwrap();
+        let mut ids = Vec::new();
+        for (name, x) in [("A", 0), ("B", 20)] {
+            s.execute("layer.new.layer", json!({"name": name})).unwrap();
+            s.execute("select.rect", json!({"x": x, "y": x, "width": 10, "height": 10})).unwrap();
+            s.execute("edit.fill", json!({"color": "#00ff00"})).unwrap();
+            ids.push(s.active().unwrap().active_layer.unwrap());
+        }
+        s.execute("select.deselect", json!({})).unwrap();
+        (s, ids[0], ids[1])
+    }
+
+    #[test]
+    fn group_target_selects_the_outermost_group() {
+        let (mut s, a, _) = two_squares();
+        s.execute("layer.select", json!({"layer": a.0})).unwrap();
+        let inner = s.execute("layer.new.groupFromLayers", json!({"name": "Inner"})).unwrap()["layer"].as_u64().unwrap();
+        let outer = s.execute("layer.new.groupFromLayers", json!({"name": "Outer"})).unwrap()["layer"].as_u64().unwrap();
+        assert_ne!(inner, outer);
+        assert_eq!(s.execute("layer.pickAt", json!({"x": 5, "y": 5, "target": "group"})).unwrap()["layer"], outer);
+        assert_eq!(s.active().unwrap().active_layer, Some(LayerId(outer)));
+        // Layer mode reaches through the groups to the pixels.
+        assert_eq!(s.execute("layer.pickAt", json!({"x": 5, "y": 5, "target": "layer"})).unwrap()["layer"], a.0);
+        // Hiding the outer group hides its layers from the pick.
+        s.execute("layer.setProps", json!({"layer": outer, "visible": false})).unwrap();
+        let bg = s.active().unwrap().doc.layers[0].id;
+        assert_eq!(s.execute("layer.pickAt", json!({"x": 5, "y": 5, "select": false})).unwrap()["layer"], bg.0);
+    }
+
+    #[test]
+    fn a_click_on_a_selected_layer_keeps_the_multi_selection() {
+        let (mut s, a, b) = two_squares();
+        s.execute("layer.select", json!({"layer": a.0})).unwrap();
+        s.execute("layer.select", json!({"layer": b.0, "mode": "add"})).unwrap();
+        assert_eq!(s.active().unwrap().selected_layers().len(), 2);
+        // Clicking A (selected) keeps both so the drag moves both.
+        s.execute("layer.pickAt", json!({"x": 5, "y": 5})).unwrap();
+        let st = s.active().unwrap();
+        assert!(st.is_layer_selected(a) && st.is_layer_selected(b));
+        // Clicking the Background (not selected) replaces the selection.
+        s.execute("layer.pickAt", json!({"x": 35, "y": 5})).unwrap();
+        assert_eq!(s.active().unwrap().selected_layers().len(), 1);
+        assert!(!s.active().unwrap().is_layer_selected(a));
+        // Shift-click (add) builds a selection up again.
+        s.execute("layer.pickAt", json!({"x": 5, "y": 5})).unwrap();
+        s.execute("layer.pickAt", json!({"x": 25, "y": 25, "mode": "add"})).unwrap();
+        let st = s.active().unwrap();
+        assert!(st.is_layer_selected(a) && st.is_layer_selected(b));
+    }
+
+    #[test]
+    fn hostile_params_are_errors_or_misses_not_panics() {
+        let (mut s, _, _) = two_squares();
+        assert!(s.execute("layer.pickAt", json!({})).is_err());
+        assert!(s.execute("layer.pickAt", json!({"x": "5", "y": 5})).is_err());
+        for (x, y) in [(-1e12, 5.0), (1e12, 1e12), (-0.5, -0.5), (40.0, 40.0)] {
+            assert_eq!(s.execute("layer.pickAt", json!({"x": x, "y": y})).unwrap()["layer"], Value::Null, "({x}, {y})");
+        }
+        let mut empty = Session::new();
+        assert!(empty.execute("layer.pickAt", json!({"x": 1, "y": 1})).is_err());
+    }
 }

crates/ui-egui/examples/layout_bench.rs

@@ -293,7 +293,7 @@ fn main() {
         (ms(t) + frame(&mut h)).max(frame(&mut h))
     });
 
-    // Move tool drags. Auto-Select off (Photoshop's default): the selected layer moves.
+    // Move tool drags. Auto-Select off: the selected layer moves.
     h.state_mut().ui.tool = Tool::Move;
     h.state_mut().ui.tool_options.move_auto_select = false;
     settle(&mut h, 3);

crates/ui-egui/src/lib.rs

@@ -1206,6 +1206,9 @@ mod input_tests;
 #[cfg(test)]
 mod pencil_tests;
 
+#[cfg(test)]
+mod move_auto_select_tests;
+
 #[cfg(test)]
 mod marquee_tests;
 

crates/ui-egui/src/move_auto_select_tests.rs

@@ -0,0 +1,127 @@
+//! Move tool Auto-Select (#296), as in Photoshop: on by default, a press picks the topmost
+//! visible layer with pixels under the pointer (or its outermost group in Group mode) and the drag
+//! then moves it; ⌘/Ctrl held at the press inverts the option for that click.
+
+use photocraft_doc::LayerId;
+use photocraft_geom::Rect;
+use serde_json::json;
+
+use crate::PhotocraftApp;
+use crate::canvas::ToolEvent;
+use crate::state::{Tool, ToolOptions};
+
+/// A 64×64 document with two painted layers: A covers (8..24)², B covers (40..56)². B is active.
+fn app() -> (PhotocraftApp, LayerId, LayerId) {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.session.execute("file.new", json!({"width": 64, "height": 64})).unwrap();
+    app.sync_views();
+    let mut ids = Vec::new();
+    for at in [8, 40] {
+        app.session.execute("layer.new.layer", json!({})).unwrap();
+        app.session
+            .edit("paint", |doc, a| {
+                doc.layer_mut(a.unwrap()).unwrap().surface_mut().unwrap().fill_rect(Rect::new(at, at, at + 16, at + 16), &[1.0, 0.0, 0.0, 1.0]);
+                Ok(())
+            })
+            .unwrap();
+        ids.push(app.session.active().unwrap().active_layer.unwrap());
+    }
+    app.ui.tool = Tool::Move;
+    // Snapping (to the document centre with a 1:1 test view) would shift the expected offsets.
+    app.ui.extras.snap = false;
+    app.ui.view.show.smart_guides = false;
+    (app, ids[0], ids[1])
+}
+
+fn bounds(app: &PhotocraftApp, id: LayerId) -> Rect {
+    app.session.active().unwrap().doc.layer(id).unwrap().surface().unwrap().content_bounds()
+}
+
+fn active(app: &PhotocraftApp) -> Option<LayerId> {
+    app.session.active().unwrap().active_layer
+}
+
+fn drag(app: &mut PhotocraftApp, from: [f64; 2], by: [f64; 2], mods: egui::Modifiers) {
+    crate::canvas::tool_event(app, ToolEvent::Down { x: from[0], y: from[1], pressure: 1.0 }, mods);
+    let mid = [from[0] + by[0] / 2.0, from[1] + by[1] / 2.0];
+    crate::canvas::tool_event(app, ToolEvent::Move { x: mid[0], y: mid[1], pressure: 1.0 }, mods);
+    let end = [from[0] + by[0], from[1] + by[1]];
+    crate::canvas::tool_event(app, ToolEvent::Move { x: end[0], y: end[1], pressure: 1.0 }, mods);
+    crate::canvas::tool_event(app, ToolEvent::Up { x: end[0], y: end[1] }, mods);
+}
+
+#[test]
+fn auto_select_is_on_by_default_with_the_layer_target() {
+    let o = ToolOptions::default();
+    assert!(o.move_auto_select, "Photoshop ships with Auto-Select checked");
+    assert_eq!(o.move_target, "layer");
+    // A saved tool preset that predates the option gets the default too.
+    let mut v = serde_json::to_value(&o).unwrap();
+    v.as_object_mut().unwrap().remove("move_auto_select");
+    let back: ToolOptions = serde_json::from_value(v).unwrap();
+    assert!(back.move_auto_select);
+}
+
+#[test]
+fn a_press_picks_the_layer_under_the_pointer_and_drags_it() {
+    let (mut app, a, b) = app();
+    assert_eq!(active(&app), Some(b));
+    drag(&mut app, [16.0, 16.0], [10.0, 4.0], egui::Modifiers::NONE);
+    assert_eq!(active(&app), Some(a), "the click selected A");
+    assert_eq!(bounds(&app, a), Rect::new(18, 12, 34, 28), "and the drag moved A");
+    assert_eq!(bounds(&app, b), Rect::new(40, 40, 56, 56), "B stays put");
+}
+
+#[test]
+fn command_click_suspends_auto_select_for_that_drag() {
+    let (mut app, a, b) = app();
+    drag(&mut app, [16.0, 16.0], [-10.0, 0.0], egui::Modifiers::COMMAND);
+    assert_eq!(active(&app), Some(b), "⌘ held: no pick");
+    assert_eq!(bounds(&app, b), Rect::new(30, 40, 46, 56), "the selected layer moved");
+    assert_eq!(bounds(&app, a), Rect::new(8, 8, 24, 24));
+}
+
+#[test]
+fn command_click_picks_when_auto_select_is_off() {
+    let (mut app, a, b) = app();
+    app.ui.tool_options.move_auto_select = false;
+    drag(&mut app, [16.0, 16.0], [0.0, 6.0], egui::Modifiers::NONE);
+    assert_eq!(active(&app), Some(b), "off: the selected layer moves");
+    assert_eq!(bounds(&app, a), Rect::new(8, 8, 24, 24));
+    drag(&mut app, [16.0, 16.0], [0.0, 6.0], egui::Modifiers::COMMAND);
+    assert_eq!(active(&app), Some(a), "⌘-click picks while the option is off");
+    assert_eq!(bounds(&app, a), Rect::new(8, 14, 24, 30));
+}
+
+#[test]
+fn group_mode_picks_and_moves_the_outermost_group() {
+    let (mut app, a, b) = app();
+    app.session.execute("layer.select", json!({"layer": a.0})).unwrap();
+    let group = LayerId(app.session.execute("layer.new.groupFromLayers", json!({"name": "G"})).unwrap()["layer"].as_u64().unwrap());
+    app.session.execute("layer.select", json!({"layer": b.0})).unwrap();
+    app.ui.tool_options.move_target = "group".into();
+    drag(&mut app, [16.0, 16.0], [0.0, 10.0], egui::Modifiers::NONE);
+    assert_eq!(active(&app), Some(group));
+    assert_eq!(bounds(&app, a), Rect::new(8, 18, 24, 34), "moving the group moves its layer");
+    assert_eq!(bounds(&app, b), Rect::new(40, 40, 56, 56));
+}
+
+#[test]
+fn dragging_one_of_several_selected_layers_moves_them_all() {
+    let (mut app, a, b) = app();
+    app.session.execute("layer.select", json!({"layer": a.0})).unwrap();
+    app.session.execute("layer.select", json!({"layer": b.0, "mode": "add"})).unwrap();
+    drag(&mut app, [16.0, 16.0], [-4.0, -4.0], egui::Modifiers::NONE);
+    assert_eq!(bounds(&app, a), Rect::new(4, 4, 20, 20));
+    assert_eq!(bounds(&app, b), Rect::new(36, 36, 52, 52));
+}
+
+#[test]
+fn shift_click_adds_the_layer_under_the_pointer() {
+    let (mut app, a, b) = app();
+    // A click without a drag only selects.
+    drag(&mut app, [16.0, 16.0], [0.0, 0.0], egui::Modifiers::SHIFT);
+    let st = app.session.active().unwrap();
+    assert!(st.is_layer_selected(a) && st.is_layer_selected(b));
+    assert_eq!(bounds(&app, a), Rect::new(8, 8, 24, 24));
+}

crates/ui-egui/src/snap_ui.rs

@@ -350,6 +350,8 @@ mod tests {
         app.ui.tool = Tool::Move;
         app.ui.view.show.smart_guides = false;
         app.ui.extras.snap = false;
+        // The presses land beside the moved layer: keep moving it (no Auto-Select pick).
+        app.ui.tool_options.move_auto_select = false;
         let m = egui::Modifiers::NONE;
         crate::canvas::tool_event(&mut app, ToolEvent::Down { x: 60.0, y: 60.0, pressure: 1.0 }, m);
         crate::canvas::tool_event(&mut app, ToolEvent::Up { x: 323.0, y: 100.0 }, m);

crates/ui-egui/src/state.rs

@@ -364,7 +364,8 @@ pub struct ToolOptions {
     #[serde(default = "one")]
     pub marquee_height: f32,
     /// Move tool: Auto-Select (with "layer" or "group" target) and Show Transform Controls.
-    #[serde(default)]
+    /// Auto-Select is on by default, as in current Photoshop; ⌘/Ctrl-click inverts it.
+    #[serde(default = "yes")]
     pub move_auto_select: bool,
     #[serde(default = "default_move_target")]
     pub move_target: String,
@@ -449,7 +450,7 @@ impl Default for ToolOptions {
             marquee_style: default_marquee_style(),
             marquee_width: 1.0,
             marquee_height: 1.0,
-            move_auto_select: false,
+            move_auto_select: true,
             move_target: default_move_target(),
             move_show_transform: false,
             crop_ratio: String::new(),