From 67217b097aa07c9fd536a5b7def4c33fe023445f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 17:37:40 +0200 Subject: [PATCH 1/2] feat(pdf): draw annotations in the browser `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) Claude-Session: https://claude.ai/code/session_018e3PEzyU2oAFSzsEoWsSmz --- docs/design/pdf-annotation.md | 35 ++- src/odr/internal/html/frontend.cpp | 395 +++++++++++++++++++++++++++++ src/odr/internal/html/frontend.hpp | 6 + src/odr/internal/html/pdf_file.cpp | 47 +++- test/browser/annotation/.gitignore | 2 + test/browser/annotation/README.md | 33 +++ test/browser/annotation/serve | 40 +++ test/browser/annotation/tests.html | 234 +++++++++++++++++ 8 files changed, 771 insertions(+), 21 deletions(-) create mode 100644 test/browser/annotation/.gitignore create mode 100644 test/browser/annotation/README.md create mode 100755 test/browser/annotation/serve create mode 100644 test/browser/annotation/tests.html diff --git a/docs/design/pdf-annotation.md b/docs/design/pdf-annotation.md index 8b0085185..1bd7de9ec 100644 --- a/docs/design/pdf-annotation.md +++ b/docs/design/pdf-annotation.md @@ -3,8 +3,8 @@ Status: **underway.** This records the architecture for adding markup annotations — text highlight and freehand drawing first — to an existing PDF, the alternatives weighed, and the effort it costs. The format model is -validated against four viewers, and Phases 0 through 3 have landed: the writer -appends, and the markup and ink annotations it carries are written. +validated against four viewers, and Phases 0 through 5 have landed: the browser +draws the markup and the writer appends it; the bindings are what is left. Scope is **markup only**: draw on top of a page, highlight/underline/strike text. Editing or removing the *existing* text of a PDF is explicitly out — that @@ -300,19 +300,28 @@ throwing per the repo's fail-fast rule. A `FileTypeCapabilities` bit for it, and the `file_type_table` row (the capability test fails if the declaration exceeds what the engine does). -### Phase 5 — browser layer (4–6 d, ~700 JS + 80 CSS) +### Phase 5 — browser layer — **done** (#849) -`pdf_annotation_js` in `frontend.cpp`, following `viewport_js`/`search_js`: +`pdf_annotation_js` and `pdf_annotation_css` in `frontend.cpp`, alongside +`viewport_js`/`search_js`, exposing `odr.annotation`. -- Per-page overlay SVG, live preview of pending annotations. -- Highlight tool: selection → `getClientRects()` → merge per line, drop the - zero-width spacer spans of the `.sel` layer, clip to the page box. -- Ink tool: pointer events, coalesced points. -- Coordinate helper: client rect → page-div rect → scale by - `divRect.width / pageWidthPt` (robust against the zoom script's CSS transform) - → `to_box⁻¹`. -- Undo/redo, colour, delete-by-hit-test. -- `odr.getAnnotations()` returning the payload above. +Each page div carries `data-odr-page` and `data-odr-space`, the latter being +`to_box⁻¹` — which is what `Transform2D::inverse` was added for. A viewport +point divides out the zoom (`rect.width / offsetWidth`), converts css pixels to +points, and goes through that matrix; the model keeps page-box points and maps +to user space only in `getAnnotations()`. + +**Two overlays per page.** A `mix-blend-mode` on a shape *inside* an svg +composites against the svg's own canvas, not against the page, so a highlight +painted that way covers the glyphs instead of letting them through. The blend +belongs on the overlay element, and the washes therefore need an overlay of +their own (`svg.an-m`) separate from the marks drawn on top (`svg.an`). + +The overlay captures pointer events only for ink; the text tools leave the +selection layer alone, which is what makes selecting text to highlight work. + +Checks in `test/browser/annotation/`, run by hand as the repo's other emitted +scripts are. ### Phase 6 — bindings (2 d, ~470 lines) diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index d2443716e..d43457fe6 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -710,6 +710,382 @@ constexpr std::string_view viewport_js = R"js( )js"; /// Text search over the rendered page, format-agnostic: it walks text nodes. +constexpr std::string_view pdf_annotation_css = R"css( +.an{position:absolute;inset:0;overflow:visible;pointer-events:none;z-index:3} +/* on the overlay, not on the shape: a blend inside an svg composites against + the svg's own canvas, so the highlight would paint over the glyphs */ +.an-m{mix-blend-mode:multiply} +.p.an-draw .an{pointer-events:auto;cursor:crosshair;touch-action:none} +.p.an-draw .t,.p.an-draw .sel{pointer-events:none} +)css"; +/// `odr.annotation`: the pending markup a viewer draws, and the payload +/// `PdfFile::annotate` takes. Geometry is kept in page-box points and mapped +/// to pdf user space only on the way out, through the `data-odr-space` each +/// page carries. +constexpr std::string_view pdf_annotation_js = R"js( +(function () { + "use strict"; + + var odr = (window.odr = window.odr || {}); + var SVG = "http://www.w3.org/2000/svg"; + + var tool = null; + var color = [1, 0.9, 0.2]; + var width = 2; + var pending = []; + var nextId = 1; + + function pages() { + return Array.prototype.slice.call( + document.querySelectorAll("[data-odr-space]") + ); + } + + function pageOf(index) { + var all = pages(); + for (var i = 0; i < all.length; ++i) { + if (+all[i].getAttribute("data-odr-page") === index) { + return all[i]; + } + } + return null; + } + + // The page box is laid out in inches, so its own layout width in css pixels + // gives the scale a zoom transform is applied on top of. + function metrics(page) { + var rect = page.getBoundingClientRect(); + var zoom = page.offsetWidth ? rect.width / page.offsetWidth : 1; + return { rect: rect, zoom: zoom, points: page.offsetWidth * 0.75 }; + } + + /// A viewport point to page-box points (y-down, the unit the overlay draws in). + function toBox(page, clientX, clientY) { + var m = metrics(page); + return [ + ((clientX - m.rect.left) / m.zoom) * 0.75, + ((clientY - m.rect.top) / m.zoom) * 0.75, + ]; + } + + /// Page-box points to pdf user space, through the page's own inverse. + function toUserSpace(page, x, y) { + var m = page.getAttribute("data-odr-space").split(",").map(Number); + return [ + m[0] * x + m[2] * y + m[4], + m[1] * x + m[3] * y + m[5], + ]; + } + + /// Two overlays per page: `multiply` for the washes that have to let the + /// text through, and a normal one for the marks drawn on top of it. + function overlay(page, multiply) { + var name = multiply ? "an an-m" : "an"; + var svg = page.querySelector( + ':scope > svg[class="' + name + '"]' + ); + if (!svg) { + svg = document.createElementNS(SVG, "svg"); + svg.setAttribute("class", name); + svg.setAttribute("preserveAspectRatio", "none"); + page.appendChild(svg); + } + var m = metrics(page); + var height = page.offsetHeight * 0.75; + svg.setAttribute("viewBox", "0 0 " + m.points + " " + height); + return svg; + } + + function css(c) { + return ( + "rgb(" + + c + .map(function (v) { + return Math.round(Math.max(0, Math.min(1, v)) * 255); + }) + .join(",") + + ")" + ); + } + + function draw(annotation) { + var page = pageOf(annotation.page); + if (!page) { + return; + } + var svg = overlay(page, annotation.type === "highlight"); + var node; + if (annotation.type === "ink") { + node = document.createElementNS(SVG, "path"); + node.setAttribute( + "d", + annotation.strokes + .map(function (s) { + var d = "M " + s[0] + " " + s[1]; + for (var i = 2; i < s.length; i += 2) { + d += " L " + s[i] + " " + s[i + 1]; + } + return d; + }) + .join(" ") + ); + node.setAttribute("fill", "none"); + node.setAttribute("stroke", css(annotation.color)); + node.setAttribute("stroke-width", annotation.width); + node.setAttribute("stroke-linecap", "round"); + node.setAttribute("stroke-linejoin", "round"); + } else { + node = document.createElementNS(SVG, "path"); + node.setAttribute("d", annotation.boxes.map(barPath(annotation.type)).join(" ")); + if (annotation.type === "squiggly") { + node.setAttribute("fill", "none"); + node.setAttribute("stroke", css(annotation.color)); + node.setAttribute("stroke-width", 1); + } else { + node.setAttribute("fill", css(annotation.color)); + } + } + node.setAttribute("data-odr-annotation", annotation.id); + svg.appendChild(node); + } + + /// The shape one covered box gets, in page-box points. + function barPath(type) { + return function (b) { + var h = b[3] - b[1]; + if (type === "highlight") { + return rect(b[0], b[1], b[2] - b[0], h); + } + if (type === "underline") { + return rect(b[0], b[3] - h / 16, b[2] - b[0], Math.max(h / 16, 0.5)); + } + if (type === "strikeOut") { + return rect(b[0], b[1] + h / 2, b[2] - b[0], Math.max(h / 16, 0.5)); + } + var step = Math.max(h / 8, 1); + var d = "M " + b[0] + " " + (b[3] - step); + var up = true; + for (var x = b[0] + step; x < b[2]; x += step, up = !up) { + d += " L " + x + " " + (up ? b[3] - step * 2 : b[3] - step); + } + return d; + }; + } + + function rect(x, y, w, h) { + return "M " + x + " " + y + " h " + w + " v " + h + " h " + -w + " Z"; + } + + function redraw() { + pages().forEach(function (page) { + page.querySelectorAll(":scope > svg.an").forEach(function (svg) { + svg.textContent = ""; + }); + }); + pending.forEach(draw); + } + + /// The boxes a selection covers, per page, in page-box points. Zero-width + /// rects are the selection layer's spacer spans and carry no text. + function selectionBoxes() { + var selection = window.getSelection(); + var byPage = {}; + if (!selection || selection.isCollapsed) { + return byPage; + } + for (var r = 0; r < selection.rangeCount; ++r) { + var rects = selection.getRangeAt(r).getClientRects(); + for (var i = 0; i < rects.length; ++i) { + var rect = rects[i]; + if (rect.width < 0.5 || rect.height < 0.5) { + continue; + } + var page = pageAt(rect.left + rect.width / 2, rect.top + rect.height / 2); + if (!page) { + continue; + } + var index = +page.getAttribute("data-odr-page"); + var a = toBox(page, rect.left, rect.top); + var b = toBox(page, rect.right, rect.bottom); + (byPage[index] = byPage[index] || []).push([a[0], a[1], b[0], b[1]]); + } + } + return byPage; + } + + function pageAt(x, y) { + var all = pages(); + for (var i = 0; i < all.length; ++i) { + var rect = all[i].getBoundingClientRect(); + if (x >= rect.left && x <= rect.right && y >= rect.top && y <= rect.bottom) { + return all[i]; + } + } + return null; + } + + function markSelection() { + var byPage = selectionBoxes(); + var added = false; + Object.keys(byPage).forEach(function (index) { + pending.push({ + id: nextId++, + page: +index, + type: tool, + boxes: byPage[index], + color: color.slice(), + }); + added = true; + }); + if (added) { + window.getSelection().removeAllRanges(); + redraw(); + } + return added; + } + + var stroke = null; + + function onPointerDown(event) { + if (tool !== "ink" || event.button !== 0) { + return; + } + var page = pageAt(event.clientX, event.clientY); + if (!page) { + return; + } + event.preventDefault(); + var p = toBox(page, event.clientX, event.clientY); + stroke = { + id: nextId++, + page: +page.getAttribute("data-odr-page"), + type: "ink", + strokes: [[p[0], p[1]]], + color: color.slice(), + width: width, + }; + pending.push(stroke); + page.setPointerCapture(event.pointerId); + } + + function onPointerMove(event) { + if (!stroke) { + return; + } + var page = pageOf(stroke.page); + var p = toBox(page, event.clientX, event.clientY); + var points = stroke.strokes[0]; + // drop the sub-point jitter a pointer emits while nearly still + if ( + Math.abs(p[0] - points[points.length - 2]) + + Math.abs(p[1] - points[points.length - 1]) < + 0.5 + ) { + return; + } + points.push(p[0], p[1]); + redraw(); + } + + function onPointerUp() { + if (stroke && stroke.strokes[0].length < 4) { + // a tap with no drag leaves a dot, which is a legitimate mark + stroke.strokes[0].push(stroke.strokes[0][0], stroke.strokes[0][1]); + } + stroke = null; + redraw(); + } + + document.addEventListener("pointerdown", onPointerDown); + document.addEventListener("pointermove", onPointerMove); + document.addEventListener("pointerup", onPointerUp); + document.addEventListener("pointercancel", onPointerUp); + document.addEventListener("selectionchange", function () { + if (tool && tool !== "ink") { + // a selection completes the mark; the pointer is already up by then + window.setTimeout(markSelection, 0); + } + }); + window.addEventListener("resize", redraw); + + odr.annotation = { + /// null, "highlight", "underline", "strikeOut", "squiggly" or "ink". + setTool: function (value) { + tool = value || null; + pages().forEach(function (page) { + page.classList.toggle("an-draw", tool === "ink"); + }); + }, + getTool: function () { + return tool; + }, + /// DeviceRGB, each component in [0, 1]. + setColor: function (value) { + color = value.slice(0, 3).map(Number); + }, + setWidth: function (value) { + width = Number(value); + }, + /// What is pending, newest last. Geometry is in page-box points. + list: function () { + return pending.slice(); + }, + remove: function (id) { + pending = pending.filter(function (a) { + return a.id !== id; + }); + redraw(); + }, + undo: function () { + pending.pop(); + redraw(); + }, + clear: function () { + pending = []; + redraw(); + }, + /// The payload `PdfFile::annotate` takes, in pdf user space. + getAnnotations: function () { + return JSON.stringify({ + version: 1, + annotations: pending.map(function (a) { + var page = pageOf(a.page); + if (a.type === "ink") { + return { + page: a.page, + type: "ink", + strokes: a.strokes.map(function (s) { + var out = []; + for (var i = 0; i < s.length; i += 2) { + var p = toUserSpace(page, s[i], s[i + 1]); + out.push(p[0], p[1]); + } + return out; + }), + width: a.width, + color: a.color, + }; + } + return { + page: a.page, + type: a.type, + quads: a.boxes.map(function (b) { + // upper-left, upper-right, lower-left, lower-right + var ul = toUserSpace(page, b[0], b[1]); + var ur = toUserSpace(page, b[2], b[1]); + var ll = toUserSpace(page, b[0], b[3]); + var lr = toUserSpace(page, b[2], b[3]); + return [ul[0], ul[1], ur[0], ur[1], ll[0], ll[1], lr[0], lr[1]]; + }), + color: a.color, + }; + }), + }); + }, + }; +})(); +)js"; + constexpr std::string_view search_js = R"js( (function () { "use strict"; @@ -1701,6 +2077,11 @@ constexpr Asset text_js_asset{HtmlResourceType::js, "text/javascript", "text.js", text_js}; constexpr Asset viewport_js_asset{HtmlResourceType::js, "text/javascript", "viewport.js", viewport_js}; +constexpr Asset pdf_annotation_css_asset{HtmlResourceType::css, "text/css", + "pdf-annotation.css", + pdf_annotation_css}; +constexpr Asset pdf_annotation_js_asset{HtmlResourceType::js, "text/javascript", + "pdf-annotation.js", pdf_annotation_js}; /// Appends @p asset to @p resources; `nullopt` to embed it. HtmlResourceLocation locate(const Asset &asset, const HtmlConfig &config, @@ -1854,6 +2235,14 @@ void html::write_text_script(const WritingState &state) { write_script(text_js_asset, state); } +void html::write_pdf_annotation_style(const WritingState &state) { + write_style(pdf_annotation_css_asset, state); +} + +void html::write_pdf_annotation_script(const WritingState &state) { + write_script(pdf_annotation_js_asset, state); +} + void html::write_viewport_script(const WritingState &state) { write_script(viewport_js_asset, state); } @@ -1883,6 +2272,12 @@ HtmlResources html::locate_viewport_resources(const HtmlConfig &config) { return locate_all(assets, config); } +HtmlResources html::locate_pdf_annotation_resources(const HtmlConfig &config) { + static constexpr std::array assets{pdf_annotation_css_asset, + pdf_annotation_js_asset}; + return locate_all(assets, config); +} + HtmlResources html::locate_media_resources(const HtmlConfig &config) { static constexpr std::array assets{media_css_asset}; return locate_all(assets, config); diff --git a/src/odr/internal/html/frontend.hpp b/src/odr/internal/html/frontend.hpp index b22754de2..3ad2672cb 100644 --- a/src/odr/internal/html/frontend.hpp +++ b/src/odr/internal/html/frontend.hpp @@ -45,6 +45,11 @@ void write_text_script(const WritingState &state); /// rest of that object, for every view rendering text, whatever the format. void write_search_script(const WritingState &state); +/// `odr.annotation`: the pending markup a viewer draws on a pdf page, and +/// `getAnnotations()`, the payload @ref odr::PdfFile::annotate takes. +void write_pdf_annotation_style(const WritingState &state); +void write_pdf_annotation_script(const WritingState &state); + /// `odr.getZoom()`, `setZoom(value, focus)`, `adjustZoom(factor, focus)`, /// `resetZoom(focus)`, `isZoomFitted()`, `getViewportRect(element)`, /// `onZoomChange`, plus the fit @ref write_zoom_style left to be measured. @@ -59,5 +64,6 @@ HtmlResources locate_xml_resources(const HtmlConfig &config); HtmlResources locate_media_resources(const HtmlConfig &config); HtmlResources locate_search_resources(const HtmlConfig &config); HtmlResources locate_viewport_resources(const HtmlConfig &config); +HtmlResources locate_pdf_annotation_resources(const HtmlConfig &config); } // namespace odr::internal::html diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index b5b759435..36ae710db 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -66,6 +67,24 @@ std::string svg_matrix(const util::math::Transform2D &m) { return std::move(f).str(); } +/// The page's index and the map back from its box to pdf user space, for the +/// annotator to place what the user draws. Empty when the transform is +/// singular and nothing can be placed. +std::string page_attributes(const std::size_t index, + const util::math::Transform2D &to_box) { + const std::optional from_box = to_box.inverse(); + if (!from_box.has_value()) { + return {}; + } + const auto n = [](const double v) { + return util::number::to_string_significant(v, 10); + }; + return R"( data-odr-page=")" + std::to_string(index) + + R"(" data-odr-space=")" + n(from_box->a) + ',' + n(from_box->b) + ',' + + n(from_box->c) + ',' + n(from_box->d) + ',' + n(from_box->e) + ',' + + n(from_box->f) + '"'; +} + /// One resolved link annotation, positioned in page-box points (y-down). struct LinkOut { double left{0}; @@ -1127,6 +1146,9 @@ class HtmlServiceImpl final : public HtmlService { HtmlServiceImpl(PdfFile pdf_file, HtmlConfig config, const Logger &logger) : HtmlService(std::move(config), logger), m_pdf_file{std::move(pdf_file)}, m_resources{locate_search_resources(this->config())} { + for (auto &&resource : locate_pdf_annotation_resources(this->config())) { + m_resources.emplace_back(std::move(resource)); + } // declared before any page is parsed, so before the views are known for (auto &&resource : locate_viewport_resources(this->config())) { m_resources.push_back(std::move(resource)); @@ -1328,6 +1350,7 @@ class HtmlServiceImpl final : public HtmlService { struct DualPageOut { std::string classes; + std::string attributes; double width{0}; double height{0}; std::vector vis_items; @@ -1401,6 +1424,8 @@ class HtmlServiceImpl final : public HtmlService { DualPageOut &page_out = pages_out.emplace_back(); page_out.classes = pb.classes; + page_out.attributes = + page_attributes(first_page_number - 1 + pages_out.size() - 1, to_box); page_out.width = width; page_out.height = height; page_out.links = @@ -1837,10 +1862,10 @@ class HtmlServiceImpl final : public HtmlService { std::size_t page_number = first_page_number; for (const DualPageOut &page : pages_out) { out.write_element_begin( - "div", - HtmlElementOptions() - .set_class(page.classes) - .set_extra(R"(id="p)" + std::to_string(page_number++) + R"(")")); + "div", HtmlElementOptions() + .set_class(page.classes) + .set_extra(R"(id="p)" + std::to_string(page_number++) + + R"(")" + page.attributes)); // Visual layer: paint-order graphics and unselectable glyphs. out.write_element_begin("div", @@ -1865,6 +1890,7 @@ class HtmlServiceImpl final : public HtmlService { out.write_element_end("div"); // .d write_search_script(state); write_viewport_script(state); + write_pdf_annotation_script(state); out.write_body_end(); out.write_end(); @@ -1909,6 +1935,7 @@ class HtmlServiceImpl final : public HtmlService { struct SinglePageOut { std::string classes; + std::string attributes; double width{0}; double height{0}; std::vector items; @@ -2046,6 +2073,8 @@ class HtmlServiceImpl final : public HtmlService { SinglePageOut &page_out = pages_out.emplace_back(); page_out.classes = pb.classes; + page_out.attributes = + page_attributes(first_page_number - 1 + pages_out.size() - 1, to_box); page_out.width = width; page_out.height = height; page_out.links = @@ -2336,10 +2365,10 @@ class HtmlServiceImpl final : public HtmlService { std::size_t page_number = first_page_number; for (const SinglePageOut &page : pages_out) { out.write_element_begin( - "div", - HtmlElementOptions() - .set_class(page.classes) - .set_extra(R"(id="p)" + std::to_string(page_number++) + R"(")")); + "div", HtmlElementOptions() + .set_class(page.classes) + .set_extra(R"(id="p)" + std::to_string(page_number++) + + R"(")" + page.attributes)); write_page_items(out, page.clip_defs, page.items, page.width, page.height, write_line); write_page_links(out, page.links); @@ -2348,6 +2377,7 @@ class HtmlServiceImpl final : public HtmlService { out.write_element_end("div"); // .d write_search_script(state); write_viewport_script(state); + write_pdf_annotation_script(state); out.write_body_end(); out.write_end(); @@ -2679,6 +2709,7 @@ class HtmlServiceImpl final : public HtmlService { styles.write_rules(out.out()); out.write_header_style_end(); write_search_style(state); + write_pdf_annotation_style(state); out.write_header_end(); } diff --git a/test/browser/annotation/.gitignore b/test/browser/annotation/.gitignore new file mode 100644 index 000000000..75034e7db --- /dev/null +++ b/test/browser/annotation/.gitignore @@ -0,0 +1,2 @@ +pdf-annotation.js +pdf-annotation.css diff --git a/test/browser/annotation/README.md b/test/browser/annotation/README.md new file mode 100644 index 000000000..f9604e96a --- /dev/null +++ b/test/browser/annotation/README.md @@ -0,0 +1,33 @@ +# `pdf-annotation.js` checks + +What the emitted annotation script does can only be seen in a browser, so these +are run by hand rather than by `odr_test`. + +```bash +test/browser/annotation/serve # extracts script and style, serves on :8733 +open http://localhost:8733/tests.html +``` + +`serve` lifts `pdf_annotation_js` and `pdf_annotation_css` out of +`src/odr/internal/html/frontend.cpp`, so what runs is what ships — renaming +either declaration breaks the harness. + +`tests.html` stands in for a rendered pdf view: two `.p` pages laid out in +inches, each carrying the `data-odr-page` and `data-odr-space` the renderer +emits, with one selectable `.sr` run on each. + +Why the harness is shaped this way: + +- **The pages declare `[1 0 0 -1 0 792]`**, the map a us-letter page with no + crop-box offset and no rotation produces. A quad's expected user-space `y` is + then `792 - top`, which the checks compute by hand rather than through the + script, so a wrong answer cannot agree with itself. +- **The quad's expected position is measured off the run's own client rect**, + not off its css: a text range's rect follows the font's metrics, not the line + box, so `top: 92pt` does not put the glyphs at 92pt. +- **Ink is driven with real `PointerEvent`s**, not by calling into the model, so + the pointer path and the coordinate mapping are both covered. +- **`setPointerCapture` is stubbed out**: a synthetic event has no real pointer + to capture and chromium throws on it. +- The blend check reads which of the two overlays a shape lands in + (`svg.an-m` multiplies, `svg.an` does not) rather than sampling pixels. diff --git a/test/browser/annotation/serve b/test/browser/annotation/serve new file mode 100755 index 000000000..40e0781cc --- /dev/null +++ b/test/browser/annotation/serve @@ -0,0 +1,40 @@ +#!/usr/bin/env python3 +"""Extracts the emitted annotation script and style, and serves the checks.""" + +import functools +import http.server +import pathlib +import socketserver + +PORT = 8733 + +HERE = pathlib.Path(__file__).resolve().parent +SOURCE = HERE.parents[2] / "src" / "odr" / "internal" / "html" / "frontend.cpp" + + +def extract(begin: str, end: str) -> str: + source = SOURCE.read_text() + start = source.index(begin) + return source[start + len(begin) : source.index(end, start)] + + +def main() -> None: + (HERE / "pdf-annotation.js").write_text( + extract('constexpr std::string_view pdf_annotation_js = R"js(', ')js";') + ) + (HERE / "pdf-annotation.css").write_text( + extract('constexpr std::string_view pdf_annotation_css = R"css(', ')css";') + ) + print(f"{SOURCE.name} -> pdf-annotation.js, pdf-annotation.css") + + handler = functools.partial( + http.server.SimpleHTTPRequestHandler, directory=str(HERE) + ) + socketserver.TCPServer.allow_reuse_address = True + with socketserver.TCPServer(("127.0.0.1", PORT), handler) as server: + print(f"http://localhost:{PORT}/tests.html") + server.serve_forever() + + +if __name__ == "__main__": + main() diff --git a/test/browser/annotation/tests.html b/test/browser/annotation/tests.html new file mode 100644 index 000000000..effc4ca92 --- /dev/null +++ b/test/browser/annotation/tests.html @@ -0,0 +1,234 @@ + + +pdf annotation checks + + + +
+ + +
+
+ Selectable text on page one +
+
+
+
+ Selectable text on page two +
+
+ + + From 98f28b08fffd25ac9c29d6f220c39d44168fa95a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 18:03:29 +0200 Subject: [PATCH 2/2] fix(html): mark a selection when the gesture ends, not on every change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) Claude-Session: https://claude.ai/code/session_0147n68S7LNAv9KynLaGgXN4 --- src/odr/internal/html/frontend.cpp | 54 ++++++++++++++++++------------ src/odr/internal/html/pdf_file.cpp | 15 ++++++--- test/browser/annotation/README.md | 4 +++ test/browser/annotation/tests.html | 34 ++++++++++++++++--- 4 files changed, 77 insertions(+), 30 deletions(-) diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index d43457fe6..a12b4728b 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -751,20 +751,15 @@ constexpr std::string_view pdf_annotation_js = R"js( return null; } - // The page box is laid out in inches, so its own layout width in css pixels - // gives the scale a zoom transform is applied on top of. - function metrics(page) { + /// A viewport point to page-box points (y-down, the unit the overlay draws + /// in). The page box is laid out in inches, so its own layout width in css + /// pixels gives the scale a zoom transform is applied on top of. + function toBox(page, clientX, clientY) { var rect = page.getBoundingClientRect(); var zoom = page.offsetWidth ? rect.width / page.offsetWidth : 1; - return { rect: rect, zoom: zoom, points: page.offsetWidth * 0.75 }; - } - - /// A viewport point to page-box points (y-down, the unit the overlay draws in). - function toBox(page, clientX, clientY) { - var m = metrics(page); return [ - ((clientX - m.rect.left) / m.zoom) * 0.75, - ((clientY - m.rect.top) / m.zoom) * 0.75, + ((clientX - rect.left) / zoom) * 0.75, + ((clientY - rect.top) / zoom) * 0.75, ]; } @@ -790,9 +785,10 @@ constexpr std::string_view pdf_annotation_js = R"js( svg.setAttribute("preserveAspectRatio", "none"); page.appendChild(svg); } - var m = metrics(page); - var height = page.offsetHeight * 0.75; - svg.setAttribute("viewBox", "0 0 " + m.points + " " + height); + svg.setAttribute( + "viewBox", + "0 0 " + page.offsetWidth * 0.75 + " " + page.offsetHeight * 0.75 + ); return svg; } @@ -945,8 +941,24 @@ constexpr std::string_view pdf_annotation_js = R"js( } var stroke = null; + var pointerDown = false; + var settle = null; + + /// A drag fires `selectionchange` on every character it covers, so the mark + /// waits for the gesture that makes it to end rather than taking the first + /// character and tearing the selection out from under the pointer. + function scheduleMark() { + if (!tool || tool === "ink" || pointerDown) { + return; + } + window.clearTimeout(settle); + settle = window.setTimeout(markSelection, 50); + } function onPointerDown(event) { + pointerDown = true; + // a new gesture supersedes a mark the previous one had queued + window.clearTimeout(settle); if (tool !== "ink" || event.button !== 0) { return; } @@ -988,7 +1000,12 @@ constexpr std::string_view pdf_annotation_js = R"js( } function onPointerUp() { - if (stroke && stroke.strokes[0].length < 4) { + pointerDown = false; + scheduleMark(); + if (!stroke) { + return; + } + if (stroke.strokes[0].length < 4) { // a tap with no drag leaves a dot, which is a legitimate mark stroke.strokes[0].push(stroke.strokes[0][0], stroke.strokes[0][1]); } @@ -1000,12 +1017,7 @@ constexpr std::string_view pdf_annotation_js = R"js( document.addEventListener("pointermove", onPointerMove); document.addEventListener("pointerup", onPointerUp); document.addEventListener("pointercancel", onPointerUp); - document.addEventListener("selectionchange", function () { - if (tool && tool !== "ink") { - // a selection completes the mark; the pointer is already up by then - window.setTimeout(markSelection, 0); - } - }); + document.addEventListener("selectionchange", scheduleMark); window.addEventListener("resize", redraw); odr.annotation = { diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 36ae710db..2846b386a 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -77,7 +77,8 @@ std::string page_attributes(const std::size_t index, return {}; } const auto n = [](const double v) { - return util::number::to_string_significant(v, 10); + // a negative zero would render as `-0`, which is only noise in the output + return util::number::to_string_significant(v == 0 ? 0.0 : v, 10); }; return R"( data-odr-page=")" + std::to_string(index) + R"(" data-odr-space=")" + n(from_box->a) + ',' + n(from_box->b) + ',' + @@ -1422,10 +1423,12 @@ class HtmlServiceImpl final : public HtmlService { const double height = pb.height; const util::math::Transform2D &to_box = pb.to_box; + // page numbers are 1-based, `data-odr-page` is the 0-based index + const std::size_t page_index = first_page_number - 1 + pages_out.size(); + DualPageOut &page_out = pages_out.emplace_back(); page_out.classes = pb.classes; - page_out.attributes = - page_attributes(first_page_number - 1 + pages_out.size() - 1, to_box); + page_out.attributes = page_attributes(page_index, to_box); page_out.width = width; page_out.height = height; page_out.links = @@ -2071,10 +2074,12 @@ class HtmlServiceImpl final : public HtmlService { const double height = pb.height; const util::math::Transform2D &to_box = pb.to_box; + // page numbers are 1-based, `data-odr-page` is the 0-based index + const std::size_t page_index = first_page_number - 1 + pages_out.size(); + SinglePageOut &page_out = pages_out.emplace_back(); page_out.classes = pb.classes; - page_out.attributes = - page_attributes(first_page_number - 1 + pages_out.size() - 1, to_box); + page_out.attributes = page_attributes(page_index, to_box); page_out.width = width; page_out.height = height; page_out.links = diff --git a/test/browser/annotation/README.md b/test/browser/annotation/README.md index f9604e96a..e3a5824ca 100644 --- a/test/browser/annotation/README.md +++ b/test/browser/annotation/README.md @@ -31,3 +31,7 @@ Why the harness is shaped this way: to capture and chromium throws on it. - The blend check reads which of the two overlays a shape lands in (`svg.an-m` multiplies, `svg.an` does not) rather than sampling pixels. +- **A drag is stepped through by hand** — pointer down, the selection extended + a character at a time, pointer up. Chromium will not select text from a + synthetic mouse event, and one `addRange` fires a single `selectionchange`, + so neither reaches the case a drag creates. diff --git a/test/browser/annotation/tests.html b/test/browser/annotation/tests.html index effc4ca92..ba264d7de 100644 --- a/test/browser/annotation/tests.html +++ b/test/browser/annotation/tests.html @@ -109,7 +109,7 @@ api.setTool("highlight"); api.setColor([1, 0.9, 0.2]); const run1 = selectRun(0); - await new Promise((r) => setTimeout(r, 20)); + await new Promise((r) => setTimeout(r, 100)); check("a selection makes one annotation", api.list().length === 1, api.list()); @@ -141,6 +141,32 @@ q ); + // --- a drag marks once, not once per character it covers ------------- + api.clear(); + api.setTool("highlight"); + const text = document.querySelector('[data-odr-page="0"] .sr').firstChild; + const pointer = (type) => + document.dispatchEvent( + new PointerEvent(type, { bubbles: true, pointerId: 2, button: 0 }) + ); + window.getSelection().removeAllRanges(); + pointer("pointerdown"); + for (let k = 1; k <= 6; ++k) { + window.getSelection().setBaseAndExtent(text, 0, text, k); + await new Promise((r) => setTimeout(r, 5)); + } + pointer("pointerup"); + await new Promise((r) => setTimeout(r, 100)); + check( + "a drag makes one annotation, not one per character", + api.list().length === 1, + api.list().length + ); + api.clear(); + api.setTool("highlight"); + selectRun(0); + await new Promise((r) => setTimeout(r, 100)); + // --- the overlay is drawn, and the wash is the one that multiplies ---- const page1 = document.querySelector('[data-odr-page="0"]'); check("an overlay is added", page1.querySelectorAll("svg.an").length >= 1); @@ -152,7 +178,7 @@ api.clear(); api.setTool("underline"); selectRun(0); - await new Promise((r) => setTimeout(r, 20)); + await new Promise((r) => setTimeout(r, 100)); check( "an underline does not multiply", page1.querySelector("svg.an-m path") === null && @@ -163,7 +189,7 @@ api.clear(); api.setTool("highlight"); selectRun(1); - await new Promise((r) => setTimeout(r, 20)); + await new Promise((r) => setTimeout(r, 100)); check("page two annotates as page 1", payload().annotations[0].page === 1); // --- ink through real pointer events --------------------------------- @@ -202,7 +228,7 @@ api.setTool("highlight"); selectRun(0); - await new Promise((r) => setTimeout(r, 20)); + await new Promise((r) => setTimeout(r, 100)); const id = api.list()[api.list().length - 1].id; api.remove(id); check("remove drops it by id", !api.list().some((a) => a.id === id));