Skip to content

feat(pdf): draw annotations in the browser - #849

Merged
andiwand merged 2 commits into
mainfrom
feat/pdf-annotation-browser
Sep 6, 2026
Merged

feat(pdf): draw annotations in the browser#849
andiwand merged 2 commits into
mainfrom
feat/pdf-annotation-browser

Conversation

@andiwand

@andiwand andiwand commented Sep 6, 2026

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

Phase 5 of docs/design/pdf-annotation.md. Stacked on #848. Third of four; bindings follow.

odr.annotation collects what a viewer marks up and hands back the payload PdfFile::annotate takes:

odr.annotation.setTool("highlight" | "underline" | "strikeOut" | "squiggly" | "ink" | null);
odr.annotation.setColor([1, 0.9, 0.2]);   // DeviceRGB
odr.annotation.setWidth(3);               // ink
odr.annotation.list(); undo(); remove(id); clear();
odr.annotation.getAnnotations();          // the json #848 consumes

Coordinates

Each page div now carries data-odr-page and data-odr-space — the latter is to_box⁻¹, which is what Transform2D::inverse was 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-mode on 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 /Multiply design 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 of test/browser/viewport/: serve lifts the script and style out of frontend.cpp so what runs is what ships, and tests.html stands in for a rendered view with two pages carrying real data-odr-space attributes. 22 checks pass, covering the quad's user-space mapping, Z-ordering, which overlay each kind lands in, page indices, ink through real PointerEvents, 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, took getAnnotations() verbatim, fed it to PdfFile::annotate, and the rendered PDF places every one exactly where the browser drew it.

379 tests pass; -Werror clean.

No changelog entry: #848 already covers the feature, and this adds no new public C++ surface.

@andiwand
andiwand force-pushed the feat/pdf-annotate-api branch from 28a9001 to 16a4509 Compare September 6, 2026 15:45
Base automatically changed from feat/pdf-annotate-api to main September 6, 2026 15:48
`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
andiwand force-pushed the feat/pdf-annotation-browser branch from 91216ac to 98f28b0 Compare September 6, 2026 16:03
@andiwand
andiwand merged commit 7722004 into main Sep 6, 2026
29 checks passed
@andiwand
andiwand deleted the feat/pdf-annotation-browser branch September 6, 2026 16:09
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant