fix(font): resolve Identity-H CJK via Encoding::Identity variant

[gandli]

Purpose

Fix a structural character-to-Unicode bug for Type0 / Identity-H / Identity-V
composite fonts whose Encoding is parsed into the Encoding::Identity
enum variant. Previously the entire Identity recovery path for such fonts
was skipped, so every CID fell through to a bare char::from_u32(cid)
fallback. This is not a single-glyph issue: it mis-decodes any glyph whose
code is not its own Unicode scalar — e.g. a subset CIDFontType2 code 0x69
emitted as i (U+0069) instead of the intended character, and silently
wrong characters wherever the raw code happens to be a valid but incorrect
code point.

Overview

font_dict.rs::char_to_unicode_uncached gated its Identity handling behind
if let Encoding::Standard(ref encoding_name) = self.encoding. But PDF
Identity-H / Identity-V names are collapsed into the Encoding::Identity
enum variant (see the Encoding doc comment), so the if let Standard(..)
arm never matched and the whole block — ToUnicode CMap, embedded TrueType
cmap, UCS2/UTF16 direct mapping, and the CID-as-Unicode last resort — was
bypassed.

The fix replaces the nested if let + if encoding_name == ... with a single
match over &self.encoding that accepts both Encoding::Identity and a
Standard string naming an Identity/UCS2/UTF16 CMap, producing a boolean
is_identity_enc that drives the (otherwise unchanged) recovery logic. All
four sub-paths below are now reached for the folded variant, exactly as they
already were for the Standard("Identity-H") string form:

1. Identity ordering + embedded TrueType cmap → CID → GID → Unicode
2. UCS2/UTF16 encodings → char code is the Unicode scalar
3. Non-Identity ordering (e.g. Adobe-GB1) → predefined CMap lookup
4. No CIDSystemInfo → CID-as-Unicode last resort

Context

The trigger is purely structural: whenever the parser folds an
Identity-H/V encoding into Encoding::Identity, the font lost all Unicode
recovery and degraded to raw-code emission. Language-agnostic — CJK, Latin,
and symbol subsets are equally affected. Root cause was the Encoding enum
folding, not the ToUnicode CMap itself. This routes Encoding::Identity
fonts through the same recovery path Encoding::Standard("Identity-H")
fonts already used.

Verification

Regression test tests/test_identity_h_encoding_variant_cjk.rs is 100%
synthetic (no third-party or external fixture). It builds Type0 / Identity-H
fonts and asserts the true text is recovered while mojibake does not surface,
covering the four sub-paths above:

• A — Identity-H + displaced ToUnicode CMap (CJK path; proves recovery
  works, not just for one glyph)
• B — Identity-H without ToUnicode, CID == Unicode (CID-as-Unicode
  fallback sub-path)
• C — Latin subset (proves the fix is structural, not CJK-specific)
• D — UCS2-style direct char-code == Unicode path

cargo test --release --test test_identity_h_encoding_variant_cjk →
4 passed; 0 failed.

Real-world confirmation: a CJK document whose text layer contained
i务流水号 under the buggy build extracts as 业务流水号 after the fix
(the first character is no longer mis-decoded as i).

Reviewer guidance

• Change is confined to the entry match of char_to_unicode_uncached;
  non-Identity code paths are untouched.
• lookup_predefined_cmap is now called with &encoding_name (the binding
  is now a String rather than a &String).
• The new test is self-contained and runs without network or fixtures.
• CFF (CIDFontType0) fonts have no TrueType cmap, so truetype_cmap() returns
  None and recovery still depends on ToUnicode / predefined CMap — identical
  to the prior behaviour for the Standard("Identity-H") form, i.e. no
  regression introduced there.

[yfedoseev]

Thanks for the contribution, and welcome! You've correctly spotted that the if let Encoding::Standard("Identity-H"|…) gate on the Priority-2 block was effectively dead for real Identity-H fonts — /Identity-H, /Identity-V, and Adobe-collection CMaps all fold to Encoding::Identity, so they never entered that block. Good catch.

Before this can merge, though, there are a couple of things to work through — the change is broader than the one glyph it targets.

1. Main concern — it changes a deliberate policy for the whole Identity-H population, not just your case. For a CID uncovered by a present-but-partial ToUnicode CMap, the existing Encoding::Identity arm (around line 4490) deliberately emits U+FFFD rather than guess — see the comment near lines 4654–4660 ("plausible-but-wrong, per-file-varying guess" is explicitly avoided). Routing these fonts through the Priority-2 block instead makes the else at ~line 4234 return char::from_u32(char_code) — the raw CID-as-Unicode guess the design rejects. That helps when the embedded cmap has the glyph, but can turn clean U+FFFD into wrong CJK (mojibake) for partial-ToUnicode subset fonts. Since nearly all Identity-H fonts are Encoding::Identity, this flips uncovered-CID behavior across the board. Is that intended? I'm running a CJK-corpus diff and will post it here — that'll tell us whether this is net-positive.

2. The test doesn't actually exercise the fix. I traced the cascade: scenarios A & C supply a complete ToUnicode CMap, so they resolve via PRIORITY 1 (which isn't encoding-gated) regardless of your change; B & D use ASCII ("OPENAI", "DATA", offset 0, no ToUnicode) where CID == Unicode, so the pre-existing CID-as-Unicode fallback (~line 4663) already handles them. So the new test almost certainly passes with font_dict.rs reverted — please verify that, and rebuild it around the real failure mode: an embedded FontFile2 with a cmap, Identity ordering, real CJK codepoints where CID ≠ Unicode, and ToUnicode absent/partial. Also, the test is titled "CJK" but only asserts romanized ASCII ("YE WU LIU SHUI HAO") — please assert an actual CJK scalar. (The negative assertion !text.contains("'<.'") also looks like it doesn't match the real mojibake it's guarding against.)

3. DRY. After this, the Priority-2 block and the line-4490 Encoding::Identity arm both do truetype-cmap + CID-as-Unicode for Identity fonts — two overlapping copies of the fallback policy, which is exactly what created concern #1. Worth consolidating into one path rather than relocating the recovery.

4. Minor. Encoding::Identity => ("Identity-H", true) collapses Identity-V into "Identity-H" — harmless for text extraction (H/V is layout, not char mapping), just noting it.

Really do appreciate the fix — Identity-H CJK extraction is a real gap. Let's get the test to reproduce the actual bug and the corpus diff to confirm direction, and I think this lands.

[yfedoseev]

Corpus regression result (419-PDF suite, main vs this PR): 0 differing files across text/markdown/html.

Detail, because "0" here is informative rather than a coverage gap: the suite has 126 PDFs using /Identity-H and 8 that extract real CJK text — so it genuinely exercises this code path — and your change alters none of them. That's reassuring on the regression front: on well-formed Identity-H/CJK documents (which carry a complete ToUnicode CMap), the change is a no-op, so the U+FFFD→raw-CID concern does not fire here.

But it also means the suite never hits the case your fix targets — a ToUnicode miss on an Identity-H CID font — so the corpus can neither confirm the fix nor exhibit the mojibake risk. Both therefore rest entirely on a test that actually reproduces the ToUnicode-miss path (embedded cmap + real CJK CID≠Unicode + absent/partial ToUnicode), plus the policy question of whether raw-CID-as-Unicode is the right fallback there vs the existing deliberate U+FFFD. That's the path to merge.

[yfedoseev]

Thanks for this, and for the unusually clear write-up of the mechanism.

Your diagnosis is correct. font_dict.rs:2686 folds "Identity-H" | "Identity-V" into Encoding::Identity, so the if let Encoding::Standard(..) gate really did make that whole Type0 block unreachable for exactly the fonts it was written for. That's a genuine dead path and worth repairing.

The problem is that I can't get the tests to demonstrate it. I merged your branch onto current main (clean merge) and ran:

• All four tests pass with your font_dict.rs change reverted. I restored just that file from main, kept your test file, and got 4 passed; 0 failed. Per CONTRIBUTING a bug-fix test has to fail before the change and pass after — these stay green either way, so they don't pin the behaviour you're fixing.
• 419-PDF regression sweep, your branch merged onto main vs main: zero documents change. No regressions, which is good news, but also no observable improvement.

Both results point the same way: the block you've made reachable is a fallback, and the earlier paths in char_to_unicode_uncached (ToUnicode in particular) already resolve these fonts before control ever gets there. So the repair may well be correct in principle while changing no outcome in practice.

To make this reviewable, the fixture needs a font where the earlier paths genuinely can't resolve the code. The most promising shape looks like a Type0/Identity-H font with a non-Identity CIDSystemInfo ordering (e.g. Adobe-GB1) and no /ToUnicode, so the predefined-CMap lookup around line 4177 is the only thing that can produce a character — that one takes encoding_name, which is precisely what your change starts supplying. If such a fixture goes red on main and green on your branch, I'd be happy with the change.

Two smaller things while you're in there:

1. cargo fmt --check isn't clean on the branch — a missing trailing comma at font_dict.rs:4125 and the block body under if is_identity_enc { never got re-indented after the nesting level was removed. CI enforces this one.
2. For Encoding::Identity you synthesise the name "Identity-H", which then reaches lookup_predefined_cmap. There's precedent for that at line 4320 so it's consistent, but a font that was actually Identity-V will look up a horizontal CMap. Probably harmless for the Unicode value; worth a comment noting the choice is deliberate.
3. The branch is 34 commits behind main. It merges cleanly, so no urgency, but a rebase before merge would keep CI honest.

Not a rejection at all — the analysis is right and I'd like to land it. It just needs a fixture that goes red without it.

[yfedoseev]

@gandli — sorry for the five-week gap. Coming back to this with a smaller ask than I left you with.

When I last wrote I asked you to work through the policy question and rebuild the test. Having spent more time in this function, most of what I was worried about turns out to be our problem rather than yours.

The block your change routes Identity fonts into is one of two implementations of CID→GID→cmap recovery in char_to_unicode_uncached, and hardening has only ever landed in the other one. That is why it is missing a bounds check the live arm has at :4546 — cid_to_gid.get_gid(char_code as u16) truncates any CID above 0xFFFF with no guard. It is also why your test appeared to pass without the fix: parse_encoding folds /Identity-H into Encoding::Identity, so the copy the test exercises is not the copy production uses. I have filed the duplication as #1119 — please do not take that on here.

What I would like in this PR:

1. The char_code <= 0xFFFF guard, matching :4546, so the block you are reviving is not missing a check the one it precedes has.
2. Route non-Identity orderings to the predefined-CMap lookup rather than the raw-CID fallback. This one is conformance rather than preference: ISO 32000-1 §9.10.2 third method applies to a composite font or one whose descendant CIDFont uses Adobe-GB1, Adobe-CNS1, Adobe-Japan1 or Adobe-Korea1 — it is disjunctive, so it holds even under /Identity-H, and CID-as-Unicode is never right for those collections.
3. The test from my earlier comment — embedded FontFile2, non-Identity ordering, real CJK where CID ≠ Unicode, ToUnicode absent or partial — and a CONTRIBUTING §4 over-collection fixture, since this broadens what the block collects.

If you would rather not carry (2), say so and I will split it out; the guard and the test are the parts I would want either way.