Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update com.google.fonts/check/varfont/regular_x_coord checks #4010

Merged
merged 11 commits into from
Jan 17, 2023
Merged
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,33 @@ A more detailed list of changes is available in the corresponding milestones for

#### On the FontWerk Profile
- **[com.fontwerk/check/inconsistencies_between_fvar_stat]:** Fixed bug that resulted in an ERROR when attempting to access `.AxisIndex` of a format 4 AxisValue table (issue #3904)
- **[com.adobe.fonts/check/stat_has_axis_value_tables]:** Fixed bug that resulted in an ERROR when attempting to access `.AxisIndex` of a format 4 AxisValue table (issue #3904)

### Changes to existing checks
#### On the OpenType Profile
- **[com.google.fonts/check/varfont/regular_wght_coord]:** The check was modified to distinguish between a font having no regular
instance (code: `no-regular-instance`) versus having a regular instance whose wght coord != 400 (existing code `wght-not-400`). (issue #4003)
- **[com.google.fonts/check/varfont/regular_wdth_coord]:** The check was modified to distinguish between a font having no regular
instance (code: `no-regular-instance`) versus having a regular instance whose wdth coord != 100 (existing code `wdth-not-100`). (issue #4003)
- **[com.google.fonts/check/varfont/regular_slnt_coord]:** The check was modified to distinguish between a font having no regular
instance (code: `no-regular-instance`) versus having a regular instance whose slnt coord != 100 (existing code `slnt-not-0`). (issue #4003)
- **[com.google.fonts/check/varfont/regular_ital_coord]:** The check was modified to distinguish between a font having no regular
instance (code: `no-regular-instance`) versus having a regular instance whose ital coord != 100 (existing code `ital-not-0`). (issue #4003)
- **[com.google.fonts/check/varfont/regular_opsz_coord]:** The check was modified to distinguish between a font having no regular
instance (code: `no-regular-instance`) versus having a regular instance whose opsz is out of range (existing code `opsz-out-of-range`). (issue #4003)
- **[com.google.fonts/check/varfont/bold_wght_coord]:** The check was modified to distinguish between a font having no bold
instance (code: `no-bold-instance`) versus having a bold instance whose wght coord != 700 (existing code `wght-not-700`). (issue #3898)

#### On the Google Fonts Profile
- **[com.google.fonts/check/vertical_metrics]:** Check for positive and negative ascender and descender values (PR #3921)

#### Overridden in the Adobe Fonts Profile
- **[com.google.fonts/check/varfont/regular_wght_coord]:** downgrade `no-regular-instance` from FAIL to WARN. (issue #4003)
- **[com.google.fonts/check/varfont/regular_wdth_coord]:** downgrade `no-regular-instance` from FAIL to WARN. (issue #4003)
- **[com.google.fonts/check/varfont/regular_slnt_coord]:** downgrade `no-regular-instance` from FAIL to WARN. (issue #4003)
- **[com.google.fonts/check/varfont/regular_ital_coord]:** downgrade `no-regular-instance` from FAIL to WARN. (issue #4003)
- **[com.google.fonts/check/varfont/regular_opsz_coord]:** downgrade `no-regular-instance` from FAIL to WARN. (issue #4003)
- **[com.google.fonts/check/varfont/bold_wght_coord]:** downgrade `no-bold-instance` from FAIL to WARN. (issue #3898)


## 0.8.10 (2022-Aug-25)
Expand Down
77 changes: 71 additions & 6 deletions Lib/fontbakery/profiles/adobefonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
SET_EXPLICIT_CHECKS = {
# This is the set of explict checks that will be invoked
# when fontbakery is run with the 'check-adobefonts' subcommand.
# The contents of this set were last updated on December 6, 2022.
# The contents of this set were last updated on December 22, 2022.
#
# =======================================
# From adobefonts.py (this file)
Expand Down Expand Up @@ -67,11 +67,11 @@
"com.adobe.fonts/check/varfont/valid_postscript_nameid",
"com.adobe.fonts/check/varfont/valid_subfamily_nameid",
"com.google.fonts/check/varfont/bold_wght_coord", # IS_OVERRIDDEN
"com.google.fonts/check/varfont/regular_ital_coord",
"com.google.fonts/check/varfont/regular_opsz_coord",
"com.google.fonts/check/varfont/regular_slnt_coord",
"com.google.fonts/check/varfont/regular_wdth_coord",
"com.google.fonts/check/varfont/regular_wght_coord",
"com.google.fonts/check/varfont/regular_ital_coord", # IS_OVERRIDDEN
"com.google.fonts/check/varfont/regular_opsz_coord", # IS_OVERRIDDEN
"com.google.fonts/check/varfont/regular_slnt_coord", # IS_OVERRIDDEN
"com.google.fonts/check/varfont/regular_wdth_coord", # IS_OVERRIDDEN
"com.google.fonts/check/varfont/regular_wght_coord", # IS_OVERRIDDEN
"com.google.fonts/check/varfont/slnt_range",
"com.google.fonts/check/varfont/wdth_valid_range",
"com.google.fonts/check/varfont/wght_valid_range",
Expand Down Expand Up @@ -232,6 +232,11 @@
"com.google.fonts/check/os2_metrics_match_hhea",
"com.google.fonts/check/valid_glyphnames",
"com.google.fonts/check/varfont/bold_wght_coord",
"com.google.fonts/check/varfont/regular_ital_coord",
"com.google.fonts/check/varfont/regular_opsz_coord",
"com.google.fonts/check/varfont/regular_slnt_coord",
"com.google.fonts/check/varfont/regular_wdth_coord",
"com.google.fonts/check/varfont/regular_wght_coord",
"com.google.fonts/check/whitespace_glyphs",
]

Expand Down Expand Up @@ -580,6 +585,66 @@ def com_adobe_fonts_check_STAT_strings(ttFont):
)


profile.check_log_override(
# From fvar.py
"com.google.fonts/check/varfont/regular_ital_coord",
overrides=(
("no-regular-instance", WARN, KEEP_ORIGINAL_MESSAGE),
),
reason=(
"Adobe strongly recommends, but does not require having a Regular instance."
),
)


profile.check_log_override(
# From fvar.py
"com.google.fonts/check/varfont/regular_opsz_coord",
overrides=(
("no-regular-instance", WARN, KEEP_ORIGINAL_MESSAGE),
),
reason=(
"Adobe strongly recommends, but does not require having a Regular instance."
),
)


profile.check_log_override(
# From fvar.py
"com.google.fonts/check/varfont/regular_slnt_coord",
overrides=(
("no-regular-instance", WARN, KEEP_ORIGINAL_MESSAGE),
),
reason=(
"Adobe strongly recommends, but does not require having a Regular instance."
),
)


profile.check_log_override(
# From fvar.py
"com.google.fonts/check/varfont/regular_wdth_coord",
overrides=(
("no-regular-instance", WARN, KEEP_ORIGINAL_MESSAGE),
),
reason=(
"Adobe strongly recommends, but does not require having a Regular instance."
),
)


profile.check_log_override(
# From fvar.py
"com.google.fonts/check/varfont/regular_wght_coord",
overrides=(
("no-regular-instance", WARN, KEEP_ORIGINAL_MESSAGE),
),
reason=(
"Adobe strongly recommends, but does not require having a Regular instance."
),
)


profile.check_log_override(
# From fvar.py
"com.adobe.fonts/check/varfont/valid_default_instance_nameids",
Expand Down
32 changes: 26 additions & 6 deletions Lib/fontbakery/profiles/fvar.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ def com_google_fonts_check_varfont_regular_wght_coord(ttFont, regular_wght_coord
"""The variable font 'wght' (Weight) axis coordinate must be 400 on the
'Regular' instance."""

if regular_wght_coord == 400:
if regular_wght_coord is None:
yield FAIL,\
Message("no-regular-instance",
'"Regular" instance not present.')
elif regular_wght_coord == 400:
yield PASS, "Regular:wght is 400."
else:
yield FAIL,\
Expand All @@ -53,7 +57,11 @@ def com_google_fonts_check_varfont_regular_wght_coord(ttFont, regular_wght_coord
def com_google_fonts_check_varfont_regular_wdth_coord(ttFont, regular_wdth_coord):
"""The variable font 'wdth' (Width) axis coordinate must be 100 on the 'Regular' instance."""

if regular_wdth_coord == 100:
if regular_wdth_coord is None:
yield FAIL,\
Message("no-regular-instance",
'"Regular" instance not present.')
elif regular_wdth_coord == 100:
yield PASS, "Regular:wdth is 100."
else:
yield FAIL,\
Expand All @@ -80,7 +88,11 @@ def com_google_fonts_check_varfont_regular_wdth_coord(ttFont, regular_wdth_coord
def com_google_fonts_check_varfont_regular_slnt_coord(ttFont, regular_slnt_coord):
"""The variable font 'slnt' (Slant) axis coordinate must be zero on the 'Regular' instance."""

if regular_slnt_coord == 0:
if regular_slnt_coord is None:
yield FAIL, \
Message("no-regular-instance",
'"Regular" instance not present.')
elif regular_slnt_coord == 0:
yield PASS, "Regular:slnt is zero."
else:
yield FAIL,\
Expand All @@ -107,7 +119,11 @@ def com_google_fonts_check_varfont_regular_slnt_coord(ttFont, regular_slnt_coord
def com_google_fonts_check_varfont_regular_ital_coord(ttFont, regular_ital_coord):
"""The variable font 'ital' (Italic) axis coordinate must be zero on the 'Regular' instance."""

if regular_ital_coord == 0:
if regular_ital_coord is None:
yield FAIL, \
Message("no-regular-instance",
'"Regular" instance not present.')
elif regular_ital_coord == 0:
yield PASS, "Regular:ital is zero."
else:
yield FAIL,\
Expand All @@ -129,13 +145,17 @@ def com_google_fonts_check_varfont_regular_ital_coord(ttFont, regular_ital_coord
a value in the range 10 to 16.
""",
conditions = ['is_variable_font',
'regular_opsz_coord'],
'has_opsz_axis'],
proposal = 'https://github.com/googlefonts/fontbakery/issues/1707'
)
def com_google_fonts_check_varfont_regular_opsz_coord(ttFont, regular_opsz_coord):
"""The variable font 'opsz' (Optical Size) axis coordinate should be between 10 and 16 on the 'Regular' instance."""

if regular_opsz_coord >= 10 and regular_opsz_coord <= 16:
if regular_opsz_coord is None:
yield FAIL, \
Message("no-regular-instance",
'"Regular" instance not present.')
elif regular_opsz_coord >= 10 and regular_opsz_coord <= 16:
yield PASS, f"Regular:opsz coordinate ({regular_opsz_coord}) looks good."
else:
yield WARN,\
Expand Down
6 changes: 6 additions & 0 deletions Lib/fontbakery/profiles/shared_conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ def has_ital_axis(ttFont):
return True


@condition
def has_opsz_axis(ttFont):
if is_variable_font(ttFont) and "opsz" in get_axis_tags_set(ttFont):
return True


def get_instance_axis_value(ttFont, instance_name, axis_tag):
if not is_variable_font(ttFont):
return None
Expand Down
21 changes: 16 additions & 5 deletions tests/profiles/fvar_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ def test_check_varfont_regular_wght_coord():
# Change the name of the first instance from 'Regular' (nameID 258)
# to 'Medium' (nameID 259). The font now has no Regular instance.
ttFont["fvar"].instances[0].subfamilyNameID = 259
assert_results_contain(check(ttFont), FAIL, "wght-not-400")
msg = assert_results_contain(check(ttFont), FAIL, "no-regular-instance")
assert msg == ('"Regular" instance not present.')

# Test with a variable font that doesn't have a 'wght' (Weight) axis.
# The check should yield SKIP.
Expand Down Expand Up @@ -76,7 +77,8 @@ def test_check_varfont_regular_wdth_coord():
# Change the name of the first instance from 'Regular' (nameID 258)
# to 'Medium' (nameID 259). The font now has no Regular instance.
ttFont["fvar"].instances[0].subfamilyNameID = 259
assert_results_contain(check(ttFont), FAIL, "wdth-not-100")
msg = assert_results_contain(check(ttFont), FAIL, "no-regular-instance")
assert msg == ('"Regular" instance not present.')

# Test with a variable font that doesn't have a 'wdth' (Width) axis.
# The check should yield SKIP.
Expand Down Expand Up @@ -127,7 +129,8 @@ def test_check_varfont_regular_slnt_coord():
# Change the name of the first instance from 'Regular' (nameID 258)
# to 'Medium' (nameID 259). The font now has no Regular instance.
first_instance.subfamilyNameID = 259
assert_results_contain(check(ttFont), FAIL, "slnt-not-0")
msg = assert_results_contain(check(ttFont), FAIL, "no-regular-instance")
assert msg == ('"Regular" instance not present.')

# Test with a variable font that doesn't have a 'slnt' (Slant) axis.
# The check should yield SKIP.
Expand Down Expand Up @@ -174,7 +177,8 @@ def test_check_varfont_regular_ital_coord():
# Change the name of the first instance from 'Regular' (nameID 258)
# to 'Medium' (nameID 259). The font now has no Regular instance.
first_instance.subfamilyNameID = 259
assert_results_contain(check(ttFont), FAIL, "ital-not-0")
msg = assert_results_contain(check(ttFont), FAIL, "no-regular-instance")
assert msg == ('"Regular" instance not present.')

# Test with a variable font that doesn't have an 'ital' (Italic) axis.
# The check should yield SKIP.
Expand Down Expand Up @@ -204,7 +208,8 @@ def test_check_varfont_regular_opsz_coord():
ttFont["fvar"].axes.append(new_axis)

# and specify a bad coordinate for the Regular:
ttFont["fvar"].instances[0].coordinates["opsz"] = 9
first_instance = ttFont["fvar"].instances[0]
first_instance.coordinates["opsz"] = 9
# Note: I know the correct instance index for this hotfix because
# I inspected the our reference CabinVF using ttx

Expand All @@ -224,6 +229,12 @@ def test_check_varfont_regular_opsz_coord():
assert_PASS(check(ttFont, {"regular_opsz_coord": value}),
f'with a good Regular:opsz coordinate ({value})...')

# Change the name of the first instance from 'Regular' (nameID 258)
# to 'Medium' (nameID 259). The font now has no Regular instance.
first_instance.subfamilyNameID = 259
msg = assert_results_contain(check(ttFont), FAIL, "no-regular-instance")
assert msg == ('"Regular" instance not present.')


def test_check_varfont_bold_wght_coord():
""" The variable font 'wght' (Weight) axis coordinate
Expand Down