Make embedded TrueType text searchable and copyable (#15) - #30
Conversation
Generates a /ToUnicode CMap for every embedded TrueType font and attaches it to the font dictionary, so text set with prTTFont can be extracted, searched and copied. Ref #15 ## Reasoning RT #123564 asked for this in 2017 and proposed a one-line change: pass ToUnicode => 1 to Text::PDF::TTFont0->new(). That was never actionable. The CMap generator behind that option is broken (RT #123562, reported the same week, still unfixed), so enabling it emits malformed data -- worse than no mapping, because a reader that trusts a broken CMap extracts wrong text rather than none. Reviewing the upstream patch for that ticket turned up two further defects of its own: code points above U+FFFF written as bare values instead of UTF-16BE surrogate pairs, and a "currendict" typo in the trailer. So the mapping is built here instead of delegated. That is possible without touching Text::PDF at all, because two things already hold: Text::PDF::Dict::outobjdeep serialises dictionary keys generically, so an externally attached /ToUnicode key is written out; and PDF::Reuse::DocProxy owns object registration, so a new stream object can be created and numbered through the existing API. Considered instead: waiting for upstream. Text::PDF last saw a release in 2016 and the relevant PR has been open since 2016; that is not a dependency this can be scheduled against. Considered also: overriding Text::PDF::Dict::$cr or monkey-patching outobjdeep to fix the upstream generator in place. Rejected -- the $cr route is defeated by the /o flag caching the compiled pattern on first use, making it silently order dependent, and patching outobjdeep means carrying a copy of a 100-line method in perpetuity. Attachment happens in prEnd() before write_objects, which is the first point at which the subset vector is complete. Mapping is therefore restricted to the glyphs the document actually uses -- 13 entries for a short line of text rather than the whole font -- and the CMap is emitted only for TrueType fonts, leaving built-in font output byte-identical. No option is provided to turn this off. It is small, it is the thing that makes the text usable, and a switch to produce deliberately unextractable text is not worth the surface. ## Verification Text extraction confirmed with two independent readers. Before, pdftotext returned "+HOOR7R8QLFRGH:RUOG" and mutool returned replacement characters; after, both return "Hello ToUnicode World". A 158-glyph document round-trips exactly, including accented latin-1 characters, and exercises the 100-entry section limit (100 + 58, both sections declaring their true counts). 33 tests pass, up from 29. The four new assertions fail against the unmodified module (tests 16-18) and pass with the change. Built-in fonts still emit no /ToUnicode, and three consecutive prTTFont sessions in one process still extract correctly, so the object lifecycle fixed in #24 and #26 is unaffected. Load-bearing: yes. Adds an object to every document using prTTFont and changes the byte content of the font dictionary.
Coverage Report for CI Build 30410486978Warning No base build found for commit Coverage: 34.09%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
The #15 regression tests hard-coded three Linux font paths, so on CI they ran only on ubuntu and skipped silently on macOS and windows-latest. A green matrix that skipped two thirds of its platforms proves less than it appears to, and the CMap code is exactly the kind of byte-level output worth checking per platform. Globs the usual font directories for Linux, macOS (/Library, /System/Library, including Supplemental) and Windows (%SystemRoot%\\Fonts and C:/Windows/Fonts) and takes the first readable TTF, rather than naming specific files that only exist on Debian derivatives. Considered bundling a font in the distribution instead, which would make the test deterministic everywhere. Rejected for now -- the smallest suitable licence-clean face here is ~750KB, which is a large addition to a CPAN dist to cover one test. Worth revisiting if a small OFL font turns out to be practical. Adds a diag() naming the font actually used, so a future failure report says which face produced it.
|
CI: 14/14 green — run 30408969440. Pushed Now globs the platform font directories instead. Confirmed executing everywhere:
Three different faces across three platforms all produce a well-formed CMap with self-consistent section counts, which is better evidence than one font on one OS would have been. Considered bundling a font in the distribution for determinism, but the smallest licence-clean face available is ~750KB — too much to add to a CPAN dist for one test. Noted in the commit as worth revisiting if a small OFL font turns out practical. Added a |
![vsits-codex-review-agent[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
![vsits-codex-review-agent[bot]](https://avatars.githubusercontent.com/u/115404?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: PR #30 TrueType ToUnicode CMap
Date: 2026-07-29
Reviewed: PR #30 as a whole at 3ae8cdf2ebfb8ab64107eb5309ce0774f960e6ea, with focus on lib/PDF/Reuse.pm and t/regression.t
Round: 1
Label applied: approved-by-codex-agent
What Is Correct
The prEnd() placement is correct. prEnd() first flushes any pending content
stream via skrivSida(), writes the page tree via skrivUtNoder(), then runs
PDF::Reuse::TTFont::attach_tounicode($docProxy) before
$docProxy->write_objects. That is the first point where all calls to
Text::PDF::TTFont0::out_text() have marked the subset vector for placed text,
and it is still before Text::PDF::TTFont0::outobjdeep() freezes and writes the
font objects. I found no other call path that writes DocProxy objects outside
this block.
The idempotency guard is sufficient for the reachable lifecycle. A normal
prEnd() writes the DocProxy objects, releases the Text::PDF::TTFont0
instances, and undefines $docProxy, so a second prEnd() on the same completed
document cannot reattach anything. If attach_tounicode() were called twice
before writing, next if $ttfont->{'ToUnicode'} prevents allocating duplicate
CMap objects for already-attached fonts.
The mapping source matches the way this wrapper emits text. out_text() uses
$font->{'cmap'}->ms_lookup($unicode) to produce two-byte glyph IDs in the
content stream; Font::TTF::Cmap::reverse() reverses the same preferred
Microsoft Unicode cmap selected by find_ms(). The resulting $rev[$gid]
index is therefore the correct key for a Type0 /Identity-H font whose content
bytes are original glyph IDs. The implementation also correctly filters on the
completed ' subvec' so the ToUnicode map covers used text glyphs rather than
the whole font.
The generated CMap shape is appropriate for a ToUnicode CMap. /CIDSystemInfo
uses the conventional Adobe/UCS identity for Unicode mapping, /CMapType 2 is
present, the codespace range covers the two-byte codes emitted by this module,
and the trailer uses currentdict. The beginbfchar chunking arithmetic is
right: full 100-entry sections declare 100, and the final partial section
declares $total % 100 when non-zero. The local sample emitted 13 entries with
a matching section count, and the PR author's 158-glyph verification exercises
the 100 + 58 case.
Leaving the CMap stream uncompressed is acceptable. PDF stream compression is
not required for validity, and keeping this small diagnostic stream readable is
a reasonable tradeoff. The existing Text::PDF::Dict::outobjdeep() path writes
the correct /Length for an unfiltered stream.
The tests are focused on the regression surface: TrueType output now contains a
/ToUnicode reference, a beginbfchar mapping, the corrected currentdict
trailer, and internally consistent section counts. The second commit fixes a
real CI blind spot by making those assertions execute on macOS and Windows when
a readable TTF is present, while keeping the test skippable on fontless systems.
Blockers
None.
What Needs Attention
The eval { $cmap->read->reverse } boundary deliberately degrades to "no
ToUnicode" if a font lacks a usable Unicode cmap or the cmap reader fails. That
matches the existing best-effort nature of optional font extraction support, but
it also means a malformed font can silently lose search/copy support rather
than failing document generation. I do not consider that blocking because
prTTFont() can already operate with externally supplied font files and the
old behavior was no ToUnicode at all.
The regression test proves CMap presence and section consistency, not actual
text extraction. The PR body and comment provide independent pdftotext and
mutool evidence, and I reproduced that locally, so this is acceptable for this
round. A future test that uses an installed extractor when available would make
the behavior check stronger, but it should remain optional to avoid adding a new
hard runtime dependency.
Font::TTF::Cmap::reverse() returns one Unicode code point per glyph unless
called with array => 1. That is consistent with the text path here, which does
not perform shaping or ligature substitution, but it means glyphs that are only
representable as multi-code-point Unicode sequences are outside this change's
scope.
Bloat / Non-Functional
None. The implementation is a narrowly scoped helper in the existing
PDF::Reuse::TTFont wrapper, adds one object only for embedded TrueType fonts,
and avoids carrying a monkey-patched copy of upstream serialization code. The
new uncompressed CMap is small and proportional to the used subset.
Recommendations
Keep the attachment in prEnd() immediately before write_objects; moving it
earlier risks an incomplete subset vector, and moving it later misses object
serialization.
Keep the isa guard defensive. DocProxy can cache multiple Text::PDF object
types, and this helper should only alter Text::PDF::TTFont0 dictionaries.
Consider, as a future non-blocking hardening step, validating utf16be_hex()
against Unicode scalar bounds if a real-world font ever exposes cmap values
outside 0 .. 0x10FFFF or surrogate code points. I found no evidence that the
selected Microsoft Unicode cmap path returns anything other than numeric code
point values for normal fonts.
Bottom Line
Approve. The change is load-bearing, but the object lifecycle, glyph-to-Unicode
mapping, CMap syntax, idempotency guard, and regression coverage all line up
with the existing prTTFont architecture. I found no blocking defects.
vsits-codex-review-agent
Bot
added
the
approved-by-codex-agent
2 participants
Generates a
/ToUnicodeCMap for every embedded TrueType font, so text set withprTTFontcan be extracted, searched and copied.Ref #15
The problem
RT #123564 asked for this in 2017 with a one-line suggestion: pass
ToUnicode => 1toText::PDF::TTFont0->new().That was never actionable. The CMap generator behind that option is broken — RT #123562, reported the same week, still unfixed — so enabling it emits malformed data. That is worse than no mapping: a reader trusting a broken CMap extracts wrong text rather than none.
Reviewing the upstream patch for that ticket turned up two further defects in the patch itself: code points above U+FFFF written as bare values instead of UTF-16BE surrogate pairs, and a
currendicttypo in the trailer. Both are fixed in silnrsi/text-pdf#3, but that PR is at a maintainer who has not responded since 2023 (the companion PR for #23 has been open since 2016).The approach
Build the mapping here rather than delegate it. This requires no changes to Text::PDF at all, because two things already hold:
Text::PDF::Dict::outobjdeepserialises dictionary keys generically, so an externally attached/ToUnicodekey is written out.PDF::Reuse::DocProxy— our own code — owns object registration, so a new stream object can be created and numbered through the existing API.Attachment happens in
prEnd()beforewrite_objects, the first point at which the subset vector is complete. Mapping is therefore restricted to glyphs the document actually uses — 13 entries for a short line, not the whole font.Alternatives considered
Text::PDF::Dict::$cr/oflag caching the compiled pattern on first use — silently order-dependent, and poisoned by any module that touches Text::PDF first.outobjdeepNo option to disable it. It is small, it is what makes the text usable, and a switch to produce deliberately unextractable text is not worth the surface.
Verification
Text extraction, two independent readers:
pdftotextmutool+HOOR7R8QLFRGH:RUOG���������Hello ToUnicode WorldHello ToUnicode World100 + 58, both sections declaring their true counts. (The upstream patch's chunking counter was the part most prone to an off-by-one — a miscount yields a CMap some readers silently reject.)/ToUnicode— verified, so non-TTF output is unchanged.prTTFontsessions in one process still extract correctly, so the object lifecycle fixed in prTTFont leaves orphaned $docProxy causing prEnd() crash #24 and Fix stale %processed cache when a filename is reused (#21, #22) #26 is unaffected.Note
lib/PDF/Reuse.pmis latin-1 (see #27) — these changes were applied byte-wise, and the diff touches zero non-ASCII lines.Load-bearing: yes. Adds an object to every document using
prTTFontand changes the byte content of the font dictionary. Worth reviewer attention: the CMap is built fromFont::TTF's reverse cmap, and the subset-vector restriction means the mapping depends onprEnd()running after all text is placed.