diff --git a/ROADMAP.md b/ROADMAP.md --- a/ROADMAP.md +++ b/ROADMAP.md @@ -745,3 +745,4 @@ Newest first: the date, what moved, and the new overall estimate. - **2026-10-18 (renderer, hayro patches 19/35):** A mesh shading (types 4–7) is sampled over its own triangles instead of the whole path: a BricsCAD plan with 310 one-triangle `sh` fills took 2 s per 1024 px tile at every zoom (and hit the 20 s watchdog on a busy machine); now about 70 ms, so zooming into dense topology drawings stays fluid. Estimates unchanged. - **2026-10-22 (PR #3227 integration):** M11: integrate upstream `80b7d215`; retain partial target-version compatibility and upstream content-boundary and image-size regression coverage. Scope and estimates unchanged. +- **2026-10-27 (M5, #3900):** E and B swap the Draw tool and the Eraser: a tap of E switches to the Eraser and stays (E again or B goes back), and holding E is the Eraser only while held. diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -54,6 +54,7 @@ impl PdfCraftApp { right_panel_closed: false, quick_tool: QuickTool::Select, space_hand: None, + eraser_key: None, full_screen_tool: None, comment_prefs: Default::default(), theme: ThemeKind::Light, diff --git a/crates/ui-egui/src/app/keys.rs b/crates/ui-egui/src/app/keys.rs --- a/crates/ui-egui/src/app/keys.rs +++ b/crates/ui-egui/src/app/keys.rs @@ -3,6 +3,7 @@ use super::*; impl PdfCraftApp { pub(super) fn shortcuts(&mut self, ctx: &egui::Context) { use egui::Key; + self.eraser_key_frame(ctx); let pasted = self.paste_key_released(ctx); // "Save changes?" is modal: its keys are its own (⌘D is Don't save there, not Document // properties; Escape cancels it rather than clearing a selection), and nothing may run @@ -81,7 +82,48 @@ impl PdfCraftApp { } } +/// How long E must be held for the Eraser to be only temporary (#3900). +use crate::comments::CommentTool; + +const ERASER_HOLD_SECS: f64 = 0.35; +const DRAW: QuickTool = QuickTool::Comment(CommentTool::Ink); +const ERASER: QuickTool = QuickTool::Comment(CommentTool::Eraser); + impl PdfCraftApp { + /// Make `tool` the active one and the one its quick-bar group shows, as choosing it from the + /// bar or its command does. + fn set_drawing_tool(&mut self, tool: CommentTool) { + self.comment_prefs.group_tool[tool.group()] = tool; + self.quick_tool = QuickTool::Comment(tool); + } + + /// The end of a held E (#3900), settled every frame, even under a dialog, so that the + /// moment the key comes up is the moment it is read. A hold of [`ERASER_HOLD_SECS`] or more + /// gives the Draw tool back; a tap leaves the Eraser. The hold is over, with nothing given + /// back, once the user has chosen another tool. Space's temporary Hand may sit on top of the + /// Eraser meanwhile: it then returns to Draw when Space is let go. + pub(super) fn eraser_key_frame(&mut self, ctx: &egui::Context) { + let Some(since) = self.eraser_key else { return }; + let on_top = self.quick_tool == QuickTool::Hand && self.space_hand == Some(ERASER); + if self.quick_tool != ERASER && !on_top { + self.eraser_key = None; + return; + } + if ctx.input(|input| input.key_down(egui::Key::E)) { + return; + } + self.eraser_key = None; + if ctx.input(|input| input.time) - since >= ERASER_HOLD_SECS { + if on_top { + // The Hand gives Draw back when Space is let go, and the bar shows it already. + self.space_hand = Some(DRAW); + self.comment_prefs.group_tool[CommentTool::Ink.group()] = CommentTool::Ink; + } else { + self.set_drawing_tool(CommentTool::Ink); + } + } + } + /// The quick tools' keys, as in Acrobat: V selects, H pans, and holding Space pans until it /// is released. Plain letters, so not while a text field, a form field, a dialog or the /// palette has the keyboard. @@ -100,6 +142,25 @@ impl PdfCraftApp { if !free || !plain { return; } + // E and B swap the Draw tool and the Eraser (#3900): a tap of E switches to the Eraser and + // stays, another tap of E or a tap of B goes back; holding E is the Eraser only while held. + // Only from one of the two, so E means nothing to any other tool. + let first_press = |key: Key| { + ctx.input(|input| input.events.iter().any(|e| matches!(e, egui::Event::Key { key: k, pressed: true, repeat: false, .. } if *k == key))) + }; + if first_press(Key::E) { + if self.quick_tool == ERASER { + self.set_drawing_tool(CommentTool::Ink); + self.eraser_key = None; + } else if self.quick_tool == DRAW { + self.set_drawing_tool(CommentTool::Eraser); + self.eraser_key = Some(ctx.input(|input| input.time)); + } + } + if first_press(Key::B) && self.quick_tool == ERASER { + self.set_drawing_tool(CommentTool::Ink); + self.eraser_key = None; + } if ctx.input(|input| input.key_pressed(Key::V)) { self.quick_tool = QuickTool::Select; self.space_hand = None; diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -305,6 +305,9 @@ impl PdfCraftApp { let Some(tool) = crate::comments::CommentTool::from_command(tool) else { return false }; self.comment_prefs.group_tool[tool.group()] = tool; self.quick_tool = crate::QuickTool::Comment(tool); + // Chosen on purpose, from the bar, a menu or a key: a held E no longer owns the + // tool, so letting it go does not take the choice back (#3900). + self.eraser_key = None; // Acrobat opens the Comments panel with the commenting tools, unless the user closed it. if self.right.is_none() && !self.comments_panel_closed { self.choose_right_panel(Some(RightPanel::Comments)); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -269,6 +269,9 @@ pub struct PdfCraftApp { pub quick_tool: QuickTool, /// The tool to go back to when Space, held for a temporary Hand, is released. space_hand: Option, + /// When E went down for the Eraser (from the Draw tool), so that holding it gives the Draw + /// tool back on release while a tap keeps the Eraser (#3900). + eraser_key: Option, /// The tool before full screen, given back on leaving it (see [`Self::apply_full_screen`]). full_screen_tool: Option, /// Comment author, per-tool colours and widths, pin. diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -644,6 +644,167 @@ fn pen_strokes_stay_unselected_so_writing_continues() { assert_eq!(h.state().views[0].comments.selected, Some((0, 2))); } +/// The Draw and Eraser tools for the E-key tests, and what the quick bar's group shows for them. +mod eraser_key { + pub use pdfcraft_ui_egui::QuickTool; + pub use pdfcraft_ui_egui::comments::CommentTool; + + pub const DRAW: QuickTool = QuickTool::Comment(CommentTool::Ink); + pub const ERASER: QuickTool = QuickTool::Comment(CommentTool::Eraser); +} + +/// The active tool, and the tool the quick bar's group shows for it, which must agree. +fn tool_and_bar(h: &Harness<'static, PdfCraftApp>) -> (QuickTool, pdfcraft_ui_egui::comments::CommentTool) { + let s = h.state(); + (s.quick_tool, s.comment_prefs.group_tool[eraser_key::CommentTool::Ink.group()]) +} + +fn eraser_harness() -> Harness<'static, PdfCraftApp> { + let mut h = harness(|app| app.set_option("quick", "ink").unwrap()); + h.state_mut().set_option("panel", "none").unwrap(); + h.run_steps(2); + h +} + +/// #3900: E and B swap the Draw tool and the Eraser. A tap of E stays on the Eraser until E or B +/// is tapped again; holding E is the Eraser only while it is held. The quick bar follows. +#[test] +fn the_e_key_toggles_the_eraser_when_tapped_and_borrows_it_when_held() { + use eraser_key::*; + let mut h = eraser_harness(); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink)); + + // A tap goes to the Eraser and stays; the next tap of E goes back. + h.key_press(egui::Key::E); + h.run_steps(4); + assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "a tap of E stays on the Eraser, and the bar shows it"); + h.key_press(egui::Key::E); + h.run_steps(4); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "tapping E again goes back to Draw"); + + // B also goes back. + h.key_press(egui::Key::E); + h.run_steps(4); + assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser)); + h.key_press(egui::Key::B); + h.run_steps(4); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "B goes back to Draw"); + + // Held: the Eraser while the key is down, Draw again the moment it is let go. + h.key_down(egui::Key::E); + h.run_steps(2); + assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "held E is the Eraser"); + h.run_steps(6); + assert_eq!(h.state().quick_tool, ERASER, "and stays while it is held"); + h.key_up(egui::Key::E); + h.run_steps(3); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "letting go gives the Draw tool back"); + + // No other tool reads E. + h.state_mut().set_option("quick", "square").unwrap(); + h.run_steps(2); + h.key_press(egui::Key::E); + h.run_steps(3); + assert_eq!(h.state().quick_tool, QuickTool::Comment(CommentTool::Rectangle), "E leaves other tools alone"); +} + +/// #3900: E let go while a dialog is open is read when it comes up, not when the dialog closes: a +/// tap stays on the Eraser however long the dialog stays, a hold gives Draw back under it. +#[test] +fn the_e_key_is_settled_when_it_comes_up_even_under_a_dialog() { + use eraser_key::*; + // A tap whose key comes up under a dialog that then stays open a long time. + let mut h = eraser_harness(); + h.key_down(egui::Key::E); + h.run_steps(1); + assert_eq!(h.state().quick_tool, ERASER); + h.state_mut().set_option("dialog", "about").unwrap(); + h.key_up(egui::Key::E); + h.run_steps(1); + h.run_steps(8); + h.state_mut().dialog = None; + h.run_steps(4); + assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "a tap is still a tap after the dialog"); + + // A hold that comes up under a dialog gives Draw back at once. + let mut h = eraser_harness(); + h.key_down(egui::Key::E); + h.run_steps(1); + h.state_mut().set_option("dialog", "about").unwrap(); + h.run_steps(4); + h.key_up(egui::Key::E); + h.run_steps(1); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "a long hold ends with the key, dialog or not"); + + // With a dialog open already, E starts nothing. + let mut h = eraser_harness(); + h.state_mut().set_option("dialog", "about").unwrap(); + h.run_steps(3); + h.key_press(egui::Key::E); + h.run_steps(3); + assert_eq!(h.state().quick_tool, DRAW, "a dialog has the keyboard"); +} + +/// #3900: choosing a tool while E is held ends the hold, even choosing the Eraser itself. +#[test] +fn choosing_a_tool_while_e_is_held_ends_the_hold() { + use eraser_key::*; + let mut h = eraser_harness(); + h.key_down(egui::Key::E); + h.run_steps(2); + assert_eq!(h.state().quick_tool, ERASER); + h.state_mut().set_option("quick", "square").unwrap(); + h.run_steps(2); + h.state_mut().execute("comment.eraser"); + h.run_steps(6); + assert_eq!(h.state().quick_tool, ERASER, "the Eraser was chosen on purpose"); + h.key_up(egui::Key::E); + h.run_steps(3); + assert_eq!(h.state().quick_tool, ERASER, "and a late release does not take it away"); + + // Choosing the Eraser itself, with nothing between: it was the Eraser already, but the + // choice still ends the hold, whether the bar, a menu or a command made it. + let mut h = eraser_harness(); + h.key_down(egui::Key::E); + h.run_steps(6); + assert!(h.state_mut().execute("comment.eraser")); + h.run_steps(1); + h.key_up(egui::Key::E); + h.run_steps(3); + assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "choosing the Eraser while E is held keeps it"); +} + +/// #3900: Space's temporary Hand on top of a held E returns to Draw, whichever key comes up first. +#[test] +fn a_held_e_and_a_held_space_unwind_in_either_order() { + use eraser_key::*; + // E up first, while Space still holds the Hand. + let mut h = eraser_harness(); + h.key_down(egui::Key::E); + h.run_steps(6); + h.key_down(egui::Key::Space); + h.run_steps(2); + assert_eq!(h.state().quick_tool, QuickTool::Hand, "Space is the Hand"); + h.key_up(egui::Key::E); + h.run_steps(2); + h.key_up(egui::Key::Space); + h.run_steps(3); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "E ended first: Draw comes back with the Hand"); + + // Space up first: the Eraser is back under the still-held E, then E gives Draw back. + let mut h = eraser_harness(); + h.key_down(egui::Key::E); + h.run_steps(6); + h.key_down(egui::Key::Space); + h.run_steps(2); + h.key_up(egui::Key::Space); + h.run_steps(2); + assert_eq!(h.state().quick_tool, ERASER, "Space up gives the Eraser back"); + h.key_up(egui::Key::E); + h.run_steps(3); + assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "and E up gives Draw"); +} + #[test] fn sticky_note_composer_posts_and_returns_to_select() { let mut h = harness(|app| app.set_option("quick", "note").unwrap()); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3739,8 +3739,8 @@ milestone = "M5" status = "shipped" commands = ["comment.eraser"] tools = [] -evidence = ["crates/annot/src/tests.rs::the_eraser_cuts_strokes_and_removes_empty_drawings", "crates/ui-egui/tests/it/comments.rs::erasing_part_of_a_drawing", "crates/ui-egui/tests/it/comments.rs::the_eraser_rubs_as_wide_as_its_thickness_setting"] -notes = "cuts strokes where the eraser passes; an emptied drawing is deleted; one undo step. Its thickness setting sizes it on screen (3 to 72 pixels across; the default 2 pt is 12), preview included (#3747)" +evidence = ["crates/annot/src/tests.rs::the_eraser_cuts_strokes_and_removes_empty_drawings", "crates/ui-egui/tests/it/comments.rs::erasing_part_of_a_drawing", "crates/ui-egui/tests/it/comments.rs::the_eraser_rubs_as_wide_as_its_thickness_setting", "crates/ui-egui/tests/it/comments.rs::the_e_key_toggles_the_eraser_when_tapped_and_borrows_it_when_held"] +notes = "cuts strokes where the eraser passes; an emptied drawing is deleted; one undo step. Its thickness setting sizes it on screen (3 to 72 pixels across; the default 2 pt is 12), preview included (#3747). E and B swap the Draw tool and the Eraser (#3900): a tap of E (under 0.35 s) switches to the Eraser and stays, a second tap of E or a tap of B goes back to Draw, and holding E is the Eraser only until it is let go. The keys act only from the Draw tool or the Eraser, and not while a text field, a form field, a dialog or the palette has the keyboard; the shortcut is not yet in the shortcuts list and cannot be rebound" [[feature]] id = "comment.line"