diff --git a/crates/engine/src/cmd/sampler_cmds.rs b/crates/engine/src/cmd/sampler_cmds.rs index 4b041901b4..f41cbfb2ed 100644 --- a/crates/engine/src/cmd/sampler_cmds.rs +++ b/crates/engine/src/cmd/sampler_cmds.rs @@ -16,6 +16,12 @@ use crate::{DocState, EngineError, Result, Session}; /// Photoshop's Color Sampler limit: at most 10 points per document. pub const MAX_SAMPLERS: usize = 10; +/// The refusal `view.colorSamplers.add` gives at [`MAX_SAMPLERS`], so a caller can tell it from +/// other failures. +pub fn limit_message() -> String { + format!("Color Sampler limit reached: at most {MAX_SAMPLERS} points per document") +} + fn bad(cmd: &str, msg: impl Into) -> EngineError { EngineError::BadParams { cmd: cmd.into(), msg: msg.into() } } @@ -103,7 +109,7 @@ fn add(s: &mut Session, p: &Value) -> Result { change(s, |st| { let pos = pixel(&st.doc, x, y).ok_or_else(|| EngineError::Other("the sample point is outside the canvas".into()))?; if st.color_samplers.len() >= MAX_SAMPLERS { - return Err(EngineError::Other(format!("Color Sampler limit reached: at most {MAX_SAMPLERS} points per document"))); + return Err(EngineError::Other(limit_message())); } st.color_samplers.push(pos); let i = st.color_samplers.len() - 1; diff --git a/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs new file mode 100644 index 0000000000..6af1042626 --- /dev/null +++ b/crates/ui-egui/src/brush_shift_alt_sampler_tests.rs @@ -0,0 +1,170 @@ +//! #11890: Shift+Alt+click with a painting tool that has the Alt eyedropper adds a colour sampler +//! at the clicked pixel, as the Color Sampler tool places one: the foreground colour stays and +//! nothing is painted. Alt+click without Shift still sets the foreground; plain clicks paint. + +use egui::Modifiers; +use serde_json::{Value, json}; + +use crate::PhotocraftApp; +use crate::canvas::{ToolEvent, tool_event}; +use crate::state::Tool; + +const SHIFT_ALT: Modifiers = Modifiers { alt: true, ctrl: false, shift: true, mac_cmd: false, command: false }; + +/// A 400 × 300 document, a red layer to sample, blue foreground. +fn app(tool: Tool) -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 400, "height": 300})).unwrap(); + app.run("edit.fill", json!({"color": "#ff0000"})).unwrap(); + app.run("tools.setColors", json!({"foreground": "#0000ff"})).unwrap(); + app.ui.tool = tool; + app.sync_views(); + app +} + +fn click(app: &mut PhotocraftApp, x: f64, y: f64, mods: Modifiers) { + tool_event(app, ToolEvent::Down { x, y, pressure: 1.0 }, mods); + // A click's own jitter: a move or two on the spot. + tool_event(app, ToolEvent::Move { x, y, pressure: 1.0 }, mods); + tool_event(app, ToolEvent::Up { x, y }, mods); +} + +fn samplers(app: &mut PhotocraftApp) -> Vec { + let r = app.run("view.colorSamplers.list", json!({})).unwrap(); + r.get("samplers").and_then(Value::as_array).cloned().unwrap_or_default() +} + +fn foreground(app: &PhotocraftApp) -> [f32; 4] { + app.session.tools.foreground +} + +fn steps(app: &PhotocraftApp) -> usize { + app.session.active().unwrap().history.past_len() +} + +#[test] +fn shift_alt_click_adds_a_sampler_with_every_alt_eyedropper_tool() { + for tool in [Tool::Brush, Tool::Pencil, Tool::Gradient, Tool::PaintBucket, Tool::BackgroundEraser] { + let mut app = app(tool); + let fg = foreground(&app); + let before = steps(&app); + click(&mut app, 199.7, 120.2, SHIFT_ALT); + let got = samplers(&mut app); + // The point the Color Sampler tool's command places for the same click. + let mut twin = self::app(tool); + twin.run("view.colorSamplers.add", json!({"x": 199.7, "y": 120.2})).unwrap(); + assert_eq!(got, samplers(&mut twin), "{tool:?}: one sampler at the clicked pixel"); + assert_eq!(got.len(), 1, "{tool:?}"); + assert_eq!(foreground(&app), fg, "{tool:?}: the foreground stays"); + assert_eq!(steps(&app), before, "{tool:?}: nothing painted"); + assert!(!app.alt_sampling, "{tool:?}: the gesture ended"); + } +} + +#[test] +fn at_the_limit_the_click_adds_nothing_and_says_so() { + let mut app = app(Tool::Brush); + for i in 0..10 { + app.run("view.colorSamplers.add", json!({"x": 10 + i * 20, "y": 10})).unwrap(); + } + let (fg, before) = (foreground(&app), steps(&app)); + click(&mut app, 200.0, 150.0, SHIFT_ALT); + assert_eq!(samplers(&mut app).len(), 10); + assert_eq!(steps(&app), before, "nothing painted"); + assert!(app.ui.status.contains("limit"), "the Color Sampler tool's limit message: {:?}", app.ui.status); + assert!(!app.ui.status_error, "reported as the Color Sampler tool does"); + assert_eq!(foreground(&app), fg); +} + +#[test] +fn alt_click_still_samples_and_a_plain_click_still_paints() { + let mut app = app(Tool::Brush); + click(&mut app, 50.0, 50.0, Modifiers::ALT); + assert_eq!(foreground(&app)[..3], [1.0, 0.0, 0.0], "Alt+click picks the red under the pointer"); + assert!(samplers(&mut app).is_empty()); + let before = steps(&app); + click(&mut app, 80.0, 80.0, Modifiers::NONE); + assert_eq!(steps(&app), before + 1, "a plain click paints a dab"); + assert!(samplers(&mut app).is_empty()); +} + +#[test] +fn a_shift_alt_click_off_the_canvas_adds_nothing_and_never_panics() { + let mut app = app(Tool::Brush); + let fg = foreground(&app); + for (x, y) in [(-20.0, 50.0), (450.0, 50.0), (f64::NAN, 10.0), (10.0, f64::INFINITY)] { + click(&mut app, x, y, SHIFT_ALT); + } + assert!(samplers(&mut app).is_empty()); + assert_eq!(foreground(&app), fg); +} + +#[test] +fn shift_alt_click_on_a_sampler_samples_the_foreground_as_before() { + // A press on an existing point wasn't measured, so it keeps the Alt eyedropper's sample. + let mut app = app(Tool::Brush); + app.run("view.colorSamplers.add", json!({"x": 120, "y": 80})).unwrap(); + let before = samplers(&mut app); + click(&mut app, 121.0, 80.5, SHIFT_ALT); + assert_eq!(samplers(&mut app), before, "no second point on the first"); + assert_eq!(foreground(&app)[..3], [1.0, 0.0, 0.0], "the red under the pointer, as an Alt+click"); +} + +#[test] +fn a_shift_alt_click_off_the_canvas_reports_nothing() { + // The Color Sampler tool places no point off the canvas and says nothing; so does this press. + let mut app = app(Tool::Brush); + app.ui.status = "Ready".into(); + for (x, y) in [(-20.0, 50.0), (450.0, 50.0), (f64::NAN, 10.0)] { + click(&mut app, x, y, SHIFT_ALT); + assert_eq!(app.ui.status, "Ready", "({x}, {y}): the status bar is left as it was"); + } +} + +#[test] +fn a_shift_alt_click_refused_for_another_reason_keeps_the_error_styling() { + // Only the limit is the Color Sampler tool's plain report. A Pen draft whose commit failed, + // left in another document, refuses every command until it is dealt with: that stays an error. + let mut app = app(Tool::Pen); + for x in [30.0, 1e40] { + tool_event(&mut app, ToolEvent::Down { x, y: 60.0, pressure: 1.0 }, Modifiers::NONE); + tool_event(&mut app, ToolEvent::Up { x, y: 60.0 }, Modifiers::NONE); + } + crate::vector_ui::pen_commit(&mut app, false); + assert!(app.ui.pen.as_ref().is_some_and(|p| p.commit_failed), "a failed Pen draft"); + app.session.execute("file.new", json!({"width": 400, "height": 300})).unwrap(); + app.ui.tool = Tool::Brush; + // The Pen failure styled the status bar as an error already: only the click may set it again. + app.ui.status_error = false; + click(&mut app, 200.0, 150.0, SHIFT_ALT); + assert!(samplers_of(&app).is_empty(), "nothing added"); + assert!(!app.ui.status.is_empty(), "the refusal is reported"); + assert!(app.ui.status_error, "and keeps its error styling: {:?}", app.ui.status); +} + +#[test] +fn a_full_document_refused_for_another_reason_keeps_the_error_styling() { + // Ten points and a failed Pen draft in another document: the refusal is the Pen's, not the + // limit's, so it stays an error. + let mut app = app(Tool::Pen); + for x in [30.0, 1e40] { + tool_event(&mut app, ToolEvent::Down { x, y: 60.0, pressure: 1.0 }, Modifiers::NONE); + tool_event(&mut app, ToolEvent::Up { x, y: 60.0 }, Modifiers::NONE); + } + crate::vector_ui::pen_commit(&mut app, false); + assert!(app.ui.pen.as_ref().is_some_and(|p| p.commit_failed), "a failed Pen draft"); + app.session.execute("file.new", json!({"width": 400, "height": 300})).unwrap(); + for i in 0..10 { + app.session.execute("view.colorSamplers.add", json!({"x": 10 + i * 20, "y": 10})).unwrap(); + } + app.ui.tool = Tool::Brush; + app.ui.status_error = false; + click(&mut app, 200.0, 150.0, SHIFT_ALT); + assert_eq!(samplers_of(&app).len(), 10, "nothing added"); + assert!(!app.ui.status.contains("limit"), "the Pen's refusal, not the limit: {:?}", app.ui.status); + assert!(app.ui.status_error, "and keeps its error styling: {:?}", app.ui.status); +} + +fn samplers_of(app: &PhotocraftApp) -> Vec<[f64; 2]> { + app.session.active().map(|d| d.color_samplers.clone()).unwrap_or_default() +} diff --git a/crates/ui-egui/src/canvas/cursors.rs b/crates/ui-egui/src/canvas/cursors.rs index efa7f23989..5c10cfd699 100644 --- a/crates/ui-egui/src/canvas/cursors.rs +++ b/crates/ui-egui/src/canvas/cursors.rs @@ -212,6 +212,24 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: return false; } match ev { + // ⇧⌥-click adds a colour sampler at the clicked pixel instead, as the Color Sampler tool + // places one: no foreground change, nothing painted (#11890). A refused point (the + // document's limit) reports in the status bar as that tool's does; off the canvas, as that + // tool, it places none and says nothing. A press on an existing point wasn't measured, so + // it samples the foreground as before. + ToolEvent::Down { x, y, .. } if mods.shift && !on_sampler(app, x, y) => { + let size = app.session.active().map(|d| d.doc.size); + let inside = size.is_some_and(|s| (0.0..f64::from(s.width)).contains(&x) && (0.0..f64::from(s.height)).contains(&y)); + if inside && let Err(e) = app.run("view.colorSamplers.add", json!({"x": x, "y": y})) { + // Only the limit is a plain report; any other refusal (a failed Pen draft left in + // another document) stays an error, also when the document is full. + let limit = e == photocraft_engine::sampler_cmds::limit_message(); + app.ui.status = e; + app.ui.status_error = !limit; + } + } + // While ⇧⌥ stay held after that press, moves sample nothing. + ToolEvent::Move { .. } if mods.shift => {} // Without ⌥: the sample sets the foreground colour, whatever the Eyedropper would do. ToolEvent::Down { x, y, .. } | ToolEvent::Move { x, y, .. } => sample_eyedropper(app, x, y, egui::Modifiers::NONE), ToolEvent::Up { .. } => app.alt_sampling = false, @@ -219,6 +237,17 @@ pub(super) fn alt_eyedropper(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui: true } +/// Is document point (x, y) on a colour sampler, within the Color Sampler tool's reach of one (7 +/// screen pixels)? +fn on_sampler(app: &PhotocraftApp, x: f64, y: f64) -> bool { + let tol = 7.0 / f64::from(app.point_zoom().max(0.01)); + app.session.active().is_some_and(|d| d.color_samplers.iter().any(|q| (q[0] - x).hypot(q[1] - y) <= tol)) +} + +#[cfg(test)] +#[path = "../brush_shift_alt_sampler_tests.rs"] +mod brush_shift_alt_sampler_tests; + /// Sample the image or targeted raster mask at (x, y) using the configured Sample Size. /// Write a valid sample to the foreground swatch, or to the background swatch with Alt. pub(super) fn sample_eyedropper(app: &mut PhotocraftApp, x: f64, y: f64, mods: egui::Modifiers) {