[feaLib] Support lookups with mixed glyph/class-based kerning

Resolves https://github.com/behdad/fonttools/issues/456

Lib/fontTools/feaLib/builder.py

@@ -617,12 +617,12 @@ class Builder(object):
 
     def add_class_pair_pos(self, location, glyphclass1, value1,
                            glyphclass2, value2):
-        lookup = self.get_lookup_(location, ClassPairPosBuilder)
+        lookup = self.get_lookup_(location, PairPosBuilder)
-        lookup.add_pair(location, glyphclass1, value1, glyphclass2, value2)
+        lookup.addClassPair(location, glyphclass1, value1, glyphclass2, value2)
 
     def add_specific_pair_pos(self, location, glyph1, value1, glyph2, value2):
-        lookup = self.get_lookup_(location, SpecificPairPosBuilder)
+        lookup = self.get_lookup_(location, PairPosBuilder)
-        lookup.add_pair(location, glyph1, value1, glyph2, value2)
+        lookup.addGlyphPair(location, glyph1, value1, glyph2, value2)
 
     def add_single_pos(self, location, prefix, suffix, pos):
         if prefix or suffix:
@@ -885,34 +885,6 @@ class MultipleSubstBuilder(LookupBuilder):
         return self.buildLookup_([subtable])
 
 
-class SpecificPairPosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 2)
-        self.pairs = {}  # (glyph1, glyph2) -> (value1, value2)
-        self.locations = {}  # (glyph1, glyph2) -> (filepath, line, column)
-
-    def add_pair(self, location, glyph1, value1, glyph2, value2):
-        key = (glyph1, glyph2)
-        oldValue = self.pairs.get(key, None)
-        if oldValue is not None:
-            otherLoc = self.locations[key]
-            raise FeatureLibError(
-                'Already defined position for pair %s %s at %s:%d:%d'
-                % (glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2]),
-                location)
-        val1, _ = makeOpenTypeValueRecord(value1)
-        val2, _ = makeOpenTypeValueRecord(value2)
-        self.pairs[key] = (val1, val2)
-        self.locations[key] = location
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and self.pairs == other.pairs)
-
-    def build(self):
-        subtables = otl.buildPairPosGlyphs(self.pairs, self.glyphMap)
-        return self.buildLookup_(subtables)
-
-
 class CursivePosBuilder(LookupBuilder):
     def __init__(self, font, location):
         LookupBuilder.__init__(self, font, location, 'GPOS', 3)
@@ -1107,29 +1079,45 @@ class ClassPairPosSubtableBuilder(object):
         self.subtables_.append(st)
 
 
-class ClassPairPosBuilder(LookupBuilder):
+class PairPosBuilder(LookupBuilder):
     SUBTABLE_BREAK_ = "SUBTABLE_BREAK"
 
     def __init__(self, font, location):
         LookupBuilder.__init__(self, font, location, 'GPOS', 2)
-        self.pairs = []  # [(location, gc1, value1, gc2, value2)*]
+        self.pairs = []  # [(gc1, value1, gc2, value2)*]
+        self.glyphPairs = {}  # (glyph1, glyph2) --> (value1, value2)
+        self.locations = {}  # (gc1, gc2) --> (filepath, line, column)
 
-    def add_pair(self, location, glyphclass1, value1, glyphclass2, value2):
+    def addClassPair(self, location, glyphclass1, value1, glyphclass2, value2):
-        self.pairs.append((location, glyphclass1, value1, glyphclass2, value2))
+        self.pairs.append((glyphclass1, value1, glyphclass2, value2))
+
+    def addGlyphPair(self, location, glyph1, value1, glyph2, value2):
+        key = (glyph1, glyph2)
+        oldValue = self.glyphPairs.get(key, None)
+        if oldValue is not None:
+            otherLoc = self.locations[key]
+            raise FeatureLibError(
+                'Already defined position for pair %s %s at %s:%d:%d'
+                % (glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2]),
+                location)
+        val1, _ = makeOpenTypeValueRecord(value1)
+        val2, _ = makeOpenTypeValueRecord(value2)
+        self.glyphPairs[key] = (val1, val2)
+        self.locations[key] = location
 
     def add_subtable_break(self, location):
-        self.pairs.append((location,
+        self.pairs.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
-                           self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
                            self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_))
 
     def equals(self, other):
         return (LookupBuilder.equals(self, other) and
+                self.glyphPairs == other.glyphPairs and
                 self.pairs == other.pairs)
 
     def build(self):
         builders = {}
         builder = None
-        for location, glyphclass1, value1, glyphclass2, value2 in self.pairs:
+        for glyphclass1, value1, glyphclass2, value2 in self.pairs:
             if glyphclass1 is self.SUBTABLE_BREAK_:
                 if builder is not None:
                     builder.addSubtableBreak()
@@ -1143,6 +1131,9 @@ class ClassPairPosBuilder(LookupBuilder):
                 builders[(valFormat1, valFormat2)] = builder
             builder.addPair(glyphclass1, val1, glyphclass2, val2)
         subtables = []
+        if self.glyphPairs:
+            subtables.extend(
+                otl.buildPairPosGlyphs(self.glyphPairs, self.glyphMap))
         for key in sorted(builders.keys()):
             subtables.extend(builders[key].subtables())
         return self.buildLookup_(subtables)

Lib/fontTools/feaLib/testdata/GPOS_2.fea

@@ -1,18 +1,28 @@
 languagesystem DFLT dflt;
 
-feature kern {
+# Mixes kerning between single glyphs, and class-based kerning.
+# https://github.com/behdad/fonttools/issues/456
+lookup MixedKerning {
+    pos v v 14;
+    pos [D O Q] [T V W] -26;
+} MixedKerning;
+
+lookup GlyphKerning {
     pos T one 100;
     pos T two 200;
     pos T two.oldstyle 200;
     pos T three 300;
     pos T four 400;
-
     pos X a 100;
     pos X b 200;
-
     pos Y a 100;
     pos Y b 200;
     pos Y c <3 3 3 3>;
+} GlyphKerning;
+
+feature kern {
+    lookup GlyphKerning;
+    lookup MixedKerning;
 } kern;
 
 feature vkrn {

Lib/fontTools/feaLib/testdata/GPOS_2.ttx

@@ -23,21 +23,67 @@
       <FeatureRecord index="0">
         <FeatureTag value="kern"/>
         <Feature>
-          <!-- LookupCount=1 -->
+          <!-- LookupCount=2 -->
-          <LookupListIndex index="0" value="0"/>
+          <LookupListIndex index="0" value="1"/>
+          <LookupListIndex index="1" value="0"/>
         </Feature>
       </FeatureRecord>
       <FeatureRecord index="1">
         <FeatureTag value="vkrn"/>
         <Feature>
           <!-- LookupCount=1 -->
-          <LookupListIndex index="0" value="1"/>
+          <LookupListIndex index="0" value="2"/>
         </Feature>
       </FeatureRecord>
     </FeatureList>
     <LookupList>
-      <!-- LookupCount=2 -->
+      <!-- LookupCount=3 -->
       <Lookup index="0">
+        <!-- LookupType=2 -->
+        <LookupFlag value="0"/>
+        <!-- SubTableCount=2 -->
+        <PairPos index="0" Format="1">
+          <Coverage>
+            <Glyph value="v"/>
+          </Coverage>
+          <ValueFormat1 value="4"/>
+          <ValueFormat2 value="0"/>
+          <!-- PairSetCount=1 -->
+          <PairSet index="0">
+            <!-- PairValueCount=1 -->
+            <PairValueRecord index="0">
+              <SecondGlyph value="v"/>
+              <Value1 XAdvance="14"/>
+            </PairValueRecord>
+          </PairSet>
+        </PairPos>
+        <PairPos index="1" Format="2">
+          <Coverage>
+            <Glyph value="D"/>
+            <Glyph value="O"/>
+            <Glyph value="Q"/>
+          </Coverage>
+          <ValueFormat1 value="4"/>
+          <ValueFormat2 value="0"/>
+          <ClassDef1>
+          </ClassDef1>
+          <ClassDef2>
+            <ClassDef glyph="T" class="1"/>
+            <ClassDef glyph="V" class="1"/>
+            <ClassDef glyph="W" class="1"/>
+          </ClassDef2>
+          <!-- Class1Count=1 -->
+          <!-- Class2Count=2 -->
+          <Class1Record index="0">
+            <Class2Record index="0">
+            </Class2Record>
+            <Class2Record index="1">
+              <Value1 XAdvance="-26"/>
+            </Class2Record>
+          </Class1Record>
+        </PairPos>
+      </Lookup>
+      <Lookup index="1">
         <!-- LookupType=2 -->
         <LookupFlag value="0"/>
         <!-- SubTableCount=2 -->
@@ -112,7 +158,7 @@
           </PairSet>
         </PairPos>
       </Lookup>
-      <Lookup index="1">
+      <Lookup index="2">
         <!-- LookupType=2 -->
         <LookupFlag value="0"/>
         <!-- SubTableCount=1 -->