From 6341ea91f4e30aa02dae5237035f858b6d25ccbf Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Sat, 3 Oct 2026 07:24:16 +1000 Subject: [PATCH 1/4] FOP-3346: ToUnicode selectors no longer drift by one after a supplementary-plane character CIDSubset.getChars builds a char[] with StringBuilder.appendCodePoint, so a character outside the BMP occupies two slots. PDFToUnicodeCMap derived each character selector from the array position, so every selector after such a character was written one too high, and the text after an emoji or a mathematical letter extracted as its neighbour. Measured on this branch, DejaVu Math TeX Gyre, the text "A" U+1D400 "BZ". The content stream uses selectors 3 4 5 6; the CMap said before <0003> <0041> <0004> <0006> <0042> <0007> <005a> after <0003> <0041> <0004> <0005> <0042> <0006> <005a> Before, selector 5 (the B) had no entry and selector 6 (the Z) was published as B. The CMap is now built from one destination per selector, a String, so a surrogate pair is one entry of length two rather than two array positions. The char[] constructor converts (toDestinations) and callers are unchanged. The range logic needs no surrogate special cases any more: an entry may join a bfrange when it is one code point, and two entries are consecutive when their code points are and their selectors share a 256 block. A destination of several characters is written as a bfchar with a string, which the format allows and FOP-3345 uses. PDFToUnicodeCMapTestCase pinned the drift (surrogatePairTest expected the entry after the pair at 0x63 to be 0x65); its expectations change accordingly, in surrogatePairTest, surrogatePairRangeTest, surrogatePairsRangeTest and rangeSizeSurrogateTest, the last of which also ran its low surrogates past U+DFFF and now starts them at U+DC00. ToUnicodeCharacterisationTestCase records the writer's output for the common shapes, so a change to the range packing has to be deliberate. Co-Authored-By: Claude Fable 5.1 --- .../org/apache/fop/pdf/PDFToUnicodeCMap.java | 377 +++++++----------- .../fop/pdf/PDFToUnicodeCMapTestCase.java | 40 +- .../ToUnicodeCharacterisationTestCase.java | 151 +++++++ 3 files changed, 329 insertions(+), 239 deletions(-) create mode 100644 fop-core/src/test/java/org/apache/fop/pdf/ToUnicodeCharacterisationTestCase.java diff --git a/fop-core/src/main/java/org/apache/fop/pdf/PDFToUnicodeCMap.java b/fop-core/src/main/java/org/apache/fop/pdf/PDFToUnicodeCMap.java index ad41e03de82..ea17da69bc3 100644 --- a/fop-core/src/main/java/org/apache/fop/pdf/PDFToUnicodeCMap.java +++ b/fop-core/src/main/java/org/apache/fop/pdf/PDFToUnicodeCMap.java @@ -42,10 +42,13 @@ public class PDFToUnicodeCMap extends PDFCMap { /** - * The array of Unicode characters ordered by character code - * (maps from character code to Unicode code point). + * One destination per character selector, in selector order: the UTF-16 text the glyph + * stands for. Usually one code point; several for a ligature or other glyph produced from + * more than one character; empty for a glyph whose text is carried by a neighbouring glyph; + * a lone high surrogate for an unpaired one, which is reported and written with a zero + * low surrogate. */ - protected char[] unicodeCharMap; + protected String[] destinations; private boolean singleByte; @@ -54,26 +57,70 @@ public class PDFToUnicodeCMap extends PDFCMap { /** * Constructor. * - * @param unicodeCharMap An array of Unicode characters ordered by character code - * (maps from character code to Unicode code point) + * @param destinations One destination string per character selector, in selector order * @param name One of the registered names found in Table 5.14 in PDF * Reference, Second Edition. * @param sysInfo The attributes of the character collection of the CIDFont. * @param singleByte true for single-byte, false for double-byte * @param eventBroadcaster Event broadcaster. May be null. */ - public PDFToUnicodeCMap(char[] unicodeCharMap, String name, PDFCIDSystemInfo sysInfo, + public PDFToUnicodeCMap(String[] destinations, String name, PDFCIDSystemInfo sysInfo, boolean singleByte, EventBroadcaster eventBroadcaster) { super(name, sysInfo); - if (singleByte && unicodeCharMap.length > 256) { + if (singleByte && destinations.length > 256) { throw new IllegalArgumentException("unicodeCharMap may not contain more than" + " 256 characters for single-byte encodings"); } - this.unicodeCharMap = unicodeCharMap; + this.destinations = destinations; this.singleByte = singleByte; this.eventBroadcaster = eventBroadcaster; } + /** + * Constructor from a positional array of UTF-16 code units, where a surrogate pair + * occupies two slots and stands for one character selector. + * + * @param unicodeCharMap An array of Unicode characters ordered by character code + * (maps from character code to Unicode code point) + * @param name One of the registered names found in Table 5.14 in PDF + * Reference, Second Edition. + * @param sysInfo The attributes of the character collection of the CIDFont. + * @param singleByte true for single-byte, false for double-byte + * @param eventBroadcaster Event broadcaster. May be null. + */ + public PDFToUnicodeCMap(char[] unicodeCharMap, String name, PDFCIDSystemInfo sysInfo, + boolean singleByte, EventBroadcaster eventBroadcaster) { + this(toDestinations(unicodeCharMap), name, sysInfo, singleByte, eventBroadcaster); + } + + /** + * Turns a positional array of UTF-16 code units into one destination per character + * selector: a high surrogate takes the unit after it as its low surrogate, and a high + * surrogate at the end of the array stands alone. + * @param unicodeCharMap the positional array + * @return one destination per selector + */ + public static String[] toDestinations(char[] unicodeCharMap) { + int count = 0; + for (int i = 0; i < unicodeCharMap.length; i++) { + if (isHighSurrogate(unicodeCharMap[i]) && i + 1 < unicodeCharMap.length) { + i++; + } + count++; + } + String[] destinations = new String[count]; + int d = 0; + for (int i = 0; i < unicodeCharMap.length; i++) { + if (isHighSurrogate(unicodeCharMap[i]) && i + 1 < unicodeCharMap.length) { + destinations[d++] = new String(unicodeCharMap, i, 2); + i++; + } else { + destinations[d++] = String.valueOf(unicodeCharMap[i]); + } + } + return destinations; + } + /** {@inheritDoc} */ protected CMapBuilder createCMapBuilder(Writer writer) { return new ToUnicodeCMapBuilder(writer); @@ -103,104 +150,64 @@ public void writeCMap() throws IOException { * Writes the character mappings for this font. */ protected void writeBFEntries() throws IOException { - if (unicodeCharMap != null) { - writeBFCharEntries(unicodeCharMap); - writeBFRangeEntries(unicodeCharMap); + if (destinations != null) { + writeBFCharEntries(); + writeBFRangeEntries(); } } /** - * Writes the entries for single characters of a base font (only characters which cannot be - * expressed as part of a character range). - * @param charArray all the characters to map - * @throws IOException + * Writes the entries for single selectors (those which cannot be expressed as part of + * a range), in sections of at most 100. + * @throws IOException if an I/O error occurs */ - protected void writeBFCharEntries(char[] charArray) throws IOException { + protected void writeBFCharEntries() throws IOException { int totalEntries = 0; - int charIndex = 0; - if (charArray.length > 0) { - do { - if (!partOfRange(charArray, charIndex)) { - totalEntries++; - } - if (isHighSurrogate(charArray[charIndex])) { - charIndex++; - } - } while (++charIndex < charArray.length); + for (int i = 0; i < destinations.length; i++) { + if (!partOfRange(i)) { + totalEntries++; + } } if (totalEntries < 1) { return; } int remainingEntries = totalEntries; - charIndex = 0; + int index = 0; do { /* Limited to 100 entries in each section */ int entriesThisSection = Math.min(remainingEntries, 100); writer.write(entriesThisSection + " beginbfchar\n"); int sectionEntryCount = 0; do { - /* Go to the next char not in a range */ - while (partOfRange(charArray, charIndex)) { - if (isHighSurrogate(charArray[charIndex])) { - charIndex++; - } - charIndex++; + /* Go to the next selector not in a range */ + while (partOfRange(index)) { + index++; } - - writer.write("<" + padCharIndex(charIndex) + "> "); - - if (isHighSurrogate(charArray[charIndex])) { - char secondChar = 0; // Invalid low surrogate (valid: 0xDC00 - 0xDFFF) - if (charIndex + 1 < charArray.length) { - secondChar = charArray[charIndex + 1]; - } else { - if (eventBroadcaster != null) { - PDFEventProducer pdfEventProducer = PDFEventProducer.Provider.get(eventBroadcaster); - pdfEventProducer.unpairedSurrogate(this); - } - } - writer.write("<" + padHexString(Integer.toHexString(charArray[charIndex]), 4) - + padHexString(Integer.toHexString(secondChar), 4) + ">\n"); - charIndex++; - } else { - writer.write("<" + padHexString(Integer.toHexString(charArray[charIndex]), 4) - + ">\n"); - } - charIndex++; + writer.write("<" + padSelector(index) + "> "); + writer.write("<" + destinationHex(index) + ">\n"); + index++; } while (++sectionEntryCount < entriesThisSection); - remainingEntries -= entriesThisSection; writer.write("endbfchar\n"); } while (remainingEntries > 0); } - private String padCharIndex(int charIndex) { - return padHexString(Integer.toHexString(charIndex), (singleByte ? 2 : 4)); - } - /** - * Writes the entries for character ranges for a base font. - * @param charArray all the characters to map - * @throws IOException + * Writes the entries for selector ranges, in sections of at most 100. + * @throws IOException if an I/O error occurs */ - protected void writeBFRangeEntries(char[] charArray) throws IOException { + protected void writeBFRangeEntries() throws IOException { int totalEntries = 0; - int charIndex = 0; - if (charArray.length > 0) { - do { - if (startOfRange(charArray, charIndex)) { - totalEntries++; - } - if (isHighSurrogate(charArray[charIndex])) { - charIndex++; - } - } while (++charIndex < charArray.length); + for (int i = 0; i < destinations.length; i++) { + if (startOfRange(i)) { + totalEntries++; + } } if (totalEntries < 1) { return; } int remainingEntries = totalEntries; - charIndex = 0; + int index = 0; do { /* Limited to 100 entries in each section */ int entriesThisSection = Math.min(remainingEntries, 100); @@ -208,182 +215,106 @@ protected void writeBFRangeEntries(char[] charArray) throws IOException { int sectionEntryCount = 0; do { /* Go to the next start of a range */ - while (!startOfRange(charArray, charIndex)) { - if (isHighSurrogate(charArray[charIndex])) { - charIndex++; - } - charIndex++; + while (!startOfRange(index)) { + index++; } - writer.write("<" + padCharIndex(charIndex) + "> "); - writer.write("<" - + padCharIndex(endOfRange(charArray, charIndex)) - + "> "); - if (isHighSurrogate(charArray[charIndex])) { - char secondChar = 0; - if (charIndex + 1 < charArray.length) { - secondChar = charArray[charIndex + 1]; - } else { - if (eventBroadcaster != null) { - PDFEventProducer pdfEventProducer = PDFEventProducer.Provider.get(eventBroadcaster); - pdfEventProducer.unpairedSurrogate(this); - } - } - writer.write("<" + padHexString(Integer.toHexString(charArray[charIndex]), 4) - + padHexString(Integer.toHexString(secondChar), 4) - + ">\n"); - } else { - writer.write("<" + padHexString(Integer.toHexString(charArray[charIndex]), 4) - + ">\n"); - } - charIndex++; + writer.write("<" + padSelector(index) + "> "); + writer.write("<" + padSelector(endOfRange(index)) + "> "); + writer.write("<" + destinationHex(index) + ">\n"); + index++; } while (++sectionEntryCount < entriesThisSection); remainingEntries -= entriesThisSection; writer.write("endbfrange\n"); } while (remainingEntries > 0); } + private String padSelector(int index) { + return padHexString(Integer.toHexString(index), (singleByte ? 2 : 4)); + } + /** - * Find the end of the current range. - * @param charArray The array which is being tested. - * @param startOfRange The index to the array element that is the start of - * the range. - * @return The index to the element that is the end of the range. + * The destination of a selector as UTF-16BE hex, four lower-case digits per code unit. + * A lone high surrogate is reported and written with a zero low surrogate, as before. */ - private int endOfRange(char[] charArray, int startOfRange) { - int i = startOfRange; - if (isHighSurrogate(charArray[i])) { - while (i < charArray.length - 3 && sameRangeEntryAsNext(charArray, i)) { - i += 2; - } - } else { - while (i < charArray.length - 1 && sameRangeEntryAsNext(charArray, i)) { - i++; + private String destinationHex(int index) { + String d = destinations[index]; + StringBuilder hex = new StringBuilder(4 * Math.max(1, d.length())); + for (int i = 0; i < d.length(); i++) { + hex.append(padHexString(Integer.toHexString(d.charAt(i)), 4)); + } + if (d.length() == 1 && isHighSurrogate(d.charAt(0))) { + if (eventBroadcaster != null) { + PDFEventProducer pdfEventProducer = PDFEventProducer.Provider.get(eventBroadcaster); + pdfEventProducer.unpairedSurrogate(this); } + hex.append("0000"); } - return i; + return hex.toString(); } /** - * Determine whether this array element should be part of a bfchar entry or - * a bfrange entry. - * @param charArray The array to be tested. - * @param arrayIndex The index to the array element to be tested. - * @return True if this array element should be included in a range. + * The value a destination contributes to a range, or -1 if it can be in no range. Only + * a destination of exactly one code point can: a single non-surrogate code unit, or a + * high surrogate followed by one more unit. Two destinations are consecutive when their + * keys differ by one, which for a pair means the same high surrogate and the next low. */ - private boolean partOfRange(char[] charArray, int arrayIndex) { - int minBytesInRange = 2; - if (isHighSurrogate(charArray[arrayIndex])) { - minBytesInRange = 4; + private long rangeKey(int index) { + String d = destinations[index]; + if (d.length() == 1 && !Character.isSurrogate(d.charAt(0))) { + return d.charAt(0); + } else if (d.length() == 2 && isHighSurrogate(d.charAt(0))) { + return ((long) d.charAt(0) << 16) | d.charAt(1); + } else { + return -1; } - if (charArray.length < minBytesInRange) { + } + + /** + * Determine whether two consecutive selectors can be in the same bfrange entry: both + * destinations are one code point, the second is the next code point, and the two + * selectors are in the same block of 256, since only the low byte may vary in a range. + * @param index the first of the two selectors + * @return true if both are in the same range + */ + private boolean sameRangeEntryAsNext(int index) { + if (index < 0 || index >= destinations.length - 1) { return false; } - if (arrayIndex == 0) { - return sameRangeEntryAsNext(charArray, 0); - } - if (isHighSurrogate(charArray[arrayIndex])) { - if (arrayIndex == charArray.length - 2) { - return sameRangeEntryAsNext(charArray, arrayIndex - 2); - } - } - if (arrayIndex == charArray.length - 1) { - return sameRangeEntryAsNext(charArray, arrayIndex - 1); - } - if (isHighSurrogate(charArray[arrayIndex])) { - if (sameRangeEntryAsNext(charArray, arrayIndex - 2)) { - return true; - } - } - if (sameRangeEntryAsNext(charArray, arrayIndex - 1)) { - return true; - } - if (sameRangeEntryAsNext(charArray, arrayIndex)) { - return true; - } - return false; + long key = rangeKey(index); + return key >= 0 && rangeKey(index + 1) == key + 1 + && index / 256 == (index + 1) / 256; } /** - * Determine whether two code points can be included in the same bfrange entry. - * Range sizes are limited to a maximum of 256 (128 for surrogate pairs). - * @param charArray The array holding the code points to be tested. - * @param firstItem The first char of the first code point in the array to be tested. - * The first byte of the second code point is firstItem + n, where n is the number - * of chars in the firstItem code point. - * @return True if both: - * 1) the next code point in the array is sequential with this one, and - * 2) this code point and the next are both NOT surrogate pairs - * or - * this code point and the next are both surrogate pairs and - * the high-surrogates are the same, and - * 3) the resulting range cannot be greater than 256 in size. + * Determine whether this selector should be part of a bfrange entry rather than a + * bfchar entry. + * @param index the selector + * @return true if it is in a range */ - private boolean sameRangeEntryAsNext(char[] charArray, int firstItem) { - boolean retval = false; - do { - if (firstItem < 0 || firstItem >= charArray.length - 1) { - break; - } - if (isHighSurrogate(charArray[firstItem])) { - if (firstItem < charArray.length - 3) { - if (charArray[firstItem + 2] == charArray[firstItem]) { - if (charArray[firstItem + 3] == charArray[firstItem + 1] + 1) { - if (firstItem / 256 == (firstItem + 2) / 256) { - retval = true; - } - } - } - } - } else { - if (charArray[firstItem] + 1 == charArray[firstItem + 1]) { - if (firstItem / 256 == (firstItem + 1) / 256) { - retval = true; - } - } - } - } while (false); - return retval; + private boolean partOfRange(int index) { + return sameRangeEntryAsNext(index - 1) || sameRangeEntryAsNext(index); } /** - * Determine whether this array element should be the start of a bfrange - * entry. - * @param charArray The array to be tested. - * @param arrayIndex The index to the array element to be tested. - * @return True if this array element is the beginning of a range. + * Determine whether this selector starts a bfrange entry. + * @param index the selector + * @return true if it is the first of a range */ - private boolean startOfRange(char[] charArray, int arrayIndex) { - // Can't be the start of a range if not part of a range. - if (!partOfRange(charArray, arrayIndex)) { - return false; - } - // If part of a range and first element in the array, must be start of a range - if (arrayIndex == 0) { - return true; - } - // If last element in the array, cannot be start of a range - if (isHighSurrogate(charArray[arrayIndex])) { - if (arrayIndex == charArray.length - 2) { - return false; - } - } - if (arrayIndex == charArray.length - 1) { - return false; - } - /* - * If part of same range as the previous element is, cannot be start - * of range. - */ - if (isHighSurrogate(charArray[arrayIndex])) { - if (sameRangeEntryAsNext(charArray, arrayIndex - 2)) { - return false; - } - } - if (sameRangeEntryAsNext(charArray, arrayIndex - 1)) { - return false; + private boolean startOfRange(int index) { + return sameRangeEntryAsNext(index) && !sameRangeEntryAsNext(index - 1); + } + + /** + * Find the end of the range that starts at a selector. + * @param startOfRange the selector that starts the range + * @return the last selector of the range + */ + private int endOfRange(int startOfRange) { + int i = startOfRange; + while (sameRangeEntryAsNext(i)) { + i++; } - // Otherwise, this is start of a range. - return true; + return i; } /** diff --git a/fop-core/src/test/java/org/apache/fop/pdf/PDFToUnicodeCMapTestCase.java b/fop-core/src/test/java/org/apache/fop/pdf/PDFToUnicodeCMapTestCase.java index 70545d327c8..dd1e6f1eecf 100644 --- a/fop-core/src/test/java/org/apache/fop/pdf/PDFToUnicodeCMapTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/pdf/PDFToUnicodeCMapTestCase.java @@ -179,6 +179,10 @@ public void rangeTest() throws IOException { /** * Checks that one surrogate pair is correctly handled, even when it crosses a section boundary. + * The pair is one character selector, so the selector after it is 0x64, not 0x65: the writer + * once numbered selectors by array position, which put every entry after a pair one selector + * too high (FOP-3346; measured on a rendered PDF, where the letter after a supplementary-plane + * character extracted as the letter after that). * @throws IOException */ @Test @@ -201,22 +205,23 @@ public void surrogatePairTest() throws IOException { + "<63> \n" + "endbfchar\n" + "56 beginbfchar\n" - + "<65> <00fc>\n" - + "<66> <00fe>"); + + "<64> <00fc>\n" + + "<65> <00fe>"); configPairs.put(false, "<0060> <00f2>\n" + "<0061> <00f4>\n" + "<0062> <00f6>\n" + "<0063> \n" + "endbfchar\n" + "56 beginbfchar\n" - + "<0065> <00fc>\n" - + "<0066> <00fe>"); + + "<0064> <00fc>\n" + + "<0065> <00fe>"); buildAndAssert(unicodeCharMap, configPairs); } /** - * Checks that a range of surrogate pairs is correctly handled. + * Checks that a range of surrogate pairs is correctly handled. Two pairs are two selectors, + * 9 and 10 (see surrogatePairTest). * @throws IOException */ @Test @@ -236,17 +241,18 @@ public void surrogatePairRangeTest() throws IOException { Map configPairs = new HashMap<>(); configPairs.put(true, "1 beginbfrange\n" - + "<09> <0b> \n" + + "<09> <0a> \n" + "endbfrange"); configPairs.put(false, "1 beginbfrange\n" - + "<0009> <000b> \n" + + "<0009> <000a> \n" + "endbfrange"); buildAndAssert(unicodeCharMap, configPairs); } /** - * Checks that CMap is correct, even when made up of just one range of surrogate pairs. + * Checks that CMap is correct, even when made up of just one range of surrogate pairs. Ten + * pairs are selectors 0 to 9 (see surrogatePairTest). * @throws IOException */ @Test @@ -264,10 +270,10 @@ public void surrogatePairsRangeTest() throws IOException { Map configPairs = new HashMap<>(); configPairs.put(true, "1 beginbfrange\n" - + "<00> <12> \n" + + "<00> <09> \n" + "endbfrange"); configPairs.put(false, "1 beginbfrange\n" - + "<0000> <0012> \n" + + "<0000> <0009> \n" + "endbfrange"); buildAndAssert(unicodeCharMap, configPairs); @@ -351,12 +357,14 @@ public void rangeSizeTest() throws IOException { } /** - * Checks that a range of surrogate pairs is limited in size. + * Checks that a range of surrogate pairs is limited in size: 256 selectors, the same as for + * any other range, since a pair is one selector (see surrogatePairTest). The low surrogates + * start at U+DC00 so that 300 of them stay valid. * @throws IOException */ @Test public void rangeSizeSurrogateTest() throws IOException { - final int charMapSize = 300; + final int charMapSize = 600; char[] unicodeCharMap = new char[charMapSize]; @@ -364,14 +372,14 @@ public void rangeSizeSurrogateTest() throws IOException { unicodeCharMap[i] = '\uD83C'; } for (int i = 0; i < charMapSize / 2; ++i) { - unicodeCharMap[1 + i * 2] = (char)('\uDF65' + i); + unicodeCharMap[1 + i * 2] = (char)('\uDC00' + i); } Map configPairs = new HashMap<>(); - // PDFToUnicodeCMap CTOR rejects unicodeCharMap with > 256 elements where singleByte is true. + // PDFToUnicodeCMap CTOR rejects a map of more than 256 selectors where singleByte is true. configPairs.put(false, "2 beginbfrange\n" - + "<0000> <00fe> \n" - + "<0100> <012a> \n" + + "<0000> <00ff> \n" + + "<0100> <012b> \n" + "endbfrange"); buildAndAssert(unicodeCharMap, configPairs); diff --git a/fop-core/src/test/java/org/apache/fop/pdf/ToUnicodeCharacterisationTestCase.java b/fop-core/src/test/java/org/apache/fop/pdf/ToUnicodeCharacterisationTestCase.java new file mode 100644 index 00000000000..4a144cc6b40 --- /dev/null +++ b/fop-core/src/test/java/org/apache/fop/pdf/ToUnicodeCharacterisationTestCase.java @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* $Id$ */ + +package org.apache.fop.pdf; + +import java.io.CharArrayWriter; +import java.io.IOException; + +import org.junit.Test; +import static org.junit.Assert.assertEquals; + +/** + * Pins the exact ToUnicode CMap this writer produces today. + * + *

These are characterisation tests, not specifications: they record current output so a + * change to the writer has to be deliberate. Every PDF FOP produces gets its text layer + * from here, so a silent change to the range packing would be invisible in rendering and + * visible only to search, copy and paste, and screen readers. FOP-3345 and FOP-3346 touch + * this class. + * + *

If one of these fails, decide whether the new output is correct before updating it. + * Do not update the expectation to make the build pass.

+ */ +public class ToUnicodeCharacterisationTestCase { + + private String cmapOf(char[] chars, boolean singleByte) throws IOException { + PDFToUnicodeCMap cmap = new PDFToUnicodeCMap(chars, PDFCMap.ENC_IDENTITY_H, + new PDFCIDSystemInfo("Adobe", "Identity", 0), singleByte, null); + CharArrayWriter writer = new CharArrayWriter(); + cmap.createCMapBuilder(writer).writeCMap(); + return writer.toString(); + } + + private String body(String cmap) { + int from = cmap.indexOf("endcodespacerange\n"); + int to = cmap.indexOf("endcmap"); + return cmap.substring(from + "endcodespacerange\n".length(), to); + } + + /** Contiguous code points pack into a single range. */ + @Test + public void testContiguousRunBecomesOneRange() throws IOException { + assertEquals("1 beginbfrange\n<0000> <0003> <0041>\nendbfrange\n", + body(cmapOf(new char[] {'A', 'B', 'C', 'D'}, false))); + } + + /** Isolated code points are written one by one. */ + @Test + public void testScatteredCodePointsBecomeChars() throws IOException { + assertEquals("3 beginbfchar\n<0000> <0041>\n<0001> <005a>\n<0002> <0072>\nendbfchar\n", + body(cmapOf(new char[] {'A', 'Z', 'r'}, false))); + } + + /** A surrogate pair is one code point across two array slots, and still ranges. */ + @Test + public void testSurrogatePairIsOneCodePoint() throws IOException { + assertEquals("1 beginbfchar\n<0000> \nendbfchar\n", + body(cmapOf(new char[] {'\uD800', '\uDF00'}, false))); + } + + /** + * The defect FOP-3345 addresses, pinned as it stands: a ligature glyph carries a + * private-use code point, and consecutive ones pack into a range, so the text layer + * says U+E000 upward rather than the letters. + */ + @Test + public void testPrivateUseLigaturesRangeTogetherToday() throws IOException { + assertEquals("1 beginbfrange\n<0000> <0002> \nendbfrange\n", + body(cmapOf(new char[] {'', '', ''}, false))); + } + + /** + * Destinations are written in lower-case hex while the code space is upper-case. Pinned + * because it is a byte-level property of every PDF FOP writes, and easy to change by + * accident when reworking the writer. + */ + @Test + public void testHexCaseIsMixedByDesign() throws IOException { + String cmap = cmapOf(new char[] {'\uABCD'}, false); + assertEquals("1 beginbfchar\n<0000> \nendbfchar\n", body(cmap)); + assertEquals(true, cmap.contains("<0000> ")); + } + + private String cmapOf(String[] destinations) throws IOException { + PDFToUnicodeCMap cmap = new PDFToUnicodeCMap(destinations, PDFCMap.ENC_IDENTITY_H, + new PDFCIDSystemInfo("Adobe", "Identity", 0), false, null); + CharArrayWriter writer = new CharArrayWriter(); + cmap.createCMapBuilder(writer).writeCMap(); + return writer.toString(); + } + + /** + * A glyph standing for several characters publishes them as a string, in a bfchar, + * never in a range; its single-character neighbours still range. + */ + @Test + public void testLigaturePublishesItsLetters() throws IOException { + assertEquals("1 beginbfchar\n<0002> <00660069>\nendbfchar\n" + + "1 beginbfrange\n<0000> <0001> <0066>\nendbfrange\n", + body(cmapOf(new String[] {"f", "g", "fi"}))); + } + + /** + * A surrogate pair is one selector, so the selectors after it do not drift by one as they + * did when the pair occupied two slots of a positional array; the positional constructor + * now gives the same CMap as the per-selector one. + */ + @Test + public void testSelectorsDoNotDriftAfterASurrogatePair() throws IOException { + String expected = "3 beginbfchar\n<0000> <0041>\n<0001> \n<0002> <0042>\nendbfchar\n"; + assertEquals(expected, body(cmapOf(new String[] {"A", "\uD835\uDC00", "B"}))); + assertEquals(expected, body(cmapOf(new char[] {'A', '\uD835', '\uDC00', 'B'}, false))); + } + + /** Consecutive supplementary-plane code points still pack into a range. */ + @Test + public void testSurrogatePairsStillRange() throws IOException { + assertEquals("1 beginbfrange\n<0000> <0001> \nendbfrange\n", + body(cmapOf(new String[] {"\uD835\uDC00", "\uD835\uDC01"}))); + } + + /** An empty destination is written as an empty string. */ + @Test + public void testEmptyDestination() throws IOException { + assertEquals("2 beginbfchar\n<0000> <0041>\n<0001> <>\nendbfchar\n", + body(cmapOf(new String[] {"A", ""}))); + } + + /** Single-byte code space, for the simple-font path. */ + @Test + public void testSingleByteCodeSpace() throws IOException { + assertEquals("1 beginbfrange\n<00> <03> <0041>\nendbfrange\n", + body(cmapOf(new char[] {'A', 'B', 'C', 'D'}, true))); + } +} From 0c8a3db6c37d776e8c9d2d5309421919ee2dd669 Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Thu, 17 Sep 2026 15:23:48 +1000 Subject: [PATCH 2/4] FOP-3340: A CJK ideograph sharing a glyph with a Kangxi radical is written to ToUnicode as the radical With complex script features enabled, CJK text is painted correctly but extracts, searches and reads out as the Kangxi radicals which share the ideographs' glyphs. One FO, one font (Source Han Sans CN), stock FOP, only the complex-script flag changed: features on 0003=U+2F63 0004=U+2F45 0005=U+2F08 0006=U+FF0C 0007=U+724B features off 0003=U+751F 0004=U+65B9 0005=U+4EBA 0006=U+FF0C 0007=U+724B The glyphs drawn are identical either way - glyph 3,4,5,6,7 at x=0,12,24,36,48, advance 1 each. U+724B, which no radical shares, is right either way: the control. MultiByteFont.performSubstitution runs the font's layout tables over characters - chars to glyphs, GSUB, then mapGlyphsToChars - and mapGlyphsToChars takes each glyph's character from findCharacterFromGlyphIndex, which returns the first code point in the cmap mapped to that glyph ("if more than one correspondence exists, then the first one is returned"). A CJK font maps a radical and the ideograph it is the radical of to one glyph: in Source Han Sans CN, U+2F63 and U+751F are both glyph 18742, U+2F45 and U+65B9 are both 14819, U+2F08 and U+4EBA are both 8966. The radical is the lower code point, so the reverse lookup hands back the radical, and that is what reaches the PDF's ToUnicode. mapGlyphsToChars now prefers the character the glyph came from, which the GlyphSequence already carries in its CharAssociation, wherever the substitution left the glyph alone - the association covers exactly one character and the character map maps that character to this same glyph. Anything the substitution did produce, a ligature or a glyph with no character of its own, still goes through findCharacterFromGlyphIndex as before, and a supplementary plane character still comes back as its surrogate pair. MultiByteFontTestCase covers all four cases against a hand-built character map holding the three radical/ideograph pairs above, so it needs no font installed. Without the fix its first case fails with the radicals for the ideographs, which is the defect above. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ae8dWi7jSKijUffjpHWdr4 --- .../org/apache/fop/fonts/MultiByteFont.java | 41 ++++- .../fop/fonts/MultiByteFontTestCase.java | 140 ++++++++++++++++++ 2 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java diff --git a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java index 959c4c10d22..9b16796cb49 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java @@ -699,6 +699,40 @@ private GlyphSequence mapCharsToGlyphs(CharSequence cs, List associations) { return new GlyphSequence(cb, gb, associations); } + /** + * Obtain the character that produced the glyph at index I of glyph sequence GS, but only if + * substitution left that glyph alone, i.e., the glyph is associated with exactly one character + * and the font's character map maps that character to this same glyph. In a CJK font a glyph is + * commonly shared by an ideograph and by the Kangxi radical (or CJK radical supplement) form of + * that ideograph, in which case the reverse lookup made by findCharacterFromGlyphIndex() returns + * the radical, it being the lower code point; keeping the originating character instead prevents + * the radical from reaching the output character sequence. + * @param gs a GlyphSequence containing glyph indices + * @param i index of glyph in glyph sequence + * @param ca character array underlying glyph sequence + * @param nc number of characters in character array + * @param gi glyph index of the glyph at index I + * @return unicode scalar value of the originating character, or zero if not applicable + */ + private int findUnsubstitutedCharacter(GlyphSequence gs, int i, int[] ca, int nc, int gi) { + if (gi == SingleByteEncoding.NOT_FOUND_CODE_POINT) { + return 0; + } + CharAssociation a = gs.getAssociation(i); + if ((a == null) || (a.getCount() != 1)) { + return 0; + } + int s = a.getStart(); + if ((s < 0) || (s >= nc) || (s >= ca.length)) { + return 0; + } + int cc = ca [ s ]; + if ((cc == 0) || (findGlyphIndex(cc) != gi)) { + return 0; + } + return cc; + } + /** * Map sequence GS, comprising a sequence of Glyph Indices, to output sequence CS, * comprising a sequence of UTF-16 encoded Unicode Code Points. @@ -709,10 +743,15 @@ private CharSequence mapGlyphsToChars(GlyphSequence gs) { int ng = gs.getGlyphCount(); int ccMissing = Typeface.NOT_FOUND; List chars = new ArrayList(gs.getUTF16CharacterCount()); + int[] ca = gs.getCharacterArray(false); + int nc = gs.getCharacterCount(); for (int i = 0, n = ng; i < n; i++) { int gi = gs.getGlyph(i); - int cc = findCharacterFromGlyphIndex(gi); + int cc = findUnsubstitutedCharacter(gs, i, ca, nc, gi); + if (cc == 0) { + cc = findCharacterFromGlyphIndex(gi); + } if ((cc == 0) || (cc > 0x10FFFF)) { cc = ccMissing; log.warn("Unable to map glyph index " + gi diff --git a/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java b/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java new file mode 100644 index 00000000000..ca27b630123 --- /dev/null +++ b/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java @@ -0,0 +1,140 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* $Id$ */ + +package org.apache.fop.fonts; + +import java.nio.IntBuffer; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.fop.complexscripts.fonts.GlyphSubstitutionTable; +import org.apache.fop.complexscripts.util.CharAssociation; +import org.apache.fop.complexscripts.util.GlyphSequence; + +/** + * Tests the reverse mapping of glyphs to characters performed at the end of + * {@link MultiByteFont#performSubstitution}. + */ +public class MultiByteFontTestCase { + + /** glyph shared by U+2F08 (Kangxi radical) and U+4EBA (ideograph) */ + private static final int GI_REN = 8966; + /** glyph shared by U+2F45 (Kangxi radical) and U+65B9 (ideograph) */ + private static final int GI_FANG = 14819; + /** glyph shared by U+2F63 (Kangxi radical) and U+751F (ideograph) */ + private static final int GI_SHENG = 18742; + /** glyph of U+724B, an ideograph no radical shares */ + private static final int GI_JIAN = 29000; + /** glyph of U+2000B, a supplementary plane ideograph */ + private static final int GI_SUPPLEMENTARY = 40000; + + /** + * A CJK font maps a Kangxi radical and the ideograph it is the radical of to one glyph, + * as Source Han Sans CN does for the three pairs used here. + */ + private MultiByteFont createFont() { + MultiByteFont font = new MultiByteFont(null, null); + font.setCMap(new CMapSegment[] { + new CMapSegment(0x2F08, 0x2F08, GI_REN), + new CMapSegment(0x2F45, 0x2F45, GI_FANG), + new CMapSegment(0x2F63, 0x2F63, GI_SHENG), + new CMapSegment(0x4EBA, 0x4EBA, GI_REN), + new CMapSegment(0x65B9, 0x65B9, GI_FANG), + new CMapSegment(0x724B, 0x724B, GI_JIAN), + new CMapSegment(0x751F, 0x751F, GI_SHENG), + new CMapSegment(0x2000B, 0x2000B, GI_SUPPLEMENTARY) + }); + return font; + } + + private GlyphSubstitutionTable mockGSUB(Answer substitution) { + GlyphSubstitutionTable gsub = mock(GlyphSubstitutionTable.class); + when(gsub.preProcess(any(CharSequence.class), anyString(), any(MultiByteFont.class), + any(List.class))).thenAnswer(new Answer() { + public CharSequence answer(InvocationOnMock invocation) { + return (CharSequence) invocation.getArguments()[0]; + } + }); + when(gsub.substitute(any(GlyphSequence.class), anyString(), anyString())).thenAnswer(substitution); + return gsub; + } + + /** A substitution which leaves every glyph of the sequence alone. */ + private static class IdentityAnswer implements Answer { + public GlyphSequence answer(InvocationOnMock invocation) { + return (GlyphSequence) invocation.getArguments()[0]; + } + } + + private CharSequence substitute(MultiByteFont font, String text) { + return font.performSubstitution(text, "hani", "dflt", new ArrayList(), false); + } + + /** + * An ideograph whose glyph is shared with a Kangxi radical must come back as the ideograph, + * not as the radical, which is merely the lower of the two code points mapped to that glyph. + */ + @Test + public void testIdeographSharingGlyphWithRadical() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(new IdentityAnswer())); + assertEquals("生方人牋", substitute(font, "生方人牋").toString()); + } + + /** A radical which really was written stays a radical. */ + @Test + public void testRadicalItself() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(new IdentityAnswer())); + assertEquals("⽣⽅⼈", substitute(font, "⽣⽅⼈").toString()); + } + + /** A glyph the substitution did produce is still mapped back through the character map. */ + @Test + public void testSubstitutedGlyphUsesCharacterMap() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(new Answer() { + public GlyphSequence answer(InvocationOnMock invocation) { + GlyphSequence gs = (GlyphSequence) invocation.getArguments()[0]; + List associations = new ArrayList(); + associations.add(new CharAssociation(0, gs.getCharacterCount())); + return new GlyphSequence(gs.getCharacters(), IntBuffer.wrap(new int[] {GI_JIAN}), + associations); + } + })); + assertEquals("牋", substitute(font, "生方").toString()); + } + + /** A supplementary plane character still comes back as its surrogate pair. */ + @Test + public void testSupplementaryPlaneCharacter() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(new IdentityAnswer())); + assertEquals("𠀋", substitute(font, "𠀋").toString()); + } +} From 1590a009922a50edd2e3a060fce7abdb71b195ee Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Sat, 3 Oct 2026 07:25:32 +1000 Subject: [PATCH 3/4] FOP-3345: A substituted glyph's ToUnicode entry is the characters it stands for MultiByteFont.performSubstitution maps characters to glyphs, runs GSUB, and maps the glyphs back to characters, because layout works on characters. A glyph that substitution produced has no character of its own unless the font's cmap happens to map one to it, so mapGlyphsToChars minted a private-use code point for it (createPrivateUseMapping, from U+E000). The painter hands that code point to the subset, and the ToUnicode CMap is built from the subset's characters. So wherever a ligature, a contextual form or a decomposition was applied, the PDF's text layer said U+E000 upward, or a presentation form, rather than the letters: search, copy and paste and screen readers lose the text, and nothing in the rendering shows it. Measured on this branch, text extracted with pdftotext: Carlito, "fifty ti office" before U+FB01 U+E000 "y " U+E001 " o" U+FB03 "ce" after "fifty ti office" Noto Sans Arabic, U+0639 U+0644 U+064A U+0643 U+0645 before U+FEDC U+FEE2 U+E001 U+E000 U+FECB U+FEE0 (presentation forms, private use) after U+0643 U+0645 U+E001 U+0639 U+0644 U+064A Every word box is identical before and after: the code point used for layout is unchanged, so no glyph, advance or position moves. The association each glyph carries out of substitution already names the characters it came from. MultiByteFont now records them per glyph (getGlyphMeaning) as it maps glyphs back, CIDSet.getUnicodeSequences returns one string per selector (the recorded characters, or the selector's own code point), and PDFFactory builds the CMap from those, which the writer of FOP-3346 accepts: a ligature glyph publishes its letters as a bfchar with a string destination, an Arabic contextual form its letter. What it deliberately does not do. The second and later glyphs that a decomposition produces from one character keep their private-use code point (the U+E001 above), because a CMap cannot say that several glyphs share one character, and an empty destination reads as U+FFFD, a space or a raw control character depending on the reader. A glyph seen with two different meanings in one document publishes neither. And the stand-in glyph drawn for a missing character records no meaning. The private-use mapping itself is untouched, so MultiByteFont.hasPrivateUseSubstitutions() (FOP-3337, AFP) answers as before. This branch carries two other fixes it is built on: FOP-3346 (the CMap writer takes one destination per selector) and FOP-3340 (an unsubstituted glyph maps back to the character it came from), each with its own pull request. MultiByteFontTestCase: a ligature records its characters, a contextual form its character, an unsubstituted glyph nothing, the second glyph of a one-character cluster nothing, a glyph with two meanings neither, the missing-character stand-in nothing. Co-Authored-By: Claude Fable 5.1 --- .../java/org/apache/fop/fonts/CIDFull.java | 12 ++ .../java/org/apache/fop/fonts/CIDSet.java | 10 ++ .../java/org/apache/fop/fonts/CIDSubset.java | 10 ++ .../org/apache/fop/fonts/MultiByteFont.java | 99 ++++++++++++++ .../java/org/apache/fop/pdf/PDFFactory.java | 2 +- .../fop/fonts/MultiByteFontTestCase.java | 129 +++++++++++++++++- 6 files changed, 260 insertions(+), 2 deletions(-) diff --git a/fop-core/src/main/java/org/apache/fop/fonts/CIDFull.java b/fop-core/src/main/java/org/apache/fop/fonts/CIDFull.java index 9d5184b026f..b291a30edcd 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/CIDFull.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/CIDFull.java @@ -107,6 +107,18 @@ public char[] getChars() { return font.getChars(); } + /** {@inheritDoc} */ + public String[] getUnicodeSequences() { + String[] sequences = org.apache.fop.pdf.PDFToUnicodeCMap.toDestinations(font.getChars()); + for (int gi = 0; gi < sequences.length; gi++) { + String meaning = font.getGlyphMeaning(gi); + if (meaning != null) { + sequences[gi] = meaning; + } + } + return sequences; + } + /** {@inheritDoc} */ public int getNumberOfGlyphs() { initGlyphIndices(); diff --git a/fop-core/src/main/java/org/apache/fop/fonts/CIDSet.java b/fop-core/src/main/java/org/apache/fop/fonts/CIDSet.java index d89c8937bdf..9d05f7fa2c1 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/CIDSet.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/CIDSet.java @@ -90,6 +90,16 @@ public interface CIDSet { */ char[] getChars(); + /** + * Returns the text each character selector stands for, one string per selector in + * selector order, for the ToUnicode CMap. Usually the one code point {@link #getUnicode} + * gives; for a glyph that substitution produced from other characters, those characters, + * so a ligature glyph reads as its letters; empty where a neighbouring glyph carries the + * text. + * @return one string per character selector + */ + String[] getUnicodeSequences(); + /** * Returns the number of glyphs in the subset. * @return the number of glyphs in the subset diff --git a/fop-core/src/main/java/org/apache/fop/fonts/CIDSubset.java b/fop-core/src/main/java/org/apache/fop/fonts/CIDSubset.java index 470e59a3753..117ef3f8b7e 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/CIDSubset.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/CIDSubset.java @@ -144,6 +144,16 @@ public char[] getChars() { return buf.toString().toCharArray(); } + /** {@inheritDoc} */ + public String[] getUnicodeSequences() { + String[] sequences = new String[usedGlyphsCount]; + for (int i = 0; i < usedGlyphsCount; i++) { + String meaning = (font == null) ? null : font.getGlyphMeaning(getOriginalGlyphIndex(i)); + sequences[i] = (meaning != null) ? meaning : new String(Character.toChars(getUnicode(i))); + } + return sequences; + } + /** {@inheritDoc} */ public int getNumberOfGlyphs() { return this.usedGlyphsCount; diff --git a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java index 9b16796cb49..fcf22d2c99d 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java @@ -25,7 +25,9 @@ import java.nio.CharBuffer; import java.nio.IntBuffer; import java.util.ArrayList; +import java.util.Arrays; import java.util.BitSet; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -68,6 +70,15 @@ public class MultiByteFont extends CIDFont implements Substitutable, Positionabl private GlyphSubstitutionTable gsub; private GlyphPositioningTable gpos; + /** + * The text a substituted glyph stands for, by glyph index, recorded by mapGlyphsToChars for + * the ToUnicode CMap: the characters of the glyph's association, so a ligature reads as its + * letters and an Arabic contextual form as its letter. A null value means the glyph has no + * one meaning, because it was seen with different associations or as the second or later + * glyph of one character, and publishes its identity code point as before. + */ + private Map glyphMeanings = new HashMap(); + /* dynamic private use (character) mappings */ private int numMapped; private int numUnmapped; @@ -733,6 +744,93 @@ private int findUnsubstitutedCharacter(GlyphSequence gs, int i, int[] ca, int nc return cc; } + /** + * The text the glyph at a glyph index stands for, if substitution gave it one. + * @param glyphIndex the glyph index in the font + * @return the UTF-16 text, or null to publish the glyph's own code point + */ + String getGlyphMeaning(int glyphIndex) { + return glyphMeanings.get(glyphIndex); + } + + /** + * Record what a substituted glyph stands for, from the association substitution left on + * it. A glyph that is the second or later output of one character (a multiple substitution + * replicates the association onto each output) gets no meaning, since a ToUnicode entry + * cannot say that several glyphs share one character; a glyph seen with two different + * meanings gets none, since one entry cannot carry both. Either is final for the glyph. + * The stand-in glyph drawn for a character the font lacks (Typeface.NOT_FOUND) never + * gets one: it is not the character, and the text layer must go on saying so. + * @param gs a GlyphSequence containing glyph indices + * @param i index of glyph in glyph sequence + * @param ca character array underlying glyph sequence + * @param nc number of characters in character array + * @param gi glyph index of the glyph at index I + */ + private void recordGlyphMeaning(GlyphSequence gs, int i, int[] ca, int nc, int gi) { + if ((gi == SingleByteEncoding.NOT_FOUND_CODE_POINT) || (gi == findGlyphIndex(Typeface.NOT_FOUND))) { + // the stand-in drawn for a character the font lacks is not that character + return; + } + CharAssociation a = gs.getAssociation(i); + if ((a == null) || (a.getCount() <= 0)) { + return; + } + if ((i > 0) && sameAssociation(a, gs.getAssociation(i - 1))) { + glyphMeanings.put(gi, null); + return; + } + String meaning = associationText(a, ca, nc); + if (meaning == null) { + return; + } + if (!glyphMeanings.containsKey(gi)) { + glyphMeanings.put(gi, meaning); + } else if (!meaning.equals(glyphMeanings.get(gi))) { + glyphMeanings.put(gi, null); + } + } + + private static boolean sameAssociation(CharAssociation a, CharAssociation b) { + return (b != null) && (a.getOffset() == b.getOffset()) && (a.getCount() == b.getCount()) + && Arrays.equals(a.getSubIntervals(), b.getSubIntervals()); + } + + /** + * The characters an association covers, as UTF-16 text; a disjoint association (a ligature + * whose components had ignored marks between them) contributes its sub-intervals only, the + * marks staying with their own glyphs. + * @return the text, or null if the association does not lie within the character array + */ + private static String associationText(CharAssociation a, int[] ca, int nc) { + StringBuilder sb = new StringBuilder(); + if (a.isDisjoint()) { + int[] si = a.getSubIntervals(); + for (int k = 0; k + 1 < si.length; k += 2) { + if (!appendCharacters(sb, ca, nc, si[k], si[k + 1])) { + return null; + } + } + } else if (!appendCharacters(sb, ca, nc, a.getStart(), a.getEnd())) { + return null; + } + return (sb.length() > 0) ? sb.toString() : null; + } + + private static boolean appendCharacters(StringBuilder sb, int[] ca, int nc, int start, int end) { + if ((start < 0) || (start >= end) || (end > nc) || (end > ca.length)) { + return false; + } + for (int k = start; k < end; k++) { + int cc = ca[k]; + if ((cc <= 0) || (cc > 0x10FFFF)) { + return false; + } + sb.appendCodePoint(cc); + } + return true; + } + /** * Map sequence GS, comprising a sequence of Glyph Indices, to output sequence CS, * comprising a sequence of UTF-16 encoded Unicode Code Points. @@ -750,6 +848,7 @@ private CharSequence mapGlyphsToChars(GlyphSequence gs) { int gi = gs.getGlyph(i); int cc = findUnsubstitutedCharacter(gs, i, ca, nc, gi); if (cc == 0) { + recordGlyphMeaning(gs, i, ca, nc, gi); cc = findCharacterFromGlyphIndex(gi); } if ((cc == 0) || (cc > 0x10FFFF)) { diff --git a/fop-core/src/main/java/org/apache/fop/pdf/PDFFactory.java b/fop-core/src/main/java/org/apache/fop/pdf/PDFFactory.java index 13404bb7ba1..266674d5ea3 100644 --- a/fop-core/src/main/java/org/apache/fop/pdf/PDFFactory.java +++ b/fop-core/src/main/java/org/apache/fop/pdf/PDFFactory.java @@ -1008,7 +1008,7 @@ public PDFFont makeFont(String fontname, String basefont, throw new RuntimeException(e); } } else { - cmap = new PDFToUnicodeCMap(cidMetrics.getCIDSet().getChars(), "fop-ucs-H", + cmap = new PDFToUnicodeCMap(cidMetrics.getCIDSet().getUnicodeSequences(), "fop-ucs-H", new PDFCIDSystemInfo("Adobe", "Identity", 0), false, eventBroadcaster); } getDocument().registerObject(cmap); diff --git a/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java b/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java index ca27b630123..66c93527217 100644 --- a/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java @@ -67,7 +67,8 @@ private MultiByteFont createFont() { new CMapSegment(0x65B9, 0x65B9, GI_FANG), new CMapSegment(0x724B, 0x724B, GI_JIAN), new CMapSegment(0x751F, 0x751F, GI_SHENG), - new CMapSegment(0x2000B, 0x2000B, GI_SUPPLEMENTARY) + new CMapSegment(0x2000B, 0x2000B, GI_SUPPLEMENTARY), + new CMapSegment(Typeface.NOT_FOUND, Typeface.NOT_FOUND, GI_NOT_FOUND) }); return font; } @@ -130,6 +131,132 @@ public GlyphSequence answer(InvocationOnMock invocation) { assertEquals("牋", substitute(font, "生方").toString()); } + /** the glyph of Typeface.NOT_FOUND, '#', drawn for a character the font lacks */ + private static final int GI_NOT_FOUND = 3; + + /** a glyph no character maps to, as a ligature glyph usually is */ + private static final int GI_LIGATURE = 50000; + /** a second such glyph */ + private static final int GI_FORM = 50001; + /** a third, a mark glyph a decomposition splits off */ + private static final int GI_MARK = 50002; + + /** A substitution that maps the whole input to the given glyphs, with the given associations. */ + private static Answer substitutionTo(final int[] glyphs, final CharAssociation... associations) { + return new Answer() { + public GlyphSequence answer(InvocationOnMock invocation) { + GlyphSequence gs = (GlyphSequence) invocation.getArguments()[0]; + List list = new ArrayList(); + for (CharAssociation a : associations) { + list.add(a); + } + return new GlyphSequence(gs.getCharacters(), IntBuffer.wrap(glyphs), list); + } + }; + } + + /** + * A ligature glyph, produced from two characters and mapped by none, still comes back + * as one private-use character for layout, but records the two characters as what it stands + * for, which is what the ToUnicode CMap publishes. + */ + @Test + public void testLigatureGlyphRecordsItsCharacters() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_LIGATURE}, new CharAssociation(0, 2)))); + CharSequence out = substitute(font, "生方"); + assertEquals(1, out.length()); + assertEquals(0xE000, out.charAt(0)); + assertEquals("生方", font.getGlyphMeaning(GI_LIGATURE)); + } + + /** A contextual form, one character to one unmapped glyph, records that character. */ + @Test + public void testContextualFormRecordsItsCharacter() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_FORM}, new CharAssociation(0, 1)))); + substitute(font, "生"); + assertEquals("生", font.getGlyphMeaning(GI_FORM)); + } + + /** A glyph substitution left alone records nothing; its character map entry is its meaning. */ + @Test + public void testUnsubstitutedGlyphRecordsNothing() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(new IdentityAnswer())); + substitute(font, "人"); + assertEquals(null, font.getGlyphMeaning(GI_REN)); + } + + /** + * A decomposition puts one character's association on each glyph it produces. The first + * glyph records the character; the second records nothing, since a ToUnicode entry cannot + * say that two glyphs share one character, and keeps its private-use code point. + */ + @Test + public void testSecondGlyphOfOneCharacterRecordsNothing() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_FORM, GI_MARK}, + new CharAssociation(0, 1), new CharAssociation(0, 1)))); + substitute(font, "生"); + assertEquals("生", font.getGlyphMeaning(GI_FORM)); + assertEquals(null, font.getGlyphMeaning(GI_MARK)); + } + + /** A glyph seen standing for two different characters has no one meaning, and records none. */ + @Test + public void testGlyphWithTwoMeaningsRecordsNone() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_FORM}, new CharAssociation(0, 1)))); + substitute(font, "生"); + assertEquals("生", font.getGlyphMeaning(GI_FORM)); + substitute(font, "方"); + assertEquals(null, font.getGlyphMeaning(GI_FORM)); + } + + /** + * A ligature whose components had an ignored glyph between them carries a disjoint + * association; it records its components only, the glyph between keeping its own. + */ + @Test + public void testDisjointAssociationRecordsItsComponentsOnly() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_LIGATURE, GI_JIAN}, + new CharAssociation(new int[] {0, 1, 2, 3}), new CharAssociation(1, 1)))); + substitute(font, "生牋方"); + assertEquals("生方", font.getGlyphMeaning(GI_LIGATURE)); + assertEquals(null, font.getGlyphMeaning(GI_JIAN)); + } + + /** + * A character the font lacks is drawn with the stand-in glyph of Typeface.NOT_FOUND. That + * glyph must not record the missing character as its meaning: it is not that character, + * and a real '#' in the same document uses the same glyph. + */ + @Test + public void testStandInForMissingCharacterRecordsNothing() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(new IdentityAnswer())); + assertEquals("#", substitute(font, "\uF0A7").toString()); + assertEquals(null, font.getGlyphMeaning(GI_NOT_FOUND)); + } + + /** The subset publishes the recorded characters for the glyph's selector, and the code point otherwise. */ + @Test + public void testSubsetPublishesTheRecordedCharacters() { + MultiByteFont font = createFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_LIGATURE}, new CharAssociation(0, 2)))); + CharSequence out = substitute(font, "生方"); + CIDSubset subset = new CIDSubset(font); + subset.mapCodePoint(GI_JIAN, 0x724B); + subset.mapCodePoint(GI_LIGATURE, out.charAt(0)); + String[] sequences = subset.getUnicodeSequences(); + assertEquals(3, sequences.length); + assertEquals("\uFFFF", sequences[0]); + assertEquals("牋", sequences[1]); + assertEquals("生方", sequences[2]); + } + /** A supplementary plane character still comes back as its surrogate pair. */ @Test public void testSupplementaryPlaneCharacter() { From 6968306523fc6c487ec0c109fa43bc15d1d3f4d2 Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Mon, 5 Oct 2026 11:46:09 +1100 Subject: [PATCH 4/4] FOP-3345: a decomposition's glyphs each publish their piece, not the precomposed letter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first glyph of a multiple substitution recorded the source character for ToUnicode. A font whose ccmp decomposes precomposed letters (Cambria Regular: ά into alpha and a tonos mark, ü into u and uni0308) then had its base glyph recorded as the precomposed letter, and since a plain letter reaches the same glyph through the cmap and records nothing, every plain α, o, e, u, c and A drawn with that glyph was published accented. Where one character is split into exactly as many glyphs as its canonical decomposition (NFD) has characters, each glyph now records its piece in order: the base its plain letter and the mark its combining character, even a mark no character maps to. Any other split keeps the first-glyph rule, so an Arabic letter drawn as a dotless base and its dots (no canonical decomposition) is unchanged. Tests: MultiByteFontTestCase, three cases; the two decomposition cases fail without the change. Co-Authored-By: Claude Opus 5.5 --- .../org/apache/fop/fonts/MultiByteFont.java | 74 +++++++++++++++---- .../fop/fonts/MultiByteFontTestCase.java | 74 +++++++++++++++++++ 2 files changed, 135 insertions(+), 13 deletions(-) diff --git a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java index fcf22d2c99d..c930f8f5697 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java @@ -24,6 +24,7 @@ import java.nio.Buffer; import java.nio.CharBuffer; import java.nio.IntBuffer; +import java.text.Normalizer; import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; @@ -73,9 +74,11 @@ public class MultiByteFont extends CIDFont implements Substitutable, Positionabl /** * The text a substituted glyph stands for, by glyph index, recorded by mapGlyphsToChars for * the ToUnicode CMap: the characters of the glyph's association, so a ligature reads as its - * letters and an Arabic contextual form as its letter. A null value means the glyph has no - * one meaning, because it was seen with different associations or as the second or later - * glyph of one character, and publishes its identity code point as before. + * letters and an Arabic contextual form as its letter. Where one character is split into as + * many glyphs as its canonical decomposition has characters, each glyph records its own piece + * of the decomposition. A null value means the glyph has no one meaning, because it was seen + * with different associations or as the second or later glyph of one character, and publishes + * its identity code point as before. */ private Map glyphMeanings = new HashMap(); @@ -755,10 +758,15 @@ String getGlyphMeaning(int glyphIndex) { /** * Record what a substituted glyph stands for, from the association substitution left on - * it. A glyph that is the second or later output of one character (a multiple substitution - * replicates the association onto each output) gets no meaning, since a ToUnicode entry - * cannot say that several glyphs share one character; a glyph seen with two different - * meanings gets none, since one entry cannot carry both. Either is final for the glyph. + * it. Where a multiple substitution splits one character into as many glyphs as the + * character's canonical decomposition has characters (a font's ccmp decomposing a precomposed + * letter into its base and a combining mark), each glyph records its own piece: the base its + * letter and the mark its combining character, so the base glyph, which plain letters use too, + * is not published as the precomposed letter. Otherwise a glyph that is the second or later + * output of one character (a multiple substitution replicates the association onto each + * output) gets no meaning, since a ToUnicode entry cannot say that several glyphs share one + * character; a glyph seen with two different meanings gets none, since one entry cannot carry + * both. Either is final for the glyph. * The stand-in glyph drawn for a character the font lacks (Typeface.NOT_FOUND) never * gets one: it is not the character, and the text layer must go on saying so. * @param gs a GlyphSequence containing glyph indices @@ -776,13 +784,16 @@ private void recordGlyphMeaning(GlyphSequence gs, int i, int[] ca, int nc, int g if ((a == null) || (a.getCount() <= 0)) { return; } - if ((i > 0) && sameAssociation(a, gs.getAssociation(i - 1))) { - glyphMeanings.put(gi, null); - return; - } - String meaning = associationText(a, ca, nc); + String meaning = decompositionPiece(gs, i, a, ca, nc); if (meaning == null) { - return; + if ((i > 0) && sameAssociation(a, gs.getAssociation(i - 1))) { + glyphMeanings.put(gi, null); + return; + } + meaning = associationText(a, ca, nc); + if (meaning == null) { + return; + } } if (!glyphMeanings.containsKey(gi)) { glyphMeanings.put(gi, meaning); @@ -791,6 +802,43 @@ private void recordGlyphMeaning(GlyphSequence gs, int i, int[] ca, int nc, int g } } + /** + * The piece of one character's canonical decomposition that the glyph at index I stands for, + * where substitution split that character into exactly as many glyphs as the decomposition has + * characters, in order: Cambria's ccmp gives U+00E0 as a and U+0300, U+03AC as alpha and a tonos + * mark. The glyphs carry one association between them, which is how a multiple substitution + * leaves them. + * @return the decomposition's character for this glyph, or null where the glyphs are not such a + * split (a ligature, a single substitution, or a decomposition the font draws in more or fewer + * glyphs than Unicode's, such as an Arabic letter drawn as a dotless base and its dots) + */ + private static String decompositionPiece(GlyphSequence gs, int i, CharAssociation a, int[] ca, int nc) { + if (a.getCount() != 1 || a.isDisjoint()) { + return null; + } + int first = i; + while ((first > 0) && sameAssociation(a, gs.getAssociation(first - 1))) { + first--; + } + int end = i + 1; + while ((end < gs.getGlyphCount()) && sameAssociation(a, gs.getAssociation(end))) { + end++; + } + if (end - first < 2) { + return null; + } + int s = a.getStart(); + if ((s < 0) || (s >= nc) || (s >= ca.length) || (ca[s] <= 0) || (ca[s] > 0x10FFFF)) { + return null; + } + String nfd = Normalizer.normalize(new String(Character.toChars(ca[s])), Normalizer.Form.NFD); + if (nfd.codePointCount(0, nfd.length()) != end - first) { + return null; + } + int at = nfd.offsetByCodePoints(0, i - first); + return new String(Character.toChars(nfd.codePointAt(at))); + } + private static boolean sameAssociation(CharAssociation a, CharAssociation b) { return (b != null) && (a.getOffset() == b.getOffset()) && (a.getCount() == b.getCount()) && Arrays.equals(a.getSubIntervals(), b.getSubIntervals()); diff --git a/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java b/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java index 66c93527217..607c45c97ad 100644 --- a/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/fonts/MultiByteFontTestCase.java @@ -264,4 +264,78 @@ public void testSupplementaryPlaneCharacter() { font.setGSUB(mockGSUB(new IdentityAnswer())); assertEquals("𠀋", substitute(font, "𠀋").toString()); } + + /** glyphs of a font whose ccmp decomposes precomposed letters, as Cambria Regular's does */ + private static final int GI_BASE_A = 60; + /** U+0300, a combining grave accent */ + private static final int GI_GRAVE = 61; + /** U+03B1, Greek small alpha */ + private static final int GI_ALPHA = 62; + /** a tonos mark no character maps to, as Cambria's glyph00646 */ + private static final int GI_TONOS = 63; + + private MultiByteFont createDecomposingFont() { + MultiByteFont font = new MultiByteFont(null, null); + font.setCMap(new CMapSegment[] { + new CMapSegment('a', 'a', GI_BASE_A), + new CMapSegment(0x0300, 0x0300, GI_GRAVE), + new CMapSegment(0x03B1, 0x03B1, GI_ALPHA), + new CMapSegment(Typeface.NOT_FOUND, Typeface.NOT_FOUND, GI_NOT_FOUND) + }); + return font; + } + + /** + * A precomposed letter split into a base and a mark, as many glyphs as its canonical + * decomposition has characters, gives each glyph its piece. The base glyph, which a plain + * letter uses too, records the plain letter, not the precomposed one, and the mark its + * combining character. + */ + @Test + public void testDecompositionGivesEachGlyphItsPiece() { + MultiByteFont font = createDecomposingFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_BASE_A, GI_GRAVE}, + new CharAssociation(0, 1), new CharAssociation(0, 1)))); + substitute(font, "\u00E0"); + assertEquals("a", font.getGlyphMeaning(GI_BASE_A)); + assertEquals("\u0300", font.getGlyphMeaning(GI_GRAVE)); + } + + /** + * A mark glyph no character maps to takes its combining character from the + * decomposition, and the subset publishes the base glyph as the plain letter, so a plain alpha + * elsewhere in the document reads as alpha, not as the alpha with tonos the font split. + */ + @Test + public void testDecomposedBaseIsPublishedAsThePlainLetter() { + MultiByteFont font = createDecomposingFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_ALPHA, GI_TONOS}, + new CharAssociation(0, 1), new CharAssociation(0, 1)))); + CharSequence out = substitute(font, "\u03AC"); + assertEquals("\u03B1", font.getGlyphMeaning(GI_ALPHA)); + assertEquals("\u0301", font.getGlyphMeaning(GI_TONOS)); + CIDSubset subset = new CIDSubset(font); + subset.mapCodePoint(GI_ALPHA, out.charAt(0)); + subset.mapCodePoint(GI_TONOS, out.charAt(1)); + String[] sequences = subset.getUnicodeSequences(); + assertEquals(3, sequences.length); + assertEquals("\u03B1", sequences[1]); + assertEquals("\u0301", sequences[2]); + } + + /** + * A split into more glyphs than the canonical decomposition has characters is not taken as + * the decomposition: the first recording the character and the + * others nothing. + */ + @Test + public void testSplitUnlikeTheDecompositionKeepsTheFirstGlyphRule() { + MultiByteFont font = createDecomposingFont(); + font.setGSUB(mockGSUB(substitutionTo(new int[] {GI_BASE_A, GI_GRAVE, GI_MARK}, + new CharAssociation(0, 1), new CharAssociation(0, 1), new CharAssociation(0, 1)))); + substitute(font, "\u00E0"); + assertEquals("\u00E0", font.getGlyphMeaning(GI_BASE_A)); + assertEquals(null, font.getGlyphMeaning(GI_GRAVE)); + assertEquals(null, font.getGlyphMeaning(GI_MARK)); + } }