FOP-3343: A font configured with kerning="false" is not kerned through GPOS either

fop-core/src/main/java/org/apache/fop/complexscripts/fonts/GlyphPositioningTable.java

@@ -234,10 +234,27 @@ public static GlyphSubtable createSubtable(int type, String id, int sequence, in
      * @return true if some adjustment is not zero; otherwise, false
      */
     public boolean position(GlyphSequence gs, String script, String language, int fontSize, int[] widths, int[][] adjustments) {
+        return position(gs, script, language, fontSize, widths, adjustments, true);
+    }
+
+    /**
+     * Perform positioning processing using all matching lookups, with or without kerning.
+     * @param gs an input glyph sequence
+     * @param script a script identifier
+     * @param language a language identifier
+     * @param fontSize size in device units
+     * @param widths array of default advancements for each glyph
+     * @param adjustments accumulated adjustments array (sequence) of 4-tuples of placement [PX,PY] and advance [AX,AY] adjustments, in that order,
+     * with one 4-tuple for each element of glyph sequence
+     * @param kerning false to leave the kern feature out, for a font configured with kerning disabled; marks are still positioned
+     * @return true if some adjustment is not zero; otherwise, false
+     */
+    public boolean position(GlyphSequence gs, String script, String language, int fontSize, int[] widths, int[][] adjustments,
+                            boolean kerning) {
         Map<LookupSpec, List<LookupTable>> lookups = matchLookups(script, language, "*");
         if ((lookups != null) && (lookups.size() > 0)) {
             ScriptProcessor sp = ScriptProcessor.getInstance(script, processors);
-            return sp.position(this, gs, script, language, fontSize, lookups, widths, adjustments);
+            return sp.position(this, gs, script, language, fontSize, lookups, widths, adjustments, kerning);
         } else {
             return false;
         }

fop-core/src/main/java/org/apache/fop/complexscripts/scripts/ScriptProcessor.java

@@ -169,7 +169,43 @@ public String[] getOptionalPositioningFeatures() {
      */
     public final boolean position(GlyphPositioningTable gpos, GlyphSequence gs, String script, String language, int fontSize,
                                   Map<GlyphTable.LookupSpec, List<GlyphTable.LookupTable>> lookups, int[] widths, int[][] adjustments) {
-        return position(gs, script, language, fontSize, assembleLookups(gpos, getPositioningFeatures(), lookups), widths, adjustments, getPositioningContextTester());
+        return position(gpos, gs, script, language, fontSize, lookups, widths, adjustments, true);
+    }
+
+    /**
+     * Perform positioning processing using a specific set of lookup tables, with or without kerning.
+     * @param gpos the glyph positioning table that applies
+     * @param gs an input glyph sequence
+     * @param script a script identifier
+     * @param language a language identifier
+     * @param fontSize size in device units
+     * @param lookups a mapping from lookup specifications to glyph subtables to use for positioning processing
+     * @param widths array of default advancements for each glyph
+     * @param adjustments accumulated adjustments array (sequence) of 4-tuples of placement [PX,PY] and advance [AX,AY] adjustments, in that order,
+     * with one 4-tuple for each element of glyph sequence
+     * @param kerning false to leave the kern feature out, for a font configured with kerning disabled; marks are still positioned
+     * @return true if some adjustment is not zero; otherwise, false
+     */
+    public final boolean position(GlyphPositioningTable gpos, GlyphSequence gs, String script, String language, int fontSize,
+                                  Map<GlyphTable.LookupSpec, List<GlyphTable.LookupTable>> lookups, int[] widths, int[][] adjustments,
+                                  boolean kerning) {
+        String[] features = kerning ? getPositioningFeatures() : withoutKerning(getPositioningFeatures());
+        return position(gs, script, language, fontSize, assembleLookups(gpos, features, lookups), widths, adjustments, getPositioningContextTester());
+    }
+
+    /**
+     * A positioning feature list with the kern feature removed.
+     * @param features the features a processor would apply
+     * @return the same features in the same order, less kern; the argument itself when it has none
+     */
+    static String[] withoutKerning(String[] features) {
+        List<String> kept = new java.util.ArrayList<String>(features.length);
+        for (String feature : features) {
+            if (!"kern".equals(feature)) {
+                kept.add(feature);
+            }
+        }
+        return (kept.size() == features.length) ? features : kept.toArray(new String[0]);
     }
 
     /**

fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java

@@ -614,7 +614,9 @@ public boolean performsPositioning() {
         if (gpos != null) {
             GlyphSequence gs = mapCharsToGlyphs(cs, null);
             int[][] adjustments = new int [ gs.getGlyphCount() ] [ 4 ];
-            if (gpos.position(gs, script, language, fontSize, this.width, adjustments)) {
+            // The configuration's kerning flag reaches GPOS too: without it the kern feature is left
+            // out, as the legacy kern table already is, and the marks are still positioned.
+            if (gpos.position(gs, script, language, fontSize, this.width, adjustments, isKerningEnabled())) {
                 return scaleAdjustments(adjustments, fontSize);
             } else {
                 return null;

fop-core/src/main/java/org/apache/fop/fonts/truetype/OFFontLoader.java

@@ -207,6 +207,9 @@ private void buildFont(OpenFont otf, String ttcFontName) {
             returnFont.setSVG(otf.svgs);
         }
 
+        // Record the configuration's kerning flag on the font: it gates GPOS kerning as well as
+        // the legacy kern table copied below.
+        returnFont.setKerningEnabled(useKerning);
         if (otf.getKerning() != null && useKerning) {
             copyKerning(otf, isCid);
         }

fop-core/src/test/java/org/apache/fop/complexscripts/scripts/PositioningWithoutKerningTestCase.java

@@ -0,0 +1,43 @@
+/*
+ * 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.complexscripts.scripts;
+
+import org.junit.Test;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertSame;
+
+/**
+ * Kerning disabled removes the kern feature from a processor's positioning features and
+ * nothing else: the marks are still positioned (FOP-3343).
+ */
+public class PositioningWithoutKerningTestCase {
+
+    @Test
+    public void testKernIsRemovedAndMarksStay() {
+        assertArrayEquals(new String[] {"mark", "mkmk"},
+                ScriptProcessor.withoutKerning(new String[] {"kern", "mark", "mkmk"}));
+    }
+
+    @Test
+    public void testListWithoutKernIsReturnedAsIs() {
+        String[] features = {"abvm", "blwm", "dist"};
+        assertSame(features, ScriptProcessor.withoutKerning(features));
+    }
+}

fop-core/src/test/java/org/apache/fop/fonts/KerningFlagTestCase.java

@@ -0,0 +1,66 @@
+/*
+ * 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.io.File;
+
+import org.junit.Test;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.fop.apps.io.InternalResourceResolver;
+import org.apache.fop.apps.io.ResourceResolverFactory;
+import org.apache.fop.complexscripts.fonts.GlyphPositioningTable;
+
+/**
+ * The font configuration's kerning attribute disables GPOS kerning as well as the legacy kern
+ * table (FOP-3343). DejaVuLGCSerif has a GPOS kern feature under its Turkish language system,
+ * which is named here by its tag so that the lookup is found whatever the fallback does.
+ */
+public class KerningFlagTestCase {
+
+    private MultiByteFont load(boolean kerning) throws Exception {
+        InternalResourceResolver resolver =
+                ResourceResolverFactory.createDefaultInternalResourceResolver(new File(".").toURI());
+        File file = new File("test/resources/fonts/ttf/DejaVuLGCSerif.ttf");
+        return (MultiByteFont) FontLoader.loadFont(new FontUris(file.toURI(), null), "", true,
+                EmbeddingMode.AUTO, EncodingMode.AUTO, kerning, true, resolver, false, false, true);
+    }
+
+    @Test
+    public void testFlagIsRecordedOnTheFont() throws Exception {
+        assertTrue(load(true).isKerningEnabled());
+        assertFalse(load(false).isKerningEnabled());
+    }
+
+    @Test
+    public void testKerningEnabledKernsThroughGpos() throws Exception {
+        int[][] gpa = load(true).performPositioning("AV", "latn", "TRK", 1000);
+        assertNotNull(gpa);
+        assertTrue(gpa[0][GlyphPositioningTable.Value.IDX_X_ADVANCE] < 0);
+    }
+
+    @Test
+    public void testKerningDisabledDoesNotKernThroughGpos() throws Exception {
+        assertNull(load(false).performPositioning("AV", "latn", "TRK", 1000));
+    }
+}