[morx] Clean up compilation of AAT action tables; add more tests

Lib/fontTools/ttLib/tables/otConverters.py

@@ -1128,17 +1128,9 @@ class STXHeader(BaseConverter):
 		if self.perGlyphLookup is not None:
 			glyphClassTableOffset += 4
 
-		actionData, actionIndex, actionOffset = None, None, None
-		if issubclass(self.tableClass, LigatureMorphAction):
-			# 4 bytes each for {action,ligComponents,ligatures}Offset
-			glyphClassTableOffset += 12
-			actionData, actionIndex = \
-				self._compileLigActions(value, font)
-		if issubclass(self.tableClass, InsertionMorphAction):
-			glyphClassTableOffset += 4  # 4 bytes for actionOffset
-			actionData, actionIndex = \
-				self.tableClass.compileActions(font, value.States)
-
+		glyphClassTableOffset += self.tableClass.actionHeaderSize
+		actionData, actionIndex = \
+			self.tableClass.compileActions(font, value.States)
 		stateArrayData, entryTableData = self._compileStates(
 			font, value.States, glyphClassCount, actionIndex)
 		stateArrayOffset = glyphClassTableOffset + len(glyphClassData)
@@ -1148,6 +1140,8 @@ class STXHeader(BaseConverter):
 			pad(self._compilePerGlyphLookups(value, font), 4)
 		if actionData is not None:
 			actionOffset = entryTableOffset + len(entryTableData)
+		else:
+			actionOffset = None
 
 		ligaturesOffset, ligComponentsOffset = None, None
 		ligComponentsData = self._compileLigComponents(value, font)
@@ -1222,35 +1216,6 @@ class STXHeader(BaseConverter):
 			writer.writeSubTable(lookupWriter)
 		return writer.getAllData()
 
-	def _compileLigActions(self, table, font):
-		assert issubclass(self.tableClass, LigatureMorphAction)
-		actions = set()
-		for state in table.States:
-			for _glyphClass, trans in state.Transitions.items():
-				actions.add(trans.compileLigActions())
-		result, actionIndex = b"", {}
-		# Sort the compiled actions in decreasing order of
-		# length, so that the longer sequence come before the
-		# shorter ones.  For each compiled action ABCD, its
-		# suffixes BCD, CD, and D do not be encoded separately
-		# (in case they occur); instead, we can just store an
-		# index that points into the middle of the longer
-		# sequence. Every compiled AAT ligature sequence is
-		# terminated with an end-of-sequence flag, which can
-		# only be set on the last element of the sequence.
-		# Therefore, it is sufficient to consider just the
-		# suffixes.
-		for a in sorted(actions, key=lambda x:(-len(x), x)):
-			if a not in actionIndex:
-				for i in range(0, len(a), 4):
-					suffix = a[i:]
-					suffixIndex = (len(result) + i) // 4
-					actionIndex.setdefault(
-						suffix, suffixIndex)
-				result += a
-		result = pad(result, 4)
-		return (result, actionIndex)
-
 	def _compileLigComponents(self, table, font):
 		if not hasattr(table, "LigComponents"):
 			return None

Lib/fontTools/ttLib/tables/otTables.py

@@ -7,7 +7,7 @@ converter objects from otConverters.py.
 """
 from __future__ import print_function, division, absolute_import, unicode_literals
 from fontTools.misc.py23 import *
-from fontTools.misc.textTools import safeEval
+from fontTools.misc.textTools import pad, safeEval
 from .otBase import BaseTable, FormatSwitchingBaseTable, ValueRecord
 import operator
 import logging
@@ -32,6 +32,10 @@ class AATState(object):
 class AATAction(object):
 	_FLAGS = None
 
+	@staticmethod
+	def compileActions(font, states):
+		return (None, None)
+
 	def _writeFlagsToXML(self, xmlWriter):
 		flags = [f for f in self._FLAGS if self.__dict__[f]]
 		if flags:
@@ -50,6 +54,7 @@ class AATAction(object):
 
 class RearrangementMorphAction(AATAction):
 	staticSize = 4
+	actionHeaderSize = 0
 	_FLAGS = ["MarkFirst", "DontAdvance", "MarkLast"]
 
 	_VERBS = {
@@ -131,6 +136,7 @@ class RearrangementMorphAction(AATAction):
 
 class ContextualMorphAction(AATAction):
 	staticSize = 8
+	actionHeaderSize = 0
 	_FLAGS = ["SetMark", "DontAdvance"]
 
 	def __init__(self):
@@ -209,6 +215,10 @@ class LigAction(object):
 
 class LigatureMorphAction(AATAction):
 	staticSize = 6
+
+	# 4 bytes for each of {action,ligComponents,ligatures}Offset
+	actionHeaderSize = 12
+
 	_FLAGS = ["SetComponent", "DontAdvance"]
 
 	def __init__(self):
@@ -251,6 +261,34 @@ class LigatureMorphAction(AATAction):
 		else:
 			self.Actions = []
 
+	@staticmethod
+	def compileActions(font, states):
+		result, actions, actionIndex = b"", set(), {}
+		for state in states:
+			for _glyphClass, trans in state.Transitions.items():
+				actions.add(trans.compileLigActions())
+		# Sort the compiled actions in decreasing order of
+		# length, so that the longer sequence come before the
+		# shorter ones.  For each compiled action ABCD, its
+		# suffixes BCD, CD, and D do not be encoded separately
+		# (in case they occur); instead, we can just store an
+		# index that points into the middle of the longer
+		# sequence. Every compiled AAT ligature sequence is
+		# terminated with an end-of-sequence flag, which can
+		# only be set on the last element of the sequence.
+		# Therefore, it is sufficient to consider just the
+		# suffixes.
+		for a in sorted(actions, key=lambda x:(-len(x), x)):
+			if a not in actionIndex:
+				for i in range(0, len(a), 4):
+					suffix = a[i:]
+					suffixIndex = (len(result) + i) // 4
+					actionIndex.setdefault(
+						suffix, suffixIndex)
+				result += a
+		result = pad(result, 4)
+		return (result, actionIndex)
+
 	def compileLigActions(self):
 		result = []
 		for i, action in enumerate(self.Actions):
@@ -319,7 +357,7 @@ class LigatureMorphAction(AATAction):
 
 class InsertionMorphAction(AATAction):
 	staticSize = 8
-
+	actionHeaderSize = 4  # 4 bytes for actionOffset
 	_FLAGS = ["SetMark", "DontAdvance",
 	          "CurrentIsKashidaLike", "MarkedIsKashidaLike",
 	          "CurrentInsertBefore", "MarkedInsertBefore"]

Tests/ttLib/tables/otTables_test.py

@@ -383,6 +383,10 @@ class RearrangementMorphActionTest(unittest.TestCase):
         r.compile(writer, self.font, actionIndex=None)
         self.assertEqual(hexStr(writer.getAllData()), "1234fffd")
 
+    def testCompileActions(self):
+        act = otTables.RearrangementMorphAction()
+        self.assertEqual(act.compileActions(self.font, []), (None, None))
+
     def testDecompileToXML(self):
         r = otTables.RearrangementMorphAction()
         r.decompile(OTTableReader(deHexStr("1234fffd")),
@@ -411,6 +415,10 @@ class ContextualMorphActionTest(unittest.TestCase):
         a.compile(writer, self.font, actionIndex=None)
         self.assertEqual(hexStr(writer.getAllData()), "1234f117deadbeef")
 
+    def testCompileActions(self):
+        act = otTables.ContextualMorphAction()
+        self.assertEqual(act.compileActions(self.font, []), (None, None))
+
     def testDecompileToXML(self):
         a = otTables.ContextualMorphAction()
         a.decompile(OTTableReader(deHexStr("1234f117deadbeef")),
@@ -447,6 +455,32 @@ class LigatureMorphActionTest(unittest.TestCase):
                 '</Transition>',
         ])
 
+    def testCompileActions_empty(self):
+        act = otTables.LigatureMorphAction()
+        actions, actionIndex = act.compileActions(self.font, [])
+        self.assertEqual(actions, b'')
+        self.assertEqual(actionIndex, {})
+
+    def testCompileActions_shouldShareSubsequences(self):
+        state = otTables.AATState()
+        t = state.Transitions = {i: otTables.LigatureMorphAction()
+                                 for i in range(3)}
+        ligs = [otTables.LigAction() for _ in range(3)]
+        for i, lig in enumerate(ligs):
+            lig.GlyphIndexDelta = i
+        t[0].Actions = ligs[1:2]
+        t[1].Actions = ligs[0:3]
+        t[2].Actions = ligs[1:3]
+        actions, actionIndex = t[0].compileActions(self.font, [state])
+        self.assertEqual(actions,
+                         deHexStr("00000000 00000001 80000002 80000001"))
+        self.assertEqual(actionIndex, {
+            deHexStr("00000000 00000001 80000002"): 0,
+            deHexStr("00000001 80000002"): 1,
+            deHexStr("80000002"): 2,
+            deHexStr("80000001"): 3,
+        })
+
 
 class InsertionMorphActionTest(unittest.TestCase):
     MORPH_ACTION_XML = [