diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 34b79e4a7c..c5d13111c8 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -129,6 +129,10 @@ pub fn invoke(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: Va /// [`invoke`] without the unsaved-changes prompt, for once the user has already answered it. pub(crate) fn invoke_unguarded(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { + // A pending Pen anchor is gesture state; Edit › Undo must match the keyboard. + if id == "edit.undo" && crate::vector_ui::pen_undo_last_point(app) { + return Ok(Value::Null); + } // Help › Discord, website, GitHub, Report an Issue. if let Some(url) = crate::links::url_for(id) { return Ok(crate::links::open(app, ctx, url)); @@ -484,6 +488,10 @@ fn open_path(app: &mut PhotocraftApp, path: &str) -> Result { } pub fn is_enabled(app: &PhotocraftApp, id: &str) -> bool { + // Draft Pen points can be undone even when the document has no History entries. + if id == "edit.undo" && app.ui.pen.as_ref().is_some_and(|pen| !pen.knots.is_empty()) { + return true; + } // Photoshop greys these for the Background layer, other layer kinds or single-layer documents. if crate::enable_rules::disabled(app, id) { return false; diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 8bbc683e23..f01b85121a 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -305,6 +305,24 @@ pub fn handle(app: &mut PhotocraftApp, ctx: &egui::Context) { } return; } + // A Pen path's points are uncommitted gesture state, not History entries (#1466). + // Intercept Cmd/Ctrl+Z before the normal Edit › Undo shortcut, as well as unmodified + // Backspace/Delete. Once the last anchor is removed, regular Undo works again. + if app.ui.pen.as_ref().is_some_and(|pen| !pen.knots.is_empty()) { + let mods = ctx.input(|i| i.modifiers); + let command_undo = effective_shortcut(app, "edit.undo", default_shortcut("edit.undo").as_deref()) + .and_then(|shortcut| parse(&shortcut)) + .is_some_and(|shortcut| consume(ctx, &shortcut)); + let remove = !mods.command + && !mods.ctrl + && !mods.shift + && !mods.alt + && ctx.input_mut(|i| i.consume_key(mods, Key::Backspace) || i.consume_key(mods, Key::Delete)); + if command_undo || remove { + crate::vector_ui::pen_undo_last_point(app); + return; + } + } // Pen path in progress: ↩ and Esc both end it as an open path and keep it, as in Photoshop (#1769). if app.ui.pen.is_some() && ctx.input_mut(|i| i.consume_key(Modifiers::NONE, Key::Enter) || i.consume_key(Modifiers::NONE, Key::Escape)) { crate::vector_ui::pen_commit(app, false); @@ -400,6 +418,70 @@ pub fn handle(app: &mut PhotocraftApp, ctx: &egui::Context) { mod tests { use super::*; + #[test] + fn pen_command_z_and_backspace_retract_points_before_document_undo() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); + app.run("file.new", serde_json::json!({"width": 200, "height": 200})).unwrap(); + app.ui.tool = crate::state::Tool::Pen; + app.run("shape.create", serde_json::json!({"kind": "rect", "rect": [10, 10, 40, 40], "fill": "#ff0000"})).unwrap(); + let history = app.session.active().unwrap().history.past_len(); + for (x, y) in [(10.0, 10.0), (50.0, 10.0), (50.0, 50.0)] { + crate::vector_ui::pen_down(&mut app, x, y); + crate::vector_ui::pen_up(&mut app); + } + let ctx = egui::Context::default(); + let press = |app: &mut PhotocraftApp, key, modifiers| { + let raw = egui::RawInput { + events: vec![egui::Event::ModifiersChanged(modifiers), egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }], + ..Default::default() + }; + ctx.begin_pass(raw); + handle(app, &ctx); + ctx.end_pass().textures_delta.clear(); + }; + + press(&mut app, Key::Z, Modifiers::COMMAND); + assert_eq!(app.ui.pen.as_ref().unwrap().knots.len(), 2); + assert_eq!(app.session.active().unwrap().history.past_len(), history); + press(&mut app, Key::Backspace, Modifiers::NONE); + assert_eq!(app.ui.pen.as_ref().unwrap().knots.len(), 1); + press(&mut app, Key::Delete, Modifiers::NONE); + assert!(app.ui.pen.is_none()); + assert_eq!(app.session.active().unwrap().history.past_len(), history); + // No unfinished Pen points remain, so regular Undo can now affect the document. + app.run("edit.undo", serde_json::json!({})).unwrap(); + assert_eq!(app.session.active().unwrap().history.past_len(), history - 1); + } + + #[test] + fn pen_custom_undo_binding_and_menu_retract_pending_points() { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); + app.run("file.new", serde_json::json!({"width": 200, "height": 200})).unwrap(); + app.ui.tool = crate::state::Tool::Pen; + for (x, y) in [(10.0, 10.0), (50.0, 10.0)] { + crate::vector_ui::pen_down(&mut app, x, y); + crate::vector_ui::pen_up(&mut app); + } + app.session.prefs.edit(|p| p.shortcuts.insert("edit.undo".into(), "Cmd+Shift+Y".into())); + let ctx = egui::Context::default(); + let press = |app: &mut PhotocraftApp, key: Key, modifiers: Modifiers| { + ctx.begin_pass(egui::RawInput { + events: vec![egui::Event::ModifiersChanged(modifiers), egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }], + ..Default::default() + }); + handle(app, &ctx); + ctx.end_pass().textures_delta.clear(); + }; + // Once rebound, the default key must no longer intercept pending points. + press(&mut app, Key::Z, Modifiers::COMMAND); + assert_eq!(app.ui.pen.as_ref().unwrap().knots.len(), 2); + press(&mut app, Key::Y, Modifiers::COMMAND | Modifiers::SHIFT); + assert_eq!(app.ui.pen.as_ref().unwrap().knots.len(), 1); + assert!(crate::menus::is_enabled(&app, "edit.undo")); + crate::menus::invoke(&mut app, &ctx, "edit.undo", serde_json::json!({})).unwrap(); + assert!(app.ui.pen.is_none()); + } + #[test] fn parses_registry_shortcuts() { let sc = parse("Cmd+Shift+N").unwrap(); diff --git a/crates/ui-egui/src/vector_ui.rs b/crates/ui-egui/src/vector_ui.rs index f5b964c7e5..ae68129838 100644 --- a/crates/ui-egui/src/vector_ui.rs +++ b/crates/ui-egui/src/vector_ui.rs @@ -175,6 +175,22 @@ pub fn pen_move(app: &mut PhotocraftApp, x: f64, y: f64) { } } +/// Undo the most recently placed Pen anchor while its path is still in progress. +/// These knots are editor-only gesture state, not document history until Enter or +/// clicking the first anchor commits the path. Therefore Cmd/Ctrl+Z must consume one +/// knot rather than undoing an unrelated, already committed document operation. +pub fn pen_undo_last_point(app: &mut PhotocraftApp) -> bool { + let Some(pen) = app.ui.pen.as_mut() else { return false }; + if pen.knots.pop().is_none() { + return false; + } + pen.dragging = false; + if pen.knots.is_empty() { + app.ui.pen = None; + } + true +} + pub fn pen_up(app: &mut PhotocraftApp) { if let Some(pen) = app.ui.pen.as_mut() { pen.dragging = false; @@ -1174,6 +1190,48 @@ mod tests { assert_eq!((wp.subpaths[0].knots[0].anchor.x, wp.subpaths[0].knots[0].anchor.y), (30.0, 25.0)); } + #[test] + fn pen_point_undo_preserves_earlier_knots_and_document_history() { + let mut app = app(); + app.ui.tool = Tool::Pen; + app.run("shape.create", json!({"kind": "rect", "rect": [10, 10, 40, 40], "fill": "#ff0000"})).unwrap(); + let old_history = app.session.active().unwrap().history.past_len(); + + pen_down(&mut app, 20.0, 20.0); + pen_up(&mut app); + pen_down(&mut app, 120.0, 20.0); + pen_move(&mut app, 120.0, 60.0); + pen_up(&mut app); + pen_down(&mut app, 120.0, 120.0); + pen_up(&mut app); + + let original_first_two = app.ui.pen.as_ref().unwrap().knots[..2].to_vec(); + assert!(pen_undo_last_point(&mut app)); + let pen = app.ui.pen.as_ref().unwrap(); + assert_eq!(pen.knots, original_first_two); + assert!(!pen.dragging, "undo releases the current Pen drag"); + assert_eq!(app.session.active().unwrap().history.past_len(), old_history, "undo must not roll back previous document changes"); + assert!(app.session.active().unwrap().doc.work_path.is_none(), "path is not committed yet"); + + pen_commit(&mut app, false); + let work = app.session.active().unwrap().doc.work_path.as_ref().unwrap(); + assert_eq!(work.subpaths[0].knots.len(), 2); + assert!(work.subpaths[0].knots[1].smooth, "retained smooth handles are unchanged"); + } + + #[test] + fn pen_undo_of_first_anchor_cancels_empty_draft() { + let mut app = app(); + assert!(!pen_undo_last_point(&mut app)); + pen_down(&mut app, 15.0, 17.0); + pen_up(&mut app); + assert!(pen_undo_last_point(&mut app)); + assert!(app.ui.pen.is_none()); + assert!(!pen_undo_last_point(&mut app)); + assert_eq!(app.session.active().unwrap().history.past_len(), 0); + assert!(app.session.active().unwrap().doc.work_path.is_none()); + } + /// #1419: Solid Color (and every new fill or adjustment layer) made while a path is selected /// in the Paths panel takes it as its vector mask; with the path deselected, the selection /// is its layer mask again.