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

Conversation

simoncozens
Copy link
Collaborator

@simoncozens simoncozens commented Sep 19, 2023

Description

This improves the com.google.fonts/check/metadata/unreachable_subsetting check in a number of ways:

  • First, the condition font_codepoints was buggy! It searched through cmap tables to find those which are platform=Windows encoding=Unicode_BMP - which is fine if your font has codepoints exclusively within the BMP! I fixed it to use ttFont.getBestCmap(), which is simpler and also allows for cmap tables containing codepoints in supplementary planes.
  • Second, we previously did not warn about codepoints which are subsetted out because they were not covered in any glyphset definition at all. This makes sense because it's not a font bug (it's a glyphsets one!) but it turns out that it's actually very useful to know how much of your font will just get dropped on the floor.
  • Finally, this check only makes sense inside google/fonts, i.e. when you have a METADATA.pb file already. But it's also useful to be able to check during development (i.e. when you are upstream of google/fonts) if any codepoints will get left behind. We do this by copying the behaviour of the packager in how it detects subsets within a font, and using that information if a METADATA.pb file is not present.

Checklist

  • update CHANGELOG.md
  • wait for the tests to pass
  • request a review

@simoncozens simoncozens force-pushed the improve-unreachable-subsetting branch from e7d5b0a to 4396cc0 Compare September 19, 2023 20:19
@simoncozens
Copy link
Collaborator Author

I'm not sure why you've assigned this back to me, and assigned it a milestone. It's ready to merge.

@felipesanches
Copy link
Collaborator

A bug in setuptools_scm caused this PR build to fail: pypa/setuptools-scm#905

Now that setuptools_scm version 8.0.0 was yanked, I'm re-triggering the build.

@felipesanches
Copy link
Collaborator

felipesanches commented Sep 20, 2023

I'm not sure why you've assigned this back to me, and assigned it a milestone. It's ready to merge.

I always do it right before merging PRs and closing issues, with the purpose of tracking history of what went into each version of the program.

@felipesanches felipesanches merged commit 09ae90a into main Sep 20, 2023
34 checks passed
@khaledhosny khaledhosny deleted the improve-unreachable-subsetting branch October 9, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants