From c1c37bdfef30d1f016543010373aad052f6c9400 Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Sat, 3 Oct 2026 07:17:50 +1000 Subject: [PATCH 1/2] FOP-3344: letter-spacing is painted on text that carries glyph position adjustments PDFPainter.drawText takes one of two paths. With no position adjustments, or DX-only ones, drawTextWithDX writes the word as one TJ array and sets Tc to the letter spacing, which the viewer adds after every glyph. With general adjustments (a GPOS kern is an x-advance adjustment, so IFUtil.isDPOnlyDX is false) drawTextWithDP writes each glyph with its own Td and Tj and advanced by the glyph width plus the adjustment. It set Tc too, but Tc acts only between glyphs inside a TJ array; with one glyph per Tj and an explicit Td before each, the letter spacing never reached the next glyph. The layout is consistent with the first path: TextLayoutManager puts the letter spaces into the word's elements and its area, and addMappingAreas computes the word-space adjustment on the assumption that the renderer adds the character spacing "even to the last character of a word and to space characters". So a letter-spaced word in a font that positions was measured with its letter spaces and painted without them, and the gap to the next word absorbed the difference: negative letter spacing opens the gap, positive closes it, to the point of overprinting. The letter spacing is now added to the advance on this path, for every glyph, spaces and the last one included, which is what Tc does on the other. Java2DPainter and PCLPainter already add it per glyph in their dp loops, PSPainter applies it through ATJ, AFPPainter converts dp to dx; only the PDF painter was missing it. Measured on this branch, Arimo 11pt, kerning on, "During repair, if however", per-glyph steps read back with mutool draw -F stext: before r>e 3.663 e>p 6.116 p>a 6.116 a>i 6.116 i>r 2.442 r>, 3.058 comma>space 3.047 after r>e 3.246 e>p 5.699 p>a 5.699 a>i 5.699 i>r 2.025 r>, 2.641 comma>space 5.549 After: each step is the advance less 0.417, and r>, carries the kern (-55/1000 em) too. PDFPainterTestCase.testDrawDpTextKeepsLetterSpacing: two zero-width glyphs, a kern of -100 and a letter spacing of 500; the second Td must be 0.4, not -0.1. Co-Authored-By: Claude Fable 5.1 --- .../org/apache/fop/render/pdf/PDFPainter.java | 9 +++++- .../fop/render/pdf/PDFPainterTestCase.java | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/fop-core/src/main/java/org/apache/fop/render/pdf/PDFPainter.java b/fop-core/src/main/java/org/apache/fop/render/pdf/PDFPainter.java index 3ac5505fc66..c6524d8de82 100644 --- a/fop-core/src/main/java/org/apache/fop/render/pdf/PDFPainter.java +++ b/fop-core/src/main/java/org/apache/fop/render/pdf/PDFPainter.java @@ -672,7 +672,14 @@ private void drawTextWithDP(int x, int y, String text, FontTriplet triplet, double yd = (yo - yoLast) / 1000f; tu.writeTd(xd, yd); tu.writeTj(mp, tf.isMultiByte(), true); - xc += xa + pa[2]; + // Each glyph is placed by its own Td, so the Tc character spacing set above never + // reaches the next glyph as it does inside the TJ array of drawTextWithDX. The + // letter spacing is added to the advance here, for every glyph including spaces and + // the last one, which is what Tc does on the other path and what the layout's word + // space adjustment assumes (TextLayoutManager.addMappingAreas). Without it a + // letter-spaced word in a font that positions (GPOS kerning) was painted at its + // bare advances while its area kept the letter spaces. + xc += xa + pa[2] + letterSpacing; yc += ya + pa[3]; xoLast = xo; yoLast = yo; diff --git a/fop-core/src/test/java/org/apache/fop/render/pdf/PDFPainterTestCase.java b/fop-core/src/test/java/org/apache/fop/render/pdf/PDFPainterTestCase.java index bd6cf5b2488..adeb3dc2114 100644 --- a/fop-core/src/test/java/org/apache/fop/render/pdf/PDFPainterTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/render/pdf/PDFPainterTestCase.java @@ -448,6 +448,36 @@ public void testDrawDpTextWithMultiByteFont() throws IFException { + "<0001> Tj\n", output.toString()); } + /** + * The position-adjustment path places every glyph with its own Td, so the Tc it sets has no + * effect on the next glyph's position; the letter spacing has to go into the advance, as it + * does through Tc on the TJ path. Two zero-width glyphs, a kern of -100 and a letter spacing + * of 500: the second glyph sits at 400, not at -100. + */ + @Test + public void testDrawDpTextKeepsLetterSpacing() throws IFException { + StringBuilder output = new StringBuilder(); + PDFDocumentHandler pdfDocumentHandler = makePDFDocumentHandler(output); + MultiByteFont font = new MultiByteFont(null, null); + font.setWidthArray(new int[10]); + font.setCMap(new CMapSegment[]{new CMapSegment(128169, 128169, 1)}); + FontInfo fi = new FontInfo(); + fi.addFontProperties("f1", new FontTriplet("a", "normal", 400)); + fi.addMetrics("f1", font); + pdfDocumentHandler.setFontInfo(fi); + MyPDFPainter pdfPainter = new MyPDFPainter(pdfDocumentHandler, null); + pdfPainter.setFont("a", "normal", 400, null, 12, null); + int[][] dp = new int[][] {{0, 0, -100, 0}, {0, 0, 0, 0}}; + pdfPainter.drawText(0, 0, 500, 0, dp, "Hi"); + assertEquals("BT\n" + + "1 0 0 -1 0 0 Tm /f1 0.012 Tf\n" + + "0 0 Td\n" + + "<0000> Tj\n" + + "0.4 0 Td\n" + + "<0000> Tj\n", output.toString()); + verify(pdfContentGenerator).updateCharacterSpacing(0.5f); + } + private PDFDocumentHandler makePDFDocumentHandler(final StringBuilder sb) throws IFException { FopFactory fopFactory = FopFactory.newInstance(new File(".").toURI()); foUserAgent = fopFactory.newFOUserAgent(); From 7bbc3c2e771fdc632abbc3d9bd1e53427c83e919 Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Sat, 3 Oct 2026 10:26:14 +1000 Subject: [PATCH 2/2] FOP-2349: letter-spacing is in a word's width on the complex-script path too GlyphMapping.processWordNoMapping adds letterSpaceIPD x count to a word's width. For a font with GSUB or GPOS tables processWordMapping is used instead. Since FOP-2722 it returns the same letter-space count, but it adds nothing to the width (it was not passed letterSpaceIPD). TextLayoutManager breaks lines on that width while the painter spaces every glyph, so letter-spaced text in an OpenType font was measured short and overran the line. processWordMapping now adds the counted spaces to the width, as the plain path does. GlyphMappingTestCase lays out "word" in DejaVuLGCSerif with no letter spacing and with 3pt; the widths must differ by the three counted letter spaces (they differed by 0). Andreas L. Delmelle traced FOP-2349 to the same "[TBD] - handle letter spacing" in 2015. This keeps FOP-2722's character count rather than counting glyphs. Stacked on FOP-3344: without it, text on the position-adjustments path is not painted with its letter spacing at all, so lines measured correctly would look short. Co-Authored-By: Claude Opus 5.5 --- .../org/apache/fop/fonts/GlyphMapping.java | 11 ++- .../fop/fonts/GlyphMappingTestCase.java | 72 +++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/fop-core/src/main/java/org/apache/fop/fonts/GlyphMapping.java b/fop-core/src/main/java/org/apache/fop/fonts/GlyphMapping.java index 1969af34bec..2c4422c5683 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/GlyphMapping.java +++ b/fop-core/src/main/java/org/apache/fop/fonts/GlyphMapping.java @@ -87,7 +87,7 @@ public static GlyphMapping doGlyphMapping(TextFragment text, int startIndex, int boolean dontOptimizeForIdentityMapping, boolean retainAssociations, boolean retainControls) { GlyphMapping mapping; if (font.performsSubstitution() || font.performsPositioning()) { - mapping = processWordMapping(text, startIndex, endIndex, font, + mapping = processWordMapping(text, startIndex, endIndex, font, letterSpaceIPD, breakOpportunityChar, endsWithHyphen, level, dontOptimizeForIdentityMapping, retainAssociations, retainControls); } else { @@ -98,7 +98,7 @@ public static GlyphMapping doGlyphMapping(TextFragment text, int startIndex, int } private static GlyphMapping processWordMapping(TextFragment text, int startIndex, - int endIndex, final Font font, final char breakOpportunityChar, + int endIndex, final Font font, MinOptMax letterSpaceIPD, final char breakOpportunityChar, final boolean endsWithHyphen, int level, boolean dontOptimizeForIdentityMapping, boolean retainAssociations, boolean retainControls) { String script = text.getScript(); @@ -173,8 +173,13 @@ private static GlyphMapping processWordMapping(TextFragment text, int startIndex ipd = ipd.plus(w); } + // The letter spaces are part of the word's width, as processWordNoMapping makes them: the + // layout manager breaks lines on this width, and the painter spaces every glyph. + int letterSpaces = calculateLetterSpaces(startIndex, endIndex, breakOpportunityChar); + ipd = ipd.plus(letterSpaceIPD.mult(letterSpaces)); + return new GlyphMapping(startIndex, endIndex, 0, - calculateLetterSpaces(startIndex, endIndex, breakOpportunityChar), ipd, endsWithHyphen, false, + letterSpaces, ipd, endsWithHyphen, false, breakOpportunityChar != 0, font, level, gpa, !dontOptimizeForIdentityMapping && CharUtilities.isSameSequence(mcs, ics) ? null : mcs.toString(), associations); diff --git a/fop-core/src/test/java/org/apache/fop/fonts/GlyphMappingTestCase.java b/fop-core/src/test/java/org/apache/fop/fonts/GlyphMappingTestCase.java index 4d619da4170..83773dd79c2 100644 --- a/fop-core/src/test/java/org/apache/fop/fonts/GlyphMappingTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/fonts/GlyphMappingTestCase.java @@ -23,6 +23,8 @@ import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.text.CharacterIterator; +import java.text.StringCharacterIterator; import javax.xml.transform.Result; import javax.xml.transform.Source; @@ -34,15 +36,19 @@ import org.junit.Test; import org.xml.sax.SAXException; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import org.apache.fop.apps.FOUserAgent; import org.apache.fop.apps.Fop; import org.apache.fop.apps.FopFactory; import org.apache.fop.apps.MimeConstants; +import org.apache.fop.apps.io.InternalResourceResolver; +import org.apache.fop.apps.io.ResourceResolverFactory; import org.apache.fop.render.intermediate.IFContext; import org.apache.fop.render.intermediate.IFDocumentHandler; import org.apache.fop.render.intermediate.IFSerializer; +import org.apache.fop.traits.MinOptMax; public class GlyphMappingTestCase { @@ -73,6 +79,72 @@ public void testSpecialCharacterSpacing() throws Exception { output.contains("£££")); } + /** + * A word's letter spaces are part of its width on both paths. On the path for fonts with + * substitution or positioning tables they were counted but left out of the width, so the + * line breaker measured letter-spaced text short and it ran past the end of the line, while + * the painter spaced every glyph. + */ + @Test + public void testLetterSpacesInTheWidthOfAWordInAFontWithLayoutTables() throws Exception { + InternalResourceResolver resolver = + ResourceResolverFactory.createDefaultInternalResourceResolver(new File(".").toURI()); + File file = new File("test/resources/fonts/ttf/DejaVuLGCSerif.ttf"); + CustomFont typeface = FontLoader.loadFont(new FontUris(file.toURI(), null), "", true, + EmbeddingMode.AUTO, EncodingMode.AUTO, false, true, resolver, false, false, true); + Font font = new Font("F1", null, typeface, 12000); + assertTrue(font.performsSubstitution() || font.performsPositioning()); + + TextFragment word = new StringFragment("word"); + GlyphMapping unspaced = GlyphMapping.doGlyphMapping(word, 0, 4, font, MinOptMax.ZERO, null, + '\0', ' ', false, 0, false, false, false); + GlyphMapping spaced = GlyphMapping.doGlyphMapping(word, 0, 4, font, MinOptMax.getInstance(3000), null, + '\0', ' ', false, 0, false, false, false); + assertEquals(3, spaced.letterSpaceCount); + assertEquals(3 * 3000, spaced.areaIPD.getOpt() - unspaced.areaIPD.getOpt()); + } + + private static final class StringFragment implements TextFragment { + + private final String text; + + private StringFragment(String text) { + this.text = text; + } + + public CharacterIterator getIterator() { + return new StringCharacterIterator(text); + } + + public int getBeginIndex() { + return 0; + } + + public int getEndIndex() { + return text.length(); + } + + public String getScript() { + return "latn"; + } + + public String getLanguage() { + return "dflt"; + } + + public int getBidiLevel() { + return 0; + } + + public char charAt(int index) { + return text.charAt(index); + } + + public CharSequence subSequence(int startIndex, int endIndex) { + return text.subSequence(startIndex, endIndex); + } + } + private String foToIF(String fo) throws SAXException, TransformerException, IOException { String fopxconf = "\n" + "../fop/test/resources/fonts/ttf\n"