From c6f58baa74fe1d8a64e13f74ac94b3624a00d782 Mon Sep 17 00:00:00 2001 From: Lombardi2003 Date: Tue, 20 Oct 2026 23:59:44 +0200 Subject: [PATCH] fix(comments): the eraser rubs exactly its preview, at any zoom or view rotation From review of #3749: the user-space radius had a 1-unit floor, so at a high zoom the eraser rubbed past its preview; and zoom was read from the screen rectangle's width, which is the page's height when the view is turned a quarter. PageXform::zoom reads the side the page's width is on; the eraser and the drawing preview use it, and the floor is gone. A page with no size on screen erases nothing. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 19 +++++++++++++++++++ crates/ui-egui/src/comments.rs | 11 ++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index eb9244a284..f66194de14 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -2121,6 +2121,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. @@ -5469,6 +5476,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 ee5214fca7..2128d7d566 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -1260,7 +1260,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 { @@ -1346,8 +1346,13 @@ fn eraser_radius_px(width: f64) -> f64 { /// 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 = (eraser_radius_px(cx.prefs.style(CommentTool::Eraser).width) / 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()