fix: emit glyph marker for glyph-index names instead of leaking them as text#302
Open
wittjeff wants to merge 1 commit into
Open
fix: emit glyph marker for glyph-index names instead of leaking them as text#302wittjeff wants to merge 1 commit into
wittjeff wants to merge 1 commit into
Conversation
…as text
Subset generators (FontForge, fontTools, mPDF, ...) name glyphs by bare
index: /gid00043, /g43, /glyph43. When /Encoding/Differences carries such
names and no /ToUnicode CMap covers the code, init_differences() kept the
raw name as reading text, so extraction produces plausible-looking
garbage ('gid00043gid00049...') that downstream quality gates cannot
detect (docling-project#238).
Detect pure glyph-index names in the last-resort fallback and emit a
GLYPH<name:...> marker instead, keeping the unresolved glyph detectable
(and stripped to a space under the default keep_glyphs=false, so no
fabricated text reaches consumers). A valid /ToUnicode CMap stays
authoritative ahead of the guard, and meaningful unknown names (custom
ligatures like /Th) keep the existing name-as-text fallback.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jeff Witt <152964771+witt3rd@users.noreply.github.com>
Contributor
|
✅ DCO Check Passed Thanks @wittjeff, all your commits are properly signed off. 🎉 |
Contributor
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
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.
Addresses the glyph-name half of #238.
Problem
Subset generators (FontForge, fontTools, mPDF, ...) name glyphs by bare index:
/gid00043,/g43,/glyph43. Such a name identifies the glyph inside the embedded font program but carries no reading text. When/Encoding/Differencesassigns these names and no/ToUnicodeCMap covers the code,init_differences()'s last-resort fallback kept the raw name as reading text:Because that is plausible-looking text rather than a
GLYPH<...>marker, downstream consumers cannot detect the corruption — docling'srate_text_quality()gate never fires and no OCR fallback triggers. This is the same fabricated-text class as #299, reached through the glyph-name path instead of the encoding path.(Note: the with-ToUnicode variant of #238 is already fixed on
main—init_differences()consults the CMap first per PDF 32000-1 §9.10.2. Released 5.11.0 still emits/gidNNNNNeven when a valid ToUnicode exists. And the file attached to #238 does not reproduce for me on 5.11.0 ormain— details in the issue thread.)Fix
The last-resort fallback (
resolve_unknown_name) now detects pure glyph-index names —(gid|glyph|g|cid|index)\d+, case-insensitive, full-match — and emits aGLYPH<name:gid00043>marker instead of the name. Under the defaultkeep_glyphs=falsethe marker is stripped to a space, so no fabricated text reaches consumers; withkeep_glyphs=truethe glyph identity is preserved for diagnostics.Deliberately unchanged:
/ToUnicodeCMap stays authoritative ahead of the guard (method 1 of §9.10.2);/Th,/ft) keep the existing name-as-text fallback;uniXXXX, ligature join/decompose,/C<digits>) all run before the guard.Recovering the actual text for these fonts would require reverse-looking-up the embedded font program's cmap table (gid → Unicode) — worth discussing as a follow-up; this PR makes the corruption detectable so OCR fallback can kick in meanwhile.
Tests
tests/test_gid_glyph_names.pybuilds the PDFs in memory (same pattern as #299's tests): gid-named Differences with and without ToUnicode, name variants (g43,glyph49,index41), and a regression guard for ligature-style names. Full suite passes (88 tests).