Skip to content

fix(pdf): place text by its own font, decode jpeg 2000, and clip to the crop box - #689

Merged
andiwand merged 3 commits into
mainfrom
fix/pdf-text-baseline-cmyk-jpx
Aug 14, 2026
Merged

fix(pdf): place text by its own font, decode jpeg 2000, and clip to the crop box#689
andiwand merged 3 commits into
mainfrom
fix/pdf-text-baseline-cmyk-jpx

Conversation

@andiwand

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

Stacked on #688 — the four new pdfs it adds are what showed all of this, compared page by page against a native viewer.

Text sat too low. A line block inherits the browser's default font size, so its strut — not the run it holds — set the line box's baseline. Every run landed 13px − ascent × font-size below where its top was derived for: ~1.7pt for a 9.5pt heading, ~4pt for 6pt body text, which is why a heading looked off-centre in its banner. The block's strut is collapsed (font-size:0) and an embedded @font-face now states the ascent and descent the placement assumes, as a substituted face already did. Measured against the same page rendered by CoreGraphics: heading centre 70.3% → 58.7% of the banner (viewer: 59.4%); body lines 26% of a banner height off → ~3%.

JPEG 2000 images were missing — all 12 image uses in that manual are JPXDecode, which nothing here decoded. New dependency openjpeg/2.5.4 (no transitive requirements; one line in conan.lock, no other pin moved) and pdf_jpx.{hpp,cpp}: a memory-stream decode to 8-bit samples, subsampled components resampled, a cdef alpha kept as the coverage plane, sYCC converted. The raster then takes the same PNG path as every other codec. CCITT and JBIG2 stay undecodable.

CMYK was the naive (1-c)(1-k), in three separate copies. One pdf::cmyk_to_rgb now carries pdf.js's polynomial fit of Adobe's transform: pure cyan #00ffff#00b9f2 (viewer: #00a3da), and cmyk black is the dark neutral Acrobat and Preview paint rather than #000.

Content outside the crop box was drawn. Laserentfernungsmesser_PLEM50_D5 places its cover title at -170.08pt, a page-width to the left of a 170.08pt-wide crop box; a viewer clips it, we drew it onto the backdrop next to the page. The page box now hides what overflows it.

Test data: the reference output is regenerated on main of both output repos and test/data.cmake moves to those pins. Only pdf output changed — 4 public and 28 private documents, nothing else drifted. Suite is 876 passing, with new tests for the colour transform and the jpx reject paths.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f885b47b15

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/odr/internal/html/pdf_file.cpp
Comment thread src/odr/internal/pdf/pdf_jpx.cpp
Comment thread src/odr/internal/pdf/pdf_jpx.cpp
Base automatically changed from fix/pdf-content-stream-comments-and-utf16 to main August 14, 2026 18:39
andiwand and others added 3 commits August 14, 2026 20:43
…rut 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8PCMZUVxuGstmmgQzEdif
…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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8PCMZUVxuGstmmgQzEdif
… 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8PCMZUVxuGstmmgQzEdif
@andiwand
andiwand force-pushed the fix/pdf-text-baseline-cmyk-jpx branch from 2aac9d2 to fab329c Compare August 14, 2026 18:43
@andiwand
andiwand merged commit 5a07030 into main Aug 14, 2026
27 checks passed
@andiwand
andiwand deleted the fix/pdf-text-baseline-cmyk-jpx branch August 14, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant