diff --git a/fontbox/src/main/java/org/apache/fontbox/cmap/CMap.java b/fontbox/src/main/java/org/apache/fontbox/cmap/CMap.java index df50a55b1ea..b0ff1eac773 100644 --- a/fontbox/src/main/java/org/apache/fontbox/cmap/CMap.java +++ b/fontbox/src/main/java/org/apache/fontbox/cmap/CMap.java @@ -65,6 +65,14 @@ public class CMap private final Map> codeToCid = new HashMap<>(); private final List codeToCidRanges = new ArrayList<>(); + // CID mappings inherited through usecmap, kept apart from this CMap's own mappings because they + // rank below them: a CMap that uses another may redefine any of the codes it inherits, and those + // redefinitions have to win. Merging the two would make the outcome depend on parse order, and + // usecmap is read from the header before the CMap's own mappings, so the inherited ones would + // always be found first. + private final Map> inheritedCodeToCid = new HashMap<>(); + private final List inheritedCodeToCidRanges = new ArrayList<>(); + // inverted map private final Map unicodeToByteCodes = new HashMap<>(); @@ -85,7 +93,8 @@ public class CMap */ public boolean hasCIDMappings() { - return !codeToCid.isEmpty() || !codeToCidRanges.isEmpty(); + return !codeToCid.isEmpty() || !codeToCidRanges.isEmpty() + || !inheritedCodeToCid.isEmpty() || !inheritedCodeToCidRanges.isEmpty(); } /** @@ -241,13 +250,19 @@ public int toCID(byte[] code) { return 0; } - Integer cid = null; - Map codeToCidMap = codeToCid.get(code.length); - if (codeToCidMap != null) + Integer cid = lookup(codeToCid, code.length, toInt(code)); + if (cid != null) + { + return cid; + } + int fromRanges = toCIDFromRanges(codeToCidRanges, code); + if (fromRanges != 0) { - cid = codeToCidMap.get(toInt(code)); + return fromRanges; } - return cid != null ? cid : toCIDFromRanges(code); + // nothing of this CMap's own matched, fall back to whatever it inherited + cid = lookup(inheritedCodeToCid, code.length, toInt(code)); + return cid != null ? cid : toCIDFromRanges(inheritedCodeToCidRanges, code); } /** @@ -290,13 +305,19 @@ public int toCID(int code, int length) { return 0; } - Integer cid = null; - Map codeToCidMap = codeToCid.get(length); - if (codeToCidMap != null) + Integer cid = lookup(codeToCid, length, code); + if (cid != null) + { + return cid; + } + int fromRanges = toCIDFromRanges(codeToCidRanges, code, length); + if (fromRanges != 0) { - cid = codeToCidMap.get(code); + return fromRanges; } - return cid != null ? cid : toCIDFromRanges(code, length); + // nothing of this CMap's own matched, fall back to whatever it inherited + cid = lookup(inheritedCodeToCid, length, code); + return cid != null ? cid : toCIDFromRanges(inheritedCodeToCidRanges, code, length); } /** @@ -306,9 +327,15 @@ public int toCID(int code, int length) * @return CID */ - private int toCIDFromRanges(int code, int length) + private static Integer lookup(Map> mappings, int length, int code) { - for (CIDRange range : codeToCidRanges) + Map codeToCidMap = mappings.get(length); + return codeToCidMap != null ? codeToCidMap.get(code) : null; + } + + private static int toCIDFromRanges(List cidRanges, int code, int length) + { + for (CIDRange range : cidRanges) { int ch = range.map(code, length); if (ch != -1) @@ -326,9 +353,9 @@ private int toCIDFromRanges(int code, int length) * @return CID */ - private int toCIDFromRanges(byte[] code) + private static int toCIDFromRanges(List cidRanges, byte[] code) { - for (CIDRange range : codeToCidRanges) + for (CIDRange range : cidRanges) { int ch = range.map(code); if (ch != -1) @@ -476,15 +503,19 @@ void useCmap(CMap cmap) } unicodeToByteCodes.put(v, bar); }); - cmap.codeToCid.forEach((key, value) -> - { - Map existingMapping = codeToCid.putIfAbsent(key, value); - if (existingMapping!=null) - { - existingMapping.putAll(value); - } - }); - codeToCidRanges.addAll(cmap.codeToCidRanges); + // CID mappings are inherited, not merged: they rank below anything this CMap defines itself + // (see inheritedCodeToCid). The copy is deliberate - sharing the other CMap's collections + // would let a later addCIDMapping here mutate a cached predefined CMap. + // A usecmap chain stays nearest-first, which needs opposite orderings for the two structures: + // the maps are consulted by key so putAll is last-wins, and the other CMap's own mappings go + // in after the ones it inherited; the range list is scanned first-match, so the other CMap's + // own ranges go in ahead of the ones it inherited. + cmap.inheritedCodeToCid.forEach((length, mappings) -> + inheritedCodeToCid.computeIfAbsent(length, k -> new HashMap<>()).putAll(mappings)); + cmap.codeToCid.forEach((length, mappings) -> + inheritedCodeToCid.computeIfAbsent(length, k -> new HashMap<>()).putAll(mappings)); + inheritedCodeToCidRanges.addAll(cmap.codeToCidRanges); + inheritedCodeToCidRanges.addAll(cmap.inheritedCodeToCidRanges); maxCodeLength = Math.max(maxCodeLength, cmap.maxCodeLength); minCodeLength = Math.min(minCodeLength, cmap.minCodeLength); maxCidLength = Math.max(maxCidLength, cmap.maxCidLength); diff --git a/fontbox/src/test/java/org/apache/fontbox/cmap/TestCMapParser.java b/fontbox/src/test/java/org/apache/fontbox/cmap/TestCMapParser.java index 9d860dcbbdc..3ceac8bb81e 100644 --- a/fontbox/src/test/java/org/apache/fontbox/cmap/TestCMapParser.java +++ b/fontbox/src/test/java/org/apache/fontbox/cmap/TestCMapParser.java @@ -261,4 +261,147 @@ void testBadIncrement() throws IOException CMap cmap = parser.parse(new RandomAccessReadBuffer(cmapData)); assertNotNull(cmap); } + + /** + * A CMap that redefines a code it inherits through usecmap must win over the CMap it uses. + * + * ETenms-B5-H exists only to do that: it uses ETen-B5-H and then remaps 0x20-0x7E to the + * proportional latin CIDs 1-95, where the parent maps them to the full width forms at 13648+. + */ + @Test + void testUseCmapOwnMappingsWin() throws IOException + { + CMap parent = new CMapParser().parsePredefined("ETen-B5-H"); + assertEquals(13681, parent.toCID(0x41, 1), "ETen-B5-H maps 0x41 to the full width form"); + + CMap cMap = new CMapParser().parsePredefined("ETenms-B5-H"); + assertEquals(34, cMap.toCID(0x41, 1), "ETenms-B5-H overrides 0x41 to the proportional form"); + assertEquals(1, cMap.toCID(0x20, 1), "ETenms-B5-H overrides 0x20 to the proportional form"); + + // codes the CMap does not redefine still come from the one it uses + assertEquals(parent.toCID(new byte[] { (byte) 0xA1, 0x40 }), + cMap.toCID(new byte[] { (byte) 0xA1, 0x40 }), + "an inherited code is unaffected"); + + // the byte[] overload repeats the lookup order of the int one, so check the override there too + assertEquals(13681, parent.toCID(new byte[] { 0x41 })); + assertEquals(34, cMap.toCID(new byte[] { 0x41 }), + "the byte[] overload has to prefer the CMap's own mapping as well"); + + // UniJIS-UCS2-HW-H likewise overrides its parent's proportional latin with the half width forms + CMap halfWidth = new CMapParser().parsePredefined("UniJIS-UCS2-HW-H"); + assertEquals(34, new CMapParser().parsePredefined("UniJIS-UCS2-H").toCID(0x41, 2), + "UniJIS-UCS2-H maps 0x0041 to the proportional form"); + assertEquals(264, halfWidth.toCID(0x41, 2), + "UniJIS-UCS2-HW-H overrides 0x0041 to the half width form"); + } + + /** + * The override has to survive a chain of usecmap: ETenms-B5-V uses ETenms-B5-H, which in turn + * uses ETen-B5-H. A code that only the middle CMap redefines has to keep that redefinition. + */ + @Test + void testUseCmapChainKeepsNearestMapping() throws IOException + { + CMap cMap = new CMapParser().parsePredefined("ETenms-B5-V"); + + assertEquals(1, cMap.getWMode(), "ETenms-B5-V is vertical"); + assertEquals(34, cMap.toCID(0x41, 1), + "ETenms-B5-V inherits the proportional override from ETenms-B5-H, not ETen-B5-H"); + } + + /** + * Both kinds of mapping a CMap declares have to beat the ranges it inherits. ETenms-B5-V + * declares six cidchars and twelve cidranges for the punctuation whose vertical form differs, + * on top of the horizontal forms it inherits from ETenms-B5-H and ETen-B5-H. + * + * The cidchars were already resolved correctly before the inherited mappings were separated out, + * a cidchar being consulted ahead of any range either way, so they are here as a guard rather + * than as a second reproducer. + */ + @Test + void testUseCmapOwnMappingsBeatInheritedRanges() throws IOException + { + CMap horizontal = new CMapParser().parsePredefined("ETenms-B5-H"); + CMap vertical = new CMapParser().parsePredefined("ETenms-B5-V"); + + // the horizontal forms come from an inherited range in both CMaps + assertEquals(110, horizontal.toCID(0xA14B, 2)); + assertEquals(111, horizontal.toCID(0xA14C, 2)); + assertEquals(121, horizontal.toCID(0xA156, 2)); + + // ETenms-B5-V's own cidchars replace them with the vertical forms + assertEquals(13646, vertical.toCID(0xA14B, 2), "own cidchar has to beat the inherited range"); + assertEquals(109, vertical.toCID(0xA14C, 2), "own cidchar has to beat the inherited range"); + assertEquals(312, vertical.toCID(0xA156, 2), "own cidchar has to beat the inherited range"); + + // and its own cidranges likewise, two usecmap levels down + assertEquals(128, horizontal.toCID(0xA15D, 2)); + assertEquals(130, vertical.toCID(0xA15D, 2), "own cidrange has to beat the inherited range"); + } + + /** + * Identity-V is the one predefined CMap that declares no cid mappings at all, it only uses + * Identity-H. Every lookup it answers is therefore an inherited one, which also makes it the + * case that proves hasCIDMappings has to account for what a CMap inherited. + */ + @Test + void testUseCmapOnlyInheritedMappings() throws IOException + { + CMap cMap = new CMapParser().parsePredefined("Identity-V"); + + assertEquals(1, cMap.getWMode(), "Identity-V is vertical"); + assertTrue(cMap.hasCIDMappings(), "Identity-V has cid mappings, all of them inherited"); + + assertEquals(65, cMap.toCID(new byte[] { 0, 65 }), "Identity-V CID 65"); + assertEquals(12345, cMap.toCID(new byte[] { 0x30, 0x39 }), "Identity-V CID 12345"); + assertEquals(0xFFFF, cMap.toCID(new byte[] { (byte) 0xFF, (byte) 0xFF }), + "Identity-V CID 0xFFFF"); + assertEquals(12345, cMap.toCID(0x3039, 2), "Identity-V CID 12345"); + } + + /** + * Predefined CMaps are cached and handed out repeatedly, so what a CMap inherits has to be a + * copy: adding a mapping to the importing CMap must not reach back into the one it used. + */ + @Test + void testUseCmapDoesNotShareMappingsWithTheUsedCMap() throws IOException + { + CMap used = new CMap(); + used.addCIDMapping(new byte[] { 0x41 }, 100); + used.addCIDRange(new byte[] { 0x50 }, new byte[] { 0x5F }, 200); + + CMap cMap = new CMap(); + cMap.useCmap(used); + assertEquals(100, cMap.toCID(0x41, 1), "the mapping is inherited"); + assertEquals(205, cMap.toCID(0x55, 1), "the range is inherited"); + + cMap.addCIDMapping(new byte[] { 0x41 }, 300); + cMap.addCIDRange(new byte[] { 0x50 }, new byte[] { 0x5F }, 400); + + assertEquals(300, cMap.toCID(0x41, 1), "the CMap's own mapping wins"); + assertEquals(405, cMap.toCID(0x55, 1), "the CMap's own range wins"); + assertEquals(100, used.toCID(0x41, 1), "the used CMap must not have been modified"); + assertEquals(205, used.toCID(0x55, 1), "the used CMap must not have been modified"); + } + + /** + * Everything a CMap declares outranks everything it inherits, so a cidrange of its own has to + * beat an inherited cidchar too, not just an inherited cidrange. No predefined CMap pairs the + * two that way round, hence the hand built pair here. + */ + @Test + void testUseCmapOwnRangeBeatsInheritedChar() throws IOException + { + CMap used = new CMap(); + used.addCIDMapping(new byte[] { 0x41 }, 100); + + CMap cMap = new CMap(); + cMap.useCmap(used); + cMap.addCIDRange(new byte[] { 0x40 }, new byte[] { 0x4F }, 200); + + assertEquals(201, cMap.toCID(0x41, 1), "the CMap's own range has to beat the inherited char"); + assertEquals(200, cMap.toCID(0x40, 1), "a code the used CMap says nothing about"); + assertEquals(100, used.toCID(0x41, 1), "the used CMap must not have been modified"); + } }