diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 15d33d1053..71fc898427 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/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() }; diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index d262c484aa..53a71b67d6 100644 --- a/crates/ui-egui/src/comments.rs +++ b/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 = 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 { } } -/// 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 { - 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 = cx .comments() diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index 36878ca35a..68185b2235 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/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 { + fn walk(shape: &egui::Shape, out: &mut Vec) { + 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. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 2eebe3df39..14f1cea68c 100644 --- a/parity/acrobat-features.toml +++ b/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"