mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-27 03:27:21 -06:00
M3: choose overlapping comment hover by annotation index (#3620)
This commit is contained in:
+3
-1
@@ -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.
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
+3
-2
@@ -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<br>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<br>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<br>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<br>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<br>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 |
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user