Pen: Ctrl/Cmd+Z undoes in-progress points (#1478)

While a Pen path is being drawn, Undo (its customisable shortcut or Edit > Undo) and Backspace/
Delete remove the last uncommitted anchor instead of undoing an earlier document step, as in
Photoshop; once the draft is empty, Undo works on the document again. Fixes #1466.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nigel Ihza Firdausy
2026-10-09 13:18:00 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent fdfa57f456
commit ffb62ec975
3 changed files with 148 additions and 0 deletions
+8
View File
@@ -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<Value, String> {
// 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<Value, String> {
}
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;
+82
View File
@@ -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();
+58
View File
@@ -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.