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

Improve unreachable subsetting check #4273

Merged
merged 8 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ A more detailed list of changes is available in the corresponding milestones for
### Bug Fixes
#### On the Universal Profile
- **[com.google.fonts/check/font_is_centered_vertically]:** Fix yet another error on fonts without ASCII letters (issue #4269)

#### On the Google Fonts Profile
- **[com.google.fonts/check/metadata/unreachable_subsetting]:** Report codepoints in supplementary Unicode planes, and codepoints with no glyphset support at all (PR #4273)

## 0.9.1 (2023-Sep-19)
### Stable release notes
Expand Down
66 changes: 40 additions & 26 deletions Lib/fontbakery/profiles/googlefonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -1043,14 +1043,7 @@ def com_google_fonts_check_vendor_id(ttFont, registered_vendor_ids):

@condition
def font_codepoints(ttFont):
codepoints = set()
for table in ttFont["cmap"].tables:
if (
table.platformID == PlatformID.WINDOWS
and table.platEncID == WindowsEncodingID.UNICODE_BMP
):
codepoints.update(table.cmap.keys())
return codepoints
return set(ttFont.getBestCmap().keys())


@check(
Expand Down Expand Up @@ -1166,33 +1159,48 @@ def com_google_fonts_check_metadata_unsupported_subsets(
will not be served in the subsetted fonts, and so will be unreachable to
the end user.
""",
conditions=["family_metadata"],
proposal="https://github.com/fonttools/fontbakery/issues/4097",
proposal=[
"https://github.com/fonttools/fontbakery/issues/4097",
"https://github.com/fonttools/fontbakery/pull/4273",
],
severity=2,
)
def com_google_fonts_check_metadata_unreachable_subsetting(
family_metadata, ttFont, font_codepoints, config
family_directory, font, ttFont, font_codepoints, config
):
"""Check for codepoints not covered by METADATA subsets."""
from glyphsets import codepoints
from fontbakery.utils import pretty_print_list
from fontbakery.profiles.googlefonts_conditions import (
metadata_file,
family_metadata,
)
import unicodedata2

codepoints.set_encoding_path(codepoints.nam_dir)

for subset in family_metadata.subsets:
# Use the METADATA.pb subsets if we have them
metadatapb = metadata_file(family_directory)
if metadatapb:
metadata = family_metadata(metadatapb)
if metadata:
subsets = metadata.subsets
else:
yield FAIL, Message(
"unparsable-metadata", "Could not parse metadata.pb file"
)
return
else:
# Follow what the packager would do
subsets = [s[0] for s in codepoints.SubsetsInFont(font, 50, 0.01)]

for subset in subsets:
font_codepoints = font_codepoints - set(codepoints.CodepointsInSubset(subset))

if not font_codepoints:
yield PASS, "OK"
return

message = """The following codepoints supported by the font are not covered by
any subsets defined in the font's metadata file, and will never
be served. You can solve this by either manually adding additional
subset declarations to METADATA.pb, or by editing the glyphset
definitions.\n\n"""

unreachable = []
subsets_for_cps = defaultdict(set)
# This is faster than calling SubsetsForCodepoint for each codepoint
Expand All @@ -1202,23 +1210,29 @@ def com_google_fonts_check_metadata_unreachable_subsetting(
subsets_for_cps[cp].add(subset)

for codepoint in sorted(font_codepoints):
subsets = subsets_for_cps[codepoint]
if not subsets:
continue
subsets_for_cp = subsets_for_cps[codepoint]

if len(subsets) > 1:
subsets = "one of: " + ", ".join(subsets)
if len(subsets_for_cp) == 0:
message = "not included in any glyphset definition"
elif len(subsets_for_cp) == 1:
message = "try adding " + ", ".join(subsets_for_cp)
else:
subsets = ", ".join(subsets)
message = "try adding one of: " + ", ".join(subsets_for_cp)

try:
name = unicodedata2.name(chr(codepoint))
except Exception:
name = ""

unreachable.append(" * U+%04X %s: try adding %s" % (codepoint, name, subsets))
unreachable.append(" * U+%04X %s: %s" % (codepoint, name, message))

message = """The following codepoints supported by the font are not covered by
any subsets defined in the font's metadata file, and will never
be served. You can solve this by either manually adding additional
subset declarations to METADATA.pb, or by editing the glyphset
definitions.\n\n"""

subsets = ", ".join(f"`{s}`" for s in family_metadata.subsets)
subsets = ", ".join(f"`{s}`" for s in subsets)
message += pretty_print_list(config, unreachable, sep="\n", glue="\n")
message += (
f"\n\nOr you can add the above codepoints to one"
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@
python_requires=">=3.8",
setup_requires=[
"setuptools>=61.2",
"setuptools_scm[toml]>=6.2",
"setuptools_scm[toml]>=6.2, !=8.0.0", # version 8.0.0 had a bug as described at
# https://github.com/pypa/setuptools_scm/issues/905
],
install_requires=[
# ---
Expand Down
8 changes: 8 additions & 0 deletions tests/profiles/googlefonts_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5188,6 +5188,14 @@ def test_check_metadata_unreachable_subsetting():
check(font), WARN, "unreachable-subsetting", "with a bad font"
)

font = TEST_FILE("playfair/Playfair-Italic[opsz,wdth,wght].ttf")
assert_results_contain(
check(font),
WARN,
"unreachable-subsetting",
"with a bad font and no METADATA.pb",
)


def test_check_alt_caron():
"""Check accent of Lcaron, dcaron, lcaron, tcaron"""
Expand Down