Skip to content

Commit

Permalink
Merge pull request #4010 from kaydeearts/kd-no-regular-instance
Browse files Browse the repository at this point in the history
Update com.google.fonts/check/varfont/regular_x_coord checks
  • Loading branch information
miguelsousa authored Jan 17, 2023
2 parents 80a7bb5 + 21de115 commit 4f3f0df
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 17 deletions.
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,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 @@ -412,6 +412,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

0 comments on commit 4f3f0df

Please sign in to comment.