[morx] Emit more meaningful subtable flags

Before this change, we were emitting XML with numeric values for `morx`
coverage flags. Now, we emit XML that makes more sense to human readers.
XML files from previous versions of fonttools can still be parsed.

Lib/fontTools/ttLib/tables/otConverters.py

@@ -883,21 +883,45 @@ class AATLookupWithDataOffset(BaseConverter):
 
 
 class MorxSubtableConverter(BaseConverter):
+	_PROCESSING_ORDERS = {
+		# bits 30 and 28 of morx.CoverageFlags; see morx spec
+		(False, False): "LayoutOrder",
+		(True, False): "ReversedLayoutOrder",
+		(False, True): "LogicalOrder",
+		(True, True): "ReversedLogicalOrder",
+	}
+
+	_PROCESSING_ORDERS_REVERSED = {
+		val: key for key, val in _PROCESSING_ORDERS.items()
+	}
+
 	def __init__(self, name, repeat, aux):
 		BaseConverter.__init__(self, name, repeat, aux)
 
+	def _setTextDirectionFromCoverageFlags(self, flags, subtable):
+		if (flags & 0x20) != 0:
+			subtable.TextDirection = "Any"
+		elif (flags & 0x80) != 0:
+			subtable.TextDirection = "Vertical"
+		else:
+			subtable.TextDirection = "Horizontal"
+
 	def read(self, reader, font, tableDict):
 		pos = reader.pos
 		m = MorxSubtable()
 		m.StructLength = reader.readULong()
-		m.CoverageFlags = reader.readUInt8()
+		flags = reader.readUInt8()
+		orderKey = ((flags & 0x40) != 0, (flags & 0x10) != 0)
+		m.ProcessingOrder = self._PROCESSING_ORDERS[orderKey]
+		self._setTextDirectionFromCoverageFlags(flags, m)
 		m.Reserved = reader.readUShort()
+		m.Reserved |= (flags & 0xF) << 16
 		m.MorphType = reader.readUInt8()
 		m.SubFeatureFlags = reader.readULong()
 		tableClass = lookupTypes["morx"].get(m.MorphType)
 		if tableClass is None:
 			assert False, ("unsupported 'morx' lookup type %s" %
-			               morphType)
+			               m.MorphType)
 		# To decode AAT ligatures, we need to know the subtable size.
 		# The easiest way to pass this along is to create a new reader
 		# that works on just the subtable as its data.
@@ -917,13 +941,14 @@ class MorxSubtableConverter(BaseConverter):
 		xmlWriter.newline()
 		xmlWriter.comment("StructLength=%d" % value.StructLength)
 		xmlWriter.newline()
-		# TODO: Emit flags in meaningful form, similar to what we
+		xmlWriter.simpletag("TextDirection", value=value.TextDirection)
-		# already do for the individual morph types.
-		xmlWriter.simpletag("CoverageFlags",
-		                    value="%d" % value.CoverageFlags)
 		xmlWriter.newline()
+		xmlWriter.simpletag("ProcessingOrder",
+		                    value=value.ProcessingOrder)
+		xmlWriter.newline()
+		if value.Reserved != 0:
 			xmlWriter.simpletag("Reserved",
-		                      value="%d" % value.Reserved)
+			                    value="0x%04x" % value.Reserved)
 			xmlWriter.newline()
 		xmlWriter.comment("MorphType=%d" % value.MorphType)
 		xmlWriter.newline()
@@ -936,11 +961,24 @@ class MorxSubtableConverter(BaseConverter):
 
 	def xmlRead(self, attrs, content, font):
 		m = MorxSubtable()
+		covFlags = 0
+		m.Reserved = 0
 		for eltName, eltAttrs, eltContent in filter(istuple, content):
-			# TODO: Parse meaningful flags, similar to what we
-			# already do for the individual morph types.
 			if eltName == "CoverageFlags":
-				m.CoverageFlags = safeEval(eltAttrs["value"])
+				# Only in XML from old versions of fonttools.
+				covFlags = safeEval(eltAttrs["value"])
+				orderKey = ((covFlags & 0x40) != 0,
+				            (covFlags & 0x10) != 0)
+				m.ProcessingOrder = self._PROCESSING_ORDERS[
+					orderKey]
+				self._setTextDirectionFromCoverageFlags(
+					covFlags, m)
+			elif eltName == "ProcessingOrder":
+				m.ProcessingOrder = eltAttrs["value"]
+				assert m.ProcessingOrder in self._PROCESSING_ORDERS_REVERSED, "unknown ProcessingOrder: %s" % m.ProcessingOrder
+			elif eltName == "TextDirection":
+				m.TextDirection = eltAttrs["value"]
+				assert m.TextDirection in {"Horizontal", "Vertical", "Any"}, "unknown TextDirection %s" % m.TextDirection
 			elif eltName == "Reserved":
 				m.Reserved = safeEval(eltAttrs["value"])
 			elif eltName == "SubFeatureFlags":
@@ -949,13 +987,27 @@ class MorxSubtableConverter(BaseConverter):
 				m.fromXML(eltName, eltAttrs, eltContent, font)
 			else:
 				assert False, eltName
+		m.Reserved = (covFlags & 0xF) << 16 | m.Reserved
 		return m
 
 	def write(self, writer, font, tableDict, value, repeatIndex=None):
+		covFlags = (value.Reserved & 0x000F0000) >> 16
+		reverseOrder, logicalOrder = self._PROCESSING_ORDERS_REVERSED[
+			value.ProcessingOrder]
+		covFlags |= 0x80 if value.TextDirection == "Vertical" else 0
+		covFlags |= 0x40 if reverseOrder else 0
+		covFlags |= 0x20 if value.TextDirection == "Any" else 0
+		covFlags |= 0x10 if logicalOrder else 0
+		value.CoverageFlags = covFlags
 		lengthIndex = len(writer.items)
 		before = writer.getDataLength()
 		value.StructLength = 0xdeadbeef
+		# The high nibble of value.Reserved is actuallly encoded
+		# into coverageFlags, so we need to clear it here.
+		origReserved = value.Reserved # including high nibble
+		value.Reserved = value.Reserved & 0xFFFF # without high nibble
 		value.compile(writer, font)
+		value.Reserved = origReserved  # restore original value
 		assert writer.items[lengthIndex] == b"\xde\xad\xbe\xef"
 		length = writer.getDataLength() - before
 		writer.items[lengthIndex] = struct.pack(">L", length)

Tests/ttLib/tables/_m_o_r_x_test.py

@@ -71,8 +71,8 @@ MORX_NONCONTEXTUAL_XML = [
     '  </MorphFeature>',
     '  <MorphSubtable index="0">',
     '    <!-- StructLength=36 -->',
-    '    <CoverageFlags value="128"/>',
+    '    <TextDirection value="Vertical"/>',
-    '    <Reserved value="0"/>',
+    '    <ProcessingOrder value="LayoutOrder"/>',
     '    <!-- MorphType=4 -->',
     '    <SubFeatureFlags value="0x00000001"/>',
     '    <NoncontextualMorph>',
@@ -130,8 +130,8 @@ MORX_REARRANGEMENT_XML = [
     '  <!-- MorphSubtableCount=1 -->',
     '  <MorphSubtable index="0">',
     '    <!-- StructLength=104 -->',
-    '    <CoverageFlags value="128"/>',
+    '    <TextDirection value="Vertical"/>',
-    '    <Reserved value="0"/>',
+    '    <ProcessingOrder value="LayoutOrder"/>',
     '    <!-- MorphType=0 -->',
     '    <SubFeatureFlags value="0x00000001"/>',
     '    <RearrangementMorph>',
@@ -339,8 +339,8 @@ MORX_CONTEXTUAL_XML = [
     '  <!-- MorphSubtableCount=1 -->',
     '  <MorphSubtable index="0">',
     '    <!-- StructLength=164 -->',
-    '    <CoverageFlags value="128"/>',
+    '    <TextDirection value="Vertical"/>',
-    '    <Reserved value="0"/>',
+    '    <ProcessingOrder value="LayoutOrder"/>',
     '    <!-- MorphType=1 -->',
     '    <SubFeatureFlags value="0x00000001"/>',
     '    <ContextualMorph>',
@@ -563,8 +563,8 @@ MORX_LIGATURE_XML = [
     '  <!-- MorphSubtableCount=1 -->',
     '  <MorphSubtable index="0">',
     '    <!-- StructLength=202 -->',
-    '    <CoverageFlags value="128"/>',
+    '    <TextDirection value="Vertical"/>',
-    '    <Reserved value="0"/>',
+    '    <ProcessingOrder value="LayoutOrder"/>',
     '    <!-- MorphType=2 -->',
     '    <SubFeatureFlags value="0x00000001"/>',
     '    <LigatureMorph>',
@@ -801,6 +801,84 @@ class MORXLigatureSubstitutionTest(unittest.TestCase):
         self.assertEqual(hexStr(table.compile(self.font)),
                          hexStr(MORX_LIGATURE_DATA))
 
+
+class MORXCoverageFlagsTest(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        cls.maxDiff = None
+        cls.font = FakeFont(['.notdef', 'A', 'B', 'C'])
+
+    def checkFlags(self, flags, textDirection, processingOrder,
+                   checkCompile=True):
+        data = bytesjoin([
+            MORX_REARRANGEMENT_DATA[:28],
+            bytechr(flags << 4),
+            MORX_REARRANGEMENT_DATA[29:]])
+        xml = []
+        for line in MORX_REARRANGEMENT_XML:
+            if line.startswith('    <TextDirection '):
+                line = '    <TextDirection value="%s"/>' % textDirection
+            elif line.startswith('    <ProcessingOrder '):
+                line = '    <ProcessingOrder value="%s"/>' % processingOrder
+            xml.append(line)
+        table1 = newTable('morx')
+        table1.decompile(data, self.font)
+        self.assertEqual(getXML(table1.toXML), xml)
+        if checkCompile:
+            table2 = newTable('morx')
+            for name, attrs, content in parseXML(xml):
+                table2.fromXML(name, attrs, content, font=self.font)
+            self.assertEqual(hexStr(table2.compile(self.font)), hexStr(data))
+
+    def test_CoverageFlags(self):
+        self.checkFlags(0x0, "Horizontal", "LayoutOrder")
+        self.checkFlags(0x1, "Horizontal", "LogicalOrder")
+        self.checkFlags(0x2, "Any", "LayoutOrder")
+        self.checkFlags(0x3, "Any", "LogicalOrder")
+        self.checkFlags(0x4, "Horizontal", "ReversedLayoutOrder")
+        self.checkFlags(0x5, "Horizontal", "ReversedLogicalOrder")
+        self.checkFlags(0x6, "Any", "ReversedLayoutOrder")
+        self.checkFlags(0x7, "Any", "ReversedLogicalOrder")
+        self.checkFlags(0x8, "Vertical", "LayoutOrder")
+        self.checkFlags(0x9, "Vertical", "LogicalOrder")
+        # We do not always check the compilation to binary data:
+        # some flag combinations do not make sense to emit in binary.
+        # Specifically, if bit 28 (TextDirection=Any) is set in
+        # CoverageFlags, bit 30 (TextDirection=Vertical) is to be
+        # ignored according to the 'morx' specification. We still want
+        # to test the _decoding_ of 'morx' subtables whose CoverageFlags
+        # have both bits 28 and 30 set, since this is a valid flag
+        # combination with defined semantics.  However, our encoder
+        # does not set TextDirection=Vertical when TextDirection=Any.
+        self.checkFlags(0xA, "Any", "LayoutOrder", checkCompile=False)
+        self.checkFlags(0xB, "Any", "LogicalOrder", checkCompile=False)
+        self.checkFlags(0xC, "Vertical", "ReversedLayoutOrder")
+        self.checkFlags(0xD, "Vertical", "ReversedLogicalOrder")
+        self.checkFlags(0xE, "Any", "ReversedLayoutOrder", checkCompile=False)
+        self.checkFlags(0xF, "Any", "ReversedLogicalOrder", checkCompile=False)
+
+    def test_ReservedCoverageFlags(self):
+        # 8A BC DE = TextDirection=Vertical, Reserved=0xABCDE
+        # Note that the lower 4 bits of the first byte are already
+        # part of the Reserved value. We test the full round-trip
+        # to encoding and decoding is quite hairy.
+        data = bytesjoin([
+            MORX_REARRANGEMENT_DATA[:28],
+            bytechr(0x8A), bytechr(0xBC), bytechr(0xDE),
+            MORX_REARRANGEMENT_DATA[31:]])
+        table = newTable('morx')
+        table.decompile(data, self.font)
+        subtable = table.table.MorphChain[0].MorphSubtable[0]
+        self.assertEqual(subtable.Reserved, 0xABCDE)
+        xml = getXML(table.toXML)
+        self.assertIn('    <Reserved value="0xabcde"/>', xml)
+        table2 = newTable('morx')
+        for name, attrs, content in parseXML(xml):
+            table2.fromXML(name, attrs, content, font=self.font)
+        self.assertEqual(hexStr(table2.compile(self.font)[28:31]), "8abcde")
+
+
 if __name__ == '__main__':
     import sys
     sys.exit(unittest.main())