From b800c40cfdf115ac831f2f4db5c291fa12e2e4e2 Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Sun, 1 Nov 2026 19:17:45 +0100 Subject: [PATCH] Don't auto-select off-canvas layers from the pasteboard (#10665) A Move-tool click on the pasteboard with Auto-Select only cleared the layer selection (#4991) when nothing was hit there, but `layers_hit` was not clipped to the canvas: fill layers and pixels hanging past the edge were hit on the pasteboard, so the click selected a layer that isn't drawn there instead. Outside the canvas of a document without artboards, nothing is hit now, and the layer marquee is clipped to the canvas the same way. Artboard documents keep picking over the pasteboard, where they draw their layers (#5712). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/pick_cmds.rs | 36 ++++++++++++++++++++++++++-- crates/ui-egui/src/tools/move_box.rs | 25 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/pick_cmds.rs b/crates/engine/src/cmd/pick_cmds.rs index c40e15e9b8..76a9e8b773 100644 --- a/crates/engine/src/cmd/pick_cmds.rs +++ b/crates/engine/src/cmd/pick_cmds.rs @@ -26,6 +26,12 @@ fn layers_hit(doc: &Document, x: i32, y: i32, type_bounds: bool) -> Vec if x.checked_add(1).is_none() || y.checked_add(1).is_none() { return Vec::new(); } + // Off the canvas nothing is drawn (only artboard documents show layers over the pasteboard, + // #5712), so pixels hanging past the edge and fill layers can't be hit there: a pasteboard + // click with Auto-Select clears the layer selection instead of picking them (#10665). + if !doc.has_artboards() && !doc.bounds().contains(x, y) { + return Vec::new(); + } let rows = doc.walk(); let mut out = Vec::new(); for (path, _, l) in rows.iter().rev() { @@ -114,13 +120,15 @@ fn pick(s: &mut Session, p: &Value) -> Result { /// Layers whose content touches document rect `r`, top to bottom: visible, not fully locked, not /// the locked Background (as Select › All Layers). A layer on an artboard counts only where the -/// board shows it. With `group`, each counts as its outermost group (deduplicated). +/// board shows it, and without artboards only where the canvas does (#10665). With `group`, each +/// counts as its outermost group (deduplicated). pub fn layers_in_rect(doc: &Document, r: Rect, group: bool) -> Vec { let mut out = Vec::new(); + let canvas = doc.bounds(); + let r = if doc.has_artboards() { r } else { r.intersect(&canvas) }; if r.is_empty() { return out; } - let canvas = doc.bounds(); for (path, _, l) in doc.walk().iter().rev() { if matches!(l.content, LayerContent::Group(_) | LayerContent::Adjustment(_)) || doc.is_background(l.id) @@ -447,6 +455,30 @@ mod tests { assert!(empty.execute("layer.pickAt", json!({"x": 1, "y": 1})).is_err()); } + /// #10665: off the canvas nothing is drawn, so a fill layer or pixels hanging past the edge + /// are neither picked there nor caught by a marquee that touches only the pasteboard. + #[test] + fn nothing_is_hit_off_the_canvas() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 40, "height": 40, "background": "transparent"})).unwrap(); + let fill = s.execute("layer.newFillLayer.solidColor", json!({"color": "#00ff00"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.new.layer", json!({"name": "Off"})).unwrap(); + let off = s.active().unwrap().active_layer.unwrap(); + s.edit("paint past the edge", |doc, _| { + doc.layer_mut(off).unwrap().surface_mut().unwrap().fill_rect(Rect::new(-20, 0, 10, 40), &[1.0, 0.0, 0.0, 1.0]); + Ok(()) + }) + .unwrap(); + let pick = |s: &mut Session, x: i32, y: i32| s.execute("layer.pickAt", json!({"x": x, "y": y, "select": false})).unwrap()["layer"].clone(); + assert_eq!(pick(&mut s, 5, 5), off.0, "on the canvas"); + assert_eq!(pick(&mut s, 20, 5), fill, "the fill layer covers the canvas"); + assert_eq!(pick(&mut s, -10, 5), Value::Null, "the pasteboard, over Off's hidden pixels and the fill"); + assert_eq!(s.execute("layer.pickAt", json!({"x": -10, "y": 5, "list": true})).unwrap()["layers"], json!([])); + let in_rect = |s: &mut Session, x: f64| s.execute("layer.selectInRect", json!({"x": x, "y": 0, "width": 5, "height": 5})).unwrap()["layers"].clone(); + assert_eq!(in_rect(&mut s, -15.0), json!([]), "a marquee on the pasteboard only"); + assert_eq!(in_rect(&mut s, -3.0), json!([off.0, fill]), "a marquee reaching onto the canvas"); + } + /// A hide-all mask at density 0 hides nothing (the compositor applies density through /// `LayerMask::value`), so the layer stays pickable; at full density it is clicked through. #[test] diff --git a/crates/ui-egui/src/tools/move_box.rs b/crates/ui-egui/src/tools/move_box.rs index f49a5061be..05f12222b4 100644 --- a/crates/ui-egui/src/tools/move_box.rs +++ b/crates/ui-egui/src/tools/move_box.rs @@ -154,6 +154,31 @@ mod tests { assert!(app.layer_box.is_none()); } + /// #10665: a fill layer, or pixels hanging past the canvas edge, aren't drawn on the + /// pasteboard, so a click there still clears the layer selection rather than picking them. + #[test] + fn pasteboard_click_deselects_over_fill_layers_and_off_canvas_pixels() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 64, "height": 64, "background": "transparent"})).unwrap(); + app.run("layer.newFillLayer.solidColor", json!({"color": "#00ff00"})).unwrap(); + app.run("layer.new.layer", json!({})).unwrap(); + let off = app.session.active().unwrap().active_layer.unwrap(); + app.session + .edit("paint past the edge", |doc, _| { + doc.layer_mut(off).unwrap().surface_mut().unwrap().fill_rect(photocraft_geom::Rect::new(-20, 0, 10, 64), &[1.0, 0.0, 0.0, 1.0]); + Ok(()) + }) + .unwrap(); + app.run("layer.new.layer", json!({})).unwrap(); + app.ui.tool = Tool::Move; + app.ui.tool_options.move_auto_select = true; + tool_event(&mut app, ToolEvent::Down { x: -10.0, y: 30.0, pressure: 1.0 }, egui::Modifiers::NONE); + tool_event(&mut app, ToolEvent::Up { x: -10.0, y: 30.0 }, egui::Modifiers::NONE); + let st = app.session.active().unwrap(); + assert!(st.selected_layers().is_empty() && st.active_layer.is_none(), "selected {:?}", st.selected_layers()); + assert_eq!(st.doc.layer(off).unwrap().surface().unwrap().content_bounds().x0, -20, "nothing moved"); + } + /// #4058: a drag that starts on the locked Background draws the layer marquee. It does not /// open the move-lock dialog, and releasing selects the unlocked layer the box touches. #[test]