Skip to content

Commit

Permalink
Font Bakery sync (#65)
Browse files Browse the repository at this point in the history
  • Loading branch information
miguelsousa authored Sep 10, 2023
2 parents 3916768 + 8896e7a commit 2f1cb54
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 29 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `shaping` subcommand and extra (#36).
- `fontwerk` extra (#37).
- `notofonts` extra (#37).
- `com.thetypefounders/check/features_default_languagesystem`: Checks if a default languagesystem statement is present in feature files and warns if the compiler will not insert one automatically (https://github.com/fonttools/fontbakery/issues/4011)

### Changed

Expand All @@ -31,13 +32,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `com.google.fonts/check/unique_glyphnames`: The check now takes into account that OpenType-CFF2 fonts with `post` table format 3 contain no glyph names, and will yield SKIP (#38).
- `com.google.fonts/check/STAT_in_statics`: The check now skips fonts that do not have a `STAT` table (#38).
- `com.google.fonts/check/family_naming_recommendations`: Two validations of PostScript name were moved out of this check and into `com.adobe.fonts/check/postscript_name` which yields FAIL (#62).
- `com.google.fonts/check/cjk_not_enough_glyphs`: This check is now only run when a font has CJK codepages or ranges declared in the `OS/2` table. Other CJK-related checks are run on fonts with a minimum of 150 CJK glyphs (https://github.com/fonttools/fontbakery/issues/3846).
- `com.google.fonts/check/family/panose_familytype` and `com.google.fonts/check/family/panose_proportion`: Failures have been downgraded to warnings (https://github.com/fonttools/fontbakery/issues/4192).

### Fixed

- `com.google.fonts/check/interpolation_issues`: The check ERRORed when ran on CFF2 variable fonts. The check is now SKIPped for such fonts because it depends on the presence of the `gvar` table, which only apply to TrueType variable fonts (#28).
- `com.google.fonts/check/fontvalidator`: ERROR caused by attempting to run FontValidator before checking if it's installed (#30).
- `com.google.fonts/check/mandatory_glyphs`: Improved the check's resilience to edge cases that could result in ERRORs (#38).
- `-L`/`--list-checks` option that can be used with subcommands. Previously this option only worked if a path to an input file was also provided in the command line (#35).
- Summary statistics on HTML reporter (https://github.com/fonttools/fontbakery/issues/3997).

## [0.1.0] - 2023-06-11

Expand Down
3 changes: 0 additions & 3 deletions Lib/openbakery/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,6 @@ class WindowsLanguageID(enum.IntEnum):
"Hangul Syllables": 56,
"CJK Unified Ideographs": 59,
"CJK Strokes": 61,
"Yi Syllables": 83,
}


Expand All @@ -602,8 +601,6 @@ class WindowsLanguageID(enum.IntEnum):
[0x31C0, 0x31EF], # CJK Strokes
[0xF900, 0xFAFF], # CJK Compatibility Ideographs (CJK Strokes)
[0x2F800, 0x2FA1F], # CJK Compatibility Ideographs Supplement (CJK Strokes)
[0xA000, 0xA48F], # Yi Syllables
[0xA490, 0xA4CF], # Yi Radicals
]

OFL_BODY_TEXT = """
Expand Down
2 changes: 2 additions & 0 deletions Lib/openbakery/profiles/fontwerk.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ def leave_this_one_out(checkid):
"com.google.fonts/check/version_bump",
"com.google.fonts/check/production_glyphs_similarity",
"com.google.fonts/check/name/line_breaks",
"com.google.fonts/check/fontdata_namecheck",
"com.google.fonts/check/meta/script_lang_tags",
# The following check they may need some improvements
# before we decide to include it:
"com.google.fonts/check/family/italics_have_roman_counterparts",
Expand Down
12 changes: 6 additions & 6 deletions Lib/openbakery/profiles/googlefonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -5650,31 +5650,31 @@ def com_google_fonts_check_cjk_vertical_metrics_regressions(

@check(
id="com.google.fonts/check/cjk_not_enough_glyphs",
conditions=["is_cjk_font"],
conditions=["is_claiming_to_be_cjk_font"],
rationale="""
Hangul has 40 characters and it's the smallest CJK writing system.
Kana has 150 characters and it's the smallest CJK writing system.
If a font contains less CJK glyphs than this writing system, we inform the
user that some glyphs may be encoded incorrectly.
""",
proposal="https://github.com/googlefonts/fontbakery/pull/3214",
)
def com_google_fonts_check_cjk_not_enough_glyphs(ttFont):
"""Does the font contain less than 40 CJK characters?"""
"""Does the font contain less than 150 CJK characters?"""
from .shared_conditions import get_cjk_glyphs

cjk_glyphs = get_cjk_glyphs(ttFont)
cjk_glyph_count = len(cjk_glyphs)
if cjk_glyph_count > 0 and cjk_glyph_count < 40:
if cjk_glyph_count > 0 and cjk_glyph_count < 150:
if cjk_glyph_count == 1:
N_CJK_glyphs = "There is only one CJK glyph"
else:
N_CJK_glyphs = f"There are only {cjk_glyph_count} CJK glyphs"

yield WARN, Message(
"cjk-not-enough-glyphs",
f"{N_CJK_glyphs} when there needs to be at least 40"
f" in order to support the smallest CJK writing system, Hangul.\n"
f"{N_CJK_glyphs} when there needs to be at least 150"
f" in order to support the smallest CJK writing system, Kana.\n"
f"The following CJK glyphs were found:\n"
f"{cjk_glyphs}\n"
f"Please check that these glyphs have the correct unicodes.",
Expand Down
4 changes: 2 additions & 2 deletions Lib/openbakery/profiles/os2.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def com_google_fonts_check_family_panose_proportion(ttFonts):
)

if not passed:
yield FAIL, Message(
yield WARN, Message(
"inconsistency",
"PANOSE proportion is not the same across this family."
" In order to fix this, please make sure that"
Expand Down Expand Up @@ -71,7 +71,7 @@ def com_google_fonts_check_family_panose_familytype(ttFonts):
)

if not passed:
yield FAIL, Message(
yield WARN, Message(
"inconsistency",
"PANOSE family type is not the same across this family."
" In order to fix this, please make sure that"
Expand Down
31 changes: 20 additions & 11 deletions Lib/openbakery/profiles/shared_conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,18 +503,21 @@ def unicoderange(ttFont):


@condition
def is_cjk_font(ttFont):
def is_claiming_to_be_cjk_font(ttFont):
"""Test font object to confirm that it meets our definition of a CJK font file.
The definition is met if any of the following conditions are True:
We do this in two ways: in some cases, we are testing the *metadata*,
i.e. what the font claims about itself, in which case the definition is
met if any of the following conditions are True:
1. The font has a CJK code page bit set in the OS/2 table
2. The font has a CJK Unicode range bit set in the OS/2 table
3. The font has any CJK Unicode code points defined in the cmap table
See below for another way of testing this.
"""
from openbakery.constants import (
CJK_CODEPAGE_BITS,
CJK_UNICODE_RANGE_BITS,
CJK_UNICODE_RANGES,
)

if not has_os2_table(ttFont):
Expand All @@ -541,17 +544,23 @@ def is_cjk_font(ttFont):
if os2.ulUnicodeRange3 & (1 << (bit - 64)):
return True

# defined CJK Unicode code point in cmap table checks
cmap = ttFont.getBestCmap()
for unicode_range in CJK_UNICODE_RANGES:
for x in range(unicode_range[0], unicode_range[1] + 1):
if int(x) in cmap:
return True

# default, return False if the above checks did not identify a CJK font
return False


@condition
def is_cjk_font(ttFont):
"""
The `is_claiming_to_be_cjk_font` condition looks up the font's metadata to see if
it is claiming to be a CJK font. But the metadata may be wrong, and the correctness
of the metadata is something what we want to check!
We also want to know if the font really is a CJK font, i.e. it contains a
significant number of CJK characters. We say that *this* definition is met if the
font has more than 150 CJK Unicode code points defined in the cmap table.
"""
return len(get_cjk_glyphs(ttFont)) > 150


@condition
def get_cjk_glyphs(ttFont):
"""Return all glyphs which belong to a CJK unicode block"""
Expand Down
49 changes: 48 additions & 1 deletion Lib/openbakery/profiles/ufo_sources.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import re

from openbakery.callable import check, condition
from openbakery.status import ERROR, FAIL, PASS, WARN
from openbakery.status import ERROR, FAIL, PASS, SKIP, WARN
from openbakery.section import Section
from openbakery.message import Message
from openbakery.fonts_profile import profile_factory
from openbakery.utils import exit_with_install_instructions


profile = profile_factory(default_section=Section("UFO Sources"))

UFO_PROFILE_CHECKS = [
Expand All @@ -16,6 +19,7 @@
"com.google.fonts/check/designspace_has_default_master",
"com.google.fonts/check/designspace_has_consistent_glyphset",
"com.google.fonts/check/designspace_has_consistent_codepoints",
"com.thetypefounders/check/features_default_languagesystem",
]


Expand Down Expand Up @@ -303,5 +307,48 @@ def com_google_fonts_check_designspace_has_consistent_codepoints(designSpace, co
yield PASS, "Unicode assignments were consistent."


@check(
id="com.thetypefounders/check/features_default_languagesystem",
conditions=["ufo_font"],
rationale="""
The feature file specification strongly recommends to use a
`languagesystem DFLT dflt` statement in your feature file. This
statement is automatically inserted when no `languagesystem`
statements are present in the feature file, *unless* there is
another `languagesystem` statement already present. If this is
the case, this behaviour could lead to unintended side effects.
This check only WARNs when this happen as there are cases where
not having a `languagesystem DFLT dflt` statement in your feature
file is technically correct.
http://adobe-type-tools.github.io/afdko/OpenTypeFeatureFileSpecification.html#4b-language-system
""",
proposal="https://github.com/googlefonts/fontbakery/issues/4011",
)
def com_thetypefounders_check_features_default_languagesystem(ufo_font):
"""Check that languagesystem DFLT dflt is present in the features.fea file."""

if ufo_font.features.text is None:
yield SKIP, "No features.fea file in font."
elif not ufo_font.features.text.strip():
yield PASS, "Default languagesystem inserted by compiler."
else:
tags = re.findall(
# pylint: disable-next=line-too-long
r"languagesystem\s+([A-Za-z0-9\._!$%&*+:?^'|~]{1,4})\s+([A-Za-z0-9\._!$%&*+:?^'|~]{1,4})", # noqa E501
ufo_font.features.text,
)

if len(tags) > 0 and ("DFLT", "dflt") != tags[0]:
tags_str = ", ".join([" ".join(t) for t in tags])
yield WARN, Message(
"default-languagesystem",
f"Default languagesystem not found in: {tags_str}.",
)
else:
yield PASS, "Default languagesystem present or automatically inserted."


profile.auto_register(globals())
profile.test_expected_checks(UFO_PROFILE_CHECKS, exclusive=True)
5 changes: 2 additions & 3 deletions Lib/openbakery/reporters/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,11 @@ def get_html(self) -> str:
section_stati_of_note = (
e for e in section["result"].elements() if e != "PASS"
)
if all([self.omit_loglevel(s) for s in section["result"].elements()]):
continue
section_stati = "".join(
EMOTICON[s] for s in sorted(section_stati_of_note, key=LOGLEVELS.index)
)
body_elements.append(f"<h2>{section_name} {section_stati}</h2>")
if not all([self.omit_loglevel(s) for s in section["result"].elements()]):
body_elements.append(f"<h2>{section_name} {section_stati}</h2>")

checks_by_id: Dict[str, List[Dict[str, str]]] = collections.defaultdict(
list
Expand Down
5 changes: 4 additions & 1 deletion tests/profiles/googlefonts_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4171,12 +4171,15 @@ def test_check_cjk_not_enough_glyphs():

ttFont = TTFont(TEST_FILE("montserrat/Montserrat-Regular.ttf"))
msg = assert_results_contain(check(ttFont), SKIP, "unfulfilled-conditions")
assert msg == "Unfulfilled Conditions: is_cjk_font"
assert msg == "Unfulfilled Conditions: is_claiming_to_be_cjk_font"

# Let's modify Montserrat's cmap so there's a cjk glyph
cmap = ttFont["cmap"].getcmap(3, 1)
# Add first character of the CJK unified Ideographs
cmap.cmap[0x4E00] = "A"
# And let's declare that we are a CJK font
ttFont["OS/2"].ulCodePageRange1 |= 1 << 17

msg = assert_results_contain(check(ttFont), WARN, "cjk-not-enough-glyphs")
assert msg.startswith("There is only one CJK glyph")

Expand Down
4 changes: 2 additions & 2 deletions tests/profiles/os2_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def test_check_family_panose_proportion(mada_ttFonts):
mada_ttFonts[0]["OS/2"].panose.bProportion = incorrect_value

assert_results_contain(
check(mada_ttFonts), FAIL, "inconsistency", "with inconsistent family."
check(mada_ttFonts), WARN, "inconsistency", "with inconsistent family."
)


Expand All @@ -78,7 +78,7 @@ def test_check_family_panose_familytype(mada_ttFonts):
mada_ttFonts[0]["OS/2"].panose.bFamilyType = incorrect_value

assert_results_contain(
check(mada_ttFonts), FAIL, "inconsistency", "with inconsistent family."
check(mada_ttFonts), WARN, "inconsistency", "with inconsistent family."
)


Expand Down
71 changes: 71 additions & 0 deletions tests/profiles/ufo_sources_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from openbakery.codetesting import (
assert_PASS,
assert_results_contain,
assert_SKIP,
CheckTester,
TEST_FILE,
)
Expand Down Expand Up @@ -171,3 +172,73 @@ def test_check_designspace_has_consistent_codepoints():
assert_results_contain(check(designspace), FAIL, "inconsistent-codepoints")

# TODO: Fix it and ensure it passes the check


def test_check_default_languagesystem_pass_without_features(empty_ufo_font):
"""Pass if the UFO source has no features."""
check = CheckTester(
ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem"
)

ufo, _ = empty_ufo_font

assert_SKIP(check(ufo), "No features.fea file in font.")


def test_check_default_languagesystem_pass_with_empty_features(empty_ufo_font):
"""Pass if the UFO source has a feature file but it is empty."""
check = CheckTester(
ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem"
)

ufo, _ = empty_ufo_font

ufo.features.text = ""

assert_PASS(check(ufo))


def test_check_default_languagesystem_pass_with_features(empty_ufo_font):
"""Pass if the font has features and no default languagesystem statements."""
check = CheckTester(
ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem"
)

ufo, _ = empty_ufo_font

ufo.features.text = "feature liga { sub f i by f_i; } liga;"

assert_PASS(check(ufo))


def test_check_default_languagesystem_warn_without_default_languagesystem(
empty_ufo_font,
):
"""Warn if `languagesystem DFLT dflt` is not present in the feature file,
but other languagesystem statements are."""
check = CheckTester(
ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem"
)

ufo, _ = empty_ufo_font

ufo.features.text = (
"languagesystem latn dflt; feature liga { sub f i by f_i; } liga;"
)

assert_results_contain(check(ufo), WARN, "default-languagesystem")


def test_check_default_languagesystem_pass_with_default_languagesystem(empty_ufo_font):
"""Pass if `languagesystem DFLT dflt` is explicitly used in the features."""
check = CheckTester(
ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem"
)

ufo, _ = empty_ufo_font

ufo.features.text = """languagesystem DFLT dflt;
languagesystem latn dflt;
feature liga { sub f i by f_i; } liga;"""

assert_PASS(check(ufo))

0 comments on commit 2f1cb54

Please sign in to comment.