Patch: History names as the tool's (#12117)

select.lasso takes an optional historyName; the Patch tool sends "Patch Tool" so its
lasso is that step (the Lasso tool keeps "Lasso"). paint.patch records "Patch Selection"
in every mode; the content-aware job keeps its progress text.
Tests: patch_history_names_tests (engine) and patch_tool_history_names_tests (ui-egui).

crates/engine/src/cmd/retouch_cmds.rs

@@ -397,3 +397,6 @@ pub fn specs() -> Vec<CommandSpec> {
 
 #[cfg(test)]
 mod tests;
+
+#[cfg(test)]
+mod patch_history_names_tests;

crates/engine/src/cmd/retouch_cmds/patch.rs

@@ -23,6 +23,8 @@ use super::{
 };
 
 const CMD: &str = "paint.patch";
+/// The History step of applying a patch, in every mode.
+const HISTORY_NAME: &str = "Patch Selection";
 
 /// Largest patch, in pixels of the selection's bounding box. The solve holds the two regions, the
 /// result and per-channel float planes of it (on the order of 150 bytes per pixel with four
@@ -151,7 +153,6 @@ struct Plan {
     /// Shift of the repaired area from the selection, and of the texture from the repaired area.
     dst_shift: (i32, i32),
     src_shift: (i32, i32),
-    destination: bool,
     offset: (i32, i32),
 }
 
@@ -185,7 +186,7 @@ fn plan(s: &Session, p: &Value) -> Result<Plan> {
         PatchMode::Source => ((0, 0), (dx, dy)),
         PatchMode::Destination => ((dx, dy), (-dx, -dy)),
     };
-    Ok(Plan { id, canvas, area, dst_shift, src_shift, destination: mode == PatchMode::Destination, offset: (dx, dy) })
+    Ok(Plan { id, canvas, area, dst_shift, src_shift, offset: (dx, dy) })
 }
 
 /// Heal `surf` at the selection (shifted by `dst_shift`) with texture read `src_shift` away, the
@@ -216,7 +217,7 @@ pub(super) fn patch(s: &mut Session, p: &Value) -> Result<Value> {
         return patch_content_aware(s, p);
     }
     let plan = plan(s, p)?;
-    let label = if plan.destination { "Patch (Destination)" } else { "Patch" };
+    let label = HISTORY_NAME;
     let dmg = run_stroke(s, label, plan.id, p, |pre, surf, _, lock| {
         let sel = pre.selection.as_ref().ok_or_else(|| bad(CMD, "select the area to patch first"))?;
         Ok(patch_surface(surf, sel, &plan, lock, 1))
@@ -263,6 +264,7 @@ fn level(p: &Value, k: &str, default: u8, lo: u8, hi: u8) -> Result<u8> {
 /// docs), as a background job.
 fn patch_content_aware(s: &mut Session, p: &Value) -> Result<Value> {
     use super::content_aware_move::{Target, land, mask, read, window};
+    // The job's progress text; its History step is `HISTORY_NAME`.
     const LABEL: &str = "Patch (Content-Aware)";
     let plan = plan(s, p)?;
     let structure = level(p, "structure", 4, 1, 7)?;
@@ -294,7 +296,7 @@ fn patch_content_aware(s: &mut Session, p: &Value) -> Result<Value> {
             }
         },
         move |s, (surf, damage)| {
-            s.edit(LABEL, |doc, _| {
+            s.edit(HISTORY_NAME, |doc, _| {
                 *crate::channel_cmds::target_surface(doc, target.id, &p)?.0 = surf;
                 Ok(())
             })?;

crates/engine/src/cmd/retouch_cmds/patch_history_names_tests.rs

@@ -0,0 +1,58 @@
+//! History step names of the Patch Tool (#12117): "Patch Tool" for the lasso it draws with and
+//! "Patch Selection" for applying a patch in every mode; the plain Lasso tool keeps "Lasso".
+
+use super::*;
+
+fn session() -> Session {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 128, "height": 64, "depth": 8, "mode": "rgb"})).unwrap();
+    s
+}
+
+fn last_step(s: &Session) -> Option<String> {
+    s.active().unwrap().history.undo_label().map(str::to_string)
+}
+
+#[test]
+fn applying_a_patch_is_one_step_named_patch_selection_in_every_mode() {
+    let modes = [
+        json!({"offset": [30, 0]}),
+        json!({"offset": [30, 0], "mode": "source"}),
+        json!({"offset": [30, 0], "mode": "destination"}),
+        json!({"offset": [30, 0], "contentAware": true}),
+        json!({"offset": [30, 0], "mode": "destination", "contentAware": true}),
+    ];
+    for p in modes {
+        let mut s = session();
+        s.execute("select.rect", json!({"x": 16, "y": 22, "width": 20, "height": 20})).unwrap();
+        let steps = s.active().unwrap().history.past_len();
+        s.execute("paint.patch", p.clone()).unwrap();
+        assert_eq!(s.active().unwrap().history.past_len(), steps + 1, "{p}: one step");
+        assert_eq!(last_step(&s).as_deref(), Some("Patch Selection"), "{p}");
+    }
+}
+
+const TRIANGLE: [[i32; 2]; 3] = [[16, 22], [36, 22], [16, 42]];
+
+#[test]
+fn the_lasso_is_named_for_the_tool_that_drew_it() {
+    let mut s = session();
+    s.execute("select.lasso", json!({"points": TRIANGLE})).unwrap();
+    assert_eq!(last_step(&s).as_deref(), Some("Lasso"), "the Lasso tool sends no name");
+    for mode in ["replace", "add", "subtract", "intersect"] {
+        s.execute("select.lasso", json!({"points": TRIANGLE, "mode": mode, "historyName": "Patch Tool"})).unwrap();
+        assert_eq!(last_step(&s).as_deref(), Some("Patch Tool"), "{mode}");
+    }
+    s.execute("select.lasso", json!({"points": TRIANGLE, "polygonal": true, "historyName": "Patch Tool"})).unwrap();
+    assert_eq!(last_step(&s).as_deref(), Some("Polygonal Lasso"), "the Polygonal Lasso keeps its own name");
+}
+
+#[test]
+fn a_hostile_history_name_is_refused_without_a_step() {
+    let mut s = session();
+    let steps = s.active().unwrap().history.past_len();
+    for name in [json!(""), json!("   "), json!("x".repeat(65)), json!("a\nb"), json!(7), json!(["Patch Tool"])] {
+        assert!(s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).is_err(), "{name}");
+    }
+    assert_eq!(s.active().unwrap().history.past_len(), steps);
+}

crates/engine/src/cmd/retouch_cmds/tests.rs

@@ -1022,7 +1022,7 @@ fn patch_content_aware_copies_the_dragged_to_content_into_the_selection() {
             let (got, want) = (rgba(&s, x, y), texture(x, y));
             assert!((got[0] - want[0]).abs() <= tol(depth), "depth {depth}: untouched {x},{y}: {got:?}");
         }
-        assert_eq!(s.active().unwrap().history.undo_label(), Some("Patch (Content-Aware)"));
+        assert_eq!(s.active().unwrap().history.undo_label(), Some("Patch Selection"));
         s.execute("edit.undo", json!({})).unwrap();
         assert_eq!(rgba(&s, 44, 24)[1], 0.0, "depth {depth}: undo brings the blemish back");
     }

crates/engine/src/cmd/selection_cmds.rs

@@ -622,7 +622,17 @@ fn lasso(s: &mut Session, p: &Value) -> Result<Value> {
         return Err(EngineError::BadParams { cmd: "select.lasso".into(), msg: "needs at least 3 points".into() });
     }
     // The History step is named after the tool: the Polygonal Lasso sends `polygonal` (Photoshop).
-    let step = if b(p, "polygonal", false) { "Polygonal Lasso" } else { "Lasso" };
+    // The Patch and Content-Aware Move tools draw with this command and send their own name as
+    // `historyName`, so the plain Lasso tool keeps "Lasso".
+    let step = if b(p, "polygonal", false) {
+        "Polygonal Lasso"
+    } else {
+        match p.get("historyName") {
+            None | Some(Value::Null) => "Lasso",
+            Some(Value::String(n)) if !n.trim().is_empty() && n.chars().count() <= 64 && !n.chars().any(char::is_control) => n.trim(),
+            Some(_) => return Err(EngineError::BadParams { cmd: "select.lasso".into(), msg: "historyName must be a short text".into() }),
+        }
+    };
     polygon_selection(s, "select.lasso", step, &pts, p)
 }
 
@@ -756,7 +766,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "select.lasso",
             "Lasso",
             [],
-            r##"{"points":[[x,y],…],"mode":"replace|add|subtract|intersect"="replace","antiAlias":bool=true,"feather":px=0,"polygonal":bool=false}"##,
+            r##"{"points":[[x,y],…],"mode":"replace|add|subtract|intersect"="replace","antiAlias":bool=true,"feather":px=0,"polygonal":bool=false,"historyName"?:text (1..64 characters, no control characters; the History step name when a tool other than the Lasso draws it, e.g. "Patch Tool"; default "Lasso")}"##,
             has_doc,
             lasso
         ),

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

@@ -144,7 +144,12 @@ fn finish(app: &mut PhotocraftApp, mut d: Drag, click: Option<bool>) {
                 let o = &app.ui.tool_options;
                 // The Patch and Content-Aware Move tools have no Feather in their options bar.
                 let feather = if d.tool == Tool::Lasso { o.feather } else { 0.0 };
-                let made = app.run("select.lasso", json!({"points": pts, "mode": mode, "antiAlias": o.anti_alias, "feather": feather}));
+                let mut params = json!({"points": pts, "mode": mode, "antiAlias": o.anti_alias, "feather": feather});
+                // The Patch tool's lasso is a step of that tool in History; the Lasso tool keeps "Lasso".
+                if d.tool == Tool::Patch {
+                    params["historyName"] = json!("Patch Tool");
+                }
+                let made = app.run("select.lasso", params);
                 // The lasso only outlines the patch; say that it is dragged next (#1715).
                 if made.is_ok() && d.tool != Tool::Lasso {
                     crate::retouch_ui::patch_hint(app);

crates/ui-egui/src/lib.rs

@@ -979,6 +979,8 @@ mod status_bar_tests;
 #[cfg(test)]
 mod stroke_timing_tests;
 
+#[cfg(test)]
+mod patch_tool_history_names_tests;
 #[cfg(test)]
 mod polygon_lasso_step_snap_tests;
 #[cfg(test)]

crates/ui-egui/src/patch_tool_history_names_tests.rs

@@ -0,0 +1,33 @@
+//! The Patch tool's lasso is a History step named "Patch Tool" (#12117); the Lasso tool keeps "Lasso".
+
+use egui::Modifiers;
+use serde_json::json;
+
+use crate::PhotocraftApp;
+use crate::canvas::{ToolEvent, tool_event};
+use crate::state::Tool;
+
+fn app_with(tool: Tool) -> PhotocraftApp {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default());
+    app.run("file.new", json!({"width": 800, "height": 600, "background": "transparent"})).unwrap();
+    app.sync_views();
+    app.ui.tool = tool;
+    app
+}
+
+/// Drag a triangle with the active tool and return the last History step.
+fn drag_lasso(app: &mut PhotocraftApp) -> Option<String> {
+    tool_event(app, ToolEvent::Down { x: 100.0, y: 100.0, pressure: 1.0 }, Modifiers::NONE);
+    for (x, y) in [(200.0, 100.0), (300.0, 100.0), (300.0, 300.0)] {
+        tool_event(app, ToolEvent::Move { x, y, pressure: 1.0 }, Modifiers::NONE);
+    }
+    tool_event(app, ToolEvent::Up { x: 200.0, y: 300.0 }, Modifiers::NONE);
+    assert!(app.session.active().unwrap().doc.selection.is_some());
+    app.session.active().unwrap().history.entries().last().cloned()
+}
+
+#[test]
+fn the_patch_tool_lasso_is_named_for_the_tool() {
+    assert_eq!(drag_lasso(&mut app_with(Tool::Patch)).as_deref(), Some("Patch Tool"));
+    assert_eq!(drag_lasso(&mut app_with(Tool::Lasso)).as_deref(), Some("Lasso"));
+}