Merge pull request #886 from googlefonts/empty-gid1-only-for-colrv0

don't reorder 'space', but simply warn when building COLRv0 if gid1 is not blank

Lib/ufo2ft/outlineCompiler.py

@@ -988,13 +988,31 @@ def setupTable_COLR(self):
                 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

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

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>

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>

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>

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>

tests/outlineCompiler_test.py

@@ -761,67 +761,6 @@ 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(
@@ -976,7 +915,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", "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 @@ def test_colr_cpal_interpolatable_ttf(self, FontClass):
             "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(