Merge PR #3749: fix(comments): the eraser's size sets how wide it rubs (#3747)

crates/ui-egui/src/canvas.rs

@@ -2124,6 +2124,13 @@ pub struct PageXform {
 }
 
 impl PageXform {
+    /// Screen pixels per page point. The view's rotation swaps the rectangle's sides, so a page
+    /// turned a quarter shows its width down the screen.
+    pub(crate) fn zoom(&self) -> f32 {
+        let across = if self.rot % 180 == 90 { self.rect.height() } else { self.rect.width() };
+        across / self.pw.max(1.0)
+    }
+
     /// Draw an image in the PDF user-space `rect` as a custom stamp's appearance draws it: turned
     /// back by `turn` (the page /Rotate it was placed for), so with the page's own rotation it
     /// reads upright as displayed.
@@ -5498,6 +5505,18 @@ mod tests {
         assert_eq!(badges.len(), 1);
     }
 
+    #[test]
+    fn page_zoom_reads_the_side_the_page_width_is_on() {
+        // A 200 x 100 pt page at 2 pixels per point: 400 x 200 on screen, or 200 x 400 turned.
+        let xf = |rot: u16, size: Vec2| PageXform { rect: Rect::from_min_size(Pos2::ZERO, size), rot, pw: 200.0, ph: 100.0 };
+        for rot in [0, 180] {
+            assert_eq!(xf(rot, vec2(400.0, 200.0)).zoom(), 2.0, "{rot}");
+        }
+        for rot in [90, 270] {
+            assert_eq!(xf(rot, vec2(200.0, 400.0)).zoom(), 2.0, "{rot}: the width runs down the screen");
+        }
+    }
+
     #[test]
     fn snapshot_tabs_read_like_their_source_plus_the_suffix() {
         let snap = Snapshot { kind: SnapshotKind::SignedVersion, revision: 1, from: "report.pdf".into() };

crates/ui-egui/src/comments.rs

@@ -1434,7 +1434,7 @@ fn paint_gesture(painter: &egui::Painter, cx: &PageCx<'_>, view: &DocView) {
 fn paint_draw(painter: &egui::Painter, cx: &PageCx<'_>, tool: &CommentTool, points: &[[f64; 2]]) {
     {
         let style = cx.prefs.style(*tool);
-        let zoom = cx.xf.rect.width() / cx.xf.pw.max(1.0);
+        let zoom = cx.xf.zoom();
         let stroke = Stroke::new((style.width as f32 * zoom).max(1.0), color32(style.color));
         let pts: Vec<Pos2> = points.iter().map(|p| cx.to_screen(*p)).collect();
         match tool {
@@ -1442,7 +1442,9 @@ fn paint_draw(painter: &egui::Painter, cx: &PageCx<'_>, tool: &CommentTool, poin
                 painter.add(egui::Shape::line(pts, stroke));
             }
             CommentTool::Eraser => {
-                painter.add(egui::Shape::line(pts, Stroke::new(12.0, Color32::from_gray(128).gamma_multiply(0.35))));
+                // As wide as what it rubs out (see `erase`).
+                let width = (2.0 * eraser_radius_px(style.width)) as f32;
+                painter.add(egui::Shape::line(pts, Stroke::new(width, Color32::from_gray(128).gamma_multiply(0.35))));
             }
             CommentTool::Line | CommentTool::Arrow if pts.len() == 2 => {
                 painter.line_segment([pts[0], pts[1]], stroke);
@@ -1511,10 +1513,23 @@ fn drawn_shape(tool: CommentTool, points: &[[f64; 2]]) -> Option<Shape> {
     }
 }
 
-/// Eraser: rub out drawings along `path` (user space); one undo step for all of them.
+/// The eraser's radius on screen, in pixels, from its thickness setting (#3747): three pixels
+/// per point, so the default 2 pt keeps the 12-pixel eraser it always had, and the thickness
+/// slider's 0.5–12 pt gives 3 to 72 pixels across. A setting that isn't a number is the default.
+fn eraser_radius_px(width: f64) -> f64 {
+    3.0 * if width.is_finite() { width.clamp(0.5, 12.0) } else { 2.0 }
+}
+
+/// Eraser: rub out drawings along `path` (user space); one undo step for all of them. It rubs
+/// as wide on screen as its preview, whatever the zoom.
 fn erase(cx: &PageCx<'_>, path: &[[f64; 2]]) -> Option<Edit> {
-    let zoom = (cx.xf.rect.width() / cx.xf.pw.max(1.0)) as f64;
-    let radius = (6.0 / zoom).max(1.0);
+    let zoom = f64::from(cx.xf.zoom());
+    // A page with no size on screen has nothing to rub out (and no finite radius).
+    if !(zoom.is_finite() && zoom > 0.0) {
+        return None;
+    }
+    // Exactly the preview's width: no floor, which at a high zoom rubbed past it.
+    let radius = eraser_radius_px(cx.prefs.style(CommentTool::Eraser).width) / zoom;
     let b = path.iter().fold([f64::MAX, f64::MAX, f64::MIN, f64::MIN], |r, p| [r[0].min(p[0]), r[1].min(p[1]), r[2].max(p[0]), r[3].max(p[1])]);
     let mut hits: Vec<usize> = cx
         .comments()

crates/ui-egui/tests/comments.rs

@@ -1651,6 +1651,69 @@ fn erasing_part_of_a_drawing() {
     assert!(comments(&h).is_empty(), "{:?}", comments(&h));
 }
 
+#[test]
+fn the_eraser_rubs_as_wide_as_its_thickness_setting() {
+    // #3747: the eraser's size changed nothing; it always rubbed 6 pixels either side.
+    let stroke_near = |width: f64| {
+        let mut h = harness(|app| app.set_option("quick", "ink").unwrap());
+        drag_pt(&mut h, (40.0, 60.0), (260.0, 60.0));
+        assert_eq!(comments(&h).len(), 1);
+        let zoom = h.state().views[0].page_screen_rect(0).expect("page 1 on screen").width() / 300.0;
+        let app = h.state_mut();
+        app.comment_prefs.set_width(pdfcraft_ui_egui::comments::CommentTool::Eraser, width);
+        app.set_option("quick", "eraser").unwrap();
+        // Along the whole drawing, 20 pixels above it: out of reach of the default eraser
+        // (6 pixels), within reach of the largest (36).
+        let y = 60.0 + 20.0 / zoom;
+        drag_pt(&mut h, (30.0, y), (270.0, y));
+        comments(&h).len()
+    };
+    assert_eq!(stroke_near(2.0), 1, "the default eraser passes by");
+    assert_eq!(stroke_near(12.0), 0, "the largest rubs the drawing out");
+}
+
+/// The widths of the eraser preview's strokes drawn in the last frame.
+fn eraser_preview_widths(h: &Harness<'static, PdfCraftApp>) -> Vec<f32> {
+    fn walk(shape: &egui::Shape, out: &mut Vec<f32>) {
+        let preview = egui::Color32::from_gray(128).gamma_multiply(0.35);
+        match shape {
+            egui::Shape::Vec(shapes) => shapes.iter().for_each(|s| walk(s, out)),
+            egui::Shape::Path(p) if p.stroke.color == egui::epaint::ColorMode::Solid(preview) => out.push(p.stroke.width),
+            _ => {}
+        }
+    }
+    let mut out = Vec::new();
+    for clipped in &h.output().shapes {
+        walk(&clipped.shape, &mut out);
+    }
+    out
+}
+
+#[test]
+fn the_eraser_preview_is_as_wide_as_it_rubs() {
+    // #3747: while the eraser is held, its preview stroke is as wide as what it rubs out
+    // (twice its radius, three pixels per point of thickness): 12 pixels by default, 72 at 12 pt.
+    for (width, across) in [(2.0, 12.0), (12.0, 72.0)] {
+        let mut h = harness(move |app| {
+            app.comment_prefs.set_width(pdfcraft_ui_egui::comments::CommentTool::Eraser, width);
+            app.set_option("quick", "eraser").unwrap();
+        });
+        let (from, to) = (at(&h, 60.0, 100.0), at(&h, 240.0, 100.0));
+        h.hover_at(from);
+        h.run_steps(1);
+        h.drag_at(from);
+        h.run_steps(1);
+        for k in 1..=3 {
+            h.hover_at(from + (to - from) * (k as f32 / 4.0));
+            h.run_steps(1);
+        }
+        assert_eq!(eraser_preview_widths(&h), [across], "{width} pt: the preview while held");
+        h.drop_at(to);
+        h.run_steps(3);
+        assert!(eraser_preview_widths(&h).is_empty(), "{width} pt: gone once released");
+    }
+}
+
 #[test]
 fn hovering_a_comment_shows_its_author_and_text() {
     // A square comment by Ada over the first page's text.

parity/acrobat-features.toml

@@ -3704,8 +3704,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/comments.rs::erasing_part_of_a_drawing"]
-notes = "cuts strokes where the eraser passes; an emptied drawing is deleted; one undo step"
+evidence = ["crates/annot/src/tests.rs::the_eraser_cuts_strokes_and_removes_empty_drawings", "crates/ui-egui/tests/comments.rs::erasing_part_of_a_drawing", "crates/ui-egui/tests/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)"
 
 [[feature]]
 id = "comment.line"