fix(pdf): flow matrix runs into one selection block - #721
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 572f986e67
ℹ️ 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".
andiwand
force-pushed
the
fix/pdf-matrix-run-spacing
branch
from
August 20, 2026 18:45
21c2771 to
8ab31a4
Compare
andiwand
force-pushed
the
fix/pdf-matrix-selection-flow
branch
from
August 20, 2026 19:15
572f986 to
0e953f5
Compare
The selection layer measured every run in page space, so two runs a general transform placed could not be compared and each got a line block of its own — one per glyph on a page laid out glyph by glyph. Those blocks carry no PDF-derived width either, so each shrink-wrapped the half-size fallback font and the marking sat beside the glyphs rather than over them. Runs one CSS matrix can place now flow inside a single block, measured in that block's own frame (origins resolved along its axes, the unit its font size), so the existing gap and line tests apply there unchanged and the widths follow. The advance of a whitespace-only run went missing along the way: it emits no `.sr` to carry it, and the spacer span was sized by the gap before it alone. One `Tj` per line hid that; word by word it cost a space per word. Only the selection layer and the width-class table move — no glyph, path or `@font-face` line differs in the reference output.
andiwand
force-pushed
the
fix/pdf-matrix-selection-flow
branch
from
August 20, 2026 19:23
e7b7577 to
34e70bb
Compare
…he block Two holes in the selection flow. A whitespace-only run emits no `.sr`, and the spacer that should carry its advance was skipped wherever a space was already there — a line opening with one, or two in a row. The advance is now owed to whatever comes next, reaching it as the spacer's width or as a `margin-left` on the run itself. A block anchors its runs to one baseline, so a run a rise lifts off it — a superscript — took the block's baseline instead of its own once the frame let it flow. It gets a block of its own again, without the separator a real line break would bring.
…dropped `read_operator_name` stopped at four of the nine delimiters of 7.2.2. `(` was not among them, so `Tm(text)Tj` let the name run on and swallow the string, and the show was dropped as an unknown operator. The whole set now ends a bareword; a stray closing one, which opens no token for any reader above to consume, is eaten so the caller's loop still makes progress. `font_is_usable` swallowed the re-encode failure with a bare `catch (...)`, though failing there swaps in a substitute and the page shows it. It now warns through the `Logger`. No corpus font takes that path, so no reference output moves.
andiwand
force-pushed
the
fix/pdf-matrix-selection-flow
branch
from
August 20, 2026 19:35
67962af to
a47cede
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Stacked on #720 — review that one first; this PR's diff is against it.
What was wrong
Marking text on a page laid out glyph by glyph highlighted a trail of narrow
boxes beside the words instead of the words:
Two things caused it.
1. Every matrix run got its own block. The selection layer compares runs by
ox/baseline, which live in page space, so once a general transform is inplay two runs of the same line are not comparable and each opened a block of
its own. Blocks are absolutely positioned, so the glyphs still landed right —
but the selection became one box per glyph.
2. Those blocks carry no width.
width_pt > 0 && !is_matrixskipped thewclass on the matrix path, so each box shrink-wrapped its content. Theselection layer renders in
sfatsize-adjust:50%(deliberately narrow, thenjustified out to the PDF advance), so with nothing to justify to, every box came
out roughly half the width of the glyphs it covers.
The fix
A run is measured in the frame its line block lays out in: page space for an
axis-aligned block, the block's own space for a matrix one — where the CSS
matrix, not the page, is what relates the runs to each other. Origins resolve
along the block's axes (
local_origin), and the unit of the frame is the run'sfont size, so
starts_new_line, the gap test and the widths all carry overunchanged. A run joins the open block when its linear part matches
(
same_linear); otherwise it opens a new one.break_spacefrom #720 then becomes simply "did the frame carry over", which iswhat that flag was reaching for.
A second bug the flow exposed
With the words merged, every line still came up short by about a space per word.
A whitespace-only run emits no
.sr— itscoreis empty — and the spacer spanbeside it was sized by the gap before it alone, so its own advance was dropped.
One
Tjper line hid this completely (the whole line is one.srand its widthis the whole advance); word by word it accumulated. The spacer now takes it.
Measured on the affected page, selection extent vs glyph extent per line:
Scope of the output change
31 documents, all pdf. Every changed line is a
.sr/.sg/.t … iselection element or a
.wNwidth rule — a scripted classification of all21799 distinct changed line shapes finds zero in the visual layer: no glyph
span, no
<path>, no@font-face, no non-iblock. Display is untouched.Selection blocks on the affected page drop from ~1900 to 33.
Review follow-ups
spacer's width, or as a
margin-lefton the run — so a line opening with aspace, or carrying two in a row, keeps its geometry.
its own again rather than taking the block's baseline, and without the
separator a real line break would bring.
Tests
PdfFile.matrix_runs_flow_into_one_selection_block: glyphs shown oneTjat atime under an anisotropic
Tmland in a single selection block, both wordsand the space between them carrying a PDF-derived width. It fails on fix(pdf): keep a matrix-placed word from spacing out its glyphs #720
(5 blocks, no merged words, no widths).