Skip to content

fix: emit glyph marker for symbolic/composite fonts with no usable cmap - #322

Closed
wittjeff wants to merge 3 commits into
docling-project:mainfrom
wittjeff:fix/symbolic-font-no-cmap-glyph-marker
Closed

wittjeff wants to merge 3 commits into
docling-project:mainfrom
wittjeff:fix/symbolic-font-no-cmap-glyph-marker

Conversation

@wittjeff

Copy link
Copy Markdown
Contributor

Follow-up to #299 (thanks @yalsaffar for the report).

The guard added in #299 only fired when a /ToUnicode cmap parsed (cmap_initialized == true) and merely missed the code. A symbolic or Type0/CID font whose /ToUnicode fails to parse (could not find cmap in '/ToUnicode'), or that carries none, sets cmap_initialized = false, skips the guard, and falls through to the Standard/WinAnsi tables — which fabricate unrelated text again (e.g. !"#).

This drops the cmap_initialized requirement for symbolic/composite fonts: their codes bear no relation to the standard encodings, so an unresolved code must surface as a GLYPH<> marker regardless of whether a cmap was parsed. Non-symbolic fonts are unaffected (they legitimately use the declared encoding).

Adds two tests: a symbolic font with no usable /ToUnicode (must emit markers, not fabricate) and a non-symbolic control (encoding preserved). Reproduced against the current build; the change is a one-line guard widening.

@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @wittjeff, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@PeterStaar-IBM
PeterStaar-IBM self-requested a review August 16, 2026 05:59
@PeterStaar-IBM

Copy link
Copy Markdown
Member

@wittjeff it seems the regression tests are failing. Any idea why?

@wittjeff

Copy link
Copy Markdown
Contributor Author

Yes — my fault, and the CI diff pinpointed it. The widening was on the wrong axis; fixed and re-pushed.

What broke. The 29 failing pages all follow one pattern: real ASCII text turned into markers (Specify → GLYPH<83>GLYPH<112>…, Convert, 2.4.). I pulled the font dictionaries of every affected document (font_01.pdf, deep-mediabox-inheritance.pdf, the five dln_* DocLayNet docs): every broken font is symbolic-flagged but declares an encoding — either /Encoding /WinAnsiEncoding outright or an /Encoding dict with /Differences. For those fonts, codes outside /Differences legitimately resolve through the declared base encoding (9.6.6); producers set the symbolic flag spuriously on such fonts all the time. By dropping the cmap_initialized requirement without checking for a declared encoding, the guard started eating the legitimate fallback.

The case #322 is actually about is narrower. @yalsaffar's logs show both warnings together:

could not find cmap in '/ToUnicode'
font-encoding not defined, falling back to STANDARD

i.e. a symbolic font with neither a usable cmap nor any declared /Encoding. With no statement of intent from the producer at all, falling through to the standard tables is fabrication — that's the only case the guard should now catch beyond the original #299 one.

The fix (one condition, pushed as a follow-up commit):

const bool has_declared_simple_encoding =
  (has_explicit_encoding and encoding != CMAP_RESOURCES) or diff_initialized;

if(subtype==TYPE_0 or
   (is_symbolic and (cmap_initialized or not has_declared_simple_encoding)))

Case table:

font cmap declared encoding result
#3802 (في → #) parsed, misses code any marker (unchanged from #299)
#322 report failed to parse / absent none marker (the new case)
font_01.pdf, dln_*, deep-mediabox none WinAnsi or Differences declared encoding honored (regression fixed)
Type0/CID — — marker (unchanged)

I also fixed the synthetic test to match the reported shape: the no-cmap symbolic test now also omits /Encoding (and uses a subset-style BaseFont name so the core-14 alias table stays out of the way), and there's a new control asserting that a symbolic-flagged font with declared WinAnsi keeps its text — which is exactly the shape that broke CI.

Verified locally against the full regression suite: test_parse.py, test_threaded_parse.py, test_threaded_render.py all green, plus the 7 unit tests in test_tounicode_fallback.py.

One residual gap, deliberately not touched here: a symbolic no-encoding font whose name matches a core-14 alias (e.g. an ABCDEF+Arial subset) resolves through the base-font AFM table before reaching this guard, so it can still fabricate. That path predates #299 and behaves identically before/after this PR; happy to look at it separately if it shows up in the wild.

Comment thread tests/test_unit_tounicode_fallback.py Resolved
@PeterStaar-IBM

Copy link
Copy Markdown
Member

@wittjeff can you resolve the mergre conflicts, I think this should be an easy one to merge then

MelvinAkothINGUBU and others added 2 commits September 10, 2026 13:34
Follow-up to docling-project#299. The guard added there only fired when a cmap parsed
(cmap_initialized == true) and merely missed the code. A symbolic or Type0/CID
font whose /ToUnicode fails to parse ("could not find cmap in '/ToUnicode'"), or
that carries none, sets cmap_initialized = false, skips the guard, and falls
through to the Standard/WinAnsi tables — which fabricate unrelated text again
(e.g. '!"#'). Drop the cmap_initialized requirement for symbolic/composite
fonts: their codes bear no relation to the standard encodings, so an unresolved
code must surface as a GLYPH marker regardless of whether a cmap was parsed.

Non-symbolic fonts are unaffected (they legitimately use the declared encoding).
Adds a test for a symbolic font with no /ToUnicode (marker) plus a non-symbolic
control (encoding preserved).

Reported by @yalsaffar in docling-project#299.

Signed-off-by: Jeff Witt <152964771+witt3rd@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ble cmap

The previous commit widened the glyph-marker guard to every symbolic font
whose cmap did not resolve a code, which broke 29 regression pages across
7 documents (font_01.pdf, deep-mediabox-inheritance.pdf, five dln_* docs):
their fonts carry a spurious symbolic flag but explicitly declare
/WinAnsiEncoding or an /Encoding dict with /Differences, and codes outside
/Differences legitimately resolve through the declared base encoding
(PDF 32000-1 9.6.6).

Only emit the marker for a simple symbolic font when neither a parsed
cmap (docling-project#299 case) nor any declared simple encoding exists — the shape in
the original report, whose logs show both 'could not find cmap in
/ToUnicode' and 'font-encoding not defined'. Type0/CID stays
unconditional.

The synthetic no-cmap test now models that shape faithfully: no /Encoding
entry and a subset-style BaseFont name so the core-14 alias table stays
out of the way. A new control asserts a symbolic-flagged font with
declared WinAnsi keeps its text — the exact shape that failed CI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
@wittjeff
wittjeff force-pushed the fix/symbolic-font-no-cmap-glyph-marker branch from 3e41494 to d3d3dcf Compare September 10, 2026 22:03
@wittjeff

Copy link
Copy Markdown
Contributor Author

Rebased onto main (7.19.0) and force-pushed. The conflict was the test file: #325 renamed tests/test_tounicode_fallback.py to tests/test_unit_tounicode_fallback.py, which is also the rename you asked for in review — the rebase carried my additions over to the new name, so both are addressed by the same push. The page_font.h change applied without conflict; the guard region is unchanged on main.

Verified locally on the rebased branch: the 7 tests in test_unit_tounicode_fallback.py, the full test_unit_*.py suite (169 passed, 1 skipped), and pre-commit (ruff, mypy, uv-lock) are all clean. I could not reach the Hugging Face corpus from here today, so the regression job on CI is the check for that.

PeterStaar-IBM
PeterStaar-IBM previously approved these changes Sep 11, 2026

@PeterStaar-IBM PeterStaar-IBM left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@PeterStaar-IBM

Copy link
Copy Markdown
Member

@wittjeff I think we are seeing here some interesting regression tests

A symbolic simple font with no /Encoding maps its codes through the
embedded program's builtin encoding (PDF 32000-1 9.6.6.2, 9.6.6.4). The
parser never consulted it for text: with no usable /ToUnicode the codes
fell through to the Standard table, which fabricates (Ł for Cyrillic и,
'&' for a glyph index) and only coincides with the font for ASCII.

Open the program with FreeType, walk codes 0-255 through the builtin
charmaps (Adobe custom/standard/Latin-1, MS symbol at 0xF000, then any
remaining table) and take the text from the glyph's PostScript name (AGL,
uniXXXX, uXXXX) or the program's Unicode cmap; a blank glyph reads as a
space, a glyph with neither stays a GLYPH marker. Precedence is
/Differences, /ToUnicode, builtin, then the existing name-matched tables
and the docling-project#299/docling-project#322 marker guard. Mirrors the renderer's
char_code_to_glyph_index so text and drawn glyph agree.

Regression corpus @ 571f34b: 16 pages in 4 documents change, all of this
font shape; the output matches PyMuPDF on those pages.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B6oAsX79utapMVcbRKU6vh
Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
@wittjeff

Copy link
Copy Markdown
Contributor Author

Interesting indeed — and they point at a real gap, so I fixed that rather than the guard.

What the 60 failing pages are. Four documents, all new to the corpus since the August run. Every affected font has the same shape: symbolic flag set, program embedded, no /Encoding, no usable /ToUnicode. That is exactly the case the guard now covers, so main was reading those codes through the Standard-encoding table and the stored groundtruth is whatever that produced. I pulled the embedded programs apart to see what the codes really mean:

document font program says main (= groundtruth)
10017889119412695965-12.pdf HeliosC-Plain/Bold (Type1C, custom encoding, Latin-1 glyph names on cp1251 codes) 232 → egrave, 50 → two GLYPH<209>GLYPH<238>…Ł (Ł is StandardEncoding's 232)
same GKGEIM+HeliosC-Italic (Type1C, afii names) 232 → afii10074 = и same
5462902444445925132-4.pdf EHDBFI+Anastasia (TrueType, (3,0) cmap, post names) 61 → equal, 50 → two =, 2 — right by coincidence
10976580690960943929_004.pdf Calibri/Arial subsets (TrueType, (3,0) cmap only, post 3.0, no names) codes 2, 5, 6, 9 … are glyph indices &, 3, 5 for codes 38, 51, 53 — fabricated
77f9f425f5fc32be_0003.pdf CFHCON+PMingLiU+3 (TrueType, one cmap entry, no names) 32 → a blank glyph space

So the guard alone is half right: it stops the fabrication (Ł for и, & for a glyph index) but also drops the digits and = that happened to be correct, because those programs' builtin encodings are ASCII-compatible in that range. Neither the old groundtruth nor plain markers is the right answer for these pages.

The right answer is the builtin encoding. A symbolic simple font without a declared base encoding maps its codes through the embedded program itself (PDF 32000-1 9.6.6.2 for Type 1/CFF, 9.6.6.4 for the TrueType (3,0) cmap). The renderer already does exactly this for glyph identity (freetype_font_cache::char_code_to_glyph_index); the parser never did it for text. The new commit adds page_font/builtin_encoding.h: on the first unresolved code of such a font it opens the embedded program with FreeType, walks codes 0–255 through the builtin charmaps (Adobe custom/standard/Latin-1, then MS symbol with the 0xF000 convention, then whatever single table a subsetter left), and takes the reading text from the glyph's PostScript name (AGL, uniXXXX, uXXXX) or the program's Unicode cmap. A glyph with neither stays a marker; a blank glyph reads as a space. Precedence in get_correct_character is /Differences → /ToUnicode → builtin → the existing name-matched tables → the #299/#322 marker guard, so nothing that resolved before resolves differently now.

Regression suite, run locally on the full corpus @ 571f34b (test_regression_parse.py; the threaded parse/render tests fail on the same cell-text assertions, so this is the set): 16 pages in 4 documents differ from the stored groundtruth, every one of them a font of the shape above. Verified cell by cell, and against PyMuPDF on the same pages:

  • 10017889119412695965-12.pdf p2, 4, 5, 9, 11 — the HeliosC-Italic pages come out as real Russian (Приемник сигналов, Дисплей, …). The Plain/Bold pages come out as Ñîäåðæàíèå: the font's own glyph names are Latin-1 names on cp1251 codes, so this is what the producer wrote; it is byte-for-byte what PyMuPDF extracts, and a reversible transcoding rather than a lossy mix of markers and Ł (pdfminer gives the old (cid:209)(cid:238)…Ł mix, i.e. the same fallback main had).
  • 5462902444445925132-4.pdf p1–5 — digits, =, Latin letters back through the post names. The high codes follow the program's names too, which for this font are as arbitrary as the Standard table was (π where the table said '); PyMuPDF reads the page identically.
  • 10976580690960943929_004.pdf p1, 4, 7, 9, 12 — the groundtruth was already mostly markers; now the Standard-table !"#$% for codes 33–37 are markers as well, and the subset's blank space glyph reads as a space, which regroups the cells. Nothing in the file carries text for these glyphs (no names, no Unicode cmap, no /ToUnicode) — PyMuPDF emits the raw control bytes here — so a marker is the honest signal; only OCR could do better.
  • 4e5aa9238544d683_0009.pdf p1 — new to the diff: a GLYPH<32> that the fix: emit glyph marker instead of fabricated encoding text for symbolic-font cmap misses #299 guard emitted for a symbolic font whose /ToUnicode misses the space now reads as a space through the blank-glyph rule.
  • 77f9f425f5fc32be_0003.pdf matches its groundtruth again (the space).

Everything else in the corpus is unchanged — including the seven documents that broke in August and the TeX/arXiv documents — so wherever the builtin path and the existing tables both apply they agree.

Tests. tests/test_unit_builtin_encoding.py builds the fonts in memory with fontTools (already in the dev environment via matplotlib): a (3,0)-cmap TrueType with and without post names, and a bare CFF with a custom encoding placing Cyrillic and Arabic at Latin codes. It covers: names resolve, nameless glyphs stay markers, blank glyph → space, index-style names (g4) stay markers, builtin outranks a core-14 alias name (ABCDEF+Arial), a declared /WinAnsiEncoding still wins, /Differences without /BaseEncoding overrides only the listed codes, and a nonsymbolic font is untouched. With the 7 existing tests, the full test_unit_* suite is 178 passed, 1 skipped.

Groundtruth. The stored groundtruth for these four documents was never right (it encodes the Standard-table fallback), so the 16 pages above need regenerating in the dataset repo once this lands. Happy to open that PR against regression-dataset-for-docling-parse, or leave it to you if you prefer to regenerate from CI.

One residual, unchanged from before: when the builtin lookup finds no text for a glyph and the font's name matches a core-14 alias, the name-matched AFM table is still consulted (JBXGIG+Arial in 10976580690960943929_004.pdf keeps its Standard-table text for symbol-coded glyphs). That path predates #299 and is a separate decision.

@wittjeff

Copy link
Copy Markdown
Contributor Author

Dataset groundtruth PR for the 16 pages above, regenerated on a641751 and verified by hash to touch nothing else: https://huggingface.co/datasets/docling-project/regression-dataset-for-docling-parse/discussions/4

@cau-git

cau-git commented Oct 28, 2026

Copy link
Copy Markdown
Member

Thanks for this contribution. This PR is on hold until it has been consolidated as described in #428. Please resubmit it as part of the topic PR listed there, with the evidence the issue asks for.

@wittjeff

Copy link
Copy Markdown
Contributor Author

Superseded by #431, which consolidates the font and ToUnicode decoding fixes as described in #428. This fix is commit 6 of 6 there, rebased on current main.

@wittjeff wittjeff closed this Oct 28, 2026
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.

4 participants