Skip to content

Stop letting map order decide what a page looks like - #14

Merged
tannevaled merged 1 commit into
mainfrom
fix/inline-image-expansion-order
Aug 27, 2026
Merged

Stop letting map order decide what a page looks like#14
tannevaled merged 1 commit into
mainfrom
fix/inline-image-expansion-order

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

An inline image dictionary may say the same thing twice and disagree with
itself: /W 20 beside /Width 10, /CS /RGB beside /ColorSpace /3chanRGB, /D
beside /Decode with the components reversed. Expanded() walked the dictionary
in one pass and wrote each key into the output, so when both spellings were
present the winner was whichever Go's map iteration happened to yield last —
and that order is deliberately randomised per range.

The same file therefore drew a different picture on different runs of the same
binary, with nothing to tell a caller which one was meant. Eight renders of
safedocs' Inline_Image_Abbreviations fixture:

before 79b9edef 79b9edef b2594aef 6bf89747 6bf89747 544c8e6b 79b9edef 43030bf3
after 87f31291 × 8

Five different answers out of eight, against one.

The scope is now known rather than guessed. Rendering the first page of 3 668
corpus files three times over — the 1 633 real forms, 1 400 arXiv papers and
all 635 vendor fixtures — turns up exactly one file that disagrees with itself,
and it is that fixture, which safedocs built for this case: its seven images
each pair an abbreviation against the written-out name, with comments marking
the line to remove "to see the effect". So the corpus measurements already
taken are not contaminated; one file is, and it is the one designed to be.

WHICH SPELLING WINS

The specification permits both in an inline image and says nothing about which
takes precedence, and the public implementations disagree: pdf.js asks for the
abbreviation and falls back to the written-out name, MuPDF asks the other way
round. The abbreviation wins here, for two reasons — it is the spelling Table
93 gives for an inline image, the written-out name being the tolerated alias;
and safedocs marks the abbreviation as the line whose removal changes what a
viewer shows, which is a statement about the reference viewer's behaviour.

Two passes settle it: the written-out names first, then the abbreviations over
them. No order within either pass can matter, because the names in each are
distinct — so the answer no longer depends on the run whichever way it is read.

The test expands the same dictionary two hundred times and requires every
answer to match the first. Against the parent commit it fails on the first
comparison, reporting Height = 40 where the first gave 10.

An inline image dictionary may say the same thing twice and disagree with
itself: /W 20 beside /Width 10, /CS /RGB beside /ColorSpace /3chanRGB, /D
beside /Decode with the components reversed. Expanded() walked the dictionary
in one pass and wrote each key into the output, so when both spellings were
present the winner was whichever Go's map iteration happened to yield last —
and that order is deliberately randomised per range.

The same file therefore drew a different picture on different runs of the same
binary, with nothing to tell a caller which one was meant. Eight renders of
safedocs' Inline_Image_Abbreviations fixture:

  before   79b9edef 79b9edef b2594aef 6bf89747 6bf89747 544c8e6b 79b9edef 43030bf3
  after    87f31291 × 8

Five different answers out of eight, against one.

The scope is now known rather than guessed. Rendering the first page of 3 668
corpus files three times over — the 1 633 real forms, 1 400 arXiv papers and
all 635 vendor fixtures — turns up exactly one file that disagrees with itself,
and it is that fixture, which safedocs built for this case: its seven images
each pair an abbreviation against the written-out name, with comments marking
the line to remove "to see the effect". So the corpus measurements already
taken are not contaminated; one file is, and it is the one designed to be.

WHICH SPELLING WINS

The specification permits both in an inline image and says nothing about which
takes precedence, and the public implementations disagree: pdf.js asks for the
abbreviation and falls back to the written-out name, MuPDF asks the other way
round. The abbreviation wins here, for two reasons — it is the spelling Table
93 gives for an inline image, the written-out name being the tolerated alias;
and safedocs marks the abbreviation as the line whose removal changes what a
viewer shows, which is a statement about the reference viewer's behaviour.

Two passes settle it: the written-out names first, then the abbreviations over
them. No order within either pass can matter, because the names in each are
distinct — so the answer no longer depends on the run whichever way it is read.

The test expands the same dictionary two hundred times and requires every
answer to match the first. Against the parent commit it fails on the first
comparison, reporting Height = 40 where the first gave 10.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tannevaled
tannevaled merged commit 62002d5 into main Aug 27, 2026
1 check passed
tannevaled added a commit to go-pdfkit/render that referenced this pull request Aug 27, 2026
This module required reader v0.4.0 and pdffont v0.2.0. Both had already been
fixed and released; nothing had brought the fixes here, so a caller who asked
only for this package got the defects — minimum version selection gives them
the version this go.mod names, and a package's own tests never see its callers'
module graph. Merging is not shipping.

  reader v0.4.1   a 219-byte file with no trailer and no startxref, whose last
                  object is numbered 2 147 483 647, took 21.2 s to open and
                  allocated not one byte — so no memory limit caught it.
                  Now under a millisecond.

  pdffont v0.3.0  a /ToUnicode map's size had nothing to do with its input:
                  10 655 bytes produced 13 107 200 entries, 1 054 MB and 10 s,
                  per font per page. Now 52 ms and 31 MB. Also the glyph names
                  of eight more languages, which had been read as nothing at
                  all — "Příliš" came back "Píliš" and still looked like a word.

deps_test.go guards the first of those with the file itself rather than with a
version string, so a later go.mod edit cannot quietly reintroduce it. It is
built in the test, not committed, so nobody else's PDF enters the repository.
Against reader v0.4.0 it fails after five seconds.

WHAT IT CHANGES ON THE CORPUS

Two binaries, identical but for pdffont, verified with `go version -m` because
this measurement has silently compared a version against itself twice on this
project. 8 935 real files — 2 268 forms and 6 667 arXiv papers — first page
each, 20-second budget:

  identical  8 929
  differing      6

Of the six, three are pages sitting at the budget: with a MaxDuration set, a
page near the limit stops at a different operation each run, so its output is
timing-dependent and nothing can be concluded from it. One is safedocs'
Inline_Image_Abbreviations fixture, which disagrees with itself between runs of
the same binary for an unrelated reason (go-pdfkit/reader#14).

That leaves two genuine changes, both in the vendor half of the forms corpus:

  gh-pdfbox/source.pdf                     +142 inked pixels
  gh-pdfbox/sample_fonts_solidconvertor    -58 inked pixels, all inside one
                                           152x212 region, 1 003 pixels changed

The second is the interesting one and it is not fully explained: the file has
no /Differences and no uniXXXX names anywhere, decompressed streams included,
so the difference cannot come from a document-supplied glyph name. It arrives
through the base-encoding table, where the added letters include the ones
WinAnsiEncoding already names — scaron, zcaron, oe. Fewer inked pixels is
consistent with a real narrow glyph replacing a wider wrong one, but that is
inference and not measurement, so it is written down as unexplained rather than
as an improvement.

No file got worse in a way that can be shown. Wall clock is not compared: four
other jobs had the machine while this ran, and a number taken under that load
would be fiction.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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