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] 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 670f14e68..b18b4fe76 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 8bb2bf4bf..75f65986d 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 a91b0d541..00f08ba89 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 2d2918057..b830b57b0 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 1d9c4d9f5..bc046ca0f 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"