From 35abce7773c6043660162380f8bef706059ae911 Mon Sep 17 00:00:00 2001 From: rmsff <17902130+rmsff@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:45:37 +0300 Subject: [PATCH 1/2] Name Convert Point's History steps Change Anchor Point and Corner Drag path.convertPoint and path.moveHandle take an optional stepName (only that one name is accepted, as with edit.transform), and the Convert Point tool sends it. Direct Selection, the Pen's Alt conversion and the Curvature Pen keep Convert Point and Drag Direction Point. --- crates/engine/src/cmd/path_edit_cmds.rs | 22 +++++++++++++++++----- crates/ui-egui/src/tools/direct_select.rs | 15 +++++++++++++-- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/path_edit_cmds.rs b/crates/engine/src/cmd/path_edit_cmds.rs index fce3a28c110..a2e94952bc7 100644 --- a/crates/engine/src/cmd/path_edit_cmds.rs +++ b/crates/engine/src/cmd/path_edit_cmds.rs @@ -46,6 +46,16 @@ fn knot(cmd: &str, p: &Value) -> Result<[usize; 2]> { Ok([index(cmd, p, "subpath")?, index(cmd, p, "knot")?]) } +/// The History step name: `default`, or `named` when `stepName` asks for it (the Convert Point +/// tool names its steps as the reference does, #12736). Any other value is refused. +fn step_name(cmd: &str, p: &Value, default: &'static str, named: &'static str) -> Result<&'static str> { + match p.get("stepName") { + None | Some(Value::Null) => Ok(default), + Some(Value::String(n)) if n == named => Ok(named), + Some(_) => Err(bad(cmd, format!("`stepName` must be \"{named}\""))), + } +} + /// Applies `f` to the targeted path in one undoable step labelled `label`. fn edit_path(s: &mut Session, cmd: &str, p: &Value, label: &str, f: impl FnOnce(&mut Path) -> std::result::Result<(), String>) -> Result { // A malformed target is an error, never a silent edit of the Work Path or the active layer. @@ -118,7 +128,8 @@ fn move_handle(s: &mut Session, p: &Value) -> Result { let h = p.get("handle").and_then(Value::as_str).and_then(Handle::parse).ok_or_else(|| bad(CMD, "`handle` must be \"in\" or \"out\""))?; let to = point(CMD, p, "to")?.ok_or_else(|| bad(CMD, "missing `to`"))?; let independent = p.get("independent").and_then(Value::as_bool).unwrap_or(false); - edit_path(s, CMD, p, "Drag Direction Point", |path| edit::move_handle(path, k, h, to, independent)) + let label = step_name(CMD, p, "Drag Direction Point", "Corner Drag")?; + edit_path(s, CMD, p, label, |path| edit::move_handle(path, k, h, to, independent)) } fn bend_segment(s: &mut Session, p: &Value) -> Result { @@ -135,11 +146,12 @@ fn bend_segment(s: &mut Session, p: &Value) -> Result { fn convert_point(s: &mut Session, p: &Value) -> Result { const CMD: &str = "path.convertPoint"; let k = knot(CMD, p)?; + let label = step_name(CMD, p, "Convert Point", "Change Anchor Point")?; if p.get("curvature").and_then(Value::as_bool).unwrap_or(false) { - return edit_path(s, CMD, p, "Convert Point", |path| edit::toggle_curvature_point(path, k)); + return edit_path(s, CMD, p, label, |path| edit::toggle_curvature_point(path, k)); } let out = point(CMD, p, "out")?; - edit_path(s, CMD, p, "Convert Point", |path| edit::convert_point(path, k, out)) + edit_path(s, CMD, p, label, |path| edit::convert_point(path, k, out)) } fn add_anchor(s: &mut Session, p: &Value) -> Result { @@ -255,7 +267,7 @@ pub fn specs() -> Vec { spec( "path.moveHandle", "Drag Direction Point", - r#""subpath":i,"knot":j,"handle":"in|out","to":[x,y],"independent":bool=false (a smooth point turns its other handle to stay collinear; independent = ⌥ / Convert Point, makes a corner)"#.into(), + r#""subpath":i,"knot":j,"handle":"in|out","to":[x,y],"independent":bool=false (a smooth point turns its other handle to stay collinear; independent = ⌥ / Convert Point, makes a corner),"stepName":"Corner Drag"? (History step name; the Convert Point tool sends it; default "Drag Direction Point"; any other value is refused)"#.into(), move_handle, ), spec( @@ -267,7 +279,7 @@ pub fn specs() -> Vec { spec( "path.convertPoint", "Convert Point", - r#""subpath":i,"knot":j,"out":[x,y]? (none: corner with retracted handles; given: smooth with this out handle and its mirror),"curvature":bool=false (the Curvature Pen's double-click: toggles smooth/corner and reflows the smooth knots of the subpath; out ignored)"#.into(), + r#""subpath":i,"knot":j,"out":[x,y]? (none: corner with retracted handles; given: smooth with this out handle and its mirror),"curvature":bool=false (the Curvature Pen's double-click: toggles smooth/corner and reflows the smooth knots of the subpath; out ignored),"stepName":"Change Anchor Point"? (History step name; the Convert Point tool sends it; default "Convert Point"; any other value is refused)"#.into(), convert_point, ), spec( diff --git a/crates/ui-egui/src/tools/direct_select.rs b/crates/ui-egui/src/tools/direct_select.rs index ba8ab3b8ac3..98844fbb871 100644 --- a/crates/ui-egui/src/tools/direct_select.rs +++ b/crates/ui-egui/src/tools/direct_select.rs @@ -430,7 +430,7 @@ pub fn pointer(app: &mut PhotocraftApp, ev: ToolEvent, mods: Modifiers) -> bool ToolEvent::Up { x, y } => match app.ui.direct_selection.drag.take() { Some(Drag::Edit { target, base, edit, start, .. }) => { let end = edit.constrain(start, [x, y], mods.shift); - release(app, target, &base, &edit, start, end); + release(app, target, &base, &edit, start, end, mods); app.ui.direct_selection.coalesce = None; true } @@ -600,7 +600,7 @@ fn curvature_press(app: &mut PhotocraftApp, p: [f64; 2]) -> bool { /// End of an edit drag: commit it as one command. A click (no movement) commits nothing, except /// Pen + ⌥ on an anchor, which makes it a corner. The Pen path being drawn takes the edit directly. -fn release(app: &mut PhotocraftApp, target: PathRef, base: &Path, edit: &Edit, start: [f64; 2], end: [f64; 2]) { +fn release(app: &mut PhotocraftApp, target: PathRef, base: &Path, edit: &Edit, start: [f64; 2], end: [f64; 2], mods: Modifiers) { let moved = (end[0] - start[0]).hypot(end[1] - start[1]) * f64::from(app.point_zoom()) >= 1.0; if target == PathRef::Pen { let edited = match edit { @@ -621,6 +621,17 @@ fn release(app: &mut PhotocraftApp, target: PathRef, base: &Path, edit: &Edit, s _ if !moved => return, e => e.command(start, end), }; + // The Convert Point tool names its steps as the reference does; the Pen's ⌥ and Direct + // Selection (Convert Point with ⌘ too) keep the commands' own names. + let convert_tool = app.ui.tool == Tool::ConvertPoint && !direct(app.ui.tool, mods); + let step_name = match edit { + Edit::Convert(_) if convert_tool => Some("Change Anchor Point"), + Edit::Handle(.., true) if convert_tool => Some("Corner Drag"), + _ => None, + }; + if let (Some(p), Some(name)) = (params.as_object_mut(), step_name) { + p.insert("stepName".into(), json!(name)); + } if let (Some(p), Some(t)) = (params.as_object_mut(), target.params(app).as_object()) { p.extend(t.clone()); } From 6c3579fb59d733e4e0301926c67fa150e7ed8917 Mon Sep 17 00:00:00 2001 From: rmsff <17902130+rmsff@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:45:37 +0300 Subject: [PATCH 2/2] Test Convert Point's History step names --- crates/ui-egui/src/tools/direct_select.rs | 3 + .../convert_point_history_tests.rs | 187 ++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 crates/ui-egui/src/tools/direct_select/convert_point_history_tests.rs diff --git a/crates/ui-egui/src/tools/direct_select.rs b/crates/ui-egui/src/tools/direct_select.rs index 98844fbb871..16121f373ed 100644 --- a/crates/ui-egui/src/tools/direct_select.rs +++ b/crates/ui-egui/src/tools/direct_select.rs @@ -765,6 +765,9 @@ pub fn draw_overlay(app: &PhotocraftApp, painter: &egui::Painter, to_scr: &dyn F #[cfg(test)] mod tests; +#[cfg(test)] +mod convert_point_history_tests; + #[cfg(test)] mod freeform_command_tests { use super::*; diff --git a/crates/ui-egui/src/tools/direct_select/convert_point_history_tests.rs b/crates/ui-egui/src/tools/direct_select/convert_point_history_tests.rs new file mode 100644 index 00000000000..7bdabd5dbcc --- /dev/null +++ b/crates/ui-egui/src/tools/direct_select/convert_point_history_tests.rs @@ -0,0 +1,187 @@ +//! #12736: the Convert Point tool's History steps read "Change Anchor Point" (a click or drag on +//! an anchor) and "Corner Drag" (a handle drag), as in the reference. Direct Selection, the Pen's +//! ⌥ conversion and the Curvature Pen's double-click keep their names. + +use egui::Modifiers; +use photocraft_doc::LayerContent; +use serde_json::json; + +use crate::PhotocraftApp; +use crate::canvas::{ToolEvent, tool_event}; +use crate::state::Tool; + +fn app(tool: Tool) -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.session.execute("file.new", json!({"width": 200, "height": 200})).unwrap(); + app.sync_views(); + app.ui.tool = tool; + app +} + +/// A square work path; knot 1 at (120, 40) is smooth with handles at (100, 40) and (140, 40). +fn with_path(app: &mut PhotocraftApp) { + let path = json!({"subpaths": [{"closed": true, "knots": [ + [40, 40], + {"anchor": [120, 40], "in": [100, 40], "out": [140, 40], "smooth": true}, + [120, 120], + [40, 120], + ]}]}); + app.run("path.set", json!({"name": "work", "path": path})).unwrap(); +} + +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: to[0], y: to[1], pressure: 1.0 }, mods); + tool_event(app, ToolEvent::Up { x: to[0], y: to[1] }, mods); +} + +fn click(app: &mut PhotocraftApp, at: [f64; 2], mods: Modifiers) { + drag(app, at, at, mods); +} + +fn names(app: &PhotocraftApp) -> Vec { + app.session.active().unwrap().history.entries() +} + +/// Runs `gesture` and returns the History steps it added. +fn added(app: &mut PhotocraftApp, gesture: impl FnOnce(&mut PhotocraftApp)) -> Vec { + let n = names(app).len(); + gesture(app); + names(app).split_off(n) +} + +/// `tool` with the smooth anchor selected so its handles show: a ⌘-click, Direct Selection in +/// each of these tools (a selection made with another tool is dropped when the Pen is taken). +fn with_shown_handles(tool: Tool) -> PhotocraftApp { + let mut app = app(tool); + with_path(&mut app); + click(&mut app, [120.0, 40.0], Modifiers::COMMAND); + assert_eq!(app.ui.direct_selection.anchors, vec![[0, 1]]); + app +} + +#[test] +fn convert_point_clicks_and_drags_on_anchors_are_change_anchor_point() { + let mut app = app(Tool::ConvertPoint); + with_path(&mut app); + let one = vec!["Change Anchor Point".to_string()]; + assert_eq!(added(&mut app, |a| click(a, [120.0, 40.0], Modifiers::NONE)), one, "click on a smooth anchor"); + assert_eq!(added(&mut app, |a| click(a, [120.0, 40.0], Modifiers::NONE)), one, "click on a corner with retracted handles"); + assert_eq!(added(&mut app, |a| drag(a, [40.0, 120.0], [40.0, 150.0], Modifiers::NONE)), one, "drag from a corner"); + assert_eq!(added(&mut app, |a| drag(a, [40.0, 120.0], [60.0, 150.0], Modifiers::NONE)), one, "drag from a smooth anchor"); + assert_eq!(added(&mut app, |a| drag(a, [40.0, 40.0], [60.0, 10.0], Modifiers::SHIFT)), one, "⇧-drag"); + assert_eq!(added(&mut app, |a| drag(a, [120.0, 120.0], [150.0, 140.0], Modifiers::ALT)), one, "⌥-drag"); + let double = added(&mut app, |a| { + click(a, [120.0, 120.0], Modifiers::NONE); + click(a, [120.0, 120.0], Modifiers::NONE); + }); + assert_eq!(double, vec!["Change Anchor Point".to_string(); 2], "a double-click is two steps"); +} + +#[test] +fn convert_point_handle_drags_are_corner_drag() { + let one = vec!["Corner Drag".to_string()]; + for mods in [Modifiers::NONE, Modifiers::SHIFT, Modifiers::ALT] { + let mut app = with_shown_handles(Tool::ConvertPoint); + assert_eq!(added(&mut app, |a| drag(a, [140.0, 40.0], [140.0, 70.0], mods)), one, "{mods:?}"); + let k = app.session.active().unwrap().doc.work_path.clone().unwrap().subpaths[0].knots[1]; + assert!(!k.smooth && k.in_ctrl == photocraft_geom::Point::new(100.0, 40.0), "the handle moved alone: {k:?}"); + let click_anchor = added(&mut app, |a| click(a, [120.0, 40.0], Modifiers::NONE)); + assert_eq!(click_anchor, ["Change Anchor Point"], "click on an anchor with broken handles"); + } +} + +#[test] +fn convert_point_names_steps_on_a_live_rectangle_and_on_vector_masks() { + let mut app = app(Tool::ConvertPoint); + let id = app.run("shape.create", json!({"kind": "rect", "rect": [20, 20, 60, 60], "fill": "#3070c0"})).unwrap()["layer"].as_u64().unwrap(); + let live = |app: &PhotocraftApp| match &app.session.active().unwrap().doc.layer(photocraft_doc::LayerId(id)).unwrap().content { + LayerContent::Shape(sh) => sh.live.is_some(), + _ => panic!("not a shape layer"), + }; + assert!(live(&app)); + assert_eq!(added(&mut app, |a| click(a, [80.0, 80.0], Modifiers::NONE)), vec!["Change Anchor Point".to_string()]); + assert!(!live(&app), "the rectangle became a regular path shape"); + assert_eq!(added(&mut app, |a| drag(a, [20.0, 20.0], [30.0, 10.0], Modifiers::NONE)), vec!["Change Anchor Point".to_string()]); + + // A pixel layer's vector mask, targeted. + let mut masked = app_with_mask(); + let before = mask_corner(&masked); + assert_eq!(added(&mut masked, |a| drag(a, [20.0, 20.0], [40.0, 30.0], Modifiers::NONE)), vec!["Change Anchor Point".to_string()]); + assert_ne!(mask_corner(&masked), before, "the mask was edited"); +} + +fn app_with_mask() -> PhotocraftApp { + let mut app = app(Tool::ConvertPoint); + app.run("layer.new.layer", json!({})).unwrap(); + let id = app.session.active().unwrap().active_layer.unwrap().0; + let rect = json!({"subpaths": [{"closed": true, "knots": [[20, 20], [180, 20], [180, 160], [20, 160]]}]}); + app.run("layer.vectorMask.add", json!({"layer": id, "path": rect})).unwrap(); + app.ui.vector_mask_target = true; + app +} + +fn mask_corner(app: &PhotocraftApp) -> photocraft_doc::Knot { + let st = app.session.active().unwrap(); + st.active_layer.and_then(|id| st.doc.layer(id)).unwrap().vector_mask.as_ref().unwrap().path.subpaths[0].knots[0] +} + +#[test] +fn other_tools_keep_their_step_names() { + let direct = vec!["Drag Direction Point".to_string()]; + for mods in [Modifiers::NONE, Modifiers::ALT] { + let mut ds = with_shown_handles(Tool::DirectSelection); + assert_eq!(added(&mut ds, |a| drag(a, [140.0, 40.0], [140.0, 70.0], mods)), direct, "Direct Selection {mods:?}"); + } + for mods in [Modifiers::COMMAND, Modifiers::COMMAND | Modifiers::ALT] { + let mut convert_cmd = with_shown_handles(Tool::ConvertPoint); + assert_eq!(added(&mut convert_cmd, |a| drag(a, [140.0, 40.0], [120.0, 60.0], mods)), direct, "Convert Point with {mods:?} is Direct Selection"); + } + let mut pen_alt = with_shown_handles(Tool::Pen); + assert_eq!(added(&mut pen_alt, |a| drag(a, [140.0, 40.0], [140.0, 70.0], Modifiers::ALT)), direct, "Pen ⌥ on a handle"); + + let convert = vec!["Convert Point".to_string()]; + let mut pen = app(Tool::Pen); + with_path(&mut pen); + assert_eq!(added(&mut pen, |a| click(a, [120.0, 40.0], Modifiers::ALT)), convert, "Pen ⌥-click"); + assert_eq!(added(&mut pen, |a| drag(a, [40.0, 120.0], [40.0, 140.0], Modifiers::ALT)), convert, "Pen ⌥-drag"); + pen.ui.tool = Tool::CurvaturePen; + assert_eq!(added(&mut pen, |a| crate::vector_ui::curvature_toggle(a, 120.0, 120.0)), convert, "Curvature Pen double-click"); +} + +#[test] +fn the_commands_take_the_step_name_and_refuse_any_other() { + let mut app = app(Tool::ConvertPoint); + with_path(&mut app); + let convert = json!({"subpath": 0, "knot": 1}); + let handle = json!({"subpath": 0, "knot": 1, "handle": "out", "to": [150, 60], "independent": true}); + let mut run = |id: &str, params: &serde_json::Value| added(&mut app, |a| a.run(id, params.clone()).map(|_| ()).unwrap()); + assert_eq!(run("path.convertPoint", &convert), ["Convert Point"]); + assert_eq!(run("path.moveHandle", &handle), ["Drag Direction Point"]); + let named = |p: &serde_json::Value, n: &str| { + let mut p = p.clone(); + p["stepName"] = json!(n); + p + }; + assert_eq!(run("path.convertPoint", &named(&convert, "Change Anchor Point")), ["Change Anchor Point"]); + assert_eq!(run("path.moveHandle", &named(&handle, "Corner Drag")), ["Corner Drag"]); + let mut curvature = named(&convert, "Change Anchor Point"); + curvature["curvature"] = json!(true); + assert_eq!(run("path.convertPoint", &curvature), ["Change Anchor Point"]); +} + +#[test] +fn a_bad_step_name_is_an_error_and_changes_nothing() { + let mut app = app(Tool::ConvertPoint); + with_path(&mut app); + let before = (app.session.active().unwrap().doc.work_path.clone(), names(&app)); + for bad in [json!("Corner Drag"), json!("Change Anchor Point "), json!(""), json!(7), json!(true), json!(["Change Anchor Point"])] { + let e = app.run("path.convertPoint", json!({"subpath": 0, "knot": 1, "stepName": bad})).unwrap_err(); + assert!(e.contains("stepName"), "{bad}: {e}"); + } + for bad in [json!("Change Anchor Point"), json!("Drag Direction Point"), json!(0), json!({})] { + let e = app.run("path.moveHandle", json!({"subpath": 0, "knot": 1, "handle": "out", "to": [150, 60], "stepName": bad})).unwrap_err(); + assert!(e.contains("stepName"), "{bad}: {e}"); + } + assert_eq!((app.session.active().unwrap().doc.work_path.clone(), names(&app)), before); +}