Merge a few checks into opentype/fvar/axis_ranges_correct

Following FontSpector. New check: - opentype/fvar/axis_ranges_correct Old checks: - opentype/varfont/ital_range - opentype/varfont/slnt_range - opentype/varfont/wdth_valid_range - opentype/varfont/wght_valid_range On the OpenType profile. (issue fonttools#4865)
felipesanches · Nov 14, 2024 · d43afb9 · d43afb9
1 parent 551b518
commit d43afb9
Show file tree

Hide file tree

Showing 7 changed files with 126 additions and 166 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,12 +6,17 @@ A more detailed list of changes is available in the corresponding milestones for
 ### Noteworthy code-changes
   - Full fix for bug where check details with more severe status would be missing from the HTML report if a check with the same ID but less severe status was omitted (issue #4687)
 
+### New checks
+  - **[opentype/fvar/axis_ranges_correct]:** Replaces **opentype/varfont/ital_range**, **opentype/varfont/slnt_range**, **opentype/varfont/wdth_valid_range and **opentype/varfont/wght_valid_range**
+
+
 ### Changes to existing checks
 ### On the Universal profile
   - **[family/control_chars]:** renamed to **control_chars** as it is not realy a family-wide check. (issue #4896)
   - **[glyf_nested_components]**  renamed to **nested_components**.
 
 
+
 ## 0.13.0a5 (2024-Nov-10)
   - **Users are encouraged to try this pre-release as we're approaching the date when we'll cut a final v0.13.0 release...**
 

diff --git a/Lib/fontbakery/checks/opentype_fvar_axis_ranges_correct.py b/Lib/fontbakery/checks/opentype_fvar_axis_ranges_correct.py
@@ -2,123 +2,73 @@
 
 
 @check(
-    id="opentype/varfont/ital_range",
+    id="opentype/fvar/axis_ranges_correct",
     rationale="""
-        The OpenType spec says at
-        https://learn.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_ital
-        that:
-
-        [...] Valid numeric range: Values must be in the range 0 to 1.
+        According to the OpenType spec's registered design-variation tags, instances in
+        a variable font should have certain prescribed values.
+        If a variable font has a 'wght' (Weight) axis, the valid coordinate range is 1-1000.
+        If a variable font has a 'wdth' (Width) axis, the valid numeric range is strictly greater than zero.
+        If a variable font has a 'slnt' (Slant) axis, then the coordinate of its 'Regular' instance is required to be 0.
+        If a variable font has a 'ital' (Slant) axis, then the coordinate of its 'Regular' instance is required to be 0.
     """,
-    conditions=["is_variable_font", "has_ital_axis"],
+    conditions=["is_variable_font"],
+    proposal=[
+        "https://github.com/fonttools/fontbakery/issues/2264",
+        "https://github.com/fonttools/fontbakery/pull/2520",
+        "https://github.com/fonttools/fontbakery/issues/2572",
+    ],
 )
-def check_varfont_ital_range(ttFont, ital_axis):
-    """The variable font 'ital' (Italic) axis coordinates
-    is in a valid range?"""
-
-    if not (ital_axis.minValue == 0 and ital_axis.maxValue == 1):
-        yield FAIL, Message(
-            "invalid-ital-range",
-            f'The range of values for the "ital" axis in'
-            f" this font is {ital_axis.minValue} to {ital_axis.maxValue}."
-            f" Italic axis range must be 0 to 1, "
-            f" where Roman is 0 and Italic 1."
-            f" If you prefer a bigger variation range consider using"
-            f' "Slant" axis instead of "Italic".',
-        )
-
-
-@check(
-    id="opentype/varfont/slnt_range",
-    rationale="""
-        The OpenType spec says at
-        https://docs.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_slnt that:
-
-        [...] the scale for the Slant axis is interpreted as the angle of slant
-        in counter-clockwise degrees from upright. This means that a typical,
-        right-leaning oblique design will have a negative slant value. This matches
-        the scale used for the italicAngle field in the post table.
-    """,
-    conditions=["is_variable_font", "has_slnt_axis"],
-    proposal="https://github.com/fonttools/fontbakery/issues/2572",
-)
-def check_varfont_slnt_range(ttFont, slnt_axis):
-    """The variable font 'slnt' (Slant) axis coordinate
-    specifies positive values in its range?"""
-
-    if not (slnt_axis.minValue < 0 and slnt_axis.maxValue >= 0):
-        yield WARN, Message(
-            "unusual-slnt-range",
-            f'The range of values for the "slnt" axis in'
-            f" this font only allows positive coordinates"
-            f" (from {slnt_axis.minValue} to {slnt_axis.maxValue}),"
-            f" indicating that this may be a back slanted design,"
-            f" which is rare. If that's not the case, then"
-            f' the "slant" axis should be a range of'
-            f" negative values instead.",
-        )
-
-
-@check(
-    id="opentype/varfont/wght_valid_range",
-    rationale="""
-        According to the OpenType spec's
-        registered design-variation tag 'wght' available at
-        https://docs.microsoft.com/en-gb/typography/opentype/spec/dvaraxistag_wght
-
-        On the 'wght' (Weight) axis, the valid coordinate range is 1-1000.
-    """,
-    conditions=["is_variable_font", "has_wght_axis"],
-    proposal="https://github.com/fonttools/fontbakery/issues/2264",
-)
-def check_varfont_wght_valid_range(ttFont):
-    """The variable font 'wght' (Weight) axis coordinate
-    must be within spec range of 1 to 1000 on all instances."""
+def check_fvar_axis_ranges_correct(ttFont, ital_axis, slnt_axis):
+    """Axes and named instances fall within correct ranges?"""
 
     for instance in ttFont["fvar"].instances:
         if "wght" in instance.coordinates:
             value = instance.coordinates["wght"]
             if value < 1 or value > 1000:
                 yield FAIL, Message(
                     "wght-out-of-range",
-                    f'Found a bad "wght" coordinate with value {value}'
-                    f" outside of the valid range from 1 to 1000.",
+                    f"Instance {instance.name} has wght coordinate"
+                    f" of {value}, expected between 1 and 1000",
                 )
                 break
 
-
-@check(
-    id="opentype/varfont/wdth_valid_range",
-    rationale="""
-        According to the OpenType spec's
-        registered design-variation tag 'wdth' available at
-        https://docs.microsoft.com/en-gb/typography/opentype/spec/dvaraxistag_wdth
-
-        On the 'wdth' (Width) axis, the valid numeric range is strictly greater than
-        zero.
-    """,
-    conditions=["is_variable_font", "has_wdth_axis"],
-    proposal="https://github.com/fonttools/fontbakery/pull/2520",
-)
-def check_varfont_wdth_valid_range(ttFont):
-    """The variable font 'wdth' (Width) axis coordinate
-    must strictly greater than zero."""
-
     for instance in ttFont["fvar"].instances:
         if "wdth" in instance.coordinates:
             value = instance.coordinates["wdth"]
             if value < 1:
                 yield FAIL, Message(
                     "wdth-out-of-range",
-                    f'Found a bad "wdth" coordinate with value {value}'
-                    f" outside of the valid range (> 0).",
+                    f"Instance {instance.name} has wdth coordinate"
+                    f" of {value}, expected at least 1",
                 )
                 break
 
             if value > 1000:
                 yield WARN, Message(
                     "wdth-greater-than-1000",
-                    f'Found a "wdth" coordinate with value {value}'
-                    f" which is valid but unusual.",
+                    f"Instance {instance.name} has wdth coordinate"
+                    f" of {value}, which is valid but unusual",
                 )
                 break
+
+    if ital_axis:
+        if not (ital_axis.minValue == 0 and ital_axis.maxValue == 1):
+            yield FAIL, Message(
+                "invalid-ital-range",
+                f'The range of values for the "ital" axis in this font is'
+                f" {ital_axis.minValue} to {ital_axis.maxValue}."
+                f" The italic axis range must be 0 to 1, where Roman is 0 and Italic 1."
+                f' If you prefer a bigger variation range consider using the "Slant"'
+                f' axis instead of "Italic".',
+            )
+
+    if slnt_axis:
+        if not (slnt_axis.minValue < 0 and slnt_axis.maxValue >= 0):
+            yield WARN, Message(
+                "unusual-slnt-range",
+                f'The range of values for the "slnt" axis in this font only allows'
+                f" positive coordinates (from {slnt_axis.minValue} to"
+                f" {slnt_axis.maxValue}), indicating that this may be a back slanted"
+                f' design, which is rare. If that\'s not the case, then the "slant"'
+                f" axis should be a range of negative values instead.",
+            )
diff --git a/Lib/fontbakery/profiles/adobefonts.py b/Lib/fontbakery/profiles/adobefonts.py
@@ -39,7 +39,6 @@
         "opentype/mac_style",
         "opentype/slant_direction",
         "opentype/varfont/family_axis_ranges",
-        "opentype/varfont/ital_range",
         "opentype/vendor_id",
         #
         "alt_caron",

diff --git a/Lib/fontbakery/profiles/opentype.py b/Lib/fontbakery/profiles/opentype.py
@@ -18,6 +18,7 @@
             "opentype/font_version",
             "opentype/fsselection",
             "opentype/fsselection_matches_macstyle",
+            "opentype/fvar/axis_ranges_correct",
             "opentype/gdef_mark_chars",
             "opentype/gdef_non_mark_chars",
             "opentype/gdef_spacing_marks",
@@ -48,21 +49,17 @@
             "opentype/varfont/distinct_instance_records",
             "opentype/varfont/family_axis_ranges",
             "opentype/varfont/foundry_defined_tag_name",
-            "opentype/varfont/ital_range",
             "opentype/varfont/regular_ital_coord",
             "opentype/varfont/regular_opsz_coord",
             "opentype/varfont/regular_slnt_coord",
             "opentype/varfont/regular_wdth_coord",
             "opentype/varfont/regular_wght_coord",
             "opentype/varfont/same_size_instance_records",
-            "opentype/varfont/slnt_range",
             "opentype/varfont/stat_axis_record_for_each_axis",
             "opentype/varfont/valid_axis_nameid",
             "opentype/varfont/valid_default_instance_nameids",
             "opentype/varfont/valid_postscript_nameid",
             "opentype/varfont/valid_subfamily_nameid",
-            "opentype/varfont/wdth_valid_range",
-            "opentype/varfont/wght_valid_range",
             "opentype/vendor_id",
             "opentype/weight_class_fvar",
             "opentype/xavgcharwidth",

diff --git a/tests/test_checks_namecheck.py b/tests/test_checks_namecheck.py
@@ -8,8 +8,12 @@
 
 
 # TODO: Maybe skip this code-test if the service is offline?
-# we could use pytest.mak.skipif here together with a piece of code that
+# we could use pytest.mark.skipif here together with a piece of code that
 # verifies whether or not the namecheck.fontdata.com website is online at the moment
+@pytest.mark.skip(
+    "The namecheck service is too unreliable"
+    " and keeps breaking our CI jobs all the time..."
+)
 @check_id("fontdata_namecheck")
 def test_check_fontdata_namecheck(check):
     """Familyname is unique according to namecheck.fontdata.com"""

diff --git a/tests/test_checks_opentype_fvar.py b/tests/test_checks_opentype_fvar.py
@@ -247,73 +247,6 @@ def test_check_varfont_regular_opsz_coord(check):
     assert msg == ('"Regular" instance not present.')
 
 
-@check_id("opentype/varfont/wght_valid_range")
-def test_check_varfont_wght_valid_range(check):
-    """The variable font 'wght' (Weight) axis coordinate
-    must be within spec range of 1 to 1000 on all instances."""
-
-    # Our reference varfont CabinVFBeta.ttf
-    # has all instances within the 1-1000 range
-    ttFont = TTFont("data/test/cabinvfbeta/CabinVFBeta.ttf")
-    assert_PASS(check(ttFont), "with a good varfont...")
-
-    # We then introduce the problem by setting a bad value:
-    ttFont["fvar"].instances[0].coordinates["wght"] = 0
-    assert_results_contain(check(ttFont), FAIL, "wght-out-of-range", "with wght=0...")
-
-    # And yet another bad value:
-    ttFont["fvar"].instances[0].coordinates["wght"] = 1001
-    assert_results_contain(
-        check(ttFont), FAIL, "wght-out-of-range", "with wght=1001..."
-    )
-
-
-@check_id("opentype/varfont/wdth_valid_range")
-def test_check_varfont_wdth_valid_range(check):
-    """The variable font 'wdth' (Width) axis coordinate
-    must be strictly greater than zero, per the spec."""
-
-    # Our reference varfont CabinVFBeta.ttf
-    # has all instances within the 1-1000 range
-    ttFont = TTFont("data/test/cabinvfbeta/CabinVFBeta.ttf")
-    assert_PASS(check(ttFont), "with a good varfont...")
-
-    # We then introduce the problem by setting a bad value:
-    ttFont["fvar"].instances[0].coordinates["wdth"] = 0
-    assert_results_contain(check(ttFont), FAIL, "wdth-out-of-range", "with wght=0...")
-
-    # A valid but unusual value:
-    ttFont["fvar"].instances[0].coordinates["wdth"] = 1001
-    assert_results_contain(
-        check(ttFont), WARN, "wdth-greater-than-1000", "with wght=1001..."
-    )
-
-
-@check_id("opentype/varfont/slnt_range")
-def test_check_varfont_slnt_range(check):
-    """The variable font 'slnt' (Slant) axis coordinate
-    specifies positive values in its range?"""
-
-    # Our reference Inter varfont has a bad slnt range
-    ttFont = TTFont("data/test/varfont/inter/Inter[slnt,wght].ttf")
-    assert_results_contain(
-        check(ttFont),
-        WARN,
-        "unusual-slnt-range",
-        'with a varfont that has an unusual "slnt" range.',
-    )
-
-    # We then fix the font-bug by flipping the slnt axis range:
-    for i, axis in enumerate(ttFont["fvar"].axes):
-        if axis.axisTag == "slnt":
-            minValue, maxValue = axis.minValue, axis.maxValue
-            ttFont["fvar"].axes[i].minValue = -maxValue
-            ttFont["fvar"].axes[i].maxValue = -minValue
-
-    # And it must now be good ;-)
-    assert_PASS(check(ttFont))
-
-
 @check_id("opentype/varfont/foundry_defined_tag_name")
 def test_check_varfont_foundry_defined_tag_name(check):
     "Validate foundry-defined design-variation axis tag names."

diff --git a/tests/test_checks_opentype_fvar_axis_ranges_correct.py b/tests/test_checks_opentype_fvar_axis_ranges_correct.py
@@ -0,0 +1,72 @@
+from fontTools.ttLib import TTFont
+
+from conftest import check_id
+from fontbakery.status import FAIL, WARN
+from fontbakery.codetesting import (
+    assert_PASS,
+    assert_results_contain,
+)
+
+
+@check_id("opentype/fvar/axis_ranges_correct")
+def test_check_varfont_wght_valid_range(check):
+    """The variable font 'wght' (Weight) axis coordinate
+    must be within spec range of 1 to 1000 on all instances."""
+    # Our reference varfont CabinVFBeta.ttf
+    # has all instances within the 1-1000 range
+    ttFont = TTFont("data/test/cabinvfbeta/CabinVFBeta.ttf")
+    assert_PASS(check(ttFont), "with a good varfont...")
+
+    # We then introduce the problem by setting a bad value:
+    ttFont["fvar"].instances[0].coordinates["wght"] = 0
+    assert_results_contain(check(ttFont), FAIL, "wght-out-of-range", "with wght=0...")
+
+    # And yet another bad value:
+    ttFont["fvar"].instances[0].coordinates["wght"] = 1001
+    assert_results_contain(
+        check(ttFont), FAIL, "wght-out-of-range", "with wght=1001..."
+    )
+
+
+@check_id("opentype/fvar/axis_ranges_correct")
+def test_check_varfont_wdth_valid_range(check):
+    """The variable font 'wdth' (Width) axis coordinate
+    must be strictly greater than zero, per the spec."""
+    # Our reference varfont CabinVFBeta.ttf
+    # has all instances within the 1-1000 range
+    ttFont = TTFont("data/test/cabinvfbeta/CabinVFBeta.ttf")
+    assert_PASS(check(ttFont), "with a good varfont...")
+
+    # We then introduce the problem by setting a bad value:
+    ttFont["fvar"].instances[0].coordinates["wdth"] = 0
+    assert_results_contain(check(ttFont), FAIL, "wdth-out-of-range", "with wght=0...")
+
+    # A valid but unusual value:
+    ttFont["fvar"].instances[0].coordinates["wdth"] = 1001
+    assert_results_contain(
+        check(ttFont), WARN, "wdth-greater-than-1000", "with wght=1001..."
+    )
+
+
+@check_id("opentype/fvar/axis_ranges_correct")
+def test_check_varfont_slnt_range(check):
+    """The variable font 'slnt' (Slant) axis coordinate
+    specifies positive values in its range?"""
+    # Our reference Inter varfont has a bad slnt range
+    ttFont = TTFont("data/test/varfont/inter/Inter[slnt,wght].ttf")
+    assert_results_contain(
+        check(ttFont),
+        WARN,
+        "unusual-slnt-range",
+        'with a varfont that has an unusual "slnt" range.',
+    )
+
+    # We then fix the font-bug by flipping the slnt axis range:
+    for i, axis in enumerate(ttFont["fvar"].axes):
+        if axis.axisTag == "slnt":
+            minValue, maxValue = axis.minValue, axis.maxValue
+            ttFont["fvar"].axes[i].minValue = -maxValue
+            ttFont["fvar"].axes[i].maxValue = -minValue
+
+    # And it must now be good ;-)
+    assert_PASS(check(ttFont))