From 6e8a91677cf8cfd8b7482260b55bff8eaa873046 Mon Sep 17 00:00:00 2001 From: Jason Harrop Date: Sat, 3 Oct 2026 07:22:18 +1000 Subject: [PATCH] FOP-3343: A font configured with kerning="false" is not kerned through GPOS either The font configuration's kerning attribute is documented as enabling or disabling kerning for the font. It did so only for the legacy kern table: OFFontLoader skips copyKerning when it is false, and GlyphMapping.useKerningAdjustments then finds no kerning info. GPOS positioning was independent of it: MultiByteFont.performPositioning ran whenever the font had a GPOS table, and every script processor's positioning feature list carries kern. So a font with a GPOS kern feature, which is nearly every current font, was kerned whatever the flag said. Underneath, the loader never recorded the flag on the font object, so CustomFont.isKerningEnabled() was true for every TrueType font however it was configured. OFFontLoader now records the flag (setKerningEnabled). MultiByteFont.performPositioning passes it down, and with kerning disabled the script processor positions with the kern feature left out of its feature list; mark and mkmk still apply, since they are not kerning. GlyphPositioningTable.position and ScriptProcessor.position gain an overload taking the flag; the existing signatures delegate with true. Measured on this branch, Carlito 14pt (no legacy kern table, GPOS kern only), no language, word widths from pdftotext -bbox: kerning="true" kerning="false" AVATAR Toffee AVATAR Toffee before 43.19 34.71 43.19 34.71 after 43.19 34.71 46.62 36.27 After, the unkerned Toffee still carries its ligature (36.27 against 36.65 unligated): substitution is unaffected. KerningFlagTestCase: the flag reaches the font, and DejaVuLGCSerif's "AV" is adjusted through GPOS with kerning on and not with it off. PositioningWithoutKerningTestCase: kern is removed from a feature list and the marks stay. Co-Authored-By: Claude Fable 5.1 --- .../fonts/GlyphPositioningTable.java | 19 +++++- .../scripts/ScriptProcessor.java | 38 ++++++++++- .../org/apache/fop/fonts/MultiByteFont.java | 4 +- .../fop/fonts/truetype/OFFontLoader.java | 3 + .../PositioningWithoutKerningTestCase.java | 43 ++++++++++++ .../apache/fop/fonts/KerningFlagTestCase.java | 66 +++++++++++++++++++ 6 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 fop-core/src/test/java/org/apache/fop/complexscripts/scripts/PositioningWithoutKerningTestCase.java create mode 100644 fop-core/src/test/java/org/apache/fop/fonts/KerningFlagTestCase.java diff --git a/fop-core/src/main/java/org/apache/fop/complexscripts/fonts/GlyphPositioningTable.java b/fop-core/src/main/java/org/apache/fop/complexscripts/fonts/GlyphPositioningTable.java index d91e389b7aa..76f9f5e869b 100644 --- a/fop-core/src/main/java/org/apache/fop/complexscripts/fonts/GlyphPositioningTable.java +++ b/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> 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; } diff --git a/fop-core/src/main/java/org/apache/fop/complexscripts/scripts/ScriptProcessor.java b/fop-core/src/main/java/org/apache/fop/complexscripts/scripts/ScriptProcessor.java index 3a6103c079d..bcd6fcff1b7 100644 --- a/fop-core/src/main/java/org/apache/fop/complexscripts/scripts/ScriptProcessor.java +++ b/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> 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> 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 kept = new java.util.ArrayList(features.length); + for (String feature : features) { + if (!"kern".equals(feature)) { + kept.add(feature); + } + } + return (kept.size() == features.length) ? features : kept.toArray(new String[0]); } /** diff --git a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java b/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java index 959c4c10d22..6f00736b2aa 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/MultiByteFont.java +++ b/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; diff --git a/fop-core/src/main/java/org/apache/fop/fonts/truetype/OFFontLoader.java b/fop-core/src/main/java/org/apache/fop/fonts/truetype/OFFontLoader.java index 895b798f713..ab882fe3212 100644 --- a/fop-core/src/main/java/org/apache/fop/fonts/truetype/OFFontLoader.java +++ b/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); } diff --git a/fop-core/src/test/java/org/apache/fop/complexscripts/scripts/PositioningWithoutKerningTestCase.java b/fop-core/src/test/java/org/apache/fop/complexscripts/scripts/PositioningWithoutKerningTestCase.java new file mode 100644 index 00000000000..5cce1752f0a --- /dev/null +++ b/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)); + } +} diff --git a/fop-core/src/test/java/org/apache/fop/fonts/KerningFlagTestCase.java b/fop-core/src/test/java/org/apache/fop/fonts/KerningFlagTestCase.java new file mode 100644 index 00000000000..55d6bc539ac --- /dev/null +++ b/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)); + } +}