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..16121f373ed 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()); } @@ -754,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); +}