[feaLib] Emit SinglePos (GPOS type 1) in format 2 when this is more compact

Lib/fontTools/feaLib/builder.py

@@ -600,20 +600,56 @@ class SinglePosBuilder(LookupBuilder):
         for glyphs in values.values():
             glyphs.sort(key=self.font.getGlyphID)
 
+        # Make a list of (glyphs, (otBase.ValueRecord, int valueFormat)).
+        # Glyphs with the same otBase.ValueRecord are grouped into one item.
+        values = [(glyphs, makeOpenTypeValueRecord(valrec))
+                  for valrec, glyphs in values.items()]
+
+        # Find out which glyphs should be encoded as SinglePos format 2.
+        # Format 2 is more compact than format 1 when multiple glyphs
+        # have different values but share the same integer valueFormat.
+        format2 = {}  # valueFormat --> [(glyph, value), (glyph, value), ...]
+        for glyphs, (value, valueFormat) in values:
+            if len(glyphs) == 1:
+                glyph = glyphs[0]
+                format2.setdefault(valueFormat, []).append((glyph, value))
+
+        # Only use format 2 if multiple glyphs share the same valueFormat.
+        # Otherwise, format 1 is more compact.
+        format2 = [(valueFormat, valueList)
+                   for valueFormat, valueList in format2.items()
+                   if len(valueList) > 1]
+        format2.sort()
+        format2Glyphs = set()  # {"A", "B", "C"}
+        for _, valueList in format2:
+            for (glyph, _) in valueList:
+                format2Glyphs.add(glyph)
+        for valueFormat, valueList in format2:
+            valueList.sort(key=lambda x: self.font.getGlyphID(x[0]))
+            st = otTables.SinglePos()
+            subtables.append(st)
+            st.Format = 2
+            st.ValueFormat = valueFormat
+            st.Coverage = otTables.Coverage()
+            st.Coverage.glyphs = [glyph for glyph, _value in valueList]
+            st.ValueCount = len(valueList)
+            st.Value = [value for _glyph, value in valueList]
+
         # To make the ordering of our subtables deterministic,
         # we sort subtables by the first glyph ID in their coverage.
         # Not doing this would be OK for OpenType, but testing the
         # compiler would be harder with non-deterministic output.
-        values = list(values.items())
+        values.sort(key=lambda x: self.font.getGlyphID(x[0][0]))
-        values.sort(key=lambda x: self.font.getGlyphID(x[1][0]))
 
-        for valrec, glyphs in values:
+        for glyphs, (value, valueFormat) in values:
+            if len(glyphs) == 1 and glyphs[0] in format2Glyphs:
+                continue  # already emitted as part of a format 2 subtable
             st = otTables.SinglePos()
             subtables.append(st)
             st.Format = 1
             st.Coverage = otTables.Coverage()
             st.Coverage.glyphs = glyphs
-            st.Value, st.ValueFormat = makeOpenTypeValueRecord(valrec)
+            st.Value, st.ValueFormat = value, valueFormat
 
         lookup = otTables.Lookup()
         lookup.SubTable = subtables

Lib/fontTools/feaLib/testdata/GPOS_1.fea

@@ -20,10 +20,18 @@ feature kern {
         <device 11 -8, 14 7> <device 13 8, 15 1>
     >;
     position four 400;
+    position four.oldstyle 401;
     position five <-80 0 -160 0>;
     position six -200;
     position @sevenEightNine -100;
 
+    pos P <1 0 800 0>;
+    pos Q <1 0 801 0>;
+    pos R <1 0 802 0>;
+    pos S <1 1 803 0>;
+    pos T <1 1 804 0>;
+    pos U <1 1 805 0>;
+
     # The AFDKO makeotf tool accepts re-definitions of previously defined
     # single adjustment positionings, provided the re-definition is using
     # the same value.  We replicate this behavior.

Lib/fontTools/feaLib/testdata/GPOS_1.ttx

@@ -53,8 +53,44 @@
       <Lookup index="0">
         <!-- LookupType=1 -->
         <LookupFlag value="0"/>
-        <!-- SubTableCount=6 -->
+        <!-- SubTableCount=7 -->
-        <SinglePos index="0" Format="1">
+        <SinglePos index="0" Format="2">
+          <Coverage>
+            <Glyph value="four"/>
+            <Glyph value="six"/>
+            <Glyph value="four.oldstyle"/>
+          </Coverage>
+          <ValueFormat value="4"/>
+          <!-- ValueCount=3 -->
+          <Value index="0" XAdvance="400"/>
+          <Value index="1" XAdvance="-200"/>
+          <Value index="2" XAdvance="401"/>
+        </SinglePos>
+        <SinglePos index="1" Format="2">
+          <Coverage>
+            <Glyph value="P"/>
+            <Glyph value="Q"/>
+            <Glyph value="R"/>
+          </Coverage>
+          <ValueFormat value="5"/>
+          <!-- ValueCount=3 -->
+          <Value index="0" XPlacement="1" XAdvance="800"/>
+          <Value index="1" XPlacement="1" XAdvance="801"/>
+          <Value index="2" XPlacement="1" XAdvance="802"/>
+        </SinglePos>
+        <SinglePos index="2" Format="2">
+          <Coverage>
+            <Glyph value="S"/>
+            <Glyph value="T"/>
+            <Glyph value="U"/>
+          </Coverage>
+          <ValueFormat value="7"/>
+          <!-- ValueCount=3 -->
+          <Value index="0" XPlacement="1" YPlacement="1" XAdvance="803"/>
+          <Value index="1" XPlacement="1" YPlacement="1" XAdvance="804"/>
+          <Value index="2" XPlacement="1" YPlacement="1" XAdvance="805"/>
+        </SinglePos>
+        <SinglePos index="3" Format="1">
           <Coverage>
             <Glyph value="one"/>
             <Glyph value="two"/>
@@ -64,21 +100,7 @@
           <ValueFormat value="5"/>
           <Value XPlacement="-80" XAdvance="-160"/>
         </SinglePos>
-        <SinglePos index="1" Format="1">
+        <SinglePos index="4" Format="1">
-          <Coverage>
-            <Glyph value="four"/>
-          </Coverage>
-          <ValueFormat value="4"/>
-          <Value XAdvance="400"/>
-        </SinglePos>
-        <SinglePos index="2" Format="1">
-          <Coverage>
-            <Glyph value="six"/>
-          </Coverage>
-          <ValueFormat value="4"/>
-          <Value XAdvance="-200"/>
-        </SinglePos>
-        <SinglePos index="3" Format="1">
           <Coverage>
             <Glyph value="seven"/>
             <Glyph value="eight"/>
@@ -87,7 +109,7 @@
           <ValueFormat value="4"/>
           <Value XAdvance="-100"/>
         </SinglePos>
-        <SinglePos index="4" Format="1">
+        <SinglePos index="5" Format="1">
           <Coverage>
             <Glyph value="A"/>
             <Glyph value="B"/>
@@ -114,7 +136,7 @@
             </XAdvDevice>
           </Value>
         </SinglePos>
-        <SinglePos index="5" Format="1">
+        <SinglePos index="6" Format="1">
           <Coverage>
             <Glyph value="C"/>
           </Coverage>