FOP-3180: SVG Glyph positions ignored when using a custom font by João André Gonçalves

6 files changed, 137 insertions, 73 deletions

diff --git a/fop-core/src/main/java/org/apache/fop/svg/NativeTextPainter.java b/fop-core/src/main/java/org/apache/fop/svg/NativeTextPainter.java
index e6a1b43dd..09cca6a03 100644
--- a/fop-core/src/main/java/org/apache/fop/svg/NativeTextPainter.java
+++ b/fop-core/src/main/java/org/apache/fop/svg/NativeTextPainter.java
@@ -134,7 +134,6 @@ public abstract class NativeTextPainter extends StrokingTextPainter {
     }
 
     protected void writeGlyphs(FOPGVTGlyphVector gv, GeneralPath debugShapes) throws IOException {
-        AffineTransform localTransform = new AffineTransform();
         Point2D prevPos = null;
         AffineTransform prevGlyphTransform = null;
         font = ((FOPGVTFont) gv.getFont()).getFont();
@@ -142,6 +141,8 @@ public abstract class NativeTextPainter extends StrokingTextPainter {
             if (!gv.isGlyphVisible(index)) {
                 continue;
             }
+
+            char glyph = (char) gv.getGlyphCode(index);
             Point2D glyphPos = gv.getGlyphPosition(index);
 
             AffineTransform glyphTransform = gv.getGlyphTransform(index);
@@ -156,22 +157,27 @@ public abstract class NativeTextPainter extends StrokingTextPainter {
                 debugShapes.append(sh, false);
             }
 
-            //Exact position of the glyph
-            localTransform.setToIdentity();
-            localTransform.translate(glyphPos.getX(), glyphPos.getY());
-            if (glyphTransform != null) {
-                localTransform.concatenate(glyphTransform);
-            }
-            localTransform.scale(1, -1);
-
             positionGlyph(prevPos, glyphPos, glyphTransform != null || prevGlyphTransform != null);
-            char glyph = (char) gv.getGlyphCode(index);
+
             //Update last position
             prevPos = glyphPos;
             prevGlyphTransform = glyphTransform;
 
-            writeGlyph(glyph, localTransform);
+            writeGlyph(glyph, getLocalTransform(glyphPos, glyphTransform));
+        }
+    }
+
+    protected AffineTransform getLocalTransform(Point2D glyphPos, AffineTransform glyphTransform) {
+        //Exact position of the glyph
+        AffineTransform localTransform = new AffineTransform();
+        localTransform.setToIdentity();
+        localTransform.translate(glyphPos.getX(), glyphPos.getY());
+        if (glyphTransform != null) {
+            localTransform.concatenate(glyphTransform);
         }
+        localTransform.scale(1, -1);
+
+        return localTransform;
     }
 
     @Override
diff --git a/fop-core/src/main/java/org/apache/fop/svg/PDFTextPainter.java b/fop-core/src/main/java/org/apache/fop/svg/PDFTextPainter.java
index 301af2070..ef5aac323 100644
--- a/fop-core/src/main/java/org/apache/fop/svg/PDFTextPainter.java
+++ b/fop-core/src/main/java/org/apache/fop/svg/PDFTextPainter.java
@@ -31,7 +31,6 @@ import java.io.IOException;
 
 import org.apache.batik.gvt.text.TextPaintInfo;
 
-import org.apache.fop.fonts.Font;
 import org.apache.fop.fonts.FontInfo;
 import org.apache.fop.svg.font.FOPGVTFont;
 import org.apache.fop.svg.font.FOPGVTGlyphVector;
@@ -48,6 +47,8 @@ import org.apache.fop.svg.font.FOPGVTGlyphVector;
  */
 class PDFTextPainter extends NativeTextPainter {
 
+    private static final int[] PA_ZERO = new int[4];
+
     private PDFGraphics2D pdf;
 
     private PDFTextUtil textUtil;
@@ -108,40 +109,50 @@ class PDFTextPainter extends NativeTextPainter {
         pdf.writeClip(clip);
     }
 
-    private static int[] paZero = new int[4];
-
+    @Override
     protected void writeGlyphs(FOPGVTGlyphVector gv, GeneralPath debugShapes) throws IOException {
         if (gv.getGlyphPositionAdjustments() == null) {
             super.writeGlyphs(gv, debugShapes);
         } else {
-            FOPGVTFont gvtFont = (FOPGVTFont) gv.getFont();
-            String fk = gvtFont.getFontKey();
-            Font f = gvtFont.getFont();
-            Point2D initialPos = gv.getGlyphPosition(0);
-            int         fs              = f.getFontSize();
-            float       fsPoints        = fs / 1000f;
-            double      xc              = 0f;
-            double      yc              = 0f;
-            double      xoLast          = 0f;
-            double      yoLast          = 0f;
-            textUtil.writeTextMatrix(new AffineTransform(1, 0, 0, -1, initialPos.getX(), initialPos.getY()));
-            textUtil.updateTf(fk, fsPoints, f.isMultiByte(), false);
-            int[][] dp = gv.getGlyphPositionAdjustments();
-            for (int i = 0, n = gv.getNumGlyphs(); i < n; i++) {
-                int     gc              = gv.getGlyphCode(i);
-                int[]   pa              = ((i > dp.length) || (dp[i] == null)) ? paZero : dp[i];
-                double  xo              = xc + pa[0];
-                double  yo              = yc + pa[1];
-                double  xa              = f.getWidth(gc);
-                double  ya              = 0;
-                double  xd              = (xo - xoLast) / 1000f;
-                double  yd              = (yo - yoLast) / 1000f;
-                textUtil.writeTd(xd, yd);
-                textUtil.writeTj((char) gc, f.isMultiByte(), false);
-                xc += xa + pa[2];
-                yc += ya + pa[3];
-                xoLast = xo;
-                yoLast = yo;
+            Point2D prevPos = null;
+            AffineTransform prevGlyphTransform = null;
+            font = ((FOPGVTFont) gv.getFont()).getFont();
+            int[][] glyphPositionAdjustments = gv.getGlyphPositionAdjustments();
+            double glyphXPos = 0f;
+            double glyphYPos = 0f;
+            double prevAdjustedGlyphXPos = 0f;
+            double prevAdjustedGlyphYPos = 0f;
+            for (int index = 0; index < gv.getNumGlyphs(); index++) {
+                if (!gv.isGlyphVisible(index)) {
+                    continue;
+                }
+                Point2D glyphPos = gv.getGlyphPosition(index);
+                AffineTransform glyphTransform = gv.getGlyphTransform(index);
+                int[] positionAdjust = ((index > glyphPositionAdjustments.length)
+                        || (glyphPositionAdjustments[index] == null)) ? PA_ZERO : glyphPositionAdjustments[index];
+                if (log.isTraceEnabled()) {
+                    log.trace("pos " + glyphPos + ", transform " + glyphTransform);
+                }
+
+                positionGlyph(prevPos, glyphPos, glyphTransform != null || prevGlyphTransform != null);
+
+                char glyph = (char) gv.getGlyphCode(index);
+                double adjustedGlyphXPos = glyphXPos + positionAdjust[0];
+                double adjustedGlyphYPos = glyphYPos + positionAdjust[1];
+                double tdXPos = (adjustedGlyphXPos - prevAdjustedGlyphXPos) / 1000f;
+                double tdYPos = (adjustedGlyphYPos - prevAdjustedGlyphYPos) / 1000f;
+
+                textUtil.writeTd(tdXPos, tdYPos);
+
+                writeGlyph(glyph, getLocalTransform(glyphPos, glyphTransform));
+
+                //Update last position
+                prevPos = glyphPos;
+                prevGlyphTransform = glyphTransform;
+                glyphXPos = glyphPos.getX() + positionAdjust[2];
+                glyphYPos = glyphPos.getY() + positionAdjust[3];
+                prevAdjustedGlyphXPos = adjustedGlyphXPos;
+                prevAdjustedGlyphYPos = adjustedGlyphYPos;
             }
         }
     }
@@ -193,9 +204,7 @@ class PDFTextPainter extends NativeTextPainter {
                 || reposition);
         if (!repositionNextGlyph) {
             double xdiff = glyphPos.getX() - prevPos.getX();
-            //Width of previous character
-            double cw = prevVisibleGlyphWidth;
-            double effxdiff = (1000 * xdiff) - cw;
+            double effxdiff = (1000 * xdiff) - prevVisibleGlyphWidth;
             if (effxdiff != 0) {
                 double adjust = (-effxdiff / font.getFontSize());
                 textUtil.adjustGlyphTJ(adjust * 1000);
diff --git a/fop-core/src/main/java/org/apache/fop/svg/font/FOPGVTGlyphVector.java b/fop-core/src/main/java/org/apache/fop/svg/font/FOPGVTGlyphVector.java
index 106469258..0f92be578 100644
--- a/fop-core/src/main/java/org/apache/fop/svg/font/FOPGVTGlyphVector.java
+++ b/fop-core/src/main/java/org/apache/fop/svg/font/FOPGVTGlyphVector.java
@@ -320,6 +320,10 @@ public class FOPGVTGlyphVector implements GVTGlyphVector {
         return gposAdjustments;
     }
 
+    public List getAssociations() {
+        return associations;
+    }
+
     public Point2D getGlyphPosition(int glyphIndex) {
         int positionIndex = glyphIndex * 2;
         return new Point2D.Float(positions[positionIndex], positions[positionIndex + 1]);
diff --git a/fop-core/src/main/java/org/apache/fop/svg/text/ComplexGlyphLayout.java b/fop-core/src/main/java/org/apache/fop/svg/text/ComplexGlyphLayout.java
index b90e89b01..a35b41250 100644
--- a/fop-core/src/main/java/org/apache/fop/svg/text/ComplexGlyphLayout.java
+++ b/fop-core/src/main/java/org/apache/fop/svg/text/ComplexGlyphLayout.java
@@ -22,14 +22,16 @@ package org.apache.fop.svg.text;
 import java.awt.font.FontRenderContext;
 import java.awt.geom.Point2D;
 import java.text.AttributedCharacterIterator;
+import java.util.List;
 
 import org.apache.batik.bridge.GlyphLayout;
 import org.apache.batik.gvt.font.GVTFont;
-import org.apache.batik.gvt.font.GVTGlyphVector;
 import org.apache.batik.gvt.text.GVTAttributedCharacterIterator;
 
+import org.apache.fop.complexscripts.util.CharAssociation;
 import org.apache.fop.fonts.Font;
 import org.apache.fop.svg.font.FOPGVTFont;
+import org.apache.fop.svg.font.FOPGVTGlyphVector;
 
 public class ComplexGlyphLayout extends GlyphLayout {
 
@@ -39,16 +41,21 @@ public class ComplexGlyphLayout extends GlyphLayout {
     }
 
     @Override
-    protected void doExplicitGlyphLayout() {
-        GVTGlyphVector gv = this.gv;
-        gv.performDefaultLayout();
-        int ng = gv.getNumGlyphs();
-        if (ng > 0) {
-            this.advance = gv.getGlyphPosition(ng);
-        } else {
-            this.advance = new Point2D.Float(0, 0);
+    protected int getAciIndex(int aciIndex, int loopIndex) {
+        if (gv instanceof FOPGVTGlyphVector) {
+            List associations = ((FOPGVTGlyphVector) gv).getAssociations();
+            // this method is called at the end of the cycle, therefore we still have the index of the current cycle
+            // since we are trying to determine the aci index for the next interation, we need to add 1 to the index
+            // the parent method does that automatically when it tries to get the character count
+            int nextIndex = loopIndex + 1;
+            if (nextIndex < associations.size() && associations.get(nextIndex) instanceof CharAssociation) {
+                CharAssociation association = (CharAssociation) associations.get(nextIndex);
+                return association.getStart();
+            }
         }
-        this.layoutApplied  = true;
+
+        //will only be used on the last iteration. the loop will stop after this and the value will not be used
+        return super.getAciIndex(aciIndex, loopIndex);
     }
 
     public static final boolean mayRequireComplexLayout(AttributedCharacterIterator aci) {
diff --git a/fop-core/src/test/java/org/apache/fop/svg/PDFTextPainterTestCase.java b/fop-core/src/test/java/org/apache/fop/svg/PDFTextPainterTestCase.java
index 317e969d2..941cdb754 100644
--- a/fop-core/src/test/java/org/apache/fop/svg/PDFTextPainterTestCase.java
+++ b/fop-core/src/test/java/org/apache/fop/svg/PDFTextPainterTestCase.java
@@ -25,9 +25,9 @@ import java.awt.geom.AffineTransform;
 import java.awt.geom.GeneralPath;
 import java.io.StringWriter;
 
-import org.junit.Assert;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -38,6 +38,7 @@ import org.apache.xmlgraphics.java2d.GraphicContext;
 
 import org.apache.fop.fonts.Font;
 import org.apache.fop.fonts.FontInfo;
+import org.apache.fop.fonts.FontMetrics;
 import org.apache.fop.fonts.FontTriplet;
 import org.apache.fop.fonts.base14.Base14FontCollection;
 import org.apache.fop.pdf.PDFDocument;
@@ -164,16 +165,42 @@ public class PDFTextPainterTestCase extends NativeTextPainterTest {
      */
     @Test
     public void testSingleByteAdjustments() throws Exception {
+        defaultSingleByteAdjustmentsTest(1, "BT\n"
+                + "3 Tr\n"
+                + "0.002 0.003 Td\n"
+                + "/F5 0.012 Tf\n"
+                + "1 0 0 -1 0 0 Tm [(\\0)] TJ\n"
+                + "ET\n");
+    }
+
+    @Test
+    public void testSingleByteAdjustmentsMultipleGlyphs() throws Exception {
+        defaultSingleByteAdjustmentsTest(3, "BT\n"
+                + "3 Tr\n"
+                + "0.002 0.003 Td\n"
+                + "/F5 0.012 Tf\n"
+                + "1 0 0 -1 0 0 Tm 0.008 0.009 Td\n"
+                + "[(\\0)] TJ\n"
+                + "1 0 0 -1 1 1 Tm 0.009 0.009 Td\n"
+                + "[(\\1)] TJ\n"
+                + "1 0 0 -1 2 2 Tm [(\\2)] TJ\n"
+                + "ET\n");
+    }
+
+    private void defaultSingleByteAdjustmentsTest(int numberOfGlyphs, String expected) throws Exception {
         FontInfo fontInfo = new FontInfo();
         new Base14FontCollection(true).setup(0, fontInfo);
 
-        PDFTextPainter painter = new PDFTextPainter(fontInfo);
+        TextPaintInfo tpi = new TextPaintInfo();
+        tpi.visible = true;
+
         PDFGraphics2D g2d = mock(PDFGraphics2D.class);
+        g2d.fontInfo = fontInfo;
         g2d.currentStream = new StringWriter();
+
+        PDFTextPainter painter = new PDFTextPainter(fontInfo);
         painter.preparePainting(g2d);
         painter.setInitialTransform(new AffineTransform());
-        TextPaintInfo tpi = new TextPaintInfo();
-        tpi.visible = true;
         painter.tpi = tpi;
         painter.beginTextObject();
 
@@ -182,24 +209,35 @@ public class PDFTextPainterTestCase extends NativeTextPainterTest {
         Font font = fontInfo.getFontInstance(triplet, 12);
 
         FOPGVTFont mockGvtFont = mock(FOPGVTFont.class);
-        org.apache.fop.fonts.FontMetrics fontMetrics = font.getFontMetrics();
-        when(mockGvtFont.getFont()).thenReturn(new Font("Times", triplet, fontMetrics, 12));
-        when(mockGvtFont.getFontKey()).thenReturn("Times");
+        FontMetrics fontMetrics = font.getFontMetrics();
+        when(mockGvtFont.getFont()).thenReturn(new Font("F5", triplet, fontMetrics, 12));
+        when(mockGvtFont.getFontKey()).thenReturn("F5");
 
         when(mockGV.getFont()).thenReturn(mockGvtFont);
-        when(mockGV.getGlyphPositionAdjustments()).thenReturn(new int[][] {{2, 3, 4, 5}, {6, 7, 8, 9}});
-        when(mockGV.getGlyphPosition(0)).thenReturn(new Point(0, 0));
-        when(mockGV.getNumGlyphs()).thenReturn(1);
-        when(mockGV.getGlyphCode(0)).thenReturn(1);
+        addGlyphs(mockGV, numberOfGlyphs);
 
         GeneralPath gp = new GeneralPath();
 
         painter.writeGlyphs(mockGV, gp);
-        Assert.assertEquals("BT\n"
-                        + "3 Tr\n"
-                        + "1 0 0 -1 0 0 Tm /Times 0.012 Tf\n"
-                        + "0.002 0.003 Td\n"
-                        + "(\\1) Tj\n",
-                g2d.currentStream.toString());
+        painter.endTextObject();
+
+        assertEquals("Must write a Text Matrix for each glyph and use the glyph adjustments",
+                expected, g2d.currentStream.toString());
+    }
+
+    private void addGlyphs(FOPGVTGlyphVector mockGV, int max) {
+        int[][] array = new int[max][4];
+        for (int i = 0; i < max; i++) {
+            when(mockGV.getGlyphPosition(i)).thenReturn(new Point(i, i));
+            when(mockGV.getNumGlyphs()).thenReturn(max);
+            when(mockGV.getGlyphCode(i)).thenReturn(i);
+            when(mockGV.isGlyphVisible(i)).thenReturn(true);
+            array[i][0] = 2 + i * 4;
+            array[i][1] = 3 + i * 4;
+            array[i][2] = 4 + i * 4;
+            array[i][3] = 5 + i * 4;
+        }
+
+        when(mockGV.getGlyphPositionAdjustments()).thenReturn(array);
     }
 }
diff --git a/pom.xml b/pom.xml
index 119c07a04..3260fdb54 100644
--- a/pom.xml
+++ b/pom.xml
@@ -13,7 +13,7 @@
   <properties>
     <ant.version>1.10.14</ant.version>
     <antrun.plugin.version>1.8</antrun.plugin.version>
-    <batik.version>1.17.0-SNAPSHOT</batik.version>
+    <batik.version>1.18.0-SNAPSHOT</batik.version>
     <build.helper.plugin.version>1.9.1</build.helper.plugin.version>
     <checkstyle.plugin.version>2.14</checkstyle.plugin.version>
     <commons.io.version>2.17.0</commons.io.version>