Skip to content

Consolidate UTF-16 and CMap decoding and fix wide ToUnicode codes - #581

Merged
Velli20 merged 1 commit into
mainfrom
consolidate-utf16-cmap-decoding
Oct 18, 2026
Merged

Velli20 merged 1 commit into
mainfrom
consolidate-utf16-cmap-decoding

Conversation

@Velli20

@Velli20 Velli20 commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • One borrowing UTF-16BE reader, Utf16BeUnits, is now shared by PDF text strings and ToUnicode destinations. It replaces the allocating BigEndianU16Units, and the hand-written surrogate decoding in cmap/unicode.rs is replaced by char::decode_utf16.
  • The ToUnicode parser builds HashMap<u32, UnicodeSequence> directly, so the second copying pass is gone. Sequential ranges no longer allocate one Vec per code, because UnicodeSequence: FromIterator<char> only allocates for mappings with two or more characters.
  • Type0 code-space decoding returns CidMapping directly, built through a new TryFrom<&[u8]> for PdfCode. The (u32, usize, u16) tuple and its re-validation are gone.
  • Type0EncodingCMap::decode_identity, decode and is_identity are removed, since only tests used them. decode is now a helper inside the tests.

Behavior changes

  • ToUnicode source codes keep their full 1–4 byte value. Previously they were cut to their last two bytes, so <AABBCC> was stored as 0xBBCC. Lookup still matches by numeric value only, so <0041> also maps the one-byte code 0x41.
  • Sequential bfrange destinations now advance their last UTF-16 unit, so <D835DC00> and <00660066> bases map correctly. Previously they mapped nothing.
  • Range spans are capped at 65,536 codes, so a malformed <00000000> <FFFFFFFF> range can't demand unbounded memory.
  • A lone high surrogate followed by a normal character, for example D83D 0041, now decodes as 'A' (standard library lossy behavior). Previously both units were dropped.

Test plan

  • cargo test passes for pdf-object-reader, pdf-cmap, pdf-font, pdf-annotation-core, pdf-canvas, pdf-content-stream and pdf-document. The full workspace wasn't run locally because the Skia build needs a download.
  • cargo clippy --all --workspace shows no warnings in changed files, and cargo fmt is applied.
  • Surrogate-pair and multi-character bfrange bases, 3-byte source codes and the range cap were checked with a temporary test, which isn't included.
  • Skia rendering and text extraction on CJK and ligature-heavy documents

🤖 Generated with Claude Code

Share one borrowing UTF-16BE reader (Utf16BeUnits) between PDF text
strings and ToUnicode destinations, replacing the allocating
BigEndianU16Units and the hand-rolled surrogate decoding with
char::decode_utf16. The ToUnicode parser now builds its final
UnicodeSequence map directly, and Type0 code-space decoding returns
CidMapping values built once through TryFrom<&[u8]> for PdfCode.
Test-only Type0EncodingCMap helpers are removed.

ToUnicode source codes keep their full one-to-four-byte value instead
of being truncated to two bytes, and sequential bfrange destinations
advance their final UTF-16 unit, so surrogate-pair and multi-character
bases map correctly. Range spans are capped at 65,536 codes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Conformance reads

Documents that Safe-PDF reads differently from the base commit, compared with PDFium. A read
regresses when Safe-PDF fails to open a document it opened before, crashes or times out, or
counts a different number of pages than PDFium.

pdf.js corpus

1008 documents compared with 4c7488c: 0 read worse, 1 read better.

Improvements

Document Base This pull request
issue17190_1 timeout: time limit exceeded (stage safe-read) read, 1 page

840 documents of the PDFium corpus read as on 4c7488c.

Details are in the job summaries of the workflow run. Rerun a case locally with
cargo conformance run --corpus <corpus> --read-only --case <id>.

@Velli20
Velli20 merged commit 5bbdabd into main Oct 18, 2026
10 checks passed
@Velli20
Velli20 deleted the consolidate-utf16-cmap-decoding branch October 18, 2026 13:06
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