FOP-2349: letter-spacing is in a word's width on the complex-script path too

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);

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;

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 = "<fop version=\"1.0\">\n"
                 + "<font-base>../fop/test/resources/fonts/ttf</font-base>\n"

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();