diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f3608a3e5..e2cd6bf17b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ A more detailed list of changes is available in the corresponding milestones for - **[com.google.fonts/check/metadata/broken_links]:** We should not keep email addresses on METADATA.pb files (issue #4110) - **[com.google.fonts/check/description/broken_links]:** We should not keep email addresses on DESCRIPTION files (issue #4110) - **[com.google.fonts/check/metadata/nameid/font_name]:** Use name id 16, when present, to compute the expected font family name (issue #4086) + - **[com.google.fonts/check/check/colorfont_tables]:** Update checking criteria according to gf-guide (issue #4131) #### On the Universal Profile - **[com.google.fonts/check/soft_hyphen]:** Improve wording of the rationale. (issue #4095) diff --git a/Lib/fontbakery/profiles/googlefonts.py b/Lib/fontbakery/profiles/googlefonts.py index 3da02883ed..55c0229166 100644 --- a/Lib/fontbakery/profiles/googlefonts.py +++ b/Lib/fontbakery/profiles/googlefonts.py @@ -6395,43 +6395,66 @@ def com_google_fonts_check_metadata_category_hint(family_metadata): @check( id = "com.google.fonts/check/colorfont_tables", rationale = """ - Colr v0 fonts are widely supported in most browsers so they do not require - an SVG color table. However, Colr v1 is only well supported in Chrome so - we need to add an SVG table to these fonts. - - To add an SVG table, run the maximum_color tool in Nano Emoji, + COLR v0 fonts are widely supported in most browsers so they do not require + an SVG color table. However, some environments (e.g. Safari, Adobe apps) + do not currently support COLR v1 so we need to add an SVG table to these fonts, + except in the case of variable fonts, since SVG does not support + OpenType Variations. + + To automatically generate compatible SVG/COLR tables, + run the maximum_color tool in nanoemoji: https://github.com/googlefonts/nanoemoji """, - proposal = 'https://github.com/googlefonts/fontbakery/issues/3886' + proposal = ['https://googlefonts.github.io/gf-guide/color.html', + 'https://github.com/googlefonts/fontbakery/issues/3886', + 'https://github.com/googlefonts/fontbakery/issues/3888', + 'https://github.com/googlefonts/fontbakery/pull/3889', + 'https://github.com/googlefonts/fontbakery/issues/4131'] ) def com_google_fonts_check_colorfont_tables(ttFont): - """Check font has the expected color font tables""" + """Check font has the expected color font tables.""" + from .shared_conditions import is_variable_font + + passed = True + NANOEMOJI_ADVICE = ("You can do it by using the maximum_color tool provided by" + " the nanoemoji project:\n" + "https://github.com/googlefonts/nanoemoji") + if "COLR" in ttFont: - colr_table = ttFont["COLR"] - if colr_table.version == 0 and "SVG " in ttFont: - yield FAIL, Message( - "drop-svg", - "Font has a COLR v0 table, which is already widely supported, " - "so the SVG table isn't needed." - ) - return - elif colr_table.version == 1 and "SVG " not in ttFont: - yield FAIL, Message( - "add-svg", - "Font has COLRv1 but no SVG table; for CORLv1, we require " - "that an SVG table is present to support environments where " - "the former is not supported yet." - ) - return - elif "SVG " in ttFont: + if ttFont["COLR"].version == 0 and "SVG " in ttFont: + passed = False + yield FAIL,\ + Message("drop-svg", + "Font has a COLR v0 table, which is already widely supported," + " so the SVG table isn't needed.") + + elif ttFont["COLR"].version == 1 and "SVG " not in ttFont\ + and not is_variable_font(ttFont): + passed = False + yield FAIL,\ + Message("add-svg", + "Font has COLRv1 but no SVG table; for CORLv1, we require" + " that an SVG table is present to support environments where" + " the former is not supported yet.\n" + NANOEMOJI_ADVICE) + + if "SVG " in ttFont: + if is_variable_font(ttFont): + passed = False + yield FAIL,\ + Message("variable-svg", + "This is a variable font and SVG does not support" + " OpenType Variations.\n" + "Please remove the SVG table from this font.") + if "COLR" not in ttFont: - yield FAIL, Message( - "add-colr", - "Font only has an SVG table. Please add a COLR table as well. " - ) - return - yield PASS, "Looks Good!" + passed = False + yield FAIL,\ + Message("add-colr", + "Font only has an SVG table." + " Please add a COLR table as well.\n" + NANOEMOJI_ADVICE) + if passed: + yield PASS, "Looks Good!" @check( diff --git a/tests/profiles/googlefonts_test.py b/tests/profiles/googlefonts_test.py index 19631b0ec0..b9c030f71f 100644 --- a/tests/profiles/googlefonts_test.py +++ b/tests/profiles/googlefonts_test.py @@ -4328,26 +4328,51 @@ def test_check_override_freetype_rasterizer(mock_import_error): def test_check_colorfont_tables(): - """Fonts must have neither or both the tables 'COLR' and 'SVG'.""" + """Check font has the expected color font tables.""" from fontTools.ttLib import newTable + from fontbakery.profiles.shared_conditions import is_variable_font + check = CheckTester(googlefonts_profile, "com.google.fonts/check/colorfont_tables") ttFont = TTFont(TEST_FILE("color_fonts/noto-glyf_colr_1.ttf")) assert 'SVG ' not in ttFont.keys() assert 'COLR' in ttFont.keys() - # Check colr v1 font has an svg table. Will fail since font - # doesn't have one. + assert ttFont["COLR"].version == 1 + # Check colr v1 static font has an svg table (since v1 is not yet broadly supported). + # Will fail since font doesn't have one. assert_results_contain(check(ttFont), FAIL, 'add-svg', - 'with a color font lacking SVG table') + 'with a static colr v1 font lacking SVG table') + + # Fake a variable font by adding an fvar table. + ttFont["fvar"] = newTable('fvar') + assert 'fvar' in ttFont.keys() + assert is_variable_font(ttFont) + + # SVG does not support OpenType Variations + assert_PASS(check(ttFont), + 'with a variable color font without SVG table') # Fake an SVG table: ttFont["SVG "] = newTable('SVG ') + assert 'SVG ' in ttFont.keys() + + assert_results_contain(check(ttFont), + FAIL, 'variable-svg', + 'with a variable color font with SVG table') + + # Make it a static again: + del ttFont["fvar"] + assert 'fvar' not in ttFont.keys() + assert is_variable_font(ttFont) == False + + assert 'SVG ' in ttFont.keys() assert 'COLR' in ttFont.keys() + assert ttFont["COLR"].version == 1 assert_PASS(check(ttFont), - f'with a font containing both tables.') + 'with a static colr v1 font containing both tables.') # Now downgrade to colr table to v0: ttFont["COLR"].version = 0 @@ -4364,7 +4389,7 @@ def test_check_colorfont_tables(): FAIL, 'add-colr', 'with a font which should have a COLR table') - #finally delete both color font tables + # Finally delete both color font tables del ttFont["SVG "] assert 'SVG ' not in ttFont.keys() assert 'COLR' not in ttFont.keys()