fix: consult base-font metrics for character decoding when the font dict carries a /Name label - #327
Conversation
…ies a /Name label A base-14 font dictionary carrying the obsolete /Name entry (e.g. << /BaseFont /Symbol /Name /F5 >>, as ReportLab emits) reports the label as its font name. get_correct_character() matched only that name against the base-14 metrics, so Symbol/ZapfDingbats text missed the built-in encoding tables and fell back to StandardEncoding: Symbol's arrowright (0256) decoded as the 'fi' ligature and ZapfDingbats' a35 star as 'H'. The bogus ligature then merged into the neighbouring line under the sanitator's cross-font ligature exemption, which is why issue docling-project#317 presented as a merging bug. Mirror the font_name-or-base_font fallback that every width/bbox lookup in page_font.h already uses. Fixes docling-project#317. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
|
✅ DCO Check Passed Thanks @wittjeff, all your commits are properly signed off. 🎉 |
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/
|
PeterStaar-IBM
left a comment
There was a problem hiding this comment.
please run the pre-commit
Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
|
Done — renamed to |
|
can you merge with main (such that we have the latest regression tests) and resolve the merge conflicts? |
…ry-builtin-encoding
|
Done — merged Full suite locally after the merge: 227 passed, 1 skipped, 3 failed — the three failures are the known I've opened a companion PR on the regression dataset with the four regenerated Once the dataset PR is merged I'll bump |
|
@wittjeff Seems we caught something in the regression testing, |
Please see https://huggingface.co/datasets/docling-project/regression-dataset-for-docling-parse/discussions/3 Are you intending for the GT dataset to include these issues? My interpretation was that if you incorporate the dataset PR following the HF link above, to fix the dataset first, then re-run the CI here, this docling-parse fix PR should clear. If I'm actually nulling out an intended error test case in the GT dataset, I'll back out the dataset PR and start over. |
Fixes #317.
A base-14 font dictionary that carries the obsolete
/Nameentry (<< /BaseFont /Symbol /Name /F5 >>, as ReportLab emits) reports the label as its font name.get_correct_character()matched only that name against the base-14 metrics, so Symbol/ZapfDingbats text missed the built-in encoding tables (Symbol.afm,ZapfDingbats.afm) and fell back to StandardEncoding: Symbol'sarrowright(0256) decoded as thefiligature, ZapfDingbats'a35star (0110) asH. The bogusfithen merged into the neighbouring Helvetica line under the sanitator's cross-font ligature exemption, which is why the issue presented as a merging bug.The fix mirrors the
font_name-or-base_fontfallback that every width/bbox lookup inpage_font.halready uses.src/parse/pdf_resources/page_font.h: one hunk inget_correct_character()tests/test_font_name_resolution.py: parse-side regression tests (Symbol arrow, ZapfDingbats star, labelled and unlabelled) alongside the existing render-side/Name-shadowing testFull test suite: 221 passed, 1 skipped, 3 failed — the three failures are a single groundtruth mismatch surfaced in
test_parse/test_threaded_parse/test_threaded_render:annots_01.pdfgroundtruth expectslwhere the fix now yields●. That document's/F3is<< /BaseFont /ZapfDingbats /Name /F3 >>with no/Encoding— the same bug pattern — so the groundtruthlis itself the misdecode (ZapfDingbats code 108,a71, is the black-circle bullet). The groundtruth dataset needs a revision bump for that file, as with the ActualText fix (#300).🤖 Generated with Claude Code