diff --git a/Lib/fontbakery/checks/opentype/fsselection.py b/Lib/fontbakery/checks/opentype/fsselection.py index 2275d268d7..c96dd36327 100644 --- a/Lib/fontbakery/checks/opentype/fsselection.py +++ b/Lib/fontbakery/checks/opentype/fsselection.py @@ -1,3 +1,4 @@ +from fontbakery.constants import FsSelection, MacStyle from fontbakery.prelude import check, Message, FAIL @@ -5,70 +6,25 @@ id="opentype/fsselection", conditions=["style"], rationale=""" - The OS/2.fsSelection field is a bit field used to specify the stylistic - qualities of the font - in particular, it specifies to some operating - systems whether the font is italic (bit 0), bold (bit 5) or regular - (bit 6). + The OS/2.fsSelection field is a bit field used to specify the stylistic + qualities of the font - in particular, it specifies to some operating + systems whether the font is italic (bit 0), bold (bit 5) or regular + (bit 6). - This check verifies that the fsSelection field is set correctly for the - font style. For a family of static fonts created in GlyphsApp, this is - set by using the style linking checkboxes in the exports settings. + This check verifies that the fsSelection field is set correctly for the + font style. For a family of static fonts created in GlyphsApp, this is + set by using the style linking checkboxes in the exports settings. + + Additionally, the bold and italic bits in OS/2.fsSelection must match the + bold and italic bits in head.macStyle per the OpenType spec. """, - proposal="https://github.com/fonttools/fontbakery/issues/4829", # legacy check + proposal=[ + "https://github.com/fonttools/fontbakery/issues/4829", # legacy check + "https://github.com/fonttools/fontbakery/pull/2382", + ], ) def check_fsselection(ttFont, style): """Checking OS/2 fsSelection value.""" - from fontbakery.constants import RIBBI_STYLE_NAMES, STATIC_STYLE_NAMES, FsSelection - from fontbakery.utils import check_bit_entry - - # Checking fsSelection REGULAR bit: - expected = "Regular" in style or ( - style in STATIC_STYLE_NAMES - and style not in RIBBI_STYLE_NAMES - and "Italic" not in style - ) - yield check_bit_entry( - ttFont, - "OS/2", - "fsSelection", - expected, - bitmask=FsSelection.REGULAR, - bitname="REGULAR", - ) - - # Checking fsSelection ITALIC bit: - expected = "Italic" in style - yield check_bit_entry( - ttFont, - "OS/2", - "fsSelection", - expected, - bitmask=FsSelection.ITALIC, - bitname="ITALIC", - ) - - # Checking fsSelection BOLD bit: - expected = style in ["Bold", "BoldItalic"] - yield check_bit_entry( - ttFont, - "OS/2", - "fsSelection", - expected, - bitmask=FsSelection.BOLD, - bitname="BOLD", - ) - - -@check( - id="opentype/fsselection_matches_macstyle", - rationale=""" - The bold and italic bits in OS/2.fsSelection must match the bold and italic - bits in head.macStyle per the OpenType spec. - """, - proposal="https://github.com/fonttools/fontbakery/pull/2382", -) -def check_fsselection_matches_macstyle(ttFont): - """Check if OS/2 fsSelection matches head macStyle bold and italic bits.""" # Check both OS/2 and head are present. missing_tables = False @@ -81,23 +37,34 @@ def check_fsselection_matches_macstyle(ttFont): if missing_tables: return - from fontbakery.constants import FsSelection, MacStyle + bold_expected = style in set("Bold", "BoldItalic") + italic_expected = "Italic" in style + regular_expected = (not bold_expected) and (not italic_expected) + bold_seen = bool(ttFont["OS/2"].fsSelection & FsSelection.BOLD) + italic_seen = bool(ttFont["OS/2"].fsSelection & FsSelection.ITALIC) + regular_seen = bool(ttFont["OS/2"].fsSelection & FsSelection.REGULAR) + + for flag, expected, label in [ + (bold_seen, bold_expected, "Bold"), + (italic_seen, italic_expected, "Italic"), + (regular_seen, regular_expected, "Regular"), + ]: + if flag != expected: + yield FAIL, Message( + f"bad-{label.upper()}", + f"fsSelection {label} flag {flag}" + f" does not match font style {style}", + ) - head_bold = (ttFont["head"].macStyle & MacStyle.BOLD) != 0 - os2_bold = (ttFont["OS/2"].fsSelection & FsSelection.BOLD) != 0 - if head_bold != os2_bold: - yield FAIL, Message( - "fsselection-macstyle-bold", - "The OS/2.fsSelection and head.macStyle bold settings do not match:\n\n" - f"* OS/2.fsSelection: BOLD is {'not ' if not os2_bold else ''}set\n" - f"* head.macStyle: BOLD is {'not ' if not head_bold else ''}set", - ) - head_italic = (ttFont["head"].macStyle & MacStyle.ITALIC) != 0 - os2_italic = (ttFont["OS/2"].fsSelection & FsSelection.ITALIC) != 0 - if head_italic != os2_italic: - yield FAIL, Message( - "fsselection-macstyle-italic", - "The OS/2.fsSelection and head.macStyle italic settings do not match.\n\n" - f"* OS/2.fsSelection: ITALIC is {'not ' if not os2_italic else ''}set\n" - f"* head.macStyle: ITALIC is {'not ' if not head_italic else ''}set", - ) + mac_bold = bool(ttFont["head"].macStyle & MacStyle.BOLD) + mac_italic = bool(ttFont["head"].macStyle & MacStyle.ITALIC) + for flag, expected, label in [ + (bold_seen, mac_bold, "Bold"), + (italic_seen, mac_italic, "Italic"), + ]: + if flag != expected: + yield FAIL, Message( + f"fsselection-macstyle-{label.lower()}", + f"fsSelection {label} flag {flag}" + f" does not match macStyle {expected} flag", + ) diff --git a/Lib/fontbakery/legacy_checkids.py b/Lib/fontbakery/legacy_checkids.py index 2ccb8d62e5..28429912e2 100644 --- a/Lib/fontbakery/legacy_checkids.py +++ b/Lib/fontbakery/legacy_checkids.py @@ -219,7 +219,7 @@ "com.google.fonts/check/family/underline_thickness": "opentype/family/underline_thickness", "com.google.fonts/check/font_version TODO: double-check this!": "opentype/font_version", "com.google.fonts/check/fsselection": "opentype/fsselection", - "com.adobe.fonts/check/fsselection_matches_macstyle": "opentype/fsselection_matches_macstyle", + "com.adobe.fonts/check/fsselection_matches_macstyle": "opentype/fsselection", "com.typenetwork/check/varfont/ital_range": "opentype/fvar/axis_ranges_correct", "com.google.fonts/check/varfont/slnt_range": "opentype/fvar/axis_ranges_correct", "com.google.fonts/check/varfont/wdth_valid_range": "opentype/fvar/axis_ranges_correct", diff --git a/Lib/fontbakery/profiles/opentype.py b/Lib/fontbakery/profiles/opentype.py index 9710a3f350..5c88295c17 100644 --- a/Lib/fontbakery/profiles/opentype.py +++ b/Lib/fontbakery/profiles/opentype.py @@ -17,7 +17,6 @@ "opentype/family/underline_thickness", "opentype/font_version", "opentype/fsselection", - "opentype/fsselection_matches_macstyle", "opentype/fvar/axis_ranges_correct", "opentype/fvar/regular_coords_correct", "opentype/gdef_mark_chars", diff --git a/tests/test_checks_opentype_os2.py b/tests/test_checks_opentype_os2.py index bb3d389ba1..41d032908d 100644 --- a/tests/test_checks_opentype_os2.py +++ b/tests/test_checks_opentype_os2.py @@ -14,7 +14,6 @@ assert_results_contain, portable_path, TEST_FILE, - MockFont, ) @@ -176,10 +175,9 @@ def test_check_xavgcharwidth(check): assert_results_contain(check(test_font), FATAL, "missing-glyphs") -@check_id("opentype/fsselection_matches_macstyle") +@check_id("opentype/fsselection") def test_check_fsselection_matches_macstyle(check): """Check if OS/2 fsSelection matches head macStyle bold and italic bits.""" - from fontbakery.constants import FsSelection test_font_path = TEST_FILE("nunito/Nunito-Regular.ttf") @@ -193,7 +191,7 @@ def test_check_fsselection_matches_macstyle(check): message = assert_results_contain( check(test_font), FAIL, "fsselection-macstyle-bold" ) - assert "bold" in message + assert "Bold" in message # now turn off bold in OS/2.fsSelection so we can focus on italic test_font["OS/2"].fsSelection &= ~FsSelection.BOLD @@ -203,7 +201,7 @@ def test_check_fsselection_matches_macstyle(check): message = assert_results_contain( check(test_font), FAIL, "fsselection-macstyle-italic" ) - assert "italic" in message + assert "Italic" in message @check_id("opentype/family/bold_italic_unique_for_nameid1") @@ -282,7 +280,6 @@ def test_check_vendor_id(check): @check_id("opentype/fsselection") def test_check_fsselection(check): """Checking OS/2 fsSelection value.""" - from fontbakery.constants import FsSelection ttFont = TTFont(TEST_FILE("cabin/Cabin-Regular.ttf")) @@ -355,19 +352,23 @@ def test_check_fsselection(check): ], ] - for fsSelection_value, style, expected, expected_message in test_cases: + for fsSelection_value, style, expected, _expected_message in test_cases: ttFont["OS/2"].fsSelection = fsSelection_value + ttFont.reader.file.name = f"Test-{style}.ttf" + print(f"Testing {ttFont.reader.file.name}...") if expected == PASS: + results = list(check(ttFont)) + # Only care about results which refer to the old FontBakery test here + results = [r for r in results if "fsselection" not in r.message.code] assert_PASS( - check(MockFont(ttFont=ttFont, style=style)), + results, "with fsSelection:{fsSelection_value} style:{style}...", ) else: message = assert_results_contain( - check(MockFont(ttFont=ttFont, style=style)), + check(ttFont), FAIL, expected, f"with fsSelection:{fsSelection_value} style:{style}...", ) - assert message == expected_message