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