From f5f54f92a5fa462db9816c8dba7c150f05864e9e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 17:49:43 +0200 Subject: [PATCH 1/3] feat(bindings): apply pdf annotations from every binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PdfFile.annotate` in python, java and swift, `annotate()` on the wasm handle, and the `annotate` capability alongside them. Each returns the annotated bytes rather than writing a file: none of these callers has a filesystem the caller would want written to, and the wasm one least of all. Each binding gets a minimal pdf fixture built the way its neighbours are — computed cross-reference offsets, no file fetched — and tests that the source comes back as a prefix of the result, that the annotation is in what follows, and that a payload this build does not understand is refused. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018e3PEzyU2oAFSzsEoWsSmz --- CHANGELOG.md | 4 ++ apple/include/OdrCoreObjC/ODRFile.h | 6 ++ apple/src/ODRFile.mm | 11 +++ apple/tests/Fixture.swift | 32 +++++++++ apple/tests/OdrCoreTests.swift | 35 ++++++++++ docs/design/pdf-annotation.md | 13 ++-- jni/CMakeLists.txt | 1 + .../core/FileTypeCapabilities.java | 7 +- jni/java/app/opendocument/core/PdfFile.java | 12 ++++ jni/src/jni_file.cpp | 11 +++ jni/src/jni_style.cpp | 5 +- .../app/opendocument/core/TestFiles.java | 29 ++++++++ .../app/opendocument/core/PdfFileTest.java | 55 +++++++++++++++ python/src/bind_file.cpp | 12 +++- python/tests/conftest.py | 30 ++++++++ python/tests/test_pdf.py | 70 +++++++++++++++++++ wasm/js/index.d.ts | 10 +++ wasm/js/index.js | 6 ++ wasm/src/odr_wasm.cpp | 1 + wasm/src/wasm_file.cpp | 13 ++++ wasm/tests/annotate.test.mjs | 63 +++++++++++++++++ wasm/tests/helper.mjs | 28 ++++++++ 22 files changed, 444 insertions(+), 10 deletions(-) create mode 100644 jni/tests/app/opendocument/core/PdfFileTest.java create mode 100644 python/tests/test_pdf.py create mode 100644 wasm/tests/annotate.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index e816214e0..5cecc3c6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ The release run heads these entries with the version and opens a fresh strike-out, squiggly and freehand ink — into a pdf as an incremental update, so the source bytes are left as they are and any viewer reads them as ordinary pdf annotations. `FileTypeCapabilities` gains an `annotate` flag. + In every binding: `PdfFile.annotate` in python, java and swift, `annotate()` + on the wasm handle. The rendered pdf view exposes `odr.annotation` — tools for + the five kinds, live preview, undo — and its `getAnnotations()` produces + exactly what `annotate` takes. - **Breaking**: `html::edit` becomes `Document::edit`, in every binding — java's `Html.edit(document, diff)` becomes `document.edit(diff)`, and so on. diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index d958a289d..6e3c48989 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -187,6 +187,8 @@ NS_SWIFT_NAME(FileTypeCapabilities) @property(nonatomic, readonly) BOOL save; /// Saving with a password is supported. @property(nonatomic, readonly) BOOL encrypt; +/// `ODRPdfFile.annotate` is supported. +@property(nonatomic, readonly) BOOL annotate; - (instancetype)init NS_UNAVAILABLE; + (instancetype)new NS_UNAVAILABLE; @@ -403,6 +405,10 @@ NS_SWIFT_NAME(DocumentFile) /// A decoded PDF — `odr::PdfFile`. NS_SWIFT_NAME(PdfFile) @interface ODRPdfFile : ODRDecodedFile +/// Applies markup annotations — the payload the rendered page's +/// `odr.annotation.getAnnotations()` collects — and returns the annotated pdf. +- (nullable NSData *)annotate:(NSString *)annotations + error:(NSError **)error NS_SWIFT_NAME(annotate(_:)); - (nullable ODRPdfFile *)decryptWithPassword:(NSString *)password error:(NSError **)error; @end diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 2acee5dbf..67a59933a 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -8,6 +8,7 @@ #include #include +#include #include using odr::apple::guarded; @@ -166,6 +167,7 @@ + (instancetype)capabilitiesWithHandle: result->_edit = handle.edit ? YES : NO; result->_save = handle.save ? YES : NO; result->_encrypt = handle.encrypt ? YES : NO; + result->_annotate = handle.annotate ? YES : NO; return result; } @@ -647,6 +649,15 @@ - (nullable ODRDocument *)documentWithError:(NSError **)error { @implementation ODRPdfFile +- (nullable NSData *)annotate:(NSString *)annotations error:(NSError **)error { + return guarded(error, [&]() -> NSData * { + std::ostringstream out; + self.handle.as_pdf_file().annotate(to_string(annotations), out); + std::istringstream in(std::move(out).str()); + return to_nsdata(in); + }); +} + - (nullable ODRPdfFile *)decryptWithPassword:(NSString *)password error:(NSError **)error { return guarded(error, [&]() -> ODRPdfFile * { diff --git a/apple/tests/Fixture.swift b/apple/tests/Fixture.swift index 68def1a62..fe5ba4ac4 100644 --- a/apple/tests/Fixture.swift +++ b/apple/tests/Fixture.swift @@ -19,6 +19,38 @@ enum Fixture { try path("mixed-layout", "odt") } + /// A one-page pdf written to a temporary file, its cross-reference offsets + /// computed so they are right. + static func pdf() throws -> String { + let objects = [ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792]" + + " /Resources << >> /Contents 4 0 R >>", + "<< /Length 5 >>\nstream\nBT ET\nendstream", + ] + + var out = "%PDF-1.7\n" + var offsets: [Int] = [] + for (index, body) in objects.enumerated() { + offsets.append(out.utf8.count) + out += "\(index + 1) 0 obj\n\(body)\nendobj\n" + } + + let start = out.utf8.count + out += "xref\n0 \(objects.count + 1)\n0000000000 65535 f \n" + for offset in offsets { + out += String(format: "%010d 00000 n \n", offset) + } + out += "trailer\n<< /Size \(objects.count + 1) /Root 1 0 R >>\n" + out += "startxref\n\(start)\n%%EOF\n" + + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("odr-minimal-\(UUID().uuidString).pdf") + try out.data(using: .isoLatin1)!.write(to: url) + return url.path + } + private static func path(_ name: String, _ extension: String) throws -> String { try XCTUnwrap( Bundle.module.url( diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index 267318c79..f0f5203bb 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -382,6 +382,41 @@ final class DocumentSaveTests: XCTestCase { } } +final class PdfAnnotationTests: XCTestCase { + private static let highlight = """ + {"version": 1, "annotations": [{"page": 0, "type": "highlight", + "quads": [[72, 700, 300, 700, 72, 688, 300, 688]], + "color": [1, 0.9, 0.2]}]} + """ + + func testAnnotateIsDeclaredForPdf() throws { + let capabilities = Odr.capabilities(fileType: .portableDocumentFormat) + XCTAssertTrue(capabilities.annotate) + } + + func testAnnotateAppendsToTheSource() throws { + let path = try Fixture.pdf() + let source = try Data(contentsOf: URL(fileURLWithPath: path)) + + let file = try DecodedFile.decode(path: path).asPdfFile() + let result = try file.annotate(Self.highlight) + + XCTAssertGreaterThan(result.count, source.count) + // the source is copied through and the annotation written after it + XCTAssertEqual(result.prefix(source.count), source) + + let text = String(decoding: result, as: UTF8.self) + XCTAssertTrue(text.contains("/Highlight")) + XCTAssertTrue(text.contains("/Subtype /Form")) + } + + func testAnnotateRefusesAPayloadItDoesNotUnderstand() throws { + let file = try DecodedFile.decode(path: try Fixture.pdf()).asPdfFile() + XCTAssertThrowsError(try file.annotate("{\"version\": 2}")) + XCTAssertThrowsError(try file.annotate("not json")) + } +} + final class TableAddressTests: XCTestCase { func testRoundTrips() throws { XCTAssertEqual(TableAddress.columnNumber(from: "C"), 2) diff --git a/docs/design/pdf-annotation.md b/docs/design/pdf-annotation.md index 1bd7de9ec..c482bde3a 100644 --- a/docs/design/pdf-annotation.md +++ b/docs/design/pdf-annotation.md @@ -1,10 +1,10 @@ # PDF annotation design -Status: **underway.** This records the architecture for adding markup +Status: **landed.** 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 5 have landed: the browser -draws the markup and the writer appends it; the bindings are what is left. +validated against four viewers, and every phase has landed: the browser draws +the markup, the writer appends it, and every binding can apply it. 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 @@ -323,10 +323,11 @@ 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) +### Phase 6 — bindings — **done** (#850) -wasm (~50 C++, ~80 TS), JNI (~60 C++, ~70 Java), Python (~40), Apple (~80 ObjC, -~90 Swift). +`annotate` and the `annotate` capability across wasm, JNI, python and Apple. +Each returns the annotated bytes rather than writing a file: none of these +callers has a filesystem the caller would want written to. ### Phase 7 — corpus and interop (2 d, ~600 test lines) diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt index 1a635d3a6..d71783d98 100644 --- a/jni/CMakeLists.txt +++ b/jni/CMakeLists.txt @@ -198,6 +198,7 @@ if (ODR_TEST AND NOT ANDROID) "tests/app/opendocument/core/HttpServerTest.java" "tests/app/opendocument/core/LoggerTest.java" "tests/app/opendocument/core/MetaTest.java" + "tests/app/opendocument/core/PdfFileTest.java" "tests/app/opendocument/core/TextEncodingTest.java" # shared with the instrumented suite of the AAR, see `android/` "testfixtures/app/opendocument/core/TestFiles.java" diff --git a/jni/java/app/opendocument/core/FileTypeCapabilities.java b/jni/java/app/opendocument/core/FileTypeCapabilities.java index 2a36b7743..e5e28012c 100644 --- a/jni/java/app/opendocument/core/FileTypeCapabilities.java +++ b/jni/java/app/opendocument/core/FileTypeCapabilities.java @@ -34,6 +34,9 @@ public final class FileTypeCapabilities { /** {@link Document#save} with a password is supported. */ public final boolean encrypt; + /** {@link PdfFile#annotate} is supported. */ + public final boolean annotate; + FileTypeCapabilities( boolean detectByContent, boolean open, @@ -42,7 +45,8 @@ public final class FileTypeCapabilities { boolean colorScheme, boolean edit, boolean save, - boolean encrypt) { + boolean encrypt, + boolean annotate) { this.detectByContent = detectByContent; this.open = open; this.decrypt = decrypt; @@ -51,5 +55,6 @@ public final class FileTypeCapabilities { this.edit = edit; this.save = save; this.encrypt = encrypt; + this.annotate = annotate; } } diff --git a/jni/java/app/opendocument/core/PdfFile.java b/jni/java/app/opendocument/core/PdfFile.java index e06f3fab7..c5b359434 100644 --- a/jni/java/app/opendocument/core/PdfFile.java +++ b/jni/java/app/opendocument/core/PdfFile.java @@ -12,5 +12,17 @@ public PdfFile decrypt(String password) { return new PdfFile(decryptPdfFileNative(handle(), password)); } + /** + * Applies markup annotations and returns the annotated pdf. + * + * @param annotations the payload the rendered page's {@code + * odr.annotation.getAnnotations()} collects. + */ + public byte[] annotate(String annotations) { + return annotateNative(handle(), annotations); + } + private native long decryptPdfFileNative(long handle, String password); + + private native byte[] annotateNative(long handle, String annotations); } diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index 5477025e3..cd329664e 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -366,6 +366,17 @@ Java_app_opendocument_core_PdfFile_decryptPdfFileNative(JNIEnv *env, jobject, }); } +extern "C" JNIEXPORT jbyteArray JNICALL +Java_app_opendocument_core_PdfFile_annotateNative(JNIEnv *env, jobject, + jlong handle, + jstring annotations) { + return guarded(env, [&] { + std::ostringstream out; + decoded(handle).as_pdf_file().annotate(to_string(env, annotations), out); + return to_jbytes(env, std::move(out).str()); + }); +} + // app.opendocument.core.FontFile extern "C" JNIEXPORT jbyteArray JNICALL diff --git a/jni/src/jni_style.cpp b/jni/src/jni_style.cpp index ac610906b..4e937c693 100644 --- a/jni/src/jni_style.cpp +++ b/jni/src/jni_style.cpp @@ -401,7 +401,7 @@ jobject make_file_type_capabilities(JNIEnv *env, const odr::FileTypeCapabilities &capabilities) { return new_object(env, "app/opendocument/core/FileTypeCapabilities", - "(ZZZZZZZZ)V", + "(ZZZZZZZZZ)V", static_cast(capabilities.detect_by_content), static_cast(capabilities.open), static_cast(capabilities.decrypt), @@ -409,7 +409,8 @@ make_file_type_capabilities(JNIEnv *env, static_cast(capabilities.color_scheme), static_cast(capabilities.edit), static_cast(capabilities.save), - static_cast(capabilities.encrypt)); + static_cast(capabilities.encrypt), + static_cast(capabilities.annotate)); } jobject html_config_to_java(JNIEnv *env, const odr::HtmlConfig &config) { diff --git a/jni/testfixtures/app/opendocument/core/TestFiles.java b/jni/testfixtures/app/opendocument/core/TestFiles.java index f178a3ead..c4844ecf6 100644 --- a/jni/testfixtures/app/opendocument/core/TestFiles.java +++ b/jni/testfixtures/app/opendocument/core/TestFiles.java @@ -66,6 +66,35 @@ static Path txtFile(Path directory) throws IOException { return path; } + /** A one-page pdf, its cross-reference offsets computed so they are right. */ + static Path pdfFile(Path directory) throws IOException { + Path path = directory.resolve("minimal.pdf"); + List objects = + Arrays.asList( + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + + "/Resources << >> /Contents 4 0 R >>", + "<< /Length 5 >>\nstream\nBT ET\nendstream"); + + StringBuilder out = new StringBuilder("%PDF-1.7\n"); + int[] offsets = new int[objects.size()]; + for (int i = 0; i < objects.size(); ++i) { + offsets[i] = out.length(); + out.append(i + 1).append(" 0 obj\n").append(objects.get(i)).append("\nendobj\n"); + } + int start = out.length(); + out.append("xref\n0 ").append(objects.size() + 1).append("\n0000000000 65535 f \n"); + for (int offset : offsets) { + out.append(String.format("%010d 00000 n %n", offset).replace(System.lineSeparator(), "\n")); + } + out.append("trailer\n<< /Size ").append(objects.size() + 1).append(" /Root 1 0 R >>\n"); + out.append("startxref\n").append(start).append("\n%%EOF\n"); + + write(path, out.toString()); + return path; + } + private static void write(Path path, String content) throws IOException { Files.write(path, content.getBytes(StandardCharsets.UTF_8)); } diff --git a/jni/tests/app/opendocument/core/PdfFileTest.java b/jni/tests/app/opendocument/core/PdfFileTest.java new file mode 100644 index 000000000..e1a25b2b3 --- /dev/null +++ b/jni/tests/app/opendocument/core/PdfFileTest.java @@ -0,0 +1,55 @@ +package app.opendocument.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class PdfFileTest { + @TempDir Path tempDir; + + private static final String HIGHLIGHT = + "{\"version\": 1, \"annotations\": [{\"page\": 0, \"type\": \"highlight\"," + + " \"quads\": [[72, 700, 300, 700, 72, 688, 300, 688]]," + + " \"color\": [1, 0.9, 0.2]}]}"; + + @Test + void annotateIsDeclaredForPdf() { + assertTrue(Odr.capabilitiesByFileType(FileType.PORTABLE_DOCUMENT_FORMAT).annotate); + } + + @Test + void annotateAppendsToTheSource() throws IOException { + Path pdf = TestFiles.pdfFile(tempDir); + byte[] source = Files.readAllBytes(pdf); + + try (DecodedFile file = Odr.open(pdf.toString())) { + byte[] result = file.asPdfFile().annotate(HIGHLIGHT); + + assertTrue(result.length > source.length); + // the source is copied and the annotation appended after it + for (int i = 0; i < source.length; ++i) { + assertEquals(source[i], result[i]); + } + String text = new String(result, StandardCharsets.ISO_8859_1); + assertTrue(text.contains("/Highlight")); + assertTrue(text.contains("/Subtype /Form")); + } + } + + @Test + void annotateRefusesAPayloadItDoesNotUnderstand() throws IOException { + Path pdf = TestFiles.pdfFile(tempDir); + try (DecodedFile file = Odr.open(pdf.toString())) { + PdfFile pdfFile = file.asPdfFile(); + assertThrows(OdrException.class, () -> pdfFile.annotate("{\"version\": 2}")); + assertThrows(OdrException.class, () -> pdfFile.annotate("not json")); + } + } +} diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index 695fa0807..fd8c930df 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -224,7 +224,8 @@ void odr_python::bind_file(py::module_ &m) { .def_readwrite("color_scheme", &odr::FileTypeCapabilities::color_scheme) .def_readwrite("edit", &odr::FileTypeCapabilities::edit) .def_readwrite("save", &odr::FileTypeCapabilities::save) - .def_readwrite("encrypt", &odr::FileTypeCapabilities::encrypt); + .def_readwrite("encrypt", &odr::FileTypeCapabilities::encrypt) + .def_readwrite("annotate", &odr::FileTypeCapabilities::annotate); py::class_(m, "File") .def(py::init<>()) @@ -327,6 +328,15 @@ void odr_python::bind_file(py::module_ &m) { .def("document", &odr::DocumentFile::document); py::class_(m, "PdfFile") + .def( + "annotate", + [](const odr::PdfFile &file, const std::string &annotations) { + std::ostringstream out; + file.annotate(annotations, out); + return py::bytes(std::move(out).str()); + }, + py::arg("annotations"), + "Apply markup annotations and return the annotated pdf.") .def("decrypt", &odr::PdfFile::decrypt, py::arg("password"), py::call_guard()); diff --git a/python/tests/conftest.py b/python/tests/conftest.py index 0adf48e6c..2691a2f10 100644 --- a/python/tests/conftest.py +++ b/python/tests/conftest.py @@ -73,6 +73,36 @@ def odt_path(tmp_path): return path +def _mini_pdf() -> bytes: + """A one-page pdf, offsets computed so the cross-reference table is right.""" + objects = [ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + b"/Resources << >> /Contents 4 0 R >>", + b"<< /Length 5 >>\nstream\nBT ET\nendstream", + ] + out = bytearray(b"%PDF-1.7\n") + offsets = [] + for index, body in enumerate(objects): + offsets.append(len(out)) + out += b"%d 0 obj\n" % (index + 1) + body + b"\nendobj\n" + start = len(out) + out += b"xref\n0 %d\n0000000000 65535 f \n" % (len(objects) + 1) + for offset in offsets: + out += b"%010d 00000 n \n" % offset + out += b"trailer\n<< /Size %d /Root 1 0 R >>\n" % (len(objects) + 1) + out += b"startxref\n%d\n%%%%EOF\n" % start + return bytes(out) + + +@pytest.fixture +def pdf_path(tmp_path): + path = tmp_path / "minimal.pdf" + path.write_bytes(_mini_pdf()) + return path + + @pytest.fixture def csv_path(tmp_path): path = tmp_path / "table.csv" diff --git a/python/tests/test_pdf.py b/python/tests/test_pdf.py new file mode 100644 index 000000000..857c5ad93 --- /dev/null +++ b/python/tests/test_pdf.py @@ -0,0 +1,70 @@ +import json + +import pytest + +import pyodr + + +def annotate(pdf_path, annotations): + return pyodr.open(str(pdf_path)).as_pdf_file().annotate(json.dumps(annotations)) + + +def test_annotate_is_declared_for_pdf(): + capabilities = pyodr.capabilities_by_file_type( + pyodr.FileType.portable_document_format + ) + assert capabilities.annotate + + +def test_annotate_appends_to_the_source(pdf_path): + source = pdf_path.read_bytes() + result = annotate( + pdf_path, + { + "version": 1, + "annotations": [ + { + "page": 0, + "type": "highlight", + "quads": [[72, 700, 300, 700, 72, 688, 300, 688]], + "color": [1, 0.9, 0.2], + } + ], + }, + ) + + assert isinstance(result, bytes) + # the source is copied and the annotation appended after it + assert result.startswith(source) + assert b"/Highlight" in result + assert b"/Subtype /Form" in result + + +def test_annotate_writes_ink(pdf_path): + result = annotate( + pdf_path, + { + "version": 1, + "annotations": [ + { + "page": 0, + "type": "ink", + "strokes": [[100, 500, 130, 540, 160, 490]], + "width": 2, + "color": [0.9, 0.1, 0.1], + } + ], + }, + ) + assert b"/Ink" in result + assert b"/InkList" in result + + +def test_annotate_refuses_a_payload_it_does_not_understand(pdf_path): + with pytest.raises(ValueError): + annotate(pdf_path, {"version": 2, "annotations": []}) + with pytest.raises(ValueError): + annotate( + pdf_path, + {"version": 1, "annotations": [{"page": 9, "type": "ink"}]}, + ) diff --git a/wasm/js/index.d.ts b/wasm/js/index.d.ts index b2bc94035..a348b1873 100644 --- a/wasm/js/index.d.ts +++ b/wasm/js/index.d.ts @@ -26,6 +26,8 @@ export interface Capabilities { edit: boolean; save: boolean; encrypt: boolean; + /** `annotate()` is supported. */ + annotate: boolean; } export interface FileTypeInfo { @@ -154,6 +156,14 @@ export declare class Document { /** Applies what the rendered page's `odr.generateDiff()` collected. * @throws OdrError `NoDocumentFile` */ edit(diff: string): this; + + /** + * Applies markup annotations to a pdf and returns the annotated bytes. + * + * @param annotations what the rendered page's + * `odr.annotation.getAnnotations()` collected. + */ + annotate(annotations: string): Uint8Array; /** The document's bytes, not the rendered html. * @throws OdrError `UnsupportedOperation` where the format cannot be saved */ save(password?: string): Uint8Array; diff --git a/wasm/js/index.js b/wasm/js/index.js index 2403cc59e..a3e012444 100644 --- a/wasm/js/index.js +++ b/wasm/js/index.js @@ -91,6 +91,12 @@ export class Document { return this; } + // The annotated pdf's bytes; what `odr.annotation.getAnnotations()` collected + // goes in. + annotate(annotations) { + return unwrap(this.#core.annotate(this.#handle, annotations)); + } + // The document's bytes, not the rendered html. save(password) { return password === undefined diff --git a/wasm/src/odr_wasm.cpp b/wasm/src/odr_wasm.cpp index 53ef0dc79..52b4a1e7a 100644 --- a/wasm/src/odr_wasm.cpp +++ b/wasm/src/odr_wasm.cpp @@ -129,6 +129,7 @@ emscripten::val to_capabilities(const FileTypeCapabilities &capabilities) { result.set("edit", capabilities.edit); result.set("save", capabilities.save); result.set("encrypt", capabilities.encrypt); + result.set("annotate", capabilities.annotate); return result; } diff --git a/wasm/src/wasm_file.cpp b/wasm/src/wasm_file.cpp index eaaaf8ff4..6e5e3beaf 100644 --- a/wasm/src/wasm_file.cpp +++ b/wasm/src/wasm_file.cpp @@ -7,6 +7,7 @@ #include +#include #include #include @@ -114,6 +115,17 @@ emscripten::val file_type(const Handle handle) { }); } +/// The annotated pdf's bytes; there is no filesystem to write to. `payload` is +/// what the rendered page's `odr.annotation.getAnnotations()` collected. +emscripten::val annotate(const Handle handle, const std::string &payload) { + return guarded([&] { + Session &s = session(handle); + std::ostringstream out; + s.file.as_pdf_file().annotate(payload, out, s.logger); + return ok(to_uint8_array(std::move(out).str())); + }); +} + emscripten::val close(const Handle handle) { return guarded([&] { return ok(emscripten::val(remove_session(handle))); }); } @@ -140,6 +152,7 @@ EMSCRIPTEN_BINDINGS(odr_file) { emscripten::function("decrypt", &odr::wasm::decrypt); emscripten::function("fileType", &odr::wasm::file_type); emscripten::function("fileName", &odr::wasm::file_name); + emscripten::function("annotate", &odr::wasm::annotate); emscripten::function("close", &odr::wasm::close); emscripten::function("closeAll", &odr::wasm::close_all); } diff --git a/wasm/tests/annotate.test.mjs b/wasm/tests/annotate.test.mjs new file mode 100644 index 000000000..cd5045943 --- /dev/null +++ b/wasm/tests/annotate.test.mjs @@ -0,0 +1,63 @@ +// The round trip the browser drives: mark a pdf up, get the bytes back. + +import assert from 'node:assert/strict'; +import { after, before, describe, it } from 'node:test'; + +import { Odr, OdrError, minimalPdf } from './helper.mjs'; + +const highlight = JSON.stringify({ + version: 1, + annotations: [ + { + page: 0, + type: 'highlight', + quads: [[72, 700, 300, 700, 72, 688, 300, 688]], + color: [1, 0.9, 0.2], + }, + ], +}); + +describe('annotate', () => { + let odr; + before(async () => { + odr = await Odr(); + }); + after(() => odr.closeAll()); + + it('declares the capability', () => { + const doc = odr.open(minimalPdf()); + try { + assert.equal(doc.capabilities().annotate, true); + } finally { + doc.close(); + } + }); + + it('appends the annotation to the source', () => { + const source = minimalPdf(); + const doc = odr.open(source); + try { + const result = doc.annotate(highlight); + + assert.ok(result.length > source.length); + // the source is copied through and the annotation written after it + assert.deepEqual(result.slice(0, source.length), source); + + const text = Buffer.from(result).toString('latin1'); + assert.match(text, /\/Highlight/); + assert.match(text, /\/Subtype \/Form/); + } finally { + doc.close(); + } + }); + + it('refuses a payload it does not understand', () => { + const doc = odr.open(minimalPdf()); + try { + assert.throws(() => doc.annotate('{"version": 2}'), OdrError); + assert.throws(() => doc.annotate('not json'), OdrError); + } finally { + doc.close(); + } + }); +}); diff --git a/wasm/tests/helper.mjs b/wasm/tests/helper.mjs index 9528fdedc..87d25f7bf 100644 --- a/wasm/tests/helper.mjs +++ b/wasm/tests/helper.mjs @@ -126,3 +126,31 @@ export function minimalOdt(text = 'hello') { }, ]); } + +// The smallest pdf that opens: one page, its cross-reference offsets computed. +export function minimalPdf() { + const objects = [ + '<< /Type /Catalog /Pages 2 0 R >>', + '<< /Type /Pages /Kids [3 0 R] /Count 1 >>', + '<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792]' + + ' /Resources << >> /Contents 4 0 R >>', + '<< /Length 5 >>\nstream\nBT ET\nendstream', + ]; + + let out = '%PDF-1.7\n'; + const offsets = []; + objects.forEach((body, index) => { + offsets.push(out.length); + out += `${index + 1} 0 obj\n${body}\nendobj\n`; + }); + + const start = out.length; + out += `xref\n0 ${objects.length + 1}\n0000000000 65535 f \n`; + for (const offset of offsets) { + out += `${String(offset).padStart(10, '0')} 00000 n \n`; + } + out += `trailer\n<< /Size ${objects.length + 1} /Root 1 0 R >>\n`; + out += `startxref\n${start}\n%%EOF\n`; + + return new Uint8Array(Buffer.from(out, 'latin1')); +} From cbe1c5ba3a866b197435aa7ae54aaacb764d31f2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 18:19:30 +0200 Subject: [PATCH 2/3] fix(bindings): release the gil while a pdf is annotated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every other binding call in `python/src` that does real work drops the GIL for it and takes it back to build the `py::bytes`; `annotate` held it for the whole parse and write, so a threaded caller stalled on it. Apple: hands the annotated bytes straight to `NSData` through a new `to_nsdata(const std::string &)` rather than wrapping them in an `istringstream` for the stream overload to drain into another `ostringstream` — three copies of the whole file for nothing. The stream overload now ends in the same place. Java: one `assertArrayEquals` for the copied prefix instead of an assertion per byte, and the xref entry's terminator written as the byte the format states rather than `%n` and a platform-separator replace. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0147n68S7LNAv9KynLaGgXN4 --- apple/src/ODRFile.mm | 3 +-- apple/src/ODRInternal.h | 4 ++-- apple/src/ODRInternal.mm | 7 +++++-- jni/testfixtures/app/opendocument/core/TestFiles.java | 2 +- jni/tests/app/opendocument/core/PdfFileTest.java | 7 +++---- python/src/bind_file.cpp | 7 ++++++- 6 files changed, 18 insertions(+), 12 deletions(-) diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 67a59933a..497d4fdbe 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -653,8 +653,7 @@ - (nullable NSData *)annotate:(NSString *)annotations error:(NSError **)error { return guarded(error, [&]() -> NSData * { std::ostringstream out; self.handle.as_pdf_file().annotate(to_string(annotations), out); - std::istringstream in(std::move(out).str()); - return to_nsdata(in); + return to_nsdata(std::move(out).str()); }); } diff --git a/apple/src/ODRInternal.h b/apple/src/ODRInternal.h index ffe41e801..9448e7503 100644 --- a/apple/src/ODRInternal.h +++ b/apple/src/ODRInternal.h @@ -28,8 +28,8 @@ std::string to_string(NSString *string); NSString *to_nsstring(const std::string &string); NSString *to_nsstring(std::string_view string); -/// Drains `stream` into an `NSData`. The stream APIs of odrcore hand out a -/// `std::istream`; ObjC callers want bytes. +/// Bytes as an `NSData`, drained from a stream where odrcore hands one out. +NSData *to_nsdata(const std::string &bytes); NSData *to_nsdata(std::istream &stream); /// Fills `*error` from the exception currently being handled. Call only from diff --git a/apple/src/ODRInternal.mm b/apple/src/ODRInternal.mm index 283eb610b..17f3e58a2 100644 --- a/apple/src/ODRInternal.mm +++ b/apple/src/ODRInternal.mm @@ -47,11 +47,14 @@ return result != nil ? result : @""; } +NSData *apple::to_nsdata(const std::string &bytes) { + return [NSData dataWithBytes:bytes.data() length:bytes.size()]; +} + NSData *apple::to_nsdata(std::istream &stream) { std::ostringstream buffer; buffer << stream.rdbuf(); - const std::string bytes = buffer.str(); - return [NSData dataWithBytes:bytes.data() length:bytes.size()]; + return to_nsdata(std::move(buffer).str()); } namespace { diff --git a/jni/testfixtures/app/opendocument/core/TestFiles.java b/jni/testfixtures/app/opendocument/core/TestFiles.java index c4844ecf6..4ca12d5ad 100644 --- a/jni/testfixtures/app/opendocument/core/TestFiles.java +++ b/jni/testfixtures/app/opendocument/core/TestFiles.java @@ -86,7 +86,7 @@ static Path pdfFile(Path directory) throws IOException { int start = out.length(); out.append("xref\n0 ").append(objects.size() + 1).append("\n0000000000 65535 f \n"); for (int offset : offsets) { - out.append(String.format("%010d 00000 n %n", offset).replace(System.lineSeparator(), "\n")); + out.append(String.format("%010d 00000 n \n", offset)); } out.append("trailer\n<< /Size ").append(objects.size() + 1).append(" /Root 1 0 R >>\n"); out.append("startxref\n").append(start).append("\n%%EOF\n"); diff --git a/jni/tests/app/opendocument/core/PdfFileTest.java b/jni/tests/app/opendocument/core/PdfFileTest.java index e1a25b2b3..75b4ffbb6 100644 --- a/jni/tests/app/opendocument/core/PdfFileTest.java +++ b/jni/tests/app/opendocument/core/PdfFileTest.java @@ -1,6 +1,6 @@ package app.opendocument.core; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -8,6 +8,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -34,9 +35,7 @@ void annotateAppendsToTheSource() throws IOException { assertTrue(result.length > source.length); // the source is copied and the annotation appended after it - for (int i = 0; i < source.length; ++i) { - assertEquals(source[i], result[i]); - } + assertArrayEquals(source, Arrays.copyOf(result, source.length)); String text = new String(result, StandardCharsets.ISO_8859_1); assertTrue(text.contains("/Highlight")); assertTrue(text.contains("/Subtype /Form")); diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index fd8c930df..ab338ea60 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -332,7 +332,12 @@ void odr_python::bind_file(py::module_ &m) { "annotate", [](const odr::PdfFile &file, const std::string &annotations) { std::ostringstream out; - file.annotate(annotations, out); + { + // scoped rather than a `call_guard`: the `py::bytes` below needs + // the GIL back + const py::gil_scoped_release release; + file.annotate(annotations, out); + } return py::bytes(std::move(out).str()); }, py::arg("annotations"), From 692c38f4c0629db7574325915d6a42a1ca2bdca6 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 18:21:49 +0200 Subject: [PATCH 3/3] docs(changelog): split the annotation entry in two Eight lines had grown around three separate things. The api and its bindings are one entry, `odr.annotation` in the rendered view another, each the length of its neighbours. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0147n68S7LNAv9KynLaGgXN4 --- CHANGELOG.md | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cecc3c6b..197c6407e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,14 +16,12 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- `PdfFile::annotate` writes markup annotations — highlight, underline, - strike-out, squiggly and freehand ink — into a pdf as an incremental update, - so the source bytes are left as they are and any viewer reads them as - ordinary pdf annotations. `FileTypeCapabilities` gains an `annotate` flag. - In every binding: `PdfFile.annotate` in python, java and swift, `annotate()` - on the wasm handle. The rendered pdf view exposes `odr.annotation` — tools for - the five kinds, live preview, undo — and its `getAnnotations()` produces - exactly what `annotate` takes. +- `PdfFile::annotate` writes highlight, underline, strike-out, squiggly and ink + annotations into a pdf as an incremental update — source bytes untouched, + any viewer reading them — in every binding, with an `annotate` capability. + +- The rendered pdf view exposes `odr.annotation`: the five tools, live preview + and undo, whose `getAnnotations()` produces exactly what `annotate` takes. - **Breaking**: `html::edit` becomes `Document::edit`, in every binding — java's `Html.edit(document, diff)` becomes `document.edit(diff)`, and so on.