Handle glyphs with no Unicode mapping: glyph-index recovery + OCR sidecar - #234
Open
lfoppiano wants to merge 6 commits into
Open
Handle glyphs with no Unicode mapping: glyph-index recovery + OCR sidecar#234lfoppiano wants to merge 6 commits into
lfoppiano wants to merge 6 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR re-enables OCR placeholder handling so pdfalto can emit a JSON sidecar describing glyphs without usable Unicode mappings for external OCR workflows.
Changes:
- Reintroduces the
-ocrCLI flag and invokes sidecar generation after page rendering. - Records placeholder glyph occurrences grouped by font and character code.
- Documents the external OCR sidecar workflow in the README.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/XmlAltoOutputDev.h |
Adds OCR glyph sidecar data structures and public writer API. |
src/XmlAltoOutputDev.cc |
Records non-Unicode glyph metadata and writes ocr-regions.json. |
src/pdfalto.cc |
Re-enables -ocr and writes the sidecar after rendering. |
Readme.md |
Documents the -ocr option and sidecar-based OCR workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+103
to
+104
| # apply corrections back: replace placeholder codepoints in out.xml | ||
| # with the recovered Unicode, matched by (page, bbox) or (placeholder) |
lfoppiano
force-pushed
the
bugfix/improve-scaling-font-dedup
branch
from
May 30, 2026 16:15
1e7f2e3 to
e6a681a
Compare
lfoppiano
force-pushed
the
bugfix/improve-scaling-font-dedup
branch
from
June 28, 2026 08:24
e6a681a to
868f7b4
Compare
Signed-off-by: Luca Foppiano <luca@foppiano.org>
Points the submodule at the MSTT/Type3 glyph-index -> Unicode recovery work (xpdf-4.05 bugfix/work-on-glyphs) that this OCR-sidecar feature builds on, so a clean clone with --recurse-submodules gets Tier 1.
…nique placeholders The -ocr sidecar predates the glyph-index recovery (Tier 1). It disabled MapNumericCharNames, which is the gate for resolving MSTT 'GNN'/'gNNN'/'cidNNN' glyph-index names to real Unicode -- so enabling -ocr defeated recovery and replaced decodable body text with placeholders (e.g. degruyter: 18034 -> 7893 Strings, 'Received' lost, ~100k glyphs placeholdered). Keep MapNumericCharNames on under -ocr so the sidecar captures only the residual glyphs recovery cannot map. Also replace the 12-entry geometric-shapes placeholder table -- which collapsed every distinct glyph past the 12th onto U+25FA -- with sequential Private Use Area allocation (U+E000..U+F8FF), so each distinct (fontName,charCode) gets a unique, collision-free placeholder (jstage: 1095 glyphs -> 1095 placeholders). Remove leftover debug printf and dead SplashCoord mat[] block; reconcile the Readme placeholder-range description (geometric shapes -> PUA).
lfoppiano
force-pushed
the
feature/handle-glyps
branch
from
June 30, 2026 07:33
a180310 to
9eeade0
Compare
Addresses the Copilot review on PR #234. The sidecar bbox was recomputed in drawChar with a rotation-0 formula, raw getFontSize() and raw GfxFont ascent/descent, so it did not match TextRawWord::addChar (rotation-aware, transformed font size, clamped TextFontInfo metrics). For Type 3 / rotated / CTM-scaled text the boxes were wrong -- e.g. jstage glyphs came out ~1pt tall, useless for cropping. Record the sidecar glyph from the actual TextChar that addChar creates (TextPage::getLastAddedChar) so the box is identical to the one ALTO uses; skip glyphs addChar filtered out (out-of-bounds/tiny/space). jstage boxes go from ~1pt to correct ~10pt height; all 1095 corrected, none invalid. Also document (fontName,charCode) as the canonical correction key (robust past the 6400-placeholder saturation point), per the same review.
The non-ocr path in drawChar() only tested sequences longer than one character, so a single invalid character was emitted verbatim and could make the ALTO output non-well-formed. Ported from the stale branch bugfix/utf8-invalid-characters_placeholder; that branch's -ocr -> -placeholder rename is obsolete here, since -ocr now means "emit an OCR sidecar" rather than "substitute placeholders". isUTF8() encoded the sequence through the UTF-8 UnicodeMap and then re-parsed the resulting bytes. That round trip was redundant: mapUTF8() encodes each codepoint independently and well-formed by construction, so the only input it can render invalid is an unpaired surrogate, and codepoints above U+10FFFF encode to zero bytes and so were, and still are, reported as valid. Replaced with the equivalent integer test, verified against the old scan over every codepoint U+0000..U+10FFFF and over multi-character sequences. This also drops a GString that leaked on all nine of the old function's return paths, which matters now that the function runs once per character rather than only for uLen > 1. Verified on a 14-PDF corpus: the isUTF8 rewrite alone is byte-identical to the previous output, and neither strip path fires on any of those documents, so the guard fix is latent until a document carries a lone surrogate.
xpdf resolves glyph names through the nameToUnicode tables plus a fallback for 'Axx'/'xx' hex-style names; it does not implement the AGL rule of stripping the suffix after a period. Names like "a.sc" or "three.taboldstyle" therefore resolve to nothing and the glyph is dropped from the output entirely -- not substituted, simply lost. Ported from the stale branch fix-non-standard-font-names, with two changes: - sc.nameToUnicode was shipped there but never referenced from xpdfrc, so it would have been dead weight; both it and others.nameToUnicode are now registered. - sc.nameToUnicode was missing k.sc (it ran a-j then l-z); added. taboldstyle.nameToUnicode previously held only eight.taboldstyle; it now covers 0-9. Verified with minimal PDFs that reference these names via an /Encoding /Differences array: a.sc b.sc k.sc z.sc before: (no output) after: a b k z three/seven/zero.taboldstyle before: (no output) after: 3 7 0 lscript partialdiff arrowhookleft before: d after: l d <hook> Note that partialdiff is already in xpdf's built-in table, so that entry is redundant; it is kept for explicitness.
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
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.
Summary
Adds end-to-end handling for PDF glyphs that carry no usable Unicode mapping (MSTT
MSTT31c…subset fonts, Type 3 fonts, symbolic/dingbat fonts with brokenToUnicode). It is a two-tier strategy:xpdf-4.05submodule, see dependency below):resolve glyph-index–named glyphs (
G34,g50,cid12, …) to real Unicode via the standard Macintosh glyph ordering, before xpdf's pass-2 heuristic garbles them (so4GEGKXG→Received).-ocrflag): for the residual glyphs that recovery genuinely cannot map, substitute a placeholder codepoint in the ALTO<String CONTENT>and emit<xml>_data/ocr-regions.jsonwith each glyph's font, char code and per-occurrence page bounding box, for an external glyph-level OCR pass.Depends on
What's in this PR
-ocrOCR sidecar (drawChar/recordNonUnicodeGlyph/writeOcrSidecarinXmlAltoOutputDev): records residual non-Unicode glyphs and writes a hand-rolled, dependency-free JSON sidecar. pdfalto intentionally does not rasterise — the caller picks the render backend, DPI and OCR engine.-ocrno longer defeats glyph recovery. The-ocrflag predated Tier 1 and disabledMapNumericCharNames, which is the exact gate for glyph-index recovery. As a result, enabling-ocrturned decodable body text back into placeholders across whole documents.-ocrnow keeps recovery active and only sidecars what's truly unmappable.U+25FA), making the in-text placeholder→glyph mapping lossy. Replaced with sequential Private Use Area allocation (U+E000…U+F8FF, 6400 slots): every distinct(fontName, charCode)gets a unique, collision-free placeholder that never clashes with real document text.printfand a deadSplashCoord mat[]block; reconciled the Readme placeholder-range description.git clone --recurse-submodulesincludes recovery.Impact (measured on local sample PDFs)
degruyter_1070850498under-ocr<String>, "Received" lost, ~100k glyphs placeholdered<String>, "Received" recovered, 0 residual glyphsjstage_1147923172sidecar (genuine Type3 residuals)Testing
scripts/regression_test.sh: 17/17 PASS — line-number/footnote suite plus the glyph cases: MSTT decode to readable text (Received/Contributions, no4GEGKXG/8QNWOGgarble), Type3 body text extracts (Vulvovaginal/vulvodynia/ISSVD), and a new Tier-2 case asserting the sidecar is valid JSON, lists >12 glyphs, and all placeholders are unique.ocr-regions.jsonalso appear in the ALTO<String CONTENT>(so a corrections step can substitute in place).Notes