From 929455ad5b14677c56a53d224a44a10a98497e99 Mon Sep 17 00:00:00 2001 From: PerchAnton <101743951+PerchAnton@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:20:56 +0300 Subject: [PATCH 1/2] M3: fix multiline annotation hover bounds and paint order (#3541) --- ROADMAP.md | 3 + crates/ui-egui/src/canvas.rs | 9 +- crates/ui-egui/src/comments.rs | 30 ++++--- crates/ui-egui/tests/comments.rs | 137 +++++++++++++++++++++++++++++++ docs/roadmap.md | 3 +- parity/acrobat-features.toml | 4 +- 6 files changed, 163 insertions(+), 23 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index b1283e7751..670f14e681 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,7 @@ **Stage: alpha** · next: beta, ~25% (ready for real work 50% → 75%) and ~400–700 h away +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M3, #3541/#3620: comment hover follows individual marked lines and paint order; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) @@ -172,6 +173,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-20 (M3, #3541/#3620):** Comment hover follows the individual marked lines instead of the enclosing multiline rectangle, and overlapping comments follow paint order. Synthetic UI regressions cover all four text-markup kinds, page/view rotations, neighboring highlights, legacy rectangle fallback and unchanged PDF bytes. Estimates unchanged. + - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index eb9244a284..8bb2bf4bfa 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3511,12 +3511,13 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // freehand, where it would cover the next stroke (#429). let freehand = matches!(tool, QuickTool::Comment(comments::CommentTool::Ink | comments::CommentTool::Eraser)); let gesturing = view.comments.gesture.is_some() || freehand; - for a in info.annotations.iter().filter(|a| a.page == i && !gesturing) { - let sr = xf.user_rect(info, i, a.rect); + // Match selection's per-line geometry and paint order: a newer highlight's + // comment must not be shadowed by an older one's enclosing rectangle. + for a in info.annotations.iter().rev().filter(|a| a.page == i && !gesturing) { let hidden = prefs.hide_empty_highlight_hover && a.subtype == "Highlight" && a.contents.as_deref().is_none_or(str::is_empty); - if sr.contains(p) - && hover_text.is_none() + if hover_text.is_none() && !hidden + && pcx.screen_rects(a).any(|r| r.contains(p)) && let Some(text) = annotation_hover_text(a) { hover_text = Some((p, text)); diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index 7fd9b7a622..eca50fa01b 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -588,21 +588,19 @@ impl PageCx<'_> { } /// Where a comment is on screen: one rectangle, or one per marked line for text markup. - fn screen_rects(&self, a: &Annotation) -> Vec { - if is_markup(&a.subtype) && !a.quads.is_empty() { - a.quads - .iter() - .map(|q| { - let xs = [q[0], q[2], q[4], q[6]]; - let ys = [q[1], q[3], q[5], q[7]]; - let (x0, x1) = (xs.iter().copied().fold(f32::MAX, f32::min), xs.iter().copied().fold(f32::MIN, f32::max)); - let (y0, y1) = (ys.iter().copied().fold(f32::MAX, f32::min), ys.iter().copied().fold(f32::MIN, f32::max)); - self.xf.user_rect(self.info, self.page, [x0, y0, x1, y1]) - }) - .collect() - } else { - vec![self.screen_rect(a)] - } + pub(crate) fn screen_rects<'b>(&'b self, a: &'b Annotation) -> impl Iterator + 'b { + let quads = if is_markup(&a.subtype) { a.quads.as_slice() } else { &[] }; + let fallback = quads.is_empty().then(|| self.screen_rect(a)); + quads + .iter() + .map(|q| { + let xs = [q[0], q[2], q[4], q[6]]; + let ys = [q[1], q[3], q[5], q[7]]; + let (x0, x1) = (xs.iter().copied().fold(f32::MAX, f32::min), xs.iter().copied().fold(f32::MIN, f32::max)); + let (y0, y1) = (ys.iter().copied().fold(f32::MAX, f32::min), ys.iter().copied().fold(f32::MIN, f32::max)); + self.xf.user_rect(self.info, self.page, [x0, y0, x1, y1]) + }) + .chain(fallback) } /// A redaction mark's areas: one per marked quad, or its rectangle. @@ -624,7 +622,7 @@ impl PageCx<'_> { /// The topmost comment under `p`. fn hit(&self, p: Pos2) -> Option<&Annotation> { - self.comments().filter(|a| self.screen_rects(a).iter().any(|r| r.expand(3.0).contains(p))).last() + self.comments().filter(|a| self.screen_rects(a).any(|r| r.expand(3.0).contains(p))).last() } pub(crate) fn get(&self, index: usize) -> Option<&Annotation> { diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index 01166a41d1..a91b0d5412 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/crates/ui-egui/tests/comments.rs @@ -1457,6 +1457,143 @@ fn preference_hides_author_only_highlight_hover_previews() { assert!(h.query_by_label("Ada").is_none(), "author-only hover must be suppressed"); } +/// Contributor-original synthetic markup, with the first highlight ending on the next line. +fn markup_hover_fixture(subtype: &str, rotation: u16, extra: &str, refs: &str) -> Vec { + let content = "BT /F1 14 Tf 20 150 Td (Before the marked words) Tj 0 -40 Td (Marked words then other words) Tj ET\n"; + String::from_utf8_lossy(TEXT_FIXTURE) + .replace("/Contents 5 0 R /Resources", &format!("/Contents 5 0 R /Rotate {rotation} /Annots [8 0 R {refs}] /Resources")) + .replace("/Length 55", &format!("/Length {}", content.len())) + .replace("BT /F1 14 Tf 20 150 Td (The quick brown fox) Tj ET\n", content) + .replace( + "trailer", + &format!( + "8 0 obj << /Type /Annot /Subtype /{subtype} /Rect [20 105 250 165] \ + /QuadPoints [120 165 250 165 120 145 250 145 20 125 100 125 20 105 100 105] \ + /C [1 1 0] /T (Ada) /Contents (First multiline comment) >> endobj\n{extra}\ntrailer" + ), + ) + .into_bytes() +} + +fn markup_hover_harness(pdf: Vec, view_rotation: u16) -> Harness<'static, PdfCraftApp> { + // Give the fixture a complete xref so byte comparisons do not measure repair-on-save. + let doc = pdfcraft_cos::Document::open(std::sync::Arc::new(pdf)).expect("synthetic fixture parses"); + let pdf = pdfcraft_cos::write_full(&doc, &Default::default()).expect("writes complete fixture"); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("markup.pdf", None, pdf).expect("opens synthetic markup"); + app.set_option("left", "closed").unwrap(); + app.set_option("panel", "none").unwrap(); + app.views[0].rotation = view_rotation; + app + }); + h.run_steps(4); + settle(&mut h); + h +} + +/// Includes the document's crop/rotation and the independent Rotate View transform. +fn hover_markup_at(h: &mut Harness<'static, PdfCraftApp>, x: f32, y: f32) { + let p = { + let app = h.state(); + let v = &app.views[0]; + let info = &app.session.get(v.id).unwrap().info; + let page = &info.pages[0]; + let xf = pdfcraft_ui_egui::canvas::PageXform { + rect: v.page_screen_rect(0).expect("page on screen"), + rot: v.rotation, + pw: page.width, + ph: page.height, + }; + xf.user_rect(info, 0, [x, y, x, y]).center() + }; + h.hover_at(p); + h.run_steps(3); +} + +#[test] +fn multiline_markup_hover_stays_on_marked_lines() { + // #3541: /Rect encloses both lines, including text before/after the selection and the gap. + for subtype in ["Highlight", "Underline", "StrikeOut", "Squiggly"] { + for (page_rotation, view_rotation) in [(0, 0), (90, 0), (180, 0), (270, 0), (0, 90), (0, 180), (0, 270), (90, 270)] { + let pdf = markup_hover_fixture(subtype, page_rotation, "", ""); + let mut h = markup_hover_harness(pdf, view_rotation); + let original = h.state().session.get(h.state().views[0].id).unwrap().saved_bytes(); + for (x, y, marked) in + [(160.0, 155.0, true), (60.0, 115.0, true), (60.0, 155.0, false), (180.0, 115.0, false), (60.0, 135.0, false), (60.0, 75.0, false)] + { + hover_markup_at(&mut h, x, y); + assert_eq!( + h.query_by_label_contains("First multiline comment").is_some(), + marked, + "{subtype}, page/view rotation {page_rotation}/{view_rotation}, user point ({x}, {y})" + ); + } + let app = h.state(); + let id = app.views[0].id; + let doc = app.session.get(id).unwrap(); + assert!(!doc.dirty, "hovering never edits the document"); + assert_eq!(doc.can_undo(), None); + assert_eq!(app.session.save_bytes(id).unwrap().as_slice(), original.as_slice()); + } + } +} + +#[test] +fn neighboring_markup_hover_changes_to_its_own_comment() { + // #3620: the second highlight is in the first one's /Rect, but outside its marked lines. + let other = "9 0 obj << /Type /Annot /Subtype /Highlight /Rect [160 105 240 125] \ + /QuadPoints [160 125 240 125 160 105 240 105] /Contents (Neighbor comment) >> endobj"; + let mut h = markup_hover_harness(markup_hover_fixture("Highlight", 0, other, "9 0 R"), 0); + for (x, y, first) in [(160.0, 155.0, true), (180.0, 115.0, false), (60.0, 115.0, true)] { + hover_markup_at(&mut h, x, y); + assert_eq!(h.query_by_label_contains("First multiline comment").is_some(), first); + assert_eq!(h.query_by_label_contains("Neighbor comment").is_some(), !first); + } +} + +#[test] +fn overlapping_markup_hover_shows_the_topmost_comment() { + // A later annotation can also cover a genuinely marked part of the older highlight. + let other = "9 0 obj << /Type /Annot /Subtype /Highlight /Rect [150 145 200 165] \ + /QuadPoints [150 165 200 165 150 145 200 145] /Contents (Topmost comment) >> endobj"; + let mut h = markup_hover_harness(markup_hover_fixture("Highlight", 0, other, "9 0 R"), 0); + hover_markup_at(&mut h, 170.0, 155.0); + h.get_by_label_contains("Topmost comment"); + assert!(h.query_by_label_contains("First multiline comment").is_none()); + hover_markup_at(&mut h, 230.0, 155.0); + h.get_by_label_contains("First multiline comment"); + assert!(h.query_by_label_contains("Topmost comment").is_none()); +} + +#[test] +fn markup_hover_skips_popup_containers() { + // /Popup holds the parent's comment window; it is not a second comment in paint order. + let popup = "9 0 obj << /Type /Annot /Subtype /Popup /Parent 8 0 R /Rect [150 145 200 165] >> endobj"; + let mut h = markup_hover_harness(markup_hover_fixture("Highlight", 0, popup, "9 0 R"), 0); + hover_markup_at(&mut h, 170.0, 155.0); + h.get_by_label_contains("First multiline comment"); + assert!(h.query_by_label("Popup").is_none()); +} + +#[test] +fn markup_hover_without_quads_keeps_the_rectangle_fallback() { + for quads in ["", "/QuadPoints [20 105 250 165]"] { + let pdf = markup_hover_fixture("Highlight", 0, "", ""); + let pdf = String::from_utf8(pdf) + .unwrap() + .replace("/QuadPoints [120 165 250 165 120 145 250 145 20 125 100 125 20 105 100 105]", quads) + .into_bytes(); + let mut h = markup_hover_harness(pdf, 0); + assert!(comments(&h)[0].quads.is_empty()); + hover_markup_at(&mut h, 180.0, 115.0); + h.get_by_label_contains("First multiline comment"); + hover_markup_at(&mut h, 270.0, 115.0); + assert!(h.query_by_label_contains("First multiline comment").is_none()); + } +} + #[test] fn the_squiggly_tool_marks_selected_text() { let mut h = harness(|_| {}); diff --git a/docs/roadmap.md b/docs/roadmap.md index 5d73511e9c..2d29180578 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,5 +1,6 @@ # PdfCraft roadmap: milestones and what's next +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M3, #3541/#3620: comment hover follows individual marked lines and paint order; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M14/#2383 GPU-limit candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -114,7 +115,7 @@ total matches the parity estimate. | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. Page marks, added content and print output retain small page-unit transform coefficients when serialized. | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. The renderer keeps its one-point crop-size floor in physical points after UserUnit scaling, with regression coverage for crop origins, rotations and f32 reciprocal rounding. Existing unit policy and nearly-zero fallback are retained. | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings | -| M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. #3149: page-list changes discard render demand based on old geometry; actual raster dimensions are checked against the GPU limit before upload. Synthetic crash and continued-rendering regressions cover the replacement path. Save-prompt shortcuts yield to dialogs above them; stacked link-consent UI regression covers Escape and Return. | +| M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. #3149: page-list changes discard render demand based on old geometry; actual raster dimensions are checked against the GPU limit before upload. Synthetic crash and continued-rendering regressions cover the replacement path. Save-prompt shortcuts yield to dialogs above them; stacked link-consent UI regression covers Escape and Return. Comment hover follows individual marked lines and paint order (#3541/#3620); synthetic rotation, overlap and unchanged-document regressions. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. Wrapped revision buttons stay above Binding in Document Properties (#2739); a synthetic UI test checks layout and saved revision content. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Later native-open replies stay queued while a modal or picker owns the foreground; regressions cover recipient ownership, completion/cancellation and one-shot wakes. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. #1980: exact quit modifiers preserve Ctrl+Alt+Q email input; Ctrl+Q/Cmd+Q asks before discarding unsaved documents. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7aca7d5c8b..1d9c4d9f50 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1190,8 +1190,8 @@ milestone = "M3" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/comments.rs::hovering_a_comment_shows_its_author_and_text"] -notes = "a popup with the author and the comment text, as Acrobat shows" +evidence = ["crates/ui-egui/tests/comments.rs::hovering_a_comment_shows_its_author_and_text", "crates/ui-egui/tests/comments.rs::multiline_markup_hover_stays_on_marked_lines", "crates/ui-egui/tests/comments.rs::neighboring_markup_hover_changes_to_its_own_comment", "crates/ui-egui/tests/comments.rs::overlapping_markup_hover_shows_the_topmost_comment", "crates/ui-egui/tests/comments.rs::markup_hover_without_quads_keeps_the_rectangle_fallback", "crates/ui-egui/tests/comments.rs::markup_hover_skips_popup_containers", "crates/ui-egui/tests/comments.rs::preference_hides_author_only_highlight_hover_previews"] +notes = "A popup with the author and comment text. Text markup uses selection's per-quad screen bounds, including page and view rotation, so unmarked parts of a multiline Rect do not claim the hover (#3541). The last-painted eligible comment wins where annotations overlap (#3620). Missing complete quads retain the Rect fallback; author-only highlight hiding and dialog suppression remain. Bounds are rectangular, not exact hit testing of skewed quadrilaterals. Pointer feedback is UI-only, reachable through the opt-in control channel, with no document automation tool." [[feature]] id = "view.page-size-hover" From 941fbe7cda4be03ab4f48fd45161c38149c818c7 Mon Sep 17 00:00:00 2001 From: PerchAnton <101743951+PerchAnton@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:58:22 +0300 Subject: [PATCH 2/2] M3: choose overlapping comment hover by annotation index (#3620) --- ROADMAP.md | 4 +++- crates/ui-egui/src/canvas.rs | 27 ++++++++++++++++----------- crates/ui-egui/tests/comments.rs | 19 +++++++++++++++++++ docs/roadmap.md | 5 +++-- parity/acrobat-features.toml | 4 ++-- 5 files changed, 43 insertions(+), 16 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 670f14e681..b18b4fe76a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25% (ready for real work 50% → 75%) and ~400–700 h away -> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M3, #3541/#3620: comment hover follows individual marked lines and paint order; estimates unchanged) +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M3, #3541/#3620: comment hover follows individual marked lines and original annotation paint order; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: CSV tab-containing headers; estimates unchanged) @@ -173,6 +173,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-20 (M3, #3620):** Overlapping comment hover uses the original annotation index independently of spatial inspection order. A synthetic UI regression covers different annotation upper edges. Estimates unchanged. + - **2026-10-20 (M3, #3541/#3620):** Comment hover follows the individual marked lines instead of the enclosing multiline rectangle, and overlapping comments follow paint order. Synthetic UI regressions cover all four text-markup kinds, page/view rotations, neighboring highlights, legacy rectangle fallback and unchanged PDF bytes. Estimates unchanged. - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 8bb2bf4bfa..75f65986d1 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3511,17 +3511,22 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // freehand, where it would cover the next stroke (#429). let freehand = matches!(tool, QuickTool::Comment(comments::CommentTool::Ink | comments::CommentTool::Eraser)); let gesturing = view.comments.gesture.is_some() || freehand; - // Match selection's per-line geometry and paint order: a newer highlight's - // comment must not be shadowed by an older one's enclosing rectangle. - for a in info.annotations.iter().rev().filter(|a| a.page == i && !gesturing) { - let hidden = prefs.hide_empty_highlight_hover && a.subtype == "Highlight" && a.contents.as_deref().is_none_or(str::is_empty); - if hover_text.is_none() - && !hidden - && pcx.screen_rects(a).any(|r| r.contains(p)) - && let Some(text) = annotation_hover_text(a) - { - hover_text = Some((p, text)); - } + // Inspection sorts annotations spatially, but the renderer paints in /Annots + // order. Keep the shared list intact and choose the last eligible raw index. + if hover_text.is_none() + && !gesturing + && let Some((_, text)) = info + .annotations + .iter() + .filter(|a| a.page == i) + .filter(|a| { + !(prefs.hide_empty_highlight_hover && a.subtype == "Highlight" && a.contents.as_deref().is_none_or(str::is_empty)) + }) + .filter(|a| pcx.screen_rects(a).any(|r| r.contains(p))) + .filter_map(|a| annotation_hover_text(a).map(|text| (a.index, text))) + .max_by_key(|(index, _)| *index) + { + hover_text = Some((p, text)); } } for (mp, mr, mc) in &view.compare_marks { diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index a91b0d5412..00f08ba895 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/crates/ui-egui/tests/comments.rs @@ -1567,6 +1567,25 @@ fn overlapping_markup_hover_shows_the_topmost_comment() { assert!(h.query_by_label_contains("Topmost comment").is_none()); } +#[test] +fn overlapping_markup_hover_uses_annotation_index_with_different_top_edges() { + // Inspection sorts by position, while /Annots determines paint order. The later + // highlight wins the overlap whether its upper edge is above or below the first's. + for top in [175, 160] { + let other = format!( + "9 0 obj << /Type /Annot /Subtype /Highlight /Rect [150 145 200 {top}] \ + /QuadPoints [150 {top} 200 {top} 150 145 200 145] /Contents (Topmost comment) >> endobj" + ); + let mut h = markup_hover_harness(markup_hover_fixture("Highlight", 0, &other, "9 0 R"), 0); + hover_markup_at(&mut h, 170.0, 155.0); + h.get_by_label_contains("Topmost comment"); + assert!(h.query_by_label_contains("First multiline comment").is_none(), "later annotation with top {top} owns the overlap"); + hover_markup_at(&mut h, 230.0, 155.0); + h.get_by_label_contains("First multiline comment"); + assert!(h.query_by_label_contains("Topmost comment").is_none()); + } +} + #[test] fn markup_hover_skips_popup_containers() { // /Popup holds the parent's comment window; it is not a second comment in paint order. diff --git a/docs/roadmap.md b/docs/roadmap.md index 2d29180578..b830b57b06 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # PdfCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M3, #3541/#3620: comment hover follows individual marked lines and paint order; estimates unchanged) +> **Last reviewed:** 2026-10-20 · **Last updated:** 2026-10-20 · **Change:** minor (M3, #3541/#3620: comment hover follows individual marked lines and original annotation paint order; estimates unchanged) > **Last reviewed:** 2026-10-18 · **Last updated:** 2026-10-18 · **Change:** minor (M5, PR #1971: bounded nested FDF export excludes source page/page-tree dictionaries; refreshed against upstream 4db6ac4a; estimates unchanged) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M14/#2383 GPU-limit candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M7: the Edit text caret is as tall as the row it is on; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -115,7 +115,7 @@ total matches the parity estimate. | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. Page marks, added content and print output retain small page-unit transform coefficients when serialized. | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings. Packed TIFF-predictor RGB rows retain later rows in decoding and rendering. The renderer keeps its one-point crop-size floor in physical points after UserUnit scaling, with regression coverage for crop origins, rotations and f32 reciprocal rounding. Existing unit policy and nearly-zero fallback are retained. | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings | -| M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. #3149: page-list changes discard render demand based on old geometry; actual raster dimensions are checked against the GPU limit before upload. Synthetic crash and continued-rendering regressions cover the replacement path. Save-prompt shortcuts yield to dialogs above them; stacked link-consent UI regression covers Escape and Return. Comment hover follows individual marked lines and paint order (#3541/#3620); synthetic rotation, overlap and unchanged-document regressions. | +| M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. #3149: page-list changes discard render demand based on old geometry; actual raster dimensions are checked against the GPU limit before upload. Synthetic crash and continued-rendering regressions cover the replacement path. Save-prompt shortcuts yield to dialogs above them; stacked link-consent UI regression covers Escape and Return. Comment hover follows individual marked lines and original annotation paint order (#3541/#3620), independently of spatial inspection sorting; synthetic rotation, differing-upper-edge overlap and unchanged-document regressions. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). Selected-text Copy in Edit and the canvas context menu is covered by focused regressions and native clipboard checks (#1225); document copy restrictions apply to the floating action too. #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Page-label navigation displays logical labels, retains the current page on unchanged submission and refreshes focused same-document replacements; clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Synthetic control regressions cover these behaviors. Wrapped revision buttons stay above Binding in Document Properties (#2739); a synthetic UI test checks layout and saved revision content. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. Later native-open replies stay queued while a modal or picker owns the foreground; regressions cover recipient ownership, completion/cancellation and one-shot wakes. | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). #1364 candidate: absolute local GoToR links ask before target access; headless callers explicitly opt in. Native-opening candidate: chosen tools survive preparation and password retries; startup intake waits for pending preparation. #1980: exact quit modifiers preserve Ctrl+Alt+Q email input; Ctrl+Q/Cmd+Q asks before discarding unsaved documents. | @@ -201,6 +201,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st | Date | Change | Summary | |---|---|---| +| 2026-10-20 | minor | M3/#3620: overlapping comment hover follows original annotation indices independently of spatial inspection sorting; a synthetic differing-upper-edge regression covers paint order. Estimates unchanged. | | 2026-10-18 | minor | M5, PR #1971: nested source `/Page` and `/Pages` dictionaries export as null without traversing page content or the page tree. Synthetic saved/reopened three-page regression retains unrelated metadata and aliases. FDF import semantics, copied-graph limits and estimates unchanged. | | 2026-10-15 | minor | M5: budget retained FDF entries before cloning; discovery keeps lazy array owners and annotation positions, with alias/cache-eviction stress fixtures. Limits cover the copied graph; source parsing/page discovery retain existing COS/parser limits, with no total source-memory bound. FDF import semantics and estimates unchanged. | | 2026-10-15 | minor | M7: Edit text caret height follows its row, with a kittest regression; estimates unchanged | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 1d9c4d9f50..bc046ca0fb 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1190,8 +1190,8 @@ milestone = "M3" status = "shipped" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/comments.rs::hovering_a_comment_shows_its_author_and_text", "crates/ui-egui/tests/comments.rs::multiline_markup_hover_stays_on_marked_lines", "crates/ui-egui/tests/comments.rs::neighboring_markup_hover_changes_to_its_own_comment", "crates/ui-egui/tests/comments.rs::overlapping_markup_hover_shows_the_topmost_comment", "crates/ui-egui/tests/comments.rs::markup_hover_without_quads_keeps_the_rectangle_fallback", "crates/ui-egui/tests/comments.rs::markup_hover_skips_popup_containers", "crates/ui-egui/tests/comments.rs::preference_hides_author_only_highlight_hover_previews"] -notes = "A popup with the author and comment text. Text markup uses selection's per-quad screen bounds, including page and view rotation, so unmarked parts of a multiline Rect do not claim the hover (#3541). The last-painted eligible comment wins where annotations overlap (#3620). Missing complete quads retain the Rect fallback; author-only highlight hiding and dialog suppression remain. Bounds are rectangular, not exact hit testing of skewed quadrilaterals. Pointer feedback is UI-only, reachable through the opt-in control channel, with no document automation tool." +evidence = ["crates/ui-egui/tests/comments.rs::hovering_a_comment_shows_its_author_and_text", "crates/ui-egui/tests/comments.rs::multiline_markup_hover_stays_on_marked_lines", "crates/ui-egui/tests/comments.rs::neighboring_markup_hover_changes_to_its_own_comment", "crates/ui-egui/tests/comments.rs::overlapping_markup_hover_shows_the_topmost_comment", "crates/ui-egui/tests/comments.rs::overlapping_markup_hover_uses_annotation_index_with_different_top_edges", "crates/ui-egui/tests/comments.rs::markup_hover_without_quads_keeps_the_rectangle_fallback", "crates/ui-egui/tests/comments.rs::markup_hover_skips_popup_containers", "crates/ui-egui/tests/comments.rs::preference_hides_author_only_highlight_hover_previews"] +notes = "A popup with the author and comment text. Text markup uses selection's per-quad screen bounds, including page and view rotation, so unmarked parts of a multiline Rect do not claim the hover (#3541). The last-painted eligible comment wins where annotations overlap (#3620), using the original /Annots index independently of spatial inspection order. Missing complete quads retain the Rect fallback; author-only highlight hiding and dialog suppression remain. Bounds are rectangular, not exact hit testing of skewed quadrilaterals. Pointer feedback is UI-only, reachable through the opt-in control channel, with no document automation tool." [[feature]] id = "view.page-size-hover"