Repository navigation
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if is_simple_font { | ||
| match entry.remapped.as_ref().map(|c| c.lookup_code(code)) { | ||
| Some(CodeMapping::Text(text)) if !text.contains('\u{FFFD}') => { | ||
| return Some(text); | ||
| } | ||
| Some(CodeMapping::ControlDestination) => control_destination = true, | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
🔴 Sparse mappings lose to font encodings
With a declared base encoding, entry.remapped never reads codes that entry.primary misses; the base returns first. A sparse ToUnicode mapping from 0x21 to c still reads ! under WinAnsi, and unreadable /Differences names drop mapped codes.
Learn more
A sparse ToUnicode CMap is moved into remapped when a font-program reading takes the primary role in cmap_entry. The new lookup occurs after differences_reading and the base-encoding return. Thus an explicit /BaseEncoding supplies a character even for a code the sparse CMap maps differently; an unreadable Differences name exits before the CMap can read the code at all. Control destinations in the sparse map likewise cannot be marked when the base encoding has already returned.
Example: A simple font with a sparse <21> <0063> ToUnicode entry, an embedded program that reads only code 0x22, and /BaseEncoding /WinAnsiEncoding reads byte 0x21 as ! rather than c. With an unresolved /Differences name at 0x21 and another readable name keeping the encoding active, it omits c altogether.
Recommended fix: After the primary program reading, consult the demoted ToUnicode map before returning a base encoding or discarding an unreadable Differences code. Preserve the existing priority for a Differences name that actually identifies a readable glyph, and apply control-destination handling consistently.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
You were right: a declared base encoding, and an unreadable /Differences name, were read before the sparse ToUnicode entry, so <21> -> c still read ! under WinAnsi. Fixed in e5cb9fd. The sparse entry is now read after a readable /Differences name and before the base encoding and unreadable names. The new test a_simple_fonts_sparse_cmap_beats_a_base_encoding_and_unreadable_differences covers both cases and failed before the change.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes a simple font's sparse ToUnicode CMap being ignored by reading it before the printable-byte guess, with tests pinning the corrected codes and control-character handling. The change is focused and clearly corrects misread text.
Turn on auto-fix | Re-trigger cubic
… its program does not A ToUnicode CMap of fewer than ten entries yields the primary role to the embedded program's own reading and is kept as the alternative. The byte-level reading of a simple font never looked at that alternative, so a code the CMap maps but the program does not read (its glyph has no usable name) fell through to the printable-byte guess: macOS Quartz writes such fonts for the few accented capitals of a heading, and `CÔNG TY` read as `CÔ!G TY`. The alternative is now read before that guess, after the program's reading, the Differences and the base encoding, so only text that was a guess changes. A simple font has no other alternative: the subset remap is made for Identity-H/V composite fonts alone.
…reads as U+FFFD The sparse ToUnicode CMap of a simple font was read for text alone. A code it maps to a control character, which nothing repaired because the program does not name the glyph, fell through to the printable-byte guess and read as its byte value. The code is now marked as any other CMap's control destination is, so it reads as U+FFFD.
ebf28a2 to
567b863
Compare
…its base encoding and unreadable Differences Agent: claude
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval skipped because cubic reviewed only this push, not the earlier force-push. Comment @cubic review to review the whole pull request.
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Re-trigger cubic
… be read by the sparse CMap Agent: claude
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval skipped because cubic reviewed only this push, not the earlier force-push. Comment @cubic review to review the whole pull request.
Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack
View guided diff | Re-trigger cubic
Fixes #620.
Summary
A ToUnicode CMap of fewer than ten entries yields the primary role to the
embedded program's reading and is kept as the alternative (
cmap_entry).The byte-level reading of a simple font never looks at that alternative.
So a code the CMap maps, but whose glyph the program cannot name, falls
through to the printable-byte guess and reads as its byte value.
macOS Quartz writes such fonts: the letters of a heading go into a small
TrueType subset with codes from
0x21and a short ToUnicode CMap. Thereproduction in #620 (Times New Roman Bold):
BIÊN BẢNBIÊ! BẢ!BIÊN BẢNpdftotextreads the same file right.Fix
In the byte-level reading of a simple font, the demoted sparse CMap is
now read right after a
/Differencesname that maps (which still wins),and before a
/Differencesname that cannot be mapped and before thebase encoding. A ToUnicode CMap says what a code is, so it beats the
encoding table for the codes it maps; the font program's own reading
still comes first. It is limited to simple fonts because their only
alternative is the demoted sparse CMap; the subset remap is made for
Identity-H/V composite fonts alone.
A code the sparse CMap maps to a control character is marked like any
other CMap's control destination: if nothing else reads it, it shows
U+FFFD instead of a guess from its byte value (second commit, from
cubic's review).
Tests
a_simple_fonts_sparse_cmap_reads_the_codes_the_program_does_not(synthetic font, no fixture file). Reads
!o$$before,coeeafter;a code both read still takes the program's reading.
a_simple_fonts_sparse_cmap_marks_its_control_destination.A code the sparse CMap maps to U+0003 reads
#before the secondcommit, U+FFFD after.
a_simple_fonts_sparse_cmap_beats_a_base_encoding_and_unreadable_differences.<21>readscunder WinAnsi and under an unreadable/Differencesname (Devin's case).
cargo fmt --all -- --check,cargo clippy -- -D warnings,cargo clippy --features ocr -- -D warnings,cargo test,scripts/version.py --check, script tests.pdf2mdoutput of all 44 PDFs intests/fixtures/was byte-identicalbefore and after when last measured, before the third unit test above
was added; not re-measured since.
BIÊN BẢNreads asBIÊ! BẢ!#620 and two more Quartz-made PDFs with the fault now readright; four from the same generator that read right before are unchanged.
Not verified
pdf-evalsis not public, sobench.py test/bench.py scorewerenot run. Please run them before merge.
The script in A simple font's short ToUnicode CMap is ignored for codes the embedded program does not name:
BIÊN BẢNreads asBIÊ! BẢ!#620 makes one on macOS.Summary by cubic
Fixes sparse ToUnicode CMap entries being ignored for simple fonts, so codes the embedded program can't name no longer misread as their byte values.
/Differencesnames, after the program's reading and mapped Differences.Written for commit dacbc57. Summary will update on new commits.