Check that particle names on STAT table match the fallback names... #3056
Conversation
There was a problem hiding this comment.
Thank you for working on this! I really appreciate it.
I find the nested loops and conditions difficult to read. It may be best to munge the axis registry and stat table into flatter data structures e.g
font_axis_values = {
"wght": {"value": 100, "name": Thin},
"wdth": {"value": 75, "name": Condensed},
...
}
axis_registry_fallbacks = {
wght: [
{"value": 100, "name": "Thin"},
...
],
...
}
It should then be much easier to make the comparison.
for axis_tag, axis_value in font_axis_values.items():
if axis_tag not in axis_registry_fallbacks:
# Warn/fail that axis isn't in the axis registry
else:
if axis_value not in axis_registry[axis_tag]:
# Fail that axis isn't in the axis registry
Besides from the readability improvements, we also need to do the following:
- support format 4 axisValues
- only compare the US english nameRecords.
- You'll need to remove spaces from the wght names in the GF Axis Registry, axisregistry: Can we remove spaces from Weight fallback names? google/fonts#2701. You may need to do this to other axes as well.
| for axis_value in ttFont['STAT'].table.AxisValueArray.AxisValue: | ||
| axis = ttFont['STAT'].table.DesignAxisRecord.Axis[axis_value.AxisIndex] | ||
| if axis.AxisTag in GFAxisRegistry.keys(): | ||
| expected = GFAxisRegistry[axis.AxisTag] |
There was a problem hiding this comment.
Format 4 AxisValue tables don't have an AxisIndex attrib.
https://docs.microsoft.com/en-us/typography/opentype/spec/stat#axis-value-table-format-4
| if axis.AxisTag in GFAxisRegistry.keys(): | ||
| expected = GFAxisRegistry[axis.AxisTag] | ||
| expected_names = [fb.name for fb in expected.fallback] | ||
| for name_entry in ttFont['name'].names: |
There was a problem hiding this comment.
It may be wise to only check nameRecords which are US English only (langid=0x409) against our axis registry. We may have fonts in the future which contain non-english nameRecords for STAT particles.
|
thanks for all the feedback! |
felipesanches
force-pushed
the
issue_3022_b
branch
from
e265a0d to
ba922f5
Compare
...in each axis registry at the Google Fonts Axis Registry (issue fonttools#3022)
felipesanches
force-pushed
the
issue_3022_b
branch
from
2b23163 to
a613e44
Compare
|
@m4rc1e, I'm merging this now because it is already useful. If possible, please open a new issue attaching a format-4 table so that we can expand the implementation to support that as well. |
| def com_google_fonts_check_STAT_gf_axisregistry_names(ttFont, GFAxisRegistry): | ||
| """ Validate STAT particle names and values match the fallback names in GFAxisRegistry. """ | ||
|
|
||
| def normalize_name(name): |
There was a problem hiding this comment.
function already exists in googlefonts_conditions
|
This looks pretty good! I agree it's now worth merging. Thank you so much for adding this. |
...in each axis registry at the Google Fonts Axis Registry
(issue #3022)
This pull request is marked as a DRAFT because it still lacks code-tests. I want to get feedback from @m4rc1e regarding whether this implementation correctly encodes the checking procedures he had in mind for the STAT table when he requested it at #3022. If Marc agrees that this is indeed what we should be checking for, then I'll put the effort to write a few test-case examples in a code-test for this check.