From 62cde99aef61f489dbb770fd75ee9d77d98bf253 Mon Sep 17 00:00:00 2001
From: LabDennis <341377186+LabDennis@users.noreply.github.com>
Date: Mon, 19 Oct 2026 10:02:56 +0200
Subject: [PATCH] M5: reveal sidebar comments at the current zoom (#3418)

---
 crates/ui-egui/src/canvas.rs     | 15 ++++++++++
 crates/ui-egui/src/panels.rs     |  6 +++-
 crates/ui-egui/tests/comments.rs | 47 ++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs
index 16ae634901..69a5823f1f 100644
--- a/crates/ui-egui/src/canvas.rs
+++ b/crates/ui-egui/src/canvas.rs
@@ -1207,6 +1207,21 @@ impl DocView {
         self.current = page;
     }
 
+    /// Reveal a user-space rectangle without changing zoom, centred where the scroll bounds allow.
+    pub(crate) fn reveal_rect(&mut self, page: usize, rect: [f32; 4], info: &DocInfo) {
+        self.go_to_page(page);
+        self.goto_point = None;
+        let Some(p) = info.pages.get(self.current) else { return };
+        if !rect.iter().all(|v| v.is_finite()) {
+            return;
+        }
+        // Halve before adding so even large finite coordinates cannot overflow the midpoint.
+        let x = rect[0] * 0.5 + rect[2] * 0.5;
+        let y = rect[1] * 0.5 + rect[3] * 0.5;
+        let [fx, fy] = p.dest_fraction(Some(x), Some(y), self.rotation);
+        self.goto_point = Some((self.current, fx, fy, vec2(self.viewport_w, self.viewport_h) / 2.0));
+    }
+
     /// Go to a destination, a bookmark's or a link's (ISO 32000-2 §12.3.2.2): its page, scrolled
     /// and zoomed as `dest` asks, as Acrobat does. `/XYZ` puts (left, top) at the window's
     /// top-left at its zoom (null or 0 keeps the zoom); `/Fit`, `/FitH` and `/FitV` switch to fit
diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs
index 0f2c601497..03c483c841 100644
--- a/crates/ui-egui/src/panels.rs
+++ b/crates/ui-egui/src/panels.rs
@@ -826,7 +826,11 @@ pub fn right_panel(app: &mut PdfCraftApp, ui: &mut egui::Ui) {
         },
         Some(Nav::Flash(p, r)) => {
             let v = &mut app.views[index];
-            v.go_to_page(p);
+            if let Some(doc) = app.session.get(id) {
+                v.reveal_rect(p, r, &doc.info);
+            } else {
+                v.go_to_page(p);
+            }
             v.flash = Some((p, r, 0.0));
         }
         None => {}
diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs
index 3d53f1e3bb..887686feef 100644
--- a/crates/ui-egui/tests/comments.rs
+++ b/crates/ui-egui/tests/comments.rs
@@ -100,6 +100,53 @@ fn comments(h: &Harness<'static, PdfCraftApp>) -> Vec<Annotation> {
     s.session.get(s.views[0].id).unwrap().info.annotations.clone()
 }
 
+/// #3418: choosing a sidebar comment must reveal the annotation, not just its page's top.
+#[test]
+fn sidebar_comment_centres_the_annotation_at_high_zoom() {
+    for (layout, rotation) in [("continuous", 0), ("single", 0), ("two-page", 0), ("continuous", 90), ("continuous", 180), ("continuous", 270)] {
+        let mut h = harness(move |app| {
+            app.set_option("panel", "comments").unwrap();
+            app.set_option("layout", layout).unwrap();
+            app.set_option("zoom", "800").unwrap();
+            app.views[0].rotation = rotation;
+            let shape = pdfcraft_engine::Shape::Note { at: [150.0, 90.0], icon: pdfcraft_engine::NoteIcon::Comment };
+            assert!(app.apply_edit(pdfcraft_engine::Edit::AddAnnotation(pdfcraft_engine::NewAnnotation {
+                page: 1,
+                style: pdfcraft_engine::Style::default_for(&shape),
+                shape,
+                contents: "Reveal this note".into(),
+                author: "Tester".into(),
+            })));
+            app.views[0].comments.selected = None;
+            app.set_option("page", "1").unwrap();
+        });
+        h.get_by_label("Reveal this note").click();
+        h.run_steps(8);
+        assert_eq!(h.state().views[0].comments.selected, Some((1, 0)), "the sidebar card was selected");
+        let a = &comments(&h)[0];
+        let view = &h.state().views[0];
+        let page = view.page_screen_rect(1).expect("the selected comment's page is drawn");
+        let (u, v) = ((a.rect[0] + a.rect[2]) / 600.0, 1.0 - (a.rect[1] + a.rect[3]) / 400.0);
+        let (u, v) = match rotation {
+            90 => (1.0 - v, u),
+            180 => (1.0 - u, 1.0 - v),
+            270 => (v, 1.0 - u),
+            _ => (u, v),
+        };
+        let centre = pos2(page.left() + u * page.width(), page.top() + v * page.height());
+        let viewport = view.viewport_rect();
+        assert!(centre.distance(viewport.center()) < 3.0, "{layout}, {rotation}°: comment at {centre:?}, viewport {viewport:?}");
+        assert!((view.zoom - 8.0).abs() < 0.01, "revealing a comment preserves zoom");
+        if layout == "continuous"
+            && rotation == 0
+            && let Ok(dir) = std::env::var("PDFCRAFT_SHOTS")
+        {
+            settle(&mut h);
+            h.render().unwrap().save(format!("{dir}/comment-centred.png")).unwrap();
+        }
+    }
+}
+
 #[test]
 fn dragging_over_text_with_the_highlighter_highlights_it() {
     let mut h = harness(|app| app.set_option("quick", "highlight").unwrap());