Repository: googlefonts/ufo2ft
Pull request #886 (2024-11-08 12:41:00)

don't reorder 'space', but simply warn when building COLRv0 if gid1 is not blank
After reverting https://github.com/googlefonts/glyphsLib/pull/1038, I also decided to revert #881 (at least the latter wasn't released yet).

It was a detail of DirectWrite COLRv0 implementation in Windows 10 that required that glyph at index 1 should be blank, but that's no longer the case for later Windows versions, and in fact the COLR OT spec also removed that.
https://github.com/MicrosoftDocs/typography-issues/issues/346

There's no guarantee that a glyph named "space" is blank, and the reverted code was only checking glyph names.
There is no reason to put a glyph named "space" in second position in fonts that don't even contain COLRv0.
And one may well build a COLRv0 font that is not intended to work on older Windows and as such doesn't require the gid1 to be blank.

All in all, I think that it's sufficient to just issue a Windows-specific warning when the built font contains any COLRv0 color glyphs and the glyph at index 1 is not blank.

The font developer can then decide what is best, whether to ignore the warning or whether to add or select an empty glyph to be placed at index 1 by setting the appropriate `public.glyphOrder` key in the lib.plist.




Patch:
diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py
index d8816c6..067d3e7 100644
--- a/Lib/ufo2ft/outlineCompiler.py
+++ b/Lib/ufo2ft/outlineCompiler.py
@@ -988,13 +988,31 @@ class BaseOutlineCompiler:
                 for glyphs, box in self.ufo.lib.get(COLR_CLIP_BOXES_KEY, ())
                 for glyphName in glyphs
             }
-            self.otf["COLR"] = buildCOLR(
+            colr = buildCOLR(
                 layerInfo,
                 glyphMap=glyphMap,
                 clipBoxes=clipBoxes,
                 allowLayerReuse=self.colrLayerReuse,
             )
 
+            # Warn if there are any COLRv0 base glyphs and the gid1 isn't blank
+            # https://github.com/MicrosoftDocs/typography-issues/issues/346
+            if (colr.version == 0 or colr.table.BaseGlyphRecordCount > 0) and len(
+                self.glyphOrder
+            ) > 1:
+                g1 = self.allGlyphs[self.glyphOrder[1]]
+                if len(g1) > 0 or len(g1.components) > 0:
+                    logger.warning(
+                        "COLRv0 might not render correctly on Windows because "
+                        "the glyph at index 1 is not empty ('%s'). DirectWrite's "
+                        "COLRv0 implementation in Windows 10 used to require glyph ID "
+                        "1 to be blank, see: "
+                        "https://github.com/MicrosoftDocs/typography-issues/issues/346",
+                        g1.name,
+                    )
+
+            self.otf["COLR"] = colr
+
     def _computeCOLRClipBoxes(self):
         if (
             "COLR" not in self.otf
diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py
index a87b76e..17d4eff 100644
--- a/Lib/ufo2ft/util.py
+++ b/Lib/ufo2ft/util.py
@@ -32,20 +32,15 @@ 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). Also, if "space" is present in the font and
-    missing from glyphOrder, force it to be the second glyph (at index 1).
+    the first glyph (at index 0).
     """
     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


Test patch:
diff --git a/tests/data/ColorTest.ufo/glyphs/contents.plist b/tests/data/ColorTest.ufo/glyphs/contents.plist
index 3a55d6a..90e348b 100644
--- a/tests/data/ColorTest.ufo/glyphs/contents.plist
+++ b/tests/data/ColorTest.ufo/glyphs/contents.plist
@@ -8,5 +8,7 @@
     <string>b.glif</string>
     <key>c</key>
     <string>c.glif</string>
+    <key>space</key>
+    <string>space.glif</string>
   </dict>
 </plist>
diff --git a/tests/data/ColorTest.ufo/glyphs/space.glif b/tests/data/ColorTest.ufo/glyphs/space.glif
new file mode 100644
index 0000000..ea10aa5
--- /dev/null
+++ b/tests/data/ColorTest.ufo/glyphs/space.glif
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<glyph name="space" format="2">
+  <unicode hex="0020"/>
+  <advance width="250"/>
+</glyph>
diff --git a/tests/data/ColorTestRaw.ufo/glyphs/contents.plist b/tests/data/ColorTestRaw.ufo/glyphs/contents.plist
index 0d46f08..e22d7b9 100644
--- a/tests/data/ColorTestRaw.ufo/glyphs/contents.plist
+++ b/tests/data/ColorTestRaw.ufo/glyphs/contents.plist
@@ -8,5 +8,7 @@
     <string>a.color1.glif</string>
     <key>a.color2</key>
     <string>a.color2.glif</string>
+    <key>space</key>
+    <string>space.glif</string>
   </dict>
 </plist>
diff --git a/tests/data/ColorTestRaw.ufo/glyphs/space.glif b/tests/data/ColorTestRaw.ufo/glyphs/space.glif
new file mode 100644
index 0000000..ea10aa5
--- /dev/null
+++ b/tests/data/ColorTestRaw.ufo/glyphs/space.glif
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<glyph name="space" format="2">
+  <unicode hex="0020"/>
+  <advance width="250"/>
+</glyph>
diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py
index 69a84f2..7219794 100644
--- a/tests/outlineCompiler_test.py
+++ b/tests/outlineCompiler_test.py
@@ -761,67 +761,6 @@ class GlyphOrderTest:
         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(
@@ -976,7 +915,7 @@ class NamesTest:
         assert long_name in result.getGlyphOrder()
 
     def test_duplicate_glyph_names(self, testufo):
-        order = ["ab", "ab.1", "a-b", "a/b", "ba", "space"]
+        order = ["ab", "ab.1", "a-b", "a/b", "ba"]
         testufo.lib["public.glyphOrder"] = order
         testufo.lib["public.postscriptNames"] = {"ba": "ab"}
         for name in order:
@@ -1070,6 +1009,21 @@ class ColrCpalTest:
             "c": [("c.color2", 1), ("c.color1", 0)],
         }
 
+    def test_colr_cpal_gid1_not_blank(self, FontClass, caplog):
+        # https://github.com/MicrosoftDocs/typography-issues/issues/346
+        testufo = FontClass(getpath("ColorTest.ufo"))
+        del testufo["space"]
+
+        with caplog.at_level(logging.WARNING, logger="ufo2ft.outlineCompiler"):
+            ttf = compileTTF(testufo)
+
+        assert ttf["COLR"].version == 0
+        assert ttf.getGlyphOrder()[1] == "a"
+        assert (
+            "COLRv0 might not render correctly on Windows because "
+            "the glyph at index 1 is not empty ('a')."
+        ) in caplog.text
+
     @pytest.mark.parametrize("compileFunc", [compileTTF, compileOTF])
     @pytest.mark.parametrize("manualClipBoxes", [True, False])
     @pytest.mark.parametrize(