feat(pdf): draw annotations in the browser - #849
Merged
Conversation
andiwand
force-pushed
the
feat/pdf-annotate-api
branch
from
September 6, 2026 15:45
28a9001 to
16a4509
Compare
`odr.annotation` collects what a viewer marks up and hands back the payload `PdfFile::annotate` takes. Each page div now carries `data-odr-page` and `data-odr-space`, the latter the inverse of the page transform, so a viewport point reaches pdf user space without the browser knowing anything else about the file. Two overlays per page rather than one: `mix-blend-mode` on a shape inside an svg composites against that svg's own canvas, so a highlight painted that way sits on top of the glyphs instead of letting them through. The blend belongs on the overlay, which means the washes need one of their own. The overlay takes pointer events only while the ink tool is active; a text tool leaves the selection layer alone, which is what lets a viewer select text to highlight it in the first place. Checks in `test/browser/annotation/`, run by hand like the other emitted scripts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018e3PEzyU2oAFSzsEoWsSmz
`selectionchange` fires on every character a drag covers, so the highlight tool took the first one and called `removeAllRanges()` while the pointer was still down — the mark was a fragment and the selection was torn away mid-drag. Stepping a selection out by hand made six annotations where one was meant. The mark now waits for the pointer that made it to come up, a new gesture cancels one still queued, and the harness steps a drag through rather than adding one range in a single call, which fired `selectionchange` once and so never reached the case. Also: `-0` out of the page's `data-odr-space`, `metrics()` folded into its one caller now that the overlay does not need a client rect to size its viewBox, and the page index spelled out rather than derived from the vector's size after the emplace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147n68S7LNAv9KynLaGgXN4
andiwand
force-pushed
the
feat/pdf-annotation-browser
branch
from
September 6, 2026 16:03
91216ac to
98f28b0
Compare
andiwand
added a commit
to opendocument-app/OpenDocument.test.output
that referenced
this pull request
Sep 6, 2026
Every pdf page gains `data-odr-page` and `data-odr-space` — the map back from the page box to pdf user space, which the annotator needs to place what a viewer draws — and the two files the overlay loads. Regenerated for opendocument-app/OpenDocument.core#849.
andiwand
added a commit
that referenced
this pull request
Sep 6, 2026
The design doc becomes a record: the decisions and the alternatives they beat, which is what a reader changing this next needs, and a short account of how it landed. The phase list, the day estimates and the line counts go — git and the pull requests hold that. Two things cost more than the estimate said and neither was found by a test, so both are written down: a `mix-blend-mode` inside an svg composites against that svg's own canvas rather than the page, and `selectionchange` fires on every character a drag covers. `pdf/AGENTS.md` no longer calls the module read-only, and its module table has the two files that write. Advances the reference-output pins for #849, which put `data-odr-page` and `data-odr-space` on every pdf page and two files in `resources/`. 829 files changed; no line differs beyond those attributes, the stylesheet link and the script tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018e3PEzyU2oAFSzsEoWsSmz
andiwand
added a commit
that referenced
this pull request
Sep 6, 2026
…#851) * docs(pdf): retire the annotation plan, its phases having landed The design doc becomes a record: the decisions and the alternatives they beat, which is what a reader changing this next needs, and a short account of how it landed. The phase list, the day estimates and the line counts go — git and the pull requests hold that. Two things cost more than the estimate said and neither was found by a test, so both are written down: a `mix-blend-mode` inside an svg composites against that svg's own canvas rather than the page, and `selectionchange` fires on every character a drag covers. `pdf/AGENTS.md` no longer calls the module read-only, and its module table has the two files that write. Advances the reference-output pins for #849, which put `data-odr-page` and `data-odr-space` on every pdf page and two files in `resources/`. 829 files changed; no line differs beyond those attributes, the stylesheet link and the script tag. * fix(pdf): answer whether a file can actually be annotated `capabilities().annotate` was true for every pdf, so a caller keying a button off it got one that throws: `IncrementalWriter` refuses a file whose cross-reference table was rebuilt, and one declaring an `/Encrypt`. `PdfFile::is_annotatable` is the counterpart of `Document::is_editable` — `FileTypeCapabilities` answers for the format, this for the file in hand — and `DecodedFile::capabilities()` narrows `annotate` by it. Bound in all four bindings, as `is_editable` is. Neither condition is `encryption_state()`. An owner-locked pdf opens with the empty password and reports itself *unencrypted*, yet its `/Encrypt` still stops us appending, so the answer is settled during the construction-time probe, where the parser that knows both is already in hand. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Phase 5 of
docs/design/pdf-annotation.md. Stacked on #848. Third of four; bindings follow.odr.annotationcollects what a viewer marks up and hands back the payloadPdfFile::annotatetakes:Coordinates
Each page div now carries
data-odr-pageanddata-odr-space— the latter isto_box⁻¹, which is whatTransform2D::inversewas added for back in #843. A viewport point divides out the zoom (rect.width / offsetWidth, robust against the zoom script's CSS transform), converts CSS pixels to points, and goes through that matrix. The model keeps page-box points and maps to user space only on the way out, so there is one representation to reason about.The bug worth recording
The highlight painted over the text.
mix-blend-modeon a shape inside an<svg>composites against that svg's own canvas, not against the page behind it — so the wash covered the glyphs instead of letting them through, exactly the failure the whole/BM /Multiplydesign exists to avoid.The blend has to sit on the overlay element, which means the washes need an overlay of their own (
svg.an-m) separate from the marks drawn on top of the text (svg.an). Two overlays per page. Caught by eye in the browser, not by a test — which is why the harness below now checks which overlay a shape lands in.Pointer handling
The overlay takes pointer events only while the ink tool is active. A text tool leaves the selection layer alone — that is what lets a viewer select text to highlight it at all, and it sidesteps the link-overlay-vs-selection conflict for the markup tools.
Verified
test/browser/annotation/— a harness in the shape oftest/browser/viewport/:servelifts the script and style out offrontend.cppso what runs is what ships, andtests.htmlstands in for a rendered view with two pages carrying realdata-odr-spaceattributes. 22 checks pass, covering the quad's user-space mapping, Z-ordering, which overlay each kind lands in, page indices, ink through realPointerEvents, and undo/remove/clear.One of those checks caught a mistake in the harness rather than the script: I had predicted the quad's position from the CSS
top, but a text range's client rect follows the font's metrics, not the line box. It now measures the run's own rect independently of the script, per the convention the viewport README sets out.End to end, against
style-various-1.pdf: marked up five annotations in a real browser, tookgetAnnotations()verbatim, fed it toPdfFile::annotate, and the rendered PDF places every one exactly where the browser drew it.379 tests pass;
-Werrorclean.No changelog entry: #848 already covers the feature, and this adds no new public C++ surface.