Merge pull request #881 from googlefonts/space-order

[outlineCompiler] Make space the 2nd glyph unless its order is explicitly set
googlefonts · Oct 16, 2024 · 5bb7139 · 5bb7139
2 parents feeb474 + 2792574
commit 5bb7139
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 2 deletions.
diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py
@@ -32,15 +32,20 @@ def makeOfficialGlyphOrder(font, glyphOrder=None):
     If not explicit glyphOrder is defined, sort glyphs alphabetically.
 
     If ".notdef" glyph is present in the font, force this to always be
-    the first glyph (at index 0).
+    the first glyph (at index 0). Also, if "space" is present in the font and
+    missing from glyphOrder, force it to be the second glyph (at index 1).
     """
     if glyphOrder is None:
         glyphOrder = getattr(font, "glyphOrder", ())
+    reorderSpace = "space" not in glyphOrder
     names = set(font.keys())
     order = []
     if ".notdef" in names:
         names.remove(".notdef")
         order.append(".notdef")
+    if reorderSpace and "space" in names:
+        names.remove("space")
+        order.append("space")
     for name in glyphOrder:
         if name not in names:
             continue

diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py
@@ -761,6 +761,67 @@ def test_compile_strange_glyph_order(self, quadufo):
         compiler.compile()
         assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
 
+    def test_compile_reorder_space_glyph(self, quadufo):
+        """
+        Test that ufo2ft always puts .notdef first, and put space second if no
+        explicit glyph order is set.
+        """
+        EXPECTED_ORDER = [
+            ".notdef",
+            "space",
+            "a",
+            "b",
+            "c",
+            "d",
+            "e",
+            "f",
+            "g",
+            "h",
+            "i",
+            "j",
+            "k",
+            "l",
+        ]
+        # Check with no public.glyphOrder
+        del quadufo.lib["public.glyphOrder"]
+        assert not quadufo.glyphOrder
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        # Empty glyphOrder is considered the same
+        quadufo.glyphOrder = []
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        # Non-empty glyphOrder without "space" is considered the same
+        quadufo.glyphOrder = [n for n in EXPECTED_ORDER if n != "space"]
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
+        EXPECTED_ORDER = [
+            ".notdef",
+            "a",
+            "b",
+            "c",
+            "d",
+            "space",
+            "e",
+            "f",
+            "g",
+            "h",
+            "i",
+            "j",
+            "k",
+            "l",
+        ]
+        quadufo.glyphOrder = EXPECTED_ORDER
+        compiler = OutlineTTFCompiler(quadufo)
+        compiler.compile()
+        assert compiler.otf.getGlyphOrder() == EXPECTED_ORDER
+
 
 class NamesTest:
     @pytest.mark.parametrize(
@@ -915,7 +976,7 @@ def test_warn_name_exceeds_max_length(self, testufo, caplog):
         assert long_name in result.getGlyphOrder()
 
     def test_duplicate_glyph_names(self, testufo):
-        order = ["ab", "ab.1", "a-b", "a/b", "ba"]
+        order = ["ab", "ab.1", "a-b", "a/b", "ba", "space"]
         testufo.lib["public.glyphOrder"] = order
         testufo.lib["public.postscriptNames"] = {"ba": "ab"}
         for name in order: