Fix ccmp rules that used spacing marks instead of non-spacing ones.

Fixes https://b.corp.google.com/issue?id=16727859.

2 files changed, 112 insertions, 6 deletions

diff --git a/scripts/run_android_tests.py b/scripts/run_android_tests.py
index 090d071..3898753 100644
--- a/scripts/run_android_tests.py
+++ b/scripts/run_android_tests.py
@@ -2,25 +2,28 @@
 """Test assumptions that Android relies on."""
 
 import glob
+import json
 import unittest
 
 from fontTools import ttLib
 from nototools import coverage
 from nototools import font_data
+from nototools import render
+from nototools import unicode_data
 
 
 def load_fonts():
     """Load all fonts built for Android."""
-    all_fonts = glob.glob('out/android/*.ttf')
-    all_fonts = [ttLib.TTFont(font) for font in all_fonts]
-    return all_fonts
+    all_font_files = glob.glob('out/android/*.ttf')
+    all_fonts = [ttLib.TTFont(font) for font in all_font_files]
+    return all_font_files, all_fonts
 
 
 class TestVerticalMetrics(unittest.TestCase):
     """Test the vertical metrics of fonts."""
 
     def setUp(self):
-        self.fonts = load_fonts()
+        _, self.fonts = load_fonts()
 
     def test_ymin_ymax(self):
         """Tests yMin and yMax to be equal to what Android expects."""
@@ -34,7 +37,7 @@ class TestDigitWidths(unittest.TestCase):
     """Tests the width of digits."""
 
     def setUp(self):
-        self.fonts = load_fonts()
+        _, self.fonts = load_fonts()
         self.digits = [
             'zero', 'one', 'two', 'three', 'four',
             'five', 'six', 'seven', 'eight', 'nine']
@@ -51,7 +54,7 @@ class TestCharacterCoverage(unittest.TestCase):
     """Tests character coverage."""
 
     def setUp(self):
-        self.fonts = load_fonts()
+        _, self.fonts = load_fonts()
 
     def test_lack_of_arrows_and_combining_keycap(self):
         """Tests that arrows and combining keycap are not in the fonts."""
@@ -70,5 +73,54 @@ class TestCharacterCoverage(unittest.TestCase):
                 'U+2117 not found in %s.' % font_data.font_name(font))
 
 
+class TestSpacingMarks(unittest.TestCase):
+    """Tests that spacing marks are indeed spacing."""
+
+    def setUp(self):
+        self.font_files, _ = load_fonts()
+        charset = coverage.character_set(self.font_files[0])
+        self.marks_to_test = [char for char in charset
+                              if unicode_data.category(char) in ['Lm', 'Sk']]
+        self.advance_cache = {}
+
+    def get_advances(self, text, font):
+        """Get a list of horizontal advances for text rendered in a font."""
+        try:
+            return self.advance_cache[(text, font)]
+        except KeyError:
+            hb_output = render.run_harfbuzz_on_text(text, font, '')
+            hb_output = json.loads(hb_output)
+            advances = [glyph['ax'] for glyph in hb_output]
+            self.advance_cache[(text, font)] = advances
+            return advances
+
+    def test_individual_spacing_marks(self):
+        """Tests that spacing marks are spacing by themselves."""
+        for font in self.font_files:
+            print 'Testing %s for stand-alone spacing marks...' % font
+            for mark in self.marks_to_test:
+                mark = unichr(mark)
+                advances = self.get_advances(mark, font)
+                assert len(advances) == 1
+                self.assertNotEqual(advances[0], 0)
+
+    def test_spacing_marks_in_combination(self):
+        """Tests that spacing marks do not combine with base letters."""
+        for font in self.font_files:
+            print 'Testing %s for spacing marks in combination...' % font
+            for base_letter in (u'A\u00C6BCDEFGHIJKLMNO\u00D8\u01A0PRST'
+                                u'U\u01AFVWXYZ'
+                                u'a\u00E6bcdefghi\u0131j\u0237klmn'
+                                u'o\u00F8\u01A1prs\u017Ftu\u01B0vwxyz'
+                                u'\u03D2'):
+                print 'Testing %s combinations' % base_letter
+                for mark in self.marks_to_test:
+                    mark = unichr(mark)
+                    advances = self.get_advances(base_letter + mark, font)
+                    self.assertEqual(len(advances), 2,
+                        'The sequence <%04X, %04X> combines, '
+                        'but it should not' % (ord(base_letter), ord(mark)))
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/scripts/touchup_for_android.py b/scripts/touchup_for_android.py
index ffed31a..1dac756 100755
--- a/scripts/touchup_for_android.py
+++ b/scripts/touchup_for_android.py
@@ -8,6 +8,7 @@ import sys
 
 from fontTools import ttLib
 from nototools import font_data
+from nototools import unicode_data
 
 
 def drop_lookup(table, lookup_number):
@@ -44,6 +45,56 @@ def fix_digit_widths(font):
             hmtx_table[digit][0] = most_common_width
 
 
+_MAP_SPACING_TO_COMBINING = {
+    'acute': 'acutecomb',
+    'breve': 'brevenosp',
+    'caron': 'uni030C',
+    'cedilla': 'cedillanosp',
+    'circumflex': 'circumflexnosp',
+    'dieresis': 'dieresisnosp',
+    'dotaccent': 'dotnosp',
+    'grave': 'gravecomb',
+    'hungarumlaut': 'acutedblnosp',
+    'macron': 'macroncomb',
+    'ogonek': 'ogoneknosp',
+    'tilde': 'tildecomb',
+    'ring': 'ringnosp',
+    'tonos': 'acutecomb',
+    'uni02F3': 'ringsubnosp',
+}
+
+def fix_ccmp_lookup(font):
+    """Fixes the broken ccmp lookup."""
+    cmap = font_data.get_cmap(font)
+    reverse_cmap = {name: code for (code, name) in cmap.items()}
+
+    # Where we know the bad 'ccmp' is
+    ccmp_lookup = font['GSUB'].table.LookupList.Lookup[2]
+    assert ccmp_lookup.LookupType == 4
+    assert ccmp_lookup.SubTableCount == 1
+    ligatures = ccmp_lookup.SubTable[0].ligatures
+    for first_char, ligtable in ligatures.iteritems():
+        ligatures_to_delete = []
+        for index, ligature in enumerate(ligtable):
+            assert len(ligature.Component) == 1
+            component = ligature.Component[0]
+            if (component.endswith('comb')
+                or component in ['commaaccent',
+                                 'commaaccentrotate',
+                                 'ringacute']):
+                continue
+            if first_char == 'a' and component == 'uni02BE':
+                ligatures_to_delete.append(index)
+                continue
+            char = reverse_cmap[component]
+            general_category = unicode_data.category(char)
+            if general_category != 'Mn': # not a combining mark
+                ligature.Component[0] = _MAP_SPACING_TO_COMBINING[component]
+        ligatures[first_char] = [
+            ligature for (index, ligature) in enumerate(ligtable)
+            if index not in ligatures_to_delete]
+
+
 def apply_temporary_fixes(font):
     """Apply some temporary fixes."""
 
@@ -63,6 +114,9 @@ def apply_temporary_fixes(font):
     # Drop the lookup forming the ff ligature
     drop_lookup(font['GSUB'], 5)
 
+    # Correct the ccmp lookup to use combining marks instead of spacing ones
+    fix_ccmp_lookup(font)
+
     # Fix the digit widths
     fix_digit_widths(font)