Merge pull request #3147 from fonttools/varlib-drop-implied-oncurves

[varlib] add --drop-implied-oncurves option

Lib/fontTools/pens/ttGlyphPen.py

@@ -144,10 +144,7 @@ def glyph(self, componentFlags: int = 0x04, dropImpliedOnCurves=False) -> Glyph:
         glyph.coordinates = GlyphCoordinates(self.points)
         glyph.endPtsOfContours = self.endPts
         glyph.flags = array("B", self.types)
-
         glyph.coordinates.toInt()
-        if dropImpliedOnCurves:
-            dropImpliedOnCurvePoints(glyph)
 
         self.init()
 
@@ -160,6 +157,8 @@ def glyph(self, componentFlags: int = 0x04, dropImpliedOnCurves=False) -> Glyph:
             glyph.numberOfContours = len(glyph.endPtsOfContours)
             glyph.program = ttProgram.Program()
             glyph.program.fromBytecode(b"")
+            if dropImpliedOnCurves:
+                dropImpliedOnCurvePoints(glyph)
 
         return glyph
 

Lib/fontTools/ttLib/tables/_g_l_y_f.py

@@ -28,6 +28,7 @@
 from fontTools.misc.filenames import userNameToFileName
 from fontTools.misc.loggingTools import deprecateFunction
 from enum import IntFlag
+from types import SimpleNamespace
 from typing import Set
 
 log = logging.getLogger(__name__)
@@ -1541,6 +1542,8 @@ def dropImpliedOnCurvePoints(*interpolatable_glyphs: Glyph) -> Set[int]:
     If more than one glyphs are passed, these are assumed to be interpolatable masters
     of the same glyph impliable, and thus only the on-curve points that are impliable
     for all of them will actually be implied.
+    Composite glyphs or empty glyphs are skipped, only simple glyphs with 1 or more
+    contours are considered.
     The input glyph(s) is/are modified in-place.
 
     Args:
@@ -1549,18 +1552,40 @@ def dropImpliedOnCurvePoints(*interpolatable_glyphs: Glyph) -> Set[int]:
     Returns:
         The set of point indices that were dropped if any.
 
+    Raises:
+        ValueError if simple glyphs are not in fact interpolatable because they have
+        different point flags or number of contours.
+
     Reference:
     https://developer.apple.com/fonts/TrueType-Reference-Manual/RM01/Chap1.html
     """
-    assert len(interpolatable_glyphs) > 0
-
+    staticAttributes = SimpleNamespace(
+        numberOfContours=None, flags=None, endPtsOfContours=None
+    )
     drop = None
-    for glyph in interpolatable_glyphs:
+    simple_glyphs = []
+    for i, glyph in enumerate(interpolatable_glyphs):
+        if glyph.numberOfContours < 1:
+            # ignore composite or empty glyphs
+            continue
+
+        for attr in staticAttributes.__dict__:
+            expected = getattr(staticAttributes, attr)
+            found = getattr(glyph, attr)
+            if expected is None:
+                setattr(staticAttributes, attr, found)
+            elif expected != found:
+                raise ValueError(
+                    f"Incompatible {attr} for glyph at master index {i}: "
+                    f"expected {expected}, found {found}"
+                )
+
         may_drop = set()
         start = 0
-        flags = glyph.flags
         coords = glyph.coordinates
-        for last in glyph.endPtsOfContours:
+        flags = staticAttributes.flags
+        endPtsOfContours = staticAttributes.endPtsOfContours
+        for last in endPtsOfContours:
             for i in range(start, last + 1):
                 if not (flags[i] & flagOnCurve):
                     continue
@@ -1583,32 +1608,39 @@ def dropImpliedOnCurvePoints(*interpolatable_glyphs: Glyph) -> Set[int]:
         else:
             drop.intersection_update(may_drop)
 
+        simple_glyphs.append(glyph)
+
     if drop:
         # Do the actual dropping
-        for glyph in interpolatable_glyphs:
+        flags = staticAttributes.flags
+        assert flags is not None
+        newFlags = array.array(
+            "B", (flags[i] for i in range(len(flags)) if i not in drop)
+        )
+
+        endPts = staticAttributes.endPtsOfContours
+        assert endPts is not None
+        newEndPts = []
+        i = 0
+        delta = 0
+        for d in sorted(drop):
+            while d > endPts[i]:
+                newEndPts.append(endPts[i] - delta)
+                i += 1
+            delta += 1
+        while i < len(endPts):
+            newEndPts.append(endPts[i] - delta)
+            i += 1
+
+        for glyph in simple_glyphs:
             coords = glyph.coordinates
             glyph.coordinates = GlyphCoordinates(
                 coords[i] for i in range(len(coords)) if i not in drop
             )
-            glyph.flags = array.array(
-                "B", (flags[i] for i in range(len(flags)) if i not in drop)
-            )
-
-            endPts = glyph.endPtsOfContours
-            newEndPts = []
-            i = 0
-            delta = 0
-            for d in sorted(drop):
-                while d > endPts[i]:
-                    newEndPts.append(endPts[i] - delta)
-                    i += 1
-                delta += 1
-            while i < len(endPts):
-                newEndPts.append(endPts[i] - delta)
-                i += 1
+            glyph.flags = newFlags
             glyph.endPtsOfContours = newEndPts
 
-    return drop
+    return drop if drop is not None else set()
 
 
 class GlyphComponent(object):

Lib/fontTools/varLib/__init__.py

@@ -24,7 +24,7 @@
 from fontTools.misc.textTools import Tag, tostr
 from fontTools.ttLib import TTFont, newTable
 from fontTools.ttLib.tables._f_v_a_r import Axis, NamedInstance
-from fontTools.ttLib.tables._g_l_y_f import GlyphCoordinates
+from fontTools.ttLib.tables._g_l_y_f import GlyphCoordinates, dropImpliedOnCurvePoints
 from fontTools.ttLib.tables.ttProgram import Program
 from fontTools.ttLib.tables.TupleVariation import TupleVariation
 from fontTools.ttLib.tables import otTables as ot
@@ -40,7 +40,7 @@
 from fontTools.colorLib.builder import buildColrV1
 from fontTools.colorLib.unbuilder import unbuildColrV1
 from functools import partial
-from collections import OrderedDict, namedtuple
+from collections import OrderedDict, defaultdict, namedtuple
 import os.path
 import logging
 from copy import deepcopy
@@ -965,13 +965,54 @@ def set_default_weight_width_slant(font, location):
             font["post"].italicAngle = italicAngle
 
 
+def drop_implied_oncurve_points(*masters: TTFont) -> int:
+    """Drop impliable on-curve points from all the simple glyphs in masters.
+
+    In TrueType glyf outlines, on-curve points can be implied when they are located
+    exactly at the midpoint of the line connecting two consecutive off-curve points.
+
+    The input masters' glyf tables are assumed to contain same-named glyphs that are
+    interpolatable. Oncurve points are only dropped if they can be implied for all
+    the masters. The fonts are modified in-place.
+
+    Args:
+        masters: The TTFont(s) to modify
+
+    Returns:
+        The total number of points that were dropped if any.
+
+    Reference:
+    https://developer.apple.com/fonts/TrueType-Reference-Manual/RM01/Chap1.html
+    """
+
+    count = 0
+    glyph_masters = defaultdict(list)
+    # multiple DS source may point to the same TTFont object and we want to
+    # avoid processing the same glyph twice as they are modified in-place
+    for font in {id(m): m for m in masters}.values():
+        glyf = font["glyf"]
+        for glyphName in glyf.keys():
+            glyph_masters[glyphName].append(glyf[glyphName])
+    count = 0
+    for glyphName, glyphs in glyph_masters.items():
+        try:
+            dropped = dropImpliedOnCurvePoints(*glyphs)
+        except ValueError as e:
+            # we don't fail for incompatible glyphs in _add_gvar so we shouldn't here
+            log.warning("Failed to drop implied oncurves for %r: %s", glyphName, e)
+        else:
+            count += len(dropped)
+    return count
+
+
 def build_many(
     designspace: DesignSpaceDocument,
     master_finder=lambda s: s,
     exclude=[],
     optimize=True,
     skip_vf=lambda vf_name: False,
     colr_layer_reuse=True,
+    drop_implied_oncurves=False,
 ):
     """
     Build variable fonts from a designspace file, version 5 which can define
@@ -1015,6 +1056,7 @@ def build_many(
                 exclude=exclude,
                 optimize=optimize,
                 colr_layer_reuse=colr_layer_reuse,
+                drop_implied_oncurves=drop_implied_oncurves,
             )[0]
             if doBuildStatFromDSv5:
                 buildVFStatTable(vf, designspace, name)
@@ -1028,6 +1070,7 @@ def build(
     exclude=[],
     optimize=True,
     colr_layer_reuse=True,
+    drop_implied_oncurves=False,
 ):
     """
     Build variation font from a designspace file.
@@ -1055,6 +1098,13 @@ def build(
         except AttributeError:
             master_ttfs.append(None)  # in-memory fonts have no path
 
+    if drop_implied_oncurves and "glyf" in master_fonts[ds.base_idx]:
+        drop_count = drop_implied_oncurve_points(*master_fonts)
+        log.info(
+            "Dropped %s on-curve points from simple glyphs in the 'glyf' table",
+            drop_count,
+        )
+
     # Copy the base master to work from it
     vf = deepcopy(master_fonts[ds.base_idx])
 
@@ -1228,6 +1278,14 @@ def main(args=None):
         action="store_false",
         help="do not rebuild variable COLR table to optimize COLR layer reuse",
     )
+    parser.add_argument(
+        "--drop-implied-oncurves",
+        action="store_true",
+        help=(
+            "drop on-curve points that can be implied when exactly in the middle of "
+            "two off-curve points (only applies to TrueType fonts)"
+        ),
+    )
     parser.add_argument(
         "--master-finder",
         default="master_ttf_interpolatable/{stem}.ttf",
@@ -1312,6 +1370,7 @@ def main(args=None):
         exclude=options.exclude,
         optimize=options.optimize,
         colr_layer_reuse=options.colr_layer_reuse,
+        drop_implied_oncurves=options.drop_implied_oncurves,
     )
 
     for vf_name, vf in vfs.items():

Tests/ttLib/tables/_g_l_y_f_test.py

@@ -860,13 +860,17 @@ def test_dropImpliedOnCurvePoints_all_quad_off_curves():
         ],
         Transform().scale(2.0),
     )
+    # also add an empty glyph (will be ignored); we use this trick for 'sparse' masters
+    glyph3 = Glyph()
+    glyph3.numberOfContours = 0
 
-    assert dropImpliedOnCurvePoints(glyph1, glyph2) == {0, 2, 4, 6}
+    assert dropImpliedOnCurvePoints(glyph1, glyph2, glyph3) == {0, 2, 4, 6}
 
     assert glyph1.flags == glyph2.flags == array.array("B", [0, 0, 0, 0])
     assert glyph1.coordinates == GlyphCoordinates([(1, 1), (1, -1), (-1, -1), (-1, 1)])
     assert glyph2.coordinates == GlyphCoordinates([(2, 2), (2, -2), (-2, -2), (-2, 2)])
     assert glyph1.endPtsOfContours == glyph2.endPtsOfContours == [3]
+    assert glyph3.numberOfContours == 0
 
 
 def test_dropImpliedOnCurvePoints_all_cubic_off_curves():
@@ -890,8 +894,10 @@ def test_dropImpliedOnCurvePoints_all_cubic_off_curves():
         ],
         Transform().translate(10.0),
     )
+    glyph3 = Glyph()
+    glyph3.numberOfContours = 0
 
-    assert dropImpliedOnCurvePoints(glyph1, glyph2) == {0, 3, 6, 9}
+    assert dropImpliedOnCurvePoints(glyph1, glyph2, glyph3) == {0, 3, 6, 9}
 
     assert glyph1.flags == glyph2.flags == array.array("B", [flagCubic] * 8)
     assert glyph1.coordinates == GlyphCoordinates(
@@ -901,6 +907,7 @@ def test_dropImpliedOnCurvePoints_all_cubic_off_curves():
         [(11, 1), (11, 1), (11, -1), (11, -1), (9, -1), (9, -1), (9, 1), (9, 1)]
     )
     assert glyph1.endPtsOfContours == glyph2.endPtsOfContours == [7]
+    assert glyph3.numberOfContours == 0
 
 
 def test_dropImpliedOnCurvePoints_not_all_impliable():
@@ -936,6 +943,66 @@ def test_dropImpliedOnCurvePoints_not_all_impliable():
     assert glyph2.flags == array.array("B", [0, flagOnCurve, 0, 0, 0])
 
 
+def test_dropImpliedOnCurvePoints_all_empty_glyphs():
+    glyph1 = Glyph()
+    glyph1.numberOfContours = 0
+    glyph2 = Glyph()
+    glyph2.numberOfContours = 0
+
+    assert dropImpliedOnCurvePoints(glyph1, glyph2) == set()
+
+
+def test_dropImpliedOnCurvePoints_incompatible_number_of_contours():
+    glyph1 = Glyph()
+    glyph1.numberOfContours = 1
+    glyph1.endPtsOfContours = [3]
+    glyph1.flags = array.array("B", [1, 1, 1, 1])
+    glyph1.coordinates = GlyphCoordinates([(0, 0), (1, 1), (2, 2), (3, 3)])
+
+    glyph2 = Glyph()
+    glyph2.numberOfContours = 2
+    glyph2.endPtsOfContours = [1, 3]
+    glyph2.flags = array.array("B", [1, 1, 1, 1])
+    glyph2.coordinates = GlyphCoordinates([(0, 0), (1, 1), (2, 2), (3, 3)])
+
+    with pytest.raises(ValueError, match="Incompatible numberOfContours"):
+        dropImpliedOnCurvePoints(glyph1, glyph2)
+
+
+def test_dropImpliedOnCurvePoints_incompatible_flags():
+    glyph1 = Glyph()
+    glyph1.numberOfContours = 1
+    glyph1.endPtsOfContours = [3]
+    glyph1.flags = array.array("B", [1, 1, 1, 1])
+    glyph1.coordinates = GlyphCoordinates([(0, 0), (1, 1), (2, 2), (3, 3)])
+
+    glyph2 = Glyph()
+    glyph2.numberOfContours = 1
+    glyph2.endPtsOfContours = [3]
+    glyph2.flags = array.array("B", [0, 0, 0, 0])
+    glyph2.coordinates = GlyphCoordinates([(0, 0), (1, 1), (2, 2), (3, 3)])
+
+    with pytest.raises(ValueError, match="Incompatible flags"):
+        dropImpliedOnCurvePoints(glyph1, glyph2)
+
+
+def test_dropImpliedOnCurvePoints_incompatible_endPtsOfContours():
+    glyph1 = Glyph()
+    glyph1.numberOfContours = 2
+    glyph1.endPtsOfContours = [2, 6]
+    glyph1.flags = array.array("B", [1, 1, 1, 1, 1, 1, 1])
+    glyph1.coordinates = GlyphCoordinates([(i, i) for i in range(7)])
+
+    glyph2 = Glyph()
+    glyph2.numberOfContours = 2
+    glyph2.endPtsOfContours = [3, 6]
+    glyph2.flags = array.array("B", [1, 1, 1, 1, 1, 1, 1])
+    glyph2.coordinates = GlyphCoordinates([(i, i) for i in range(7)])
+
+    with pytest.raises(ValueError, match="Incompatible endPtsOfContours"):
+        dropImpliedOnCurvePoints(glyph1, glyph2)
+
+
 if __name__ == "__main__":
     import sys
 

Tests/varLib/data/DropOnCurves.designspace

@@ -0,0 +1,20 @@
+<?xml version='1.0' encoding='utf-8'?>
+<designspace format="3">
+    <axes>
+        <axis default="400" maximum="1000" minimum="400" name="weight" tag="wght" />
+    </axes>
+    <sources>
+        <source familyname="Test Family" filename="master_ufo/TestFamily-Master1.ttx" name="master_1" stylename="Master1">
+            <location>
+                <dimension name="weight" xvalue="400" />
+            </location>
+        </source>
+        <source familyname="Test Family" filename="master_ufo/TestFamily-Master2.ttx" name="master_2" stylename="Master2">
+            <location>
+                <dimension name="weight" xvalue="1000" />
+            </location>
+        </source>
+    </sources>
+    <instances>
+    </instances>
+</designspace>