diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py index d8816c629..067d3e7b7 100644 --- a/Lib/ufo2ft/outlineCompiler.py +++ b/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 diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py index a87b76ec4..17d4eff12 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 diff --git a/tests/data/ColorTest.ufo/glyphs/contents.plist b/tests/data/ColorTest.ufo/glyphs/contents.plist index 3a55d6a56..90e348beb 100644 --- a/tests/data/ColorTest.ufo/glyphs/contents.plist +++ b/tests/data/ColorTest.ufo/glyphs/contents.plist @@ -8,5 +8,7 @@ b.glif c c.glif + space + space.glif diff --git a/tests/data/ColorTest.ufo/glyphs/space.glif b/tests/data/ColorTest.ufo/glyphs/space.glif new file mode 100644 index 000000000..ea10aa54d --- /dev/null +++ b/tests/data/ColorTest.ufo/glyphs/space.glif @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/data/ColorTestRaw.ufo/glyphs/contents.plist b/tests/data/ColorTestRaw.ufo/glyphs/contents.plist index 0d46f0819..e22d7b96e 100644 --- a/tests/data/ColorTestRaw.ufo/glyphs/contents.plist +++ b/tests/data/ColorTestRaw.ufo/glyphs/contents.plist @@ -8,5 +8,7 @@ a.color1.glif a.color2 a.color2.glif + space + space.glif diff --git a/tests/data/ColorTestRaw.ufo/glyphs/space.glif b/tests/data/ColorTestRaw.ufo/glyphs/space.glif new file mode 100644 index 000000000..ea10aa54d --- /dev/null +++ b/tests/data/ColorTestRaw.ufo/glyphs/space.glif @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py index 69a84f255..7219794bc 100644 --- a/tests/outlineCompiler_test.py +++ b/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(