Skip to content

fix: treat hexadecimal glyph-index names (Skia g%X) as unmapped glyphs - #389

Open
wittjeff wants to merge 2 commits into
docling-project:mainfrom
wittjeff:fix/hex-glyph-index-names
Open

wittjeff wants to merge 2 commits into
docling-project:mainfrom
wittjeff:fix/hex-glyph-index-names

Conversation

@wittjeff

@wittjeff wittjeff commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Skia/PDF (Chrome print-to-PDF, Notion exports, Flutter) names Type 3 glyphs with characterName.printf("g%X", gID) (SkPDFFont.cpp:758), so names such as /g15C and /gA1 carry hexadecimal digits. re_gid only accepted \d+, so those names fell through to the "unknown glyph-name" branch and leaked into the extracted text as g15C, the failure class of #238 and #302.

Before this change, with /Differences [65 /<name>], ToUnicode <0000> and no /ActualText:

glyph name result
gid00043, g348 GLYPH<name:…> / U+FFFD, as designed
g15C, g15c, gA1 literal name leaked as text

Found while tracing opendataloader-project/opendataloader-pdf#767: Skia maps Inter's cap-height hyphen alternate to <0000> in ToUnicode. docling-parse recovers that file through the /ActualText (-) span, so the leak only shows when a producer omits ActualText (older Skia) or when the alternate is not wrapped.

std::regex_match requires a full match, so AGL names such as gbreve and gcaron still do not match the widened class.

Regression corpus

test_reference_documents_from_filenames passes on upstream main and fails on this branch for two pages, both intentional: the pages mix fonts whose glyph names are G<hex> with no ToUnicode entry, and the stored ground truth already contains the inconsistency this fixes (GLYPH<name:G27> GLYPH<name:G29> GDB GLYPH<name:G67> on one line).

document page char cells changed distinct names
9acb62b4-4449-48d0-8127-f2be26349a6a-6.pdf 1 944 of 1792 137
ba55e6a2-1a86-460d-bf2c-e01a6771657b-2.pdf 1 2239 of 6121 109

Every change is a bare name wrapped into GLYPH<name:...>; no coordinate, font-key or cell-count differences, and every other document in the corpus is unchanged. The regenerated json.gz, char.txt, word.txt and line.txt for both pages are in regression-dataset PR #7. HF_DATASET_REVISION is pinned to that PR's head commit 4103ae9b so the regression lane runs against it; re-point it at the merge commit when the dataset PR lands.

Checklist:

  • Documentation has been updated, if necessary.
  • Examples have been added, if necessary.
  • Tests have been added, if necessary.

Skia/PDF names Type 3 glyphs with printf("g%X", gid), so /g15C or /gA1
carry hexadecimal digits and escaped the decimal-only glyph-index
pattern. Such names then leaked into the extracted text as 'g15C'
instead of the GLYPH<name:...> marker (the docling-project#238 / docling-project#302 failure class).

Widen the character class to [0-9A-Fa-f]; regex_match still requires a
full match, so AGL names such as gbreve or gcaron are unaffected.

Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

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

@mergify

mergify Bot commented Oct 6, 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)(?:\(.+\))?(!)?:

Points HF_DATASET_REVISION at the head of
docling-project/regression-dataset-for-docling-parse PR docling-project#7, which wraps
the bare G<hex> names on two pages in GLYPH<name:...> markers. Re-point
at the merge commit once that PR lands.

Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
@PeterStaar-IBM
PeterStaar-IBM self-requested a review October 7, 2026 05:38

This branch has not been deployed

No deployments
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