From 24e3fb3305aacd01028356a3d1415e367334bcf8 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 14 Aug 2026 19:39:50 +0200 Subject: [PATCH 1/3] fix(pdf): put a line's baseline where its font says, not where the strut does A line block inherits the default font size, and its strut then outranks the run it holds and takes the line box's baseline. Every run sat below the `ascent` its `top` was derived from, the more so the smaller the text - body text by ~4pt at 6pt, which is why a heading looked low in its banner. The block's strut collapses (`font-size:0`), and an embedded face states the ascent and descent the placement assumes, as a substituted one already did. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PCMZUVxuGstmmgQzEdif --- src/odr/internal/html/pdf_file.cpp | 37 ++++++++++++++++++------------ 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index ce7e2132..cff05a02 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -2196,7 +2196,7 @@ class HtmlServiceImpl final : public HtmlService { .m = m, .invisible = invisible, .is_matrix = is_matrix, - .asc = ascent_em(text), + .asc = ascent_em(text.font), .scale = is_matrix ? 1.0 : m.a, .ox = m.e, .baseline = m.f, @@ -2361,8 +2361,10 @@ class HtmlServiceImpl final : public HtmlService { out.out() << ".p{position:relative;margin:0 16px;background:#fff;" "box-shadow:0 1px 4px rgba(0,0,0,.5)}"; // `.t`: shared base for all absolutely-positioned line blocks. + // `font-size:0` collapses its strut, which outranks the run it holds and + // would take the line box's baseline. out.out() << ".t{position:absolute;left:0;top:0;transform-origin:0 0;" - "white-space:pre;line-height:1;font-kerning:none;" + "white-space:pre;line-height:1;font-size:0;font-kerning:none;" "font-variant-ligatures:none}"; write_mode_css(); // SVG overlay covering the page box (visual graphics layer). @@ -2472,11 +2474,15 @@ class HtmlServiceImpl final : public HtmlService { } const std::string url = file_to_url(reencoded, "font/ttf"); const std::string n = std::to_string(index + 1); - font_faces += "@font-face{font-family:'odr-f"; - font_faces += n; - font_faces += "';src:url("; - font_faces += url; - font_faces += ");}"; + // The overrides sum to one em, so `line-height:1` puts the baseline at + // exactly the `ascent_em` a run's `top` is derived from. + const double ascent = ascent_em(&font); + std::ostringstream face; + face << "@font-face{font-family:'odr-f" << n << "';src:url(" << url + << ");ascent-override:" << round2(ascent * 100.0) + << "%;descent-override:" << round2((1.0 - ascent) * 100.0) + << "%;line-gap-override:0%}"; + font_faces += std::move(face).str(); const auto rule = [&](const char *cls, const char *color) { font_styles += '.'; font_styles += cls; @@ -2495,19 +2501,20 @@ class HtmlServiceImpl final : public HtmlService { } } - static double ascent_em(const pdf::TextElement &text) { + /// Baseline offset below a line block's `top`, in em. Capped at one em so + /// the `@font-face` descent can make the two sum to it. + static double ascent_em(const pdf::Font *font) { double em = 0.8; - if (text.font != nullptr && text.font->descriptor_ascent) { - em = *text.font->descriptor_ascent; - } else if (text.font != nullptr && text.font->embedded_font != nullptr) { - const std::uint16_t units = text.font->embedded_font->units_per_em(); + if (font != nullptr && font->descriptor_ascent) { + em = *font->descriptor_ascent; + } else if (font != nullptr && font->embedded_font != nullptr) { + const std::uint16_t units = font->embedded_font->units_per_em(); if (units != 0) { - em = static_cast( - text.font->embedded_font->bounding_box().y_max) / + em = static_cast(font->embedded_font->bounding_box().y_max) / units; } } - return std::clamp(em, 0.5, 1.2); + return std::clamp(em, 0.5, 1.0); } static std::string glyph_run_str(const pdf::Font &font, From ab66497ea1c985f4329a77502cf0a8bb79b5a1a7 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 14 Aug 2026 20:04:48 +0200 Subject: [PATCH 2/3] feat(pdf): decode jpeg 2000, convert cmyk as adobe does, clip to the crop box Three things a manual in the corpus showed against a native viewer. Its photographs were missing: they are `JPXDecode`, which nothing here decoded. `openjpeg` decodes the codestream and the raster takes the same path as any other - subsampled components resampled, a `cdef` alpha kept, sYCC converted. Its cyan read as `#00ffff`. DeviceCMYK went through the naive `(1-c)(1-k)` in three separate places; one `cmyk_to_rgb` now carries pdf.js's fit of Adobe's transform, so a process cyan is one and cmyk black is the dark neutral a viewer paints. Its cover carried text a page-width to the left of the crop box, which a viewer clips and we drew on the backdrop. A page box hides what overflows it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PCMZUVxuGstmmgQzEdif --- CHANGELOG.md | 8 + CMakeLists.txt | 3 + conan.lock | 1 + conanfile.py | 1 + src/odr/internal/html/pdf_file.cpp | 18 +- src/odr/internal/pdf/AGENTS.md | 6 +- src/odr/internal/pdf/pdf_color.cpp | 38 +++- src/odr/internal/pdf/pdf_color.hpp | 4 + src/odr/internal/pdf/pdf_document_element.hpp | 4 +- src/odr/internal/pdf/pdf_document_parser.cpp | 2 +- src/odr/internal/pdf/pdf_image.cpp | 53 ++++- src/odr/internal/pdf/pdf_image.hpp | 5 +- src/odr/internal/pdf/pdf_jpx.cpp | 207 ++++++++++++++++++ src/odr/internal/pdf/pdf_jpx.hpp | 24 ++ src/odr/internal/pdf/pdf_page_extractor.cpp | 14 +- test/CMakeLists.txt | 1 + test/data.cmake | 4 +- test/src/internal/pdf/pdf_color.cpp | 19 +- test/src/internal/pdf/pdf_jpx.cpp | 23 ++ 19 files changed, 395 insertions(+), 40 deletions(-) create mode 100644 src/odr/internal/pdf/pdf_jpx.cpp create mode 100644 src/odr/internal/pdf/pdf_jpx.hpp create mode 100644 test/src/internal/pdf/pdf_jpx.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 65bf4e89..9de6ad2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,14 @@ The release run heads these entries with the version and opens a fresh white space wherever it stands, not the start of a token. - Text that decodes to half a surrogate pair costs that character a replacement mark instead of the whole document. +- A pdf's text sits where the file puts it: a line was placed against the + browser's default font rather than its own, which dropped small text by + several points. +- A pdf's cmyk colours are converted as Adobe converts them, so a process cyan + reads as one instead of as pure `#00ffff`. +- A pdf page shows what is on it and no more: content outside the crop box is + clipped, as a viewer clips it. +- A pdf's JPEG 2000 images render. New dependency: `openjpeg`. ## v6.5.0 - 2026-08-10 diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c8e480d..6c5babc5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -57,6 +57,7 @@ find_package(pugixml REQUIRED) find_package(miniz REQUIRED) find_package(cryptopp REQUIRED) find_package(nlohmann_json REQUIRED) +find_package(OpenJPEG REQUIRED) find_package(uchardet REQUIRED) find_package(utf8cpp REQUIRED) @@ -216,6 +217,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/pdf/pdf_filter.cpp" "src/odr/internal/pdf/pdf_function.cpp" "src/odr/internal/pdf/pdf_image.cpp" + "src/odr/internal/pdf/pdf_jpx.cpp" "src/odr/internal/pdf/pdf_graphics_operator_parser.cpp" "src/odr/internal/pdf/pdf_graphics_state.cpp" "src/odr/internal/pdf/pdf_object.cpp" @@ -276,6 +278,7 @@ target_link_libraries(odr miniz::miniz cryptopp::cryptopp nlohmann_json::nlohmann_json + openjp2 uchardet::uchardet utf8::cpp ) diff --git a/conan.lock b/conan.lock index 005ba383..95b848de 100644 --- a/conan.lock +++ b/conan.lock @@ -8,6 +8,7 @@ "uchardet/0.0.8#6ab25e452021fcdb560f4e37f4a27bc1%1759735438.978", "pybind11/2.13.6#42746850cd4c68d1b1ea42de456c2182%1755673714.548", "pugixml/1.15#979e88f4fafbfe3585d2c0510a071cc7%1739435725.483", + "openjpeg/2.5.4#372fbc2b4348d45ab0c0a62a8475dc2f%1760446899.685", "nlohmann_json/3.12.0#2d634ab0ec8d9f56353e5ccef6d6612c%1744735883.94", "miniz/3.0.2#bfbce07c6654293cce27ee24129d2df7%1743673472.805", "gtest/1.14.0#f8f0757a574a8dd747d16af62d6eb1b7%1743410807.169", diff --git a/conanfile.py b/conanfile.py index 8996baaf..d3806476 100644 --- a/conanfile.py +++ b/conanfile.py @@ -52,6 +52,7 @@ def requirements(self): self.requires("cryptopp/8.9.0") self.requires("miniz/3.0.2") self.requires("nlohmann_json/3.12.0") + self.requires("openjpeg/2.5.4") self.requires("uchardet/0.0.8") self.requires("utfcpp/4.0.9") if self.options.get_safe("with_http_server", False): diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index cff05a02..7382425b 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -316,14 +317,11 @@ std::string device_color_to_css(const pdf::GraphicsState::Color &color) { b = to255(color.rgb[2]); break; case pdf::ColorSpace::device_cmyk: { - // Naive CMYK -> RGB (no ICC). - const double c = color.cmyk[0]; - const double m = color.cmyk[1]; - const double y = color.cmyk[2]; - const double k = color.cmyk[3]; - r = to255((1 - c) * (1 - k)); - g = to255((1 - m) * (1 - k)); - b = to255((1 - y) * (1 - k)); + const std::array rgb = pdf::cmyk_to_rgb( + color.cmyk[0], color.cmyk[1], color.cmyk[2], color.cmyk[3]); + r = to255(rgb[0]); + g = to255(rgb[1]); + b = to255(rgb[2]); break; } case pdf::ColorSpace::unknown: @@ -2358,8 +2356,10 @@ class HtmlServiceImpl final : public HtmlService { // side margin is part of that width, so a phone screen keeps a gutter. out.out() << ".d{display:flex;flex-direction:column;align-items:center;" "gap:16px;padding:16px 0;width:max-content;min-width:100%}"; + // `overflow:hidden` clips to the crop box, as a viewer does: content may + // sit outside it (a bleed, or an InDesign spread's other page). out.out() << ".p{position:relative;margin:0 16px;background:#fff;" - "box-shadow:0 1px 4px rgba(0,0,0,.5)}"; + "overflow:hidden;box-shadow:0 1px 4px rgba(0,0,0,.5)}"; // `.t`: shared base for all absolutely-positioned line blocks. // `font-size:0` collapses its strut, which outranks the run it holds and // would take the line box's baseline. diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 41e3ef30..690c112a 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -146,10 +146,12 @@ Things the code won't shout at you: is unlocked with the empty password first so `/Info` decrypts). It is all-or-nothing: a malformed structure leaves `document_type` at `unknown` rather than half-filling the fields. XMP is not parsed — the strings are `/Info`-only. -- **Image codecs are deliberately not decoded** in the filter framework +- **Image codecs are not decoded** in the filter framework (DCTDecode/JPXDecode/CCITTFaxDecode/JBIG2Decode): `decode()` stops and hands back the still-encoded payload for the image path; `read_decoded_stream` treats - them as an error. `Crypt` passes through only as `Identity`. + them as an error. A JPEG then passes through to the browser and a JPEG 2000 + goes to `pdf_jpx` (openjpeg) to be re-encoded as PNG like any other raster; + CCITT and JBIG2 remain undecodable. `Crypt` passes through only as `Identity`. - **Inherited page attributes** (`Resources`/`MediaBox`/`CropBox`/`Rotate`, Table 30) are resolved by threading an accumulator down the `Pages` recursion — *not* by a `Parent` walk. Lenience (all with a `Logger` warning): `CropBox` ← diff --git a/src/odr/internal/pdf/pdf_color.cpp b/src/odr/internal/pdf/pdf_color.cpp index 2650f657..4c18ffba 100644 --- a/src/odr/internal/pdf/pdf_color.cpp +++ b/src/odr/internal/pdf/pdf_color.cpp @@ -11,12 +11,6 @@ namespace { double clamp01(const double v) { return std::clamp(v, 0.0, 1.0); } -/// Naive DeviceCMYK -> RGB (no ICC), matching the HTML emitter's conversion. -std::array cmyk_to_rgb(const double c, const double m, - const double y, const double k) { - return {(1 - c) * (1 - k), (1 - m) * (1 - k), (1 - y) * (1 - k)}; -} - /// sRGB gamma encode of a linear component (IEC 61966-2-1). double linear_to_srgb(const double c) { const double v = clamp01(c); @@ -183,6 +177,38 @@ std::vector ColorSpaceDef::initial_components() const { namespace odr::internal { +std::array pdf::cmyk_to_rgb(const double c, const double m, + const double y, const double k) { + const double r = + 255 + + c * (-4.387332384609988 * c + 54.48615194189176 * m + + 18.82290502165302 * y + 212.25662451639585 * k - 285.2331026137004) + + m * (1.7149763477362134 * m - 5.6096736904047315 * y - + 17.873870861415444 * k - 5.497006427196366) + + y * (-2.5217340131683033 * y - 21.248923337353073 * k + + 17.5119270841813) + + k * (-21.86122147463605 * k - 189.48180835922747); + const double g = + 255 + + c * (8.841041422036149 * c + 60.118027045597366 * m + + 6.871425592049007 * y + 31.159100130055922 * k - 79.2970844816548) + + m * (-15.310361306967817 * m + 17.575251261109482 * y + + 131.35250912493976 * k - 190.9453302588951) + + y * (4.444339102852739 * y + 9.8632861493405 * k - 24.86741582555878) + + k * (-20.737325471181034 * k - 187.80453709719578); + const double b = 255 + + c * (0.8842522430003296 * c + 8.078677503112928 * m + + 30.89978309703729 * y - 0.23883238689178934 * k - + 14.183576799673286) + + m * (10.49593273432072 * m + 63.02378494754052 * y + + 50.606957656360734 * k - 112.23884253719248) + + y * (0.03296041114873217 * y + 115.60384449646641 * k - + 193.58209356861505) + + k * (-22.33816807309886 * k - 180.12613974708367); + return {pdf::clamp01(r / 255.0), pdf::clamp01(g / 255.0), + pdf::clamp01(b / 255.0)}; +} + std::shared_ptr pdf::parse_color_space(const Object &object, const ColorSpaceContext &context) { const Object resolved = context.resolve(object); diff --git a/src/odr/internal/pdf/pdf_color.hpp b/src/odr/internal/pdf/pdf_color.hpp index 99570b58..73dd1174 100644 --- a/src/odr/internal/pdf/pdf_color.hpp +++ b/src/odr/internal/pdf/pdf_color.hpp @@ -73,6 +73,10 @@ struct ColorSpaceContext { std::function(const std::string &)> named; }; +/// DeviceCMYK -> sRGB without an ICC engine: pdf.js's polynomial fit of Adobe's +/// transform. The naive `(1-c)(1-k)` reads pure cyan as `#00ffff`. +std::array cmyk_to_rgb(double c, double m, double y, double k); + /// Build a colour space from its PDF object — a name (`/DeviceRGB`, …) or an /// array (`[/ICCBased 5 0 R]`, `[/Separation …]`, …). Returns `nullptr` for an /// unsupported or malformed definition. diff --git a/src/odr/internal/pdf/pdf_document_element.hpp b/src/odr/internal/pdf/pdf_document_element.hpp index f8737691..569cf22f 100644 --- a/src/odr/internal/pdf/pdf_document_element.hpp +++ b/src/odr/internal/pdf/pdf_document_element.hpp @@ -161,8 +161,8 @@ struct XObject final : Element { // --- image (`/Subtype /Image`) --- /// Browser-ready bytes: a `DCTDecode` JPEG passed through, or a raster /// re-encoded as PNG (with any `/SMask`/`/Mask` composited into RGBA). Empty - /// for an undecodable codec (JPX/CCITT/JBIG2) and for a stencil, so `Do` - /// skips it. + /// for an undecodable codec (CCITT/JBIG2) and for a stencil, so `Do` skips + /// it. std::string image_data; std::string image_mime; diff --git a/src/odr/internal/pdf/pdf_document_parser.cpp b/src/odr/internal/pdf/pdf_document_parser.cpp index 9449b065..253c1cff 100644 --- a/src/odr/internal/pdf/pdf_document_parser.cpp +++ b/src/odr/internal/pdf/pdf_document_parser.cpp @@ -749,7 +749,7 @@ std::vector resolve_mask_alpha(DocumentParser &parser, DecodeResult result = decode(filter, decode_parms, parser.read_object_stream(object)); if (result.stopped_at_filter.has_value()) { - return {}; // an image codec we cannot decode (CCITT/JBIG2/JPX) + return {}; // an image codec we cannot decode (CCITT/JBIG2) } return decode_mask_alpha( result.data, image_int(parser, dictionary, "Width", 0), diff --git a/src/odr/internal/pdf/pdf_image.cpp b/src/odr/internal/pdf/pdf_image.cpp index 89033f45..5315581f 100644 --- a/src/odr/internal/pdf/pdf_image.cpp +++ b/src/odr/internal/pdf/pdf_image.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -68,8 +69,49 @@ std::uint8_t to_byte(const double v) { return static_cast(scaled); } -} // namespace +/// A decoded JPEG 2000 raster as a PNG, through the image's own colour space +/// when the count matches and a device space of its component count otherwise +/// (ISO 32000-1 8.9.5.1: `/ColorSpace` is optional for `JPXDecode`). +std::optional encode_jpx(const std::string &data, + const ColorSpaceDef *color_space, + const std::vector &decode_array, + const std::vector &alpha, + const std::vector &color_key) { + const std::optional image = decode_jpx(data); + if (!image.has_value()) { + return std::nullopt; + } + + ColorSpaceDef device; + device.components = image->components; + switch (image->components) { + case 1: + device.kind = ColorSpaceKind::device_gray; + break; + case 3: + device.kind = ColorSpaceKind::device_rgb; + break; + case 4: + device.kind = ColorSpaceKind::device_cmyk; + break; + default: + return std::nullopt; + } + const ColorSpaceDef &space = + color_space != nullptr && color_space->components == image->components + ? *color_space + : device; + + const std::string png = encode_image_png( + image->samples, image->width, image->height, 8, space, decode_array, + alpha.empty() ? image->alpha : alpha, color_key); + if (png.empty()) { + return std::nullopt; + } + return EncodedImage{png, "image/png"}; +} +} // namespace } // namespace odr::internal::pdf namespace odr::internal { @@ -313,8 +355,15 @@ std::optional pdf::encode_image( } return std::nullopt; } + if (terminal == "JPXDecode") { + DecodeResult result = decode(filter, decode_parms, std::move(raw)); + if (result.stopped_at_filter != "JPXDecode") { + return std::nullopt; + } + return encode_jpx(result.data, color_space, decode_array, alpha, color_key); + } if (terminal.has_value()) { - return std::nullopt; // JPX/CCITT/JBIG2: not yet a pass-through + return std::nullopt; // CCITT/JBIG2: not decodable } // A fully decodable raster: decode, assemble samples and PNG-encode. diff --git a/src/odr/internal/pdf/pdf_image.hpp b/src/odr/internal/pdf/pdf_image.hpp index 4bd5d375..0cb7c1ab 100644 --- a/src/odr/internal/pdf/pdf_image.hpp +++ b/src/odr/internal/pdf/pdf_image.hpp @@ -22,8 +22,9 @@ struct EncodedImage { /// `DCTDecode` JPEG passes through, any other decodable raster is re-encoded as /// PNG through `color_space` (required only on that path). `alpha` and /// `color_key` (see `encode_image_png`) make the raster RGBA and are ignored by -/// the JPEG pass-through. `nullopt` for an undecodable codec -/// (JPX/CCITTFax/JBIG2) or an inconsistent raster. +/// the JPEG pass-through. A `JPXDecode` raster comes through `decode_jpx`. +/// `nullopt` for an undecodable codec (CCITTFax/JBIG2) or an inconsistent +/// raster. std::optional encode_image(std::string raw, const Object &filter, const Object &decode_parms, std::int32_t width, std::int32_t height, diff --git a/src/odr/internal/pdf/pdf_jpx.cpp b/src/odr/internal/pdf/pdf_jpx.cpp new file mode 100644 index 00000000..febd122c --- /dev/null +++ b/src/odr/internal/pdf/pdf_jpx.cpp @@ -0,0 +1,207 @@ +#include + +#include +#include +#include +#include + +#include + +namespace odr::internal { + +namespace { + +/// Cursor over the payload for openjpeg's stream callbacks. +struct MemoryStream { + const std::string *data{nullptr}; + std::size_t position{0}; +}; + +OPJ_SIZE_T stream_read(void *buffer, const OPJ_SIZE_T size, void *user) { + auto *stream = static_cast(user); + const std::size_t left = stream->data->size() - stream->position; + if (left == 0) { + return static_cast(-1); + } + const std::size_t n = std::min(size, left); + std::memcpy(buffer, stream->data->data() + stream->position, n); + stream->position += n; + return n; +} + +OPJ_OFF_T stream_skip(const OPJ_OFF_T size, void *user) { + auto *stream = static_cast(user); + const std::size_t left = stream->data->size() - stream->position; + const auto n = static_cast(std::max(size, 0)); + if (n > left) { + stream->position = stream->data->size(); + return static_cast(-1); + } + stream->position += n; + return static_cast(n); +} + +OPJ_BOOL stream_seek(const OPJ_OFF_T position, void *user) { + auto *stream = static_cast(user); + if (position < 0 || + static_cast(position) > stream->data->size()) { + return OPJ_FALSE; + } + stream->position = static_cast(position); + return OPJ_TRUE; +} + +/// JP2 signature box (ISO/IEC 15444-1 I.5.1) vs a bare codestream's SOC + SIZ. +OPJ_CODEC_FORMAT codec_format(const std::string &data) { + static constexpr std::array jp2_signature{ + 0x00, 0x00, 0x00, 0x0c, 0x6a, 0x50, 0x20, 0x20, 0x0d, 0x0a, 0x87, 0x0a}; + if (data.size() >= jp2_signature.size() && + std::memcmp(data.data(), jp2_signature.data(), jp2_signature.size()) == + 0) { + return OPJ_CODEC_JP2; + } + return OPJ_CODEC_J2K; +} + +/// One component's sample at (x, y) of the *image* grid, scaled to 8 bits. +/// A component may be subsampled (`dx`/`dy`), which the nearest sample covers. +std::uint8_t sample_at(const opj_image_comp_t &comp, const std::int32_t x, + const std::int32_t y) { + const std::uint32_t cx = + std::min(static_cast(x) / std::max(comp.dx, 1u), + comp.w == 0 ? 0 : comp.w - 1); + const std::uint32_t cy = + std::min(static_cast(y) / std::max(comp.dy, 1u), + comp.h == 0 ? 0 : comp.h - 1); + std::int32_t value = comp.data[cy * comp.w + cx]; + if (comp.sgnd != 0) { + value += 1 << (comp.prec - 1); + } + const std::int32_t max = (1 << comp.prec) - 1; + value = std::clamp(value, 0, max); + return static_cast(comp.prec >= 8 ? value >> (comp.prec - 8) + : value << (8 - comp.prec)); +} + +/// YCbCr -> RGB in place (ITU-R BT.601, the `sYCC` space JP2 may declare). +void sycc_to_rgb(std::string &samples) { + for (std::size_t i = 0; i + 2 < samples.size(); i += 3) { + const auto y = static_cast(static_cast(samples[i])); + const double cb = + static_cast(static_cast(samples[i + 1])) - 128.0; + const double cr = + static_cast(static_cast(samples[i + 2])) - 128.0; + const auto to_byte = [](const double v) { + return static_cast( + static_cast(std::clamp(v, 0.0, 255.0))); + }; + samples[i] = to_byte(y + 1.402 * cr); + samples[i + 1] = to_byte(y - 0.344136 * cb - 0.714136 * cr); + samples[i + 2] = to_byte(y + 1.772 * cb); + } +} + +} // namespace + +std::optional pdf::decode_jpx(const std::string &data) { + if (data.empty()) { + return std::nullopt; + } + + const std::unique_ptr codec( + opj_create_decompress(codec_format(data)), &opj_destroy_codec); + if (codec == nullptr) { + return std::nullopt; + } + opj_dparameters_t parameters; + opj_set_default_decoder_parameters(¶meters); + if (opj_setup_decoder(codec.get(), ¶meters) == OPJ_FALSE) { + return std::nullopt; + } + + MemoryStream memory{&data, 0}; + const std::unique_ptr stream( + opj_stream_default_create(OPJ_TRUE), &opj_stream_destroy); + if (stream == nullptr) { + return std::nullopt; + } + opj_stream_set_user_data(stream.get(), &memory, nullptr); + opj_stream_set_user_data_length(stream.get(), data.size()); + opj_stream_set_read_function(stream.get(), stream_read); + opj_stream_set_skip_function(stream.get(), stream_skip); + opj_stream_set_seek_function(stream.get(), stream_seek); + + opj_image_t *raw_image = nullptr; + if (opj_read_header(stream.get(), codec.get(), &raw_image) == OPJ_FALSE) { + opj_image_destroy(raw_image); + return std::nullopt; + } + const std::unique_ptr image( + raw_image, &opj_image_destroy); + if (opj_decode(codec.get(), stream.get(), image.get()) == OPJ_FALSE || + opj_end_decompress(codec.get(), stream.get()) == OPJ_FALSE) { + return std::nullopt; + } + + const std::int32_t width = static_cast(image->x1) - + static_cast(image->x0); + const std::int32_t height = static_cast(image->y1) - + static_cast(image->y0); + if (width <= 0 || height <= 0 || image->numcomps == 0) { + return std::nullopt; + } + for (std::uint32_t i = 0; i < image->numcomps; ++i) { + const opj_image_comp_t &comp = image->comps[i]; + if (comp.data == nullptr || comp.w == 0 || comp.h == 0 || comp.prec == 0 || + comp.prec > 16) { + return std::nullopt; + } + } + + // A component the `cdef` box marks as opacity is the alpha plane; the rest + // are colour, in codestream order. + std::vector colour; + std::optional alpha_index; + for (std::uint32_t i = 0; i < image->numcomps; ++i) { + if (image->comps[i].alpha != 0 && !alpha_index.has_value()) { + alpha_index = i; + } else { + colour.push_back(i); + } + } + if (colour.empty() || colour.size() > 4) { + return std::nullopt; + } + + JpxImage result; + result.width = width; + result.height = height; + result.components = static_cast(colour.size()); + result.samples.resize(static_cast(width) * height * + colour.size()); + if (alpha_index.has_value()) { + result.alpha.resize(static_cast(width) * height); + } + + std::size_t out = 0; + for (std::int32_t y = 0; y < height; ++y) { + for (std::int32_t x = 0; x < width; ++x) { + for (const std::uint32_t c : colour) { + result.samples[out++] = + static_cast(sample_at(image->comps[c], x, y)); + } + if (alpha_index.has_value()) { + result.alpha[static_cast(y) * width + x] = + sample_at(image->comps[*alpha_index], x, y); + } + } + } + + if (image->color_space == OPJ_CLRSPC_SYCC && result.components == 3) { + sycc_to_rgb(result.samples); + } + + return result; +} + +} // namespace odr::internal diff --git a/src/odr/internal/pdf/pdf_jpx.hpp b/src/odr/internal/pdf/pdf_jpx.hpp new file mode 100644 index 00000000..16becb1c --- /dev/null +++ b/src/odr/internal/pdf/pdf_jpx.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include +#include +#include +#include + +namespace odr::internal::pdf { + +/// A JPEG 2000 codestream decoded to 8-bit interleaved samples. +struct JpxImage { + std::int32_t width{0}; + std::int32_t height{0}; + std::int32_t components{0}; ///< colour components, alpha excluded + std::string samples; ///< `components` bytes per pixel, row-major + std::vector alpha; ///< empty unless the image carries one +}; + +/// Decode a `JPXDecode` payload — a JP2 container or a bare J2K codestream +/// (ISO 32000-1 7.4.9). `nullopt` for a codestream openjpeg rejects or a +/// layout we do not map (more than four colour components). +std::optional decode_jpx(const std::string &data); + +} // namespace odr::internal::pdf diff --git a/src/odr/internal/pdf/pdf_page_extractor.cpp b/src/odr/internal/pdf/pdf_page_extractor.cpp index d4014327..c75362e3 100644 --- a/src/odr/internal/pdf/pdf_page_extractor.cpp +++ b/src/odr/internal/pdf/pdf_page_extractor.cpp @@ -276,21 +276,17 @@ void set_color(GraphicsState::Color &color, const GraphicsOperator &op) { /// Resolve a graphics-state colour to sRGB in [0, 1]. Non-device spaces have /// already been converted to `rgb` by `set_color`/`set_color_space` (they set -/// `space` to `device_rgb`); CMYK uses the same naive conversion as the HTML -/// emitter. Used to paint a stencil image mask in the current fill colour. +/// `space` to `device_rgb`). Used to paint a stencil image mask in the current +/// fill colour. std::array color_to_rgb(const GraphicsState::Color &color) { switch (color.space) { case ColorSpace::device_grey: return {color.grey, color.grey, color.grey}; case ColorSpace::device_rgb: return {color.rgb[0], color.rgb[1], color.rgb[2]}; - case ColorSpace::device_cmyk: { - const double c = color.cmyk[0]; - const double m = color.cmyk[1]; - const double y = color.cmyk[2]; - const double k = color.cmyk[3]; - return {(1 - c) * (1 - k), (1 - m) * (1 - k), (1 - y) * (1 - k)}; - } + case ColorSpace::device_cmyk: + return cmyk_to_rgb(color.cmyk[0], color.cmyk[1], color.cmyk[2], + color.cmyk[3]); case ColorSpace::unknown: break; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7389278d..e7b1056c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -77,6 +77,7 @@ add_executable(odr_test "src/internal/pdf/pdf_font.cpp" "src/internal/pdf/pdf_function.cpp" "src/internal/pdf/pdf_image.cpp" + "src/internal/pdf/pdf_jpx.cpp" "src/internal/util/math_util_test.cpp" "src/internal/pdf/pdf_object.cpp" "src/internal/pdf/pdf_object_parser.cpp" diff --git a/test/data.cmake b/test/data.cmake index 93e0d8e6..332c2dc0 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "6d1ed797b3833b99b09ce359f2689c5a3af105c1") + REVISION "a77c80e08f8104b6110d9a47b092f1720a12b6f3") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "90841441c293ec665632b570bdfe63842926859c") + REVISION "2947be444b5f860442e163d2d94ba80ce54020df") diff --git a/test/src/internal/pdf/pdf_color.cpp b/test/src/internal/pdf/pdf_color.cpp index 19666b34..64bbd7d8 100644 --- a/test/src/internal/pdf/pdf_color.cpp +++ b/test/src/internal/pdf/pdf_color.cpp @@ -37,7 +37,9 @@ ColorSpaceDef device(const ColorSpaceKind kind, const int components) { } // namespace -// The device spaces convert as expected (CMYK by the naive formula). +// The device spaces convert as expected. CMYK follows Adobe's transform, so no +// ink is white, full key is a dark neutral rather than `#000`, and full cyan is +// a process cyan. TEST(PdfColor, device_spaces) { EXPECT_EQ(device(ColorSpaceKind::device_gray, 1).to_rgb({0.5}), (std::array{0.5, 0.5, 0.5})); @@ -45,8 +47,16 @@ TEST(PdfColor, device_spaces) { (std::array{0.2, 0.4, 0.6})); EXPECT_EQ(device(ColorSpaceKind::device_cmyk, 4).to_rgb({0, 0, 0, 0}), (std::array{1, 1, 1})); - EXPECT_EQ(device(ColorSpaceKind::device_cmyk, 4).to_rgb({0, 0, 0, 1}), - (std::array{0, 0, 0})); + const std::array key = + device(ColorSpaceKind::device_cmyk, 4).to_rgb({0, 0, 0, 1}); + EXPECT_NEAR(key[0], 0.17, 0.01); + EXPECT_NEAR(key[1], 0.18, 0.01); + EXPECT_NEAR(key[2], 0.21, 0.01); + const std::array cyan = + device(ColorSpaceKind::device_cmyk, 4).to_rgb({1, 0, 0, 0}); + EXPECT_NEAR(cyan[0], 0.0, 0.01); + EXPECT_NEAR(cyan[1], 0.72, 0.01); + EXPECT_NEAR(cyan[2], 0.95, 0.01); } // L*a*b* maps the lightness extremes to white and black under the default @@ -133,8 +143,7 @@ TEST(PdfColor, initial_components) { // so a resource alias to /DeviceCMYK matches a direct /DeviceCMYK selection. const ColorSpaceDef cmyk = device(ColorSpaceKind::device_cmyk, 4); EXPECT_EQ(cmyk.initial_components(), (std::vector{0, 0, 0, 1})); - EXPECT_EQ(cmyk.to_rgb(cmyk.initial_components()), - (std::array{0, 0, 0})); + EXPECT_EQ(cmyk.to_rgb(cmyk.initial_components()), cmyk_to_rgb(0, 0, 0, 1)); } // A name resolves to the matching device space. diff --git a/test/src/internal/pdf/pdf_jpx.cpp b/test/src/internal/pdf/pdf_jpx.cpp new file mode 100644 index 00000000..d29e5156 --- /dev/null +++ b/test/src/internal/pdf/pdf_jpx.cpp @@ -0,0 +1,23 @@ +#include + +#include + +#include + +using namespace odr::internal::pdf; + +// Bytes that are no codestream are rejected, not decoded into a raster. +TEST(PdfJpx, rejects_non_codestream) { + EXPECT_FALSE(decode_jpx("").has_value()); + EXPECT_FALSE(decode_jpx("not a codestream").has_value()); + EXPECT_FALSE(decode_jpx(std::string(64, '\0')).has_value()); +} + +// A JP2 signature box with nothing behind it, and a truncated raw codestream: +// both take the header path far enough to matter. +TEST(PdfJpx, rejects_truncated_input) { + const std::string signature( + "\x00\x00\x00\x0c\x6a\x50\x20\x20\x0d\x0a\x87\x0a", 12); + EXPECT_FALSE(decode_jpx(signature).has_value()); + EXPECT_FALSE(decode_jpx(std::string("\xff\x4f\xff\x51", 4)).has_value()); +} From fab329c483df7bbe879ec456793bd37a6deccf05 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 14 Aug 2026 20:32:13 +0200 Subject: [PATCH 3/3] fix(pdf): size a page by its crop box, and take a jpx alpha only when asked Two from review. A page was sized and translated from its media box, so clipping to it left a bleed showing and the page kept a size no viewer shows; the crop box, which falls back to the media box, decides both, and either corner order reads. A JPX codestream's own opacity channel was always applied. `/SMaskInData` says whether it counts at all - it does not by default - and `2` says the colour is premultiplied by it (Table 89). Not taken: routing openjpeg's diagnostics through `Logger`. Its default handlers are `opj_default_callback`, which does nothing, so a malformed codestream writes nothing to stderr. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PCMZUVxuGstmmgQzEdif --- CHANGELOG.md | 4 +-- src/odr/internal/html/pdf_file.cpp | 16 ++++++--- src/odr/internal/pdf/pdf_document_parser.cpp | 7 +++- src/odr/internal/pdf/pdf_image.cpp | 36 +++++++++++++++++--- src/odr/internal/pdf/pdf_image.hpp | 20 +++++------ test/data.cmake | 4 +-- 6 files changed, 63 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9de6ad2a..e404ba67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,8 +37,8 @@ The release run heads these entries with the version and opens a fresh several points. - A pdf's cmyk colours are converted as Adobe converts them, so a process cyan reads as one instead of as pure `#00ffff`. -- A pdf page shows what is on it and no more: content outside the crop box is - clipped, as a viewer clips it. +- A pdf page is the size of its crop box and shows what is on it and no more, + as a viewer shows it. - A pdf's JPEG 2000 images render. New dependency: `openjpeg`. ## v6.5.0 - 2026-08-10 diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 7382425b..1e91e109 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -2253,11 +2253,17 @@ class HtmlServiceImpl final : public HtmlService { template static PageBox begin_page(const pdf::Page &page, AddClass &&add_class) { - const pdf::Array &page_box = page.media_box.as_array(); - const double box_x0 = page_box[0].as_real(); - const double box_y0 = page_box[1].as_real(); - const double width = page_box[2].as_real() - box_x0; - const double height = page_box[3].as_real() - box_y0; + // The crop box is what a viewer shows (14.11.2); it falls back to the media + // box. Its corners may be given in either order (7.9.5). + const pdf::Array &page_box = page.crop_box.as_array(); + const double box_x0 = + std::min(page_box[0].as_real(), page_box[2].as_real()); + const double box_y0 = + std::min(page_box[1].as_real(), page_box[3].as_real()); + const double width = + std::max(page_box[0].as_real(), page_box[2].as_real()) - box_x0; + const double height = + std::max(page_box[1].as_real(), page_box[3].as_real()) - box_y0; std::string classes = "p"; { diff --git a/src/odr/internal/pdf/pdf_document_parser.cpp b/src/odr/internal/pdf/pdf_document_parser.cpp index 253c1cff..390620c6 100644 --- a/src/odr/internal/pdf/pdf_document_parser.cpp +++ b/src/odr/internal/pdf/pdf_document_parser.cpp @@ -841,10 +841,15 @@ void parse_image_data(DocumentParser &parser, const Dictionary &dictionary, } } + // A JPX codestream may carry its own opacity; `/SMaskInData` says whether it + // counts (8.9.5.4). + const std::int32_t smask_in_data = + image_int(parser, dictionary, "SMaskInData", 0); + if (std::optional encoded = encode_image(parser.read_object_stream(object), filter, decode_parms, width, height, bits_per_component, color_space.get(), - decode_array, alpha, color_key)) { + decode_array, alpha, color_key, smask_in_data)) { x_object.image_data = std::move(encoded->data); x_object.image_mime = std::move(encoded->mime); } diff --git a/src/odr/internal/pdf/pdf_image.cpp b/src/odr/internal/pdf/pdf_image.cpp index 5315581f..7dff54b4 100644 --- a/src/odr/internal/pdf/pdf_image.cpp +++ b/src/odr/internal/pdf/pdf_image.cpp @@ -69,6 +69,25 @@ std::uint8_t to_byte(const double v) { return static_cast(scaled); } +/// Undo the premultiplication `/SMaskInData 2` declares, leaving the straight +/// colour a PNG carries. +void unpremultiply(std::string &samples, const std::int32_t components, + const std::vector &alpha) { + for (std::size_t pixel = 0; pixel < alpha.size(); ++pixel) { + const std::uint8_t a = alpha[pixel]; + for (std::int32_t c = 0; c < components; ++c) { + const std::size_t i = pixel * static_cast(components) + + static_cast(c); + if (i >= samples.size()) { + return; + } + const auto value = static_cast(samples[i]); + samples[i] = static_cast( + a == 0 ? 0 : to_byte(std::min(1.0, value / static_cast(a)))); + } + } +} + /// A decoded JPEG 2000 raster as a PNG, through the image's own colour space /// when the count matches and a device space of its component count otherwise /// (ISO 32000-1 8.9.5.1: `/ColorSpace` is optional for `JPXDecode`). @@ -76,11 +95,19 @@ std::optional encode_jpx(const std::string &data, const ColorSpaceDef *color_space, const std::vector &decode_array, const std::vector &alpha, - const std::vector &color_key) { - const std::optional image = decode_jpx(data); + const std::vector &color_key, + const std::int32_t smask_in_data) { + std::optional image = decode_jpx(data); if (!image.has_value()) { return std::nullopt; } + // Table 89: the codestream's own opacity counts only where the image asks for + // it, and `2` says the colour is premultiplied by it. + if (smask_in_data == 0) { + image->alpha.clear(); + } else if (smask_in_data == 2) { + unpremultiply(image->samples, image->components, image->alpha); + } ColorSpaceDef device; device.components = image->components; @@ -344,7 +371,7 @@ std::optional pdf::encode_image( const std::int32_t bits_per_component, const ColorSpaceDef *color_space, const std::vector &decode_array, const std::vector &alpha, - const std::vector &color_key) { + const std::vector &color_key, const std::int32_t smask_in_data) { const std::optional terminal = terminal_image_codec(filter); if (terminal == "DCTDecode") { @@ -360,7 +387,8 @@ std::optional pdf::encode_image( if (result.stopped_at_filter != "JPXDecode") { return std::nullopt; } - return encode_jpx(result.data, color_space, decode_array, alpha, color_key); + return encode_jpx(result.data, color_space, decode_array, alpha, color_key, + smask_in_data); } if (terminal.has_value()) { return std::nullopt; // CCITT/JBIG2: not decodable diff --git a/src/odr/internal/pdf/pdf_image.hpp b/src/odr/internal/pdf/pdf_image.hpp index 0cb7c1ab..5c856021 100644 --- a/src/odr/internal/pdf/pdf_image.hpp +++ b/src/odr/internal/pdf/pdf_image.hpp @@ -22,16 +22,16 @@ struct EncodedImage { /// `DCTDecode` JPEG passes through, any other decodable raster is re-encoded as /// PNG through `color_space` (required only on that path). `alpha` and /// `color_key` (see `encode_image_png`) make the raster RGBA and are ignored by -/// the JPEG pass-through. A `JPXDecode` raster comes through `decode_jpx`. -/// `nullopt` for an undecodable codec (CCITTFax/JBIG2) or an inconsistent -/// raster. -std::optional -encode_image(std::string raw, const Object &filter, const Object &decode_parms, - std::int32_t width, std::int32_t height, - std::int32_t bits_per_component, const ColorSpaceDef *color_space, - const std::vector &decode, - const std::vector &alpha = {}, - const std::vector &color_key = {}); +/// the JPEG pass-through. A `JPXDecode` raster comes through `decode_jpx`, its +/// own opacity channel taken only as `smask_in_data` says (Table 89: 0 ignores +/// it, 2 says the colour is premultiplied by it). `nullopt` for an undecodable +/// codec (CCITTFax/JBIG2) or an inconsistent raster. +std::optional encode_image( + std::string raw, const Object &filter, const Object &decode_parms, + std::int32_t width, std::int32_t height, std::int32_t bits_per_component, + const ColorSpaceDef *color_space, const std::vector &decode, + const std::vector &alpha = {}, + const std::vector &color_key = {}, std::int32_t smask_in_data = 0); /// Assemble decoded image samples (ISO 32000-1 8.9.5: MSB-first, rows padded /// to a byte boundary, `bits_per_component` of 1/2/4/8/16) into an 8-bit PNG, diff --git a/test/data.cmake b/test/data.cmake index 332c2dc0..63114a56 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "a77c80e08f8104b6110d9a47b092f1720a12b6f3") + REVISION "b8d4a6de30ba901dc120573e7ceb9ac4ecef71b7") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "2947be444b5f860442e163d2d94ba80ce54020df") + REVISION "6d22a2435767a97f51fab53b2a6ac9f11ebd8198")