[
https://issues.apache.org/jira/browse/PDFBOX-6251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111530#comment-18111530
]
Tilman Hausherr commented on PDFBOX-6251:
-----------------------------------------
I have now understood the problem but I'm not persuaded by the solution,
although it obviously works. According to the specification, "CMap files can be
nested to five levels". (I didn't find out whether there can be several usecmap
operators; CharGPT says it can happen but has weird behaviour as if it has
never encountered one) Maybe this doesn't exist, but it could be. How about a
different approach for the fix:
- usecmap parses a cmap and keeps that cmap as a referenced object without
doing anything more
- for each of the public calls (like {{cmap.toCID()}}), find out whether there
are "usecmap" cmaps, and get their response (and check if there a response at
all e.g. with {{cmap.hasCIDMappings()}}). Then look at ones own, and if there
is, ignore the previous response.
This would be easier to understand IMHO. I always prefer simplicity.
> A CMap's own cid mappings are overridden by the ones it inherits via
> usecmap, so vertical CJK draws horizontal glyph forms
> ---------------------------------------------------------------------------------------------------------------------------
>
> Key: PDFBOX-6251
> URL: https://issues.apache.org/jira/browse/PDFBOX-6251
> Project: PDFBox
> Issue Type: Bug
> Components: FontBox
> Affects Versions: 2.0.37, 3.0.8 PDFBox
> Reporter: Patrick Corless
> Priority: Major
> Fix For: 2.0.38, 3.0.9 PDFBox, 4.0.0
>
> Attachments: 84700501c.pdf, GOOD ONE.pdf, JSPOverview.pdf
>
>
> *Description*
> A CMap that starts with `usecmap` is meant to be able to redefine any code it
> inherits, but in `org.apache.fontbox.cmap.CMap` the inherited
> `cidchar`/`cidrange` mappings are merged into the same collections as the
> importing CMap's own. `usecmap` is processed while the header is read, so the
> parent's mappings land in `codeToCid` / `codeToCidRanges` _before_ the CMap's
> own are parsed and appended. `toCID` consults the map and then scans
> `codeToCidRanges` first-match, so an inherited range that covers a code beats
> the mapping the importing CMap declared for it, and the override is silently
> lost.
> {code:java}
> CMap cmap = new CMapParser().parsePredefined("ETenms-B5-H");
> System.out.println(cmap.toCID(0x41, 1)); // prints 13681, expected 34{code}
> `ETenms-B5-H` exists purely to do this override: it uses `ETen-B5-H` and then
> remaps 0x20-0x7E to the proportional latin CIDs 1-95, where the parent maps
> them to the fullwidth forms at 13648+. So 0x41 ('A') must be CID 34, not
> 13681 (the fullwidth 'A').
>
> Dumping the parsed ranges of `ETenms-B5-H` shows both present, inherited
> first:
> {code:java}
> [0] 32..126 -> 13648 (inherited from ETen-B5-H, always matches first)
> ...
> [226] 32..126 -> 1 (ETenms-B5-H's own, never reached){code}
> *Scope*
> 31 of the 92 bundled predefined CMaps use `usecmap` *and* declare mappings of
> their own, and all 31 are measurably affected — the sweep below reaches every
> one of them and nothing else. Horizontal: `ETenms-B5-H`, `UniJIS-UCS2-HW-H`.
> The remaining 29 are the `-V` variants, whose own mappings select the
> vertical glyph forms — so vertical CJK is affected too. `UniJIS-UCS2-HW-H` is
> the same shape as ETenms: its parent `UniJIS-UCS2-H` maps 0x0041 to the
> proportional CID 34, and the `-HW` CMap overrides it to the halfwidth CID 264.
> *Downstream symptom*
> Wrong CID means the wrong glyph is drawn, and because `/W` is indexed by CID,
> the wrong CID usually falls outside the font's `/W` array, so the advance
> silently falls back to `/DW` = 1000 — every latin glyph becomes a full em
> wide (rendering as `J a v a S e r v e r`). Documents using `ETenms-B5-H` with
> a `/W` array that starts at CID 1 are proof the producer expected the
> proportional CIDs.
> *Proposed fix*
> Keep inherited CID mappings in their own map and range list
> (`inheritedCodeToCid` `inheritedCodeToCidRanges`) and consult them only
> after the CMap's own map and ranges: own map -> own ranges -> inherited map
> -> inherited ranges.
> A `usecmap` chain keeps nearest-wins ordering, which needs opposite insertion
> orders for the two structures: the maps are looked up by key so `putAll` is
> last-wins (deepest applied first, the
> nearer CMap's own on top), while the range list is scanned first-match so the
> nearer CMap's ranges are added ahead of the ones it inherited.
> Three details worth calling out:
> - `hasCIDMappings` has to account for the inherited mappings.* `Identity-V`
> declares no cid mappings of its own at all, it only uses `Identity-H`, so
> once the inherited ones live in separate collections `hasCIDMappings` would
> report false and `toCID` would return 0 for every code. Covered by
> `testUseCmapOnlyInheritedMappings`.
> - The maps are copied, not shared.* This also removes a latent aliasing bug:
> the previous
> `codeToCid.putIfAbsent(length, mappings)` stored the used CMap's inner map
> _by reference_ whenever the importing CMap had no mapping of that code
> length yet, so a later `addCIDMapping` on the importing CMap wrote straight
> into the cached predefined parent, corrupting it for every later user.
> Covered by `testUseCmapDoesNotShareMappingsWithTheUsedCMap`.
> - CID 0 is a valid mapping target, not a "not found" marker.* The private
> range scan returned 0 for both, which with two range lists would make a
> CMap's own `cidrange` to CID 0 fall through to the inherited mappings. It
> now returns -1 for "no range covers this code"; the public `toCID` still
> returns 0. Covered by `testUseCmapOwnMappingToCidZeroIsNotAFallthrough`
> *Testing*
> Seven tests added to `TestCMapParser`:
> ||test||covers||
> |testUseCmapOwnMappingsWin|the reproducer above, both `toCID` overloads|
> |testUseCmapChainKeepsNearestMapping|a two level `usecmap` chain stays
> nearest-wins |
> |testUseCmapOwnMappingsBeatInheritedRanges| own cidchar and own cidrange both
> beat an inherited range|
> |testUseCmapOnlyInheritedMappings|`Identity-V`, where every mapping is
> inherited |
> |testUseCmapDoesNotShareMappingsWithTheUsedCMap|the used CMap is not mutated|
> |testUseCmapOwnRangeBeatsInheritedChar|own cidrange beats an inherited
> cidchar|
> |testUseCmapOwnMappingToCidZeroIsNotAFallthrough|CID 0 is a mapping, not a
> miss|
> `mvn -pl fontbox -am test` is green (214 tests)
> *toCID sweep over the bundled predefined CMaps*
> All 92 bundled predefined CMaps were swept with both `toCID` overloads and
> the length guessing one: the 1 and 2 byte code spaces exhaustively (0x00-0xFF
> and 0x0000-0xFFFF), the 3 byte boundaries, and for 4 byte codes every code
> covered by a declared `cidrange`/`cidchar` plus a margin either side of each
> bound, plus a strided sample of the whole 32 bit space (99,434 probes). That
> is 2,889,588 non-zero results per build.
> Each result was checked against an oracle built independently of the patched
> code, by reading the CMap resource files directly and resolving each
> `usecmap` chain nearest-wins (a CMap's own `cidchar`/`cidrange` shadowing
> what it uses, `cidchar` ahead of `cidrange`, earlier range first):
> ||Build||non-zero results||disagreeing with the oracle||
> |trunk (before the fix)|2,889,588|2,714, across exactly the 31 CMaps named
> under Scope|
> | with the fix|2,889,588|0|
> So the fix does not merely change behaviour, it makes `toCID` agree with what
> the CMap files declare on every code swept, and it changes nothing on the
> other 61 CMaps. All 2,714 corrected lookups resolve to the nearest
> declaration in the CMap's own `usecmap` chain.
> The CID 0 sentinel change is behaviour preserving for the bundled set: the
> sweeps taken with and without it are byte identical. It is there because a
> CMap *may* legitimately map a code to CID 0, and the tests cover that case
> directly.
> Patch / PR attached.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
