From dc8b41f2bb987b818063b236478b22d866c32f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Corr=C3=AAa=20da=20Silva=20Sanches?= Date: Fri, 27 Dec 2024 03:26:34 -0300 Subject: [PATCH] new check: STAT/ital_axis Added to the OpenType profile. Replaces the old checks (also from OpenType profile): - opentype/italic_axis_in_stat - opentype/italic_axis_in_stat_is_boolean - opentype/italic_axis_last Large chunk of code back-ported from FontSpector. (issue #4865) --- CHANGELOG.md | 4 + .../checks/opentype/STAT/ital_axis.py | 289 +++++++++--------- Lib/fontbakery/legacy_checkids.py | 7 +- Lib/fontbakery/profiles/adobefonts.py | 4 +- Lib/fontbakery/profiles/googlefonts.py | 2 +- Lib/fontbakery/profiles/opentype.py | 4 +- tests/test_checks_googlefonts_overrides.py | 10 +- tests/test_checks_opentype_stat.py | 42 +-- 8 files changed, 176 insertions(+), 186 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67afa0b3ec..8c03316161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ A more detailed list of changes is available in the corresponding milestones for - to generate documentation - to implement a backwards compatibility mechanism +### New checks +#### Added to the OpenType profile + - **[STAT/ital_axis]**: Replaces the old checks (**opentype/italic_axis_in_stat**, **opentype/italic_axis_in_stat_is_boolean** and **opentype/italic_axis_last**) from the same profile (issue #4865) + ### Migration of checks #### Moved from Google Fonts to Universal profile - **[googlefonts/varfont/duplicate_instance_names]**: Renamed to **varfont/duplicate_instance_names** (PR #4937) diff --git a/Lib/fontbakery/checks/opentype/STAT/ital_axis.py b/Lib/fontbakery/checks/opentype/STAT/ital_axis.py index 4f6f833ee9..4c8778e4a6 100644 --- a/Lib/fontbakery/checks/opentype/STAT/ital_axis.py +++ b/Lib/fontbakery/checks/opentype/STAT/ital_axis.py @@ -1,185 +1,170 @@ import os from fontbakery.prelude import check, Message, FAIL, PASS, WARN, SKIP +from fontTools.ttLib import TTFont -@check( - id="opentype/italic_axis_in_STAT", - rationale=""" - Check that related Upright and Italic VFs have a - 'ital' axis in STAT table. - """, - proposal="https://github.com/fonttools/fontbakery/issues/2934", -) -def check_italic_axis_in_STAT(fonts, config): - """Ensure VFs have 'ital' STAT axis.""" - from fontTools.ttLib import TTFont +def get_STAT_axis(ttFont, tag): + for axis in ttFont["STAT"].table.DesignAxisRecord.Axis: + if axis.AxisTag == tag: + return axis + return None - font_filenames = [f.file for f in fonts] - italics = [f for f in font_filenames if "Italic" in f] - missing_roman = [] - italic_to_roman_mapping = {} - for italic in italics: - style_from_filename = os.path.basename(italic).split("-")[-1].split(".")[0] - is_varfont = "[" in style_from_filename - - # to remove the axes syntax used on variable-font filenames: - if is_varfont: - style_from_filename = style_from_filename.split("[")[0] - - if style_from_filename == "Italic": - if is_varfont: - # "Familyname-Italic[wght,wdth].ttf" => "Familyname[wght,wdth].ttf" - roman_counterpart = italic.replace("-Italic", "") - else: - # "Familyname-Italic.ttf" => "Familyname-Regular.ttf" - roman_counterpart = italic.replace("Italic", "Regular") - else: - # "Familyname-BoldItalic[wght,wdth].ttf" => "Familyname-Bold[wght,wdth].ttf" - roman_counterpart = italic.replace("Italic", "") - if is_varfont: - if roman_counterpart not in font_filenames: - missing_roman.append(italic) - else: - italic_to_roman_mapping[italic] = roman_counterpart +def get_STAT_axis_value(ttFont, tag): + for i, axis in enumerate(ttFont["STAT"].table.DesignAxisRecord.Axis): + if axis.AxisTag == tag: + for axisValue in ttFont["STAT"].table.AxisValueArray.AxisValue: + if axisValue.AxisIndex == i: + linked_value = None + if hasattr(axisValue, "LinkedValue"): + linked_value = axisValue.LinkedValue + return axisValue.Value, axisValue.Flags, linked_value + return None, None, None - if missing_roman: - from fontbakery.utils import pretty_print_list - missing_roman = pretty_print_list(config, missing_roman) +def check_has_ital(font): + + if "STAT" not in font.ttFont: yield FAIL, Message( - "missing-roman", - f"Italics missing a Roman counterpart, so couldn't check" - f" both Roman and Italic for 'ital' axis: {missing_roman}", + "no-stat", + f"Font {font.file} has no STAT table", ) return - # Actual check starts here - for italic_filename in italic_to_roman_mapping: - italic = italic_filename - upright = italic_to_roman_mapping[italic_filename] + if "ital" not in [ + axis.AxisTag for axis in font.ttFont["STAT"].table.DesignAxisRecord.Axis + ]: + yield FAIL, Message( + "missing-ital-axis", + f"Font {font.file} lacks an 'ital' axis in the STAT table.", + ) - for filepath in (upright, italic): - ttFont = TTFont(filepath) - if "ital" not in [ - axis.AxisTag for axis in ttFont["STAT"].table.DesignAxisRecord.Axis - ]: - yield FAIL, Message( - "missing-ital-axis", - f"Font {os.path.basename(filepath)}" f" is missing an 'ital' axis.", - ) +def check_ital_is_binary_and_last(font, is_italic): -@check( - id="opentype/italic_axis_in_STAT_is_boolean", - conditions=["style", "has_STAT_table"], - rationale=""" - Check that the value of the 'ital' STAT axis is boolean (either 0 or 1), - and elided for the Upright and not elided for the Italic, - and that the Upright is linked to the Italic. - """, - proposal="https://github.com/fonttools/fontbakery/issues/3668", -) -def check_italic_axis_in_STAT_is_boolean(ttFont, style): - """Ensure 'ital' STAT axis is boolean value""" - - def get_STAT_axis(font, tag): - for axis in font["STAT"].table.DesignAxisRecord.Axis: - if axis.AxisTag == tag: - return axis - return None - - def get_STAT_axis_value(font, tag): - for i, axis in enumerate(font["STAT"].table.DesignAxisRecord.Axis): - if axis.AxisTag == tag: - for axisValue in font["STAT"].table.AxisValueArray.AxisValue: - if axisValue.AxisIndex == i: - linkedValue = None - if hasattr(axisValue, "LinkedValue"): - linkedValue = axisValue.LinkedValue - return axisValue.Value, axisValue.Flags, linkedValue - return None, None, None - - if not get_STAT_axis(ttFont, "ital"): - yield SKIP, "Font doesn't have an ital axis" + if "STAT" not in font.ttFont: return - value, flags, linkedValue = get_STAT_axis_value(ttFont, "ital") - if (value, flags, linkedValue) == (None, None, None): + if not get_STAT_axis(font.ttFont, "ital"): + yield SKIP, "Font {font.file} doesn't have an ital axis" + return + + tags = [axis.AxisTag for axis in font.ttFont["STAT"].table.DesignAxisRecord.Axis] + ital_pos = tags.index("ital") + if ital_pos != len(tags) - 1: + yield WARN, Message( + "ital-axis-not-last", + f"Font {font.file} has 'ital' axis in position" + f" {ital_pos + 1} of {len(tags)}.", + ) + + value, flags, linked_value = get_STAT_axis_value(font.ttFont, "ital") + if (value, flags, linked_value) == (None, None, None): yield SKIP, "No 'ital' axis in STAT." return - passed = True - # Font has an 'ital' axis in STAT - if "Italic" in style: - if value != 1: - passed = False - yield WARN, Message( - "wrong-ital-axis-value", - f"STAT table 'ital' axis has wrong value." - f" Expected: 1, got '{value}'.", - ) - if flags != 0: - passed = False - yield WARN, Message( - "wrong-ital-axis-flag", - f"STAT table 'ital' axis flag is wrong." - f" Expected: 0 (not elided), got '{flags}'.", - ) + if is_italic: + expected_value = 1.0 # Italic + expected_flags = 0x0000 # AxisValueTableFlags empty else: - if value != 0: - passed = False - yield WARN, Message( - "wrong-ital-axis-value", - f"STAT table 'ital' axis has wrong value." - f" Expected: 0, got '{value}'.", - ) - if flags != 2: - passed = False - yield WARN, Message( - "wrong-ital-axis-flag", - f"STAT table 'ital' axis flag is wrong.\n" - f"Expected: 2 (elided)\n" - f"Got: '{flags}'", - ) - if linkedValue != 1: - passed = False + expected_value = 0.0 # Upright + expected_flags = 0x0002 # ElidableAxisValueName + + if value != expected_value: + yield WARN, Message( + "wrong-ital-axis-value", + f"{font.file} has STAT table 'ital' axis with wrong value." + f" Expected: {expected_value}, got '{value}'", + ) + + if flags != expected_flags: + yield WARN, Message( + "wrong-ital-axis-flag", + f"{font.file} has STAT table 'ital' axis with wrong flags." + f" Expected: {expected_flags}, got '{flags}'", + ) + + # If we are Roman, check for the linked value + if not is_italic: + if linked_value != 1.0: # Roman should be linked to a fully-italic. yield WARN, Message( "wrong-ital-axis-linkedvalue", - "STAT table 'ital' axis is not linked to Italic.", + f"{font.file} has STAT table 'ital' axis with wrong linked value." + f" Expected: 1.0, got '{linked_value}'", ) - if passed: - yield PASS, "STAT table ital axis values are good." + +def segment_vf_collection(fonts): + roman_italic = [] + italics = [] + non_italics = [] + for font in fonts: + if "-Italic[" in font.file: + italics.append(font) + else: + non_italics.append(font) + + for italic in italics: + # Find a matching roman + suspected_roman = italic.file.replace("-Italic[", "[") + + found = False + for non_italic in non_italics: + if non_italic.file == suspected_roman: + found = True + roman_italic.append((non_italic, italic)) + non_italics.remove(non_italic) + break + + if not found: + roman_italic.append((None, italic)) + + # Now add all the remaining non-italic fonts + for roman in non_italics: + roman_italic.append((roman, None)) + + return roman_italic @check( - id="opentype/italic_axis_last", - conditions=["style", "has_STAT_table"], + id="opentype/STAT/ital_axis", rationale=""" - Check that the 'ital' STAT axis is last in axis order. - """, - proposal="https://github.com/fonttools/fontbakery/issues/3669", -) -def check_italic_axis_last(ttFont, style): - """Ensure 'ital' STAT axis is last.""" + Check that related Upright and Italic VFs have an + 'ital' axis in the STAT table. - def get_STAT_axis(font, tag): - for axis in font["STAT"].table.DesignAxisRecord.Axis: - if axis.AxisTag == tag: - return axis - return None + Since the STAT table can be used to create new instances, it is + important to ensure that such an 'ital' axis be the last one + declared in the STAT table so that the eventual naming of new + instances follows the subfamily traditional scheme (RIBBI / WWS) + where "Italic" is always last. - axis = get_STAT_axis(ttFont, "ital") - if not axis: - yield SKIP, "No 'ital' axis in STAT." - return + The 'ital' axis should also be strictly boolean, only accepting + values of 0 (for Uprights) or 1 (for Italics). This usually works + as a mechanism for selecting between two linked variable font files. - if ttFont["STAT"].table.DesignAxisRecord.Axis[-1].AxisTag != "ital": - yield WARN, Message( - "ital-axis-not-last", - "STAT table 'ital' axis is not the last in the axis order.", - ) - else: - yield PASS, "STAT table ital axis order is good." + Also, the axis value name for uprights must be set as elidable. + """, + proposal=[ + "https://github.com/fonttools/fontbakery/issues/2934", + "https://github.com/fonttools/fontbakery/issues/3668", + "https://github.com/fonttools/fontbakery/issues/3669", + ], +) +def check_STAT_ital_axis(fonts, config): + """Ensure VFs have 'ital' STAT axis.""" + + for roman, italic in segment_vf_collection(fonts): + if roman and italic: + # These should definitely both have an ital axis + yield from check_has_ital(roman) + yield from check_has_ital(italic) + yield from check_ital_is_binary_and_last(roman, False) + yield from check_ital_is_binary_and_last(italic, True) + elif italic: + yield FAIL, Message( + "missing-roman", + f"Italic font {italic.file} has no matching Roman font.", + ) + elif roman: + yield from check_ital_is_binary_and_last(roman, False) diff --git a/Lib/fontbakery/legacy_checkids.py b/Lib/fontbakery/legacy_checkids.py index 8ac00d29b3..6855d9eb9a 100644 --- a/Lib/fontbakery/legacy_checkids.py +++ b/Lib/fontbakery/legacy_checkids.py @@ -233,9 +233,6 @@ "com.google.fonts/check/glyf_non_transformed_duplicate_components": "opentype/glyf_non_transformed_duplicate_components", "com.google.fonts/check/glyf_unused_data": "opentype/glyf_unused_data", "com.google.fonts/check/italic_angle": "opentype/italic_angle", - "com.google.fonts/check/italic_axis_in_stat": "opentype/italic_axis_in_STAT", - "com.google.fonts/check/italic_axis_in_stat_is_boolean": "opentype/italic_axis_in_STAT_is_boolean", - "com.google.fonts/check/italic_axis_last": "opentype/italic_axis_last", "com.google.fonts/check/kern_table": "opentype/kern_table", "com.google.fonts/check/layout_valid_feature_tags": "opentype/layout_valid_feature_tags", "com.google.fonts/check/layout_valid_language_tags": "opentype/layout_valid_language_tags", @@ -252,7 +249,9 @@ "com.google.fonts/check/post_table_version": "opentype/post_table_version", "com.adobe.fonts/check/postscript_name": "opentype/postscript_name", "com.google.fonts/check/slant_direction": "opentype/slant_direction", - # TODO: "opentype/STAT/ital_axis", + "com.google.fonts/check/italic_axis_in_stat": "opentype/STAT/ital_axis", + "com.google.fonts/check/italic_axis_in_stat_is_boolean": "opentype/STAT/ital_axis", + "com.google.fonts/check/italic_axis_last": "opentype/STAT/ital_axis", "com.google.fonts/check/unitsperem": "opentype/unitsperem", "com.adobe.fonts/check/varfont/distinct_instance_records": "opentype/varfont/distinct_instance_records", "com.google.fonts/check/varfont/family_axis_ranges": "opentype/varfont/family_axis_ranges", diff --git a/Lib/fontbakery/profiles/adobefonts.py b/Lib/fontbakery/profiles/adobefonts.py index a1c24be532..087fe059c4 100644 --- a/Lib/fontbakery/profiles/adobefonts.py +++ b/Lib/fontbakery/profiles/adobefonts.py @@ -34,11 +34,9 @@ "opentype/gdef_non_mark_chars", "opentype/gdef_spacing_marks", "opentype/italic_angle", - "opentype/italic_axis_in_STAT", - "opentype/italic_axis_in_STAT_is_boolean", - "opentype/italic_axis_last", "opentype/mac_style", "opentype/slant_direction", + "opentype/STAT/ital_axis", "opentype/varfont/family_axis_ranges", "opentype/vendor_id", # diff --git a/Lib/fontbakery/profiles/googlefonts.py b/Lib/fontbakery/profiles/googlefonts.py index 23118512ca..bc033c888d 100644 --- a/Lib/fontbakery/profiles/googlefonts.py +++ b/Lib/fontbakery/profiles/googlefonts.py @@ -204,7 +204,7 @@ "reason": "For Google Fonts, all messages from this check are considered FAILs", }, ], - "opentype/italic_axis_last": [ + "opentype/STAT/ital_axis": [ { "code": "ital-axis-not-last", "status": "FAIL", diff --git a/Lib/fontbakery/profiles/opentype.py b/Lib/fontbakery/profiles/opentype.py index 5c88295c17..f8cad1c365 100644 --- a/Lib/fontbakery/profiles/opentype.py +++ b/Lib/fontbakery/profiles/opentype.py @@ -25,9 +25,6 @@ "opentype/glyf_non_transformed_duplicate_components", "opentype/glyf_unused_data", "opentype/italic_angle", - "opentype/italic_axis_in_STAT", - "opentype/italic_axis_in_STAT_is_boolean", - "opentype/italic_axis_last", "opentype/kern_table", "opentype/layout_valid_feature_tags", "opentype/layout_valid_language_tags", @@ -44,6 +41,7 @@ "opentype/postscript_name", "opentype/post_table_version", "opentype/slant_direction", + "opentype/STAT/ital_axis", "opentype/unitsperem", "opentype/varfont/distinct_instance_records", "opentype/varfont/family_axis_ranges", diff --git a/tests/test_checks_googlefonts_overrides.py b/tests/test_checks_googlefonts_overrides.py index 83a5f7ca62..ec2bd67bc4 100644 --- a/tests/test_checks_googlefonts_overrides.py +++ b/tests/test_checks_googlefonts_overrides.py @@ -63,8 +63,8 @@ def test_check_italic_angle(check): ) -@check_id("opentype/italic_axis_in_STAT_is_boolean", profile=googlefonts_profile) -def test_check_italic_axis_in_STAT_is_boolean(check): +@check_id("opentype/STAT/ital_axis", profile=googlefonts_profile) +def test_check_STAT_ital_axis__axes_values_and_flags(check): """Ensure 'ital' STAT axis is boolean value""" # PASS @@ -101,9 +101,9 @@ def test_check_italic_axis_in_STAT_is_boolean(check): assert_results_contain(check(ttFont), FAIL, "wrong-ital-axis-linkedvalue") -@check_id("opentype/italic_axis_last", profile=googlefonts_profile) -def test_check_italic_axis_last(check): - """Ensure 'ital' STAT axis is boolean value""" +@check_id("opentype/STAT/ital_axis", profile=googlefonts_profile) +def test_check_STAT_ital_axis(check): + """Ensure VFs have 'ital' STAT axis.""" font = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") ttFont = TTFont(font) diff --git a/tests/test_checks_opentype_stat.py b/tests/test_checks_opentype_stat.py index d63e706345..1d8bae9533 100644 --- a/tests/test_checks_opentype_stat.py +++ b/tests/test_checks_opentype_stat.py @@ -36,10 +36,9 @@ def test_check_varfont_STAT_axis_record_for_each_axis(check): assert "Unfulfilled Conditions: is_variable_font" in msg -@check_id("opentype/italic_axis_in_STAT") +@check_id("opentype/STAT/ital_axis") def test_check_italic_axis_in_STAT(check): """Ensure VFs have 'ital' STAT axis.""" - # PASS fonts = [ TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf"), @@ -77,58 +76,65 @@ def test_check_italic_axis_in_STAT(check): os.remove(font) -@check_id("opentype/italic_axis_in_STAT_is_boolean") +@check_id("opentype/STAT/ital_axis") def test_check_italic_axis_in_STAT_is_boolean(check): """Ensure 'ital' STAT axis is boolean value""" # PASS font = TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf") - assert_PASS(check(TTFont(font))) + results = check(TTFont(font)) + results = [r for r in results if r.message.code == "wrong-ital-axis-value"] + assert_PASS(results) font = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") - assert_PASS(check(TTFont(font))) + results = check(TTFont(font)) + results = [r for r in results if r.message.code == "wrong-ital-axis-value"] + assert_PASS(results) # FAIL font = TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf") ttFont = TTFont(font) - ttFont["STAT"].table.AxisValueArray.AxisValue[6].Value = 1 + ttFont["STAT"].table.AxisValueArray.AxisValue[13].Value = 1 assert_results_contain(check(ttFont), WARN, "wrong-ital-axis-value") font = TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf") ttFont = TTFont(font) - ttFont["STAT"].table.AxisValueArray.AxisValue[6].Flags = 0 + ttFont["STAT"].table.AxisValueArray.AxisValue[13].Flags = 0 assert_results_contain(check(ttFont), WARN, "wrong-ital-axis-flag") - font = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") + font = TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf") ttFont = TTFont(font) - ttFont["STAT"].table.AxisValueArray.AxisValue[6].Value = 0 - assert_results_contain(check(ttFont), WARN, "wrong-ital-axis-value") + italfont = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") + ital_ttFont = TTFont(italfont) + ital_ttFont["STAT"].table.AxisValueArray.AxisValue[13].Value = 0 + assert_results_contain(check([ttFont, ital_ttFont]), WARN, "wrong-ital-axis-value") - font = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") - ttFont = TTFont(font) - ttFont["STAT"].table.AxisValueArray.AxisValue[6].Flags = 2 - assert_results_contain(check(ttFont), WARN, "wrong-ital-axis-flag") + ital_ttFont = TTFont(italfont) + ital_ttFont["STAT"].table.AxisValueArray.AxisValue[13].Flags = 2 + assert_results_contain(check([ttFont, ital_ttFont]), WARN, "wrong-ital-axis-flag") font = TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf") ttFont = TTFont(font) - ttFont["STAT"].table.AxisValueArray.AxisValue[6].LinkedValue = None + ttFont["STAT"].table.AxisValueArray.AxisValue[13].LinkedValue = 0.4 assert_results_contain(check(ttFont), WARN, "wrong-ital-axis-linkedvalue") -@check_id("opentype/italic_axis_last") +@check_id("opentype/STAT/ital_axis") def test_check_italic_axis_last(check): """Ensure 'ital' STAT axis is last.""" + font_roman = TEST_FILE("shantell/ShantellSans[BNCE,INFM,SPAC,wght].ttf") + ttFont_roman = TTFont(font_roman) font = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") ttFont = TTFont(font) # Move last axis (ital) to the front ttFont["STAT"].table.DesignAxisRecord.Axis = [ ttFont["STAT"].table.DesignAxisRecord.Axis[-1] ] + ttFont["STAT"].table.DesignAxisRecord.Axis[:-1] - assert_results_contain(check(ttFont), WARN, "ital-axis-not-last") + assert_results_contain(check([ttFont_roman, ttFont]), WARN, "ital-axis-not-last") font = TEST_FILE("shantell/ShantellSans-Italic[BNCE,INFM,SPAC,wght].ttf") - assert_PASS(check(font)) + assert_PASS(check([font_roman, font])) @check_id("opentype/weight_class_fvar")