Skip to content

Commit f5750bd

Browse files
committed
More universal/opentype checks
1 parent 8f4e609 commit f5750bd

File tree

5 files changed

+294
-39
lines changed

5 files changed

+294
-39
lines changed

fontspector-cli/src/main.rs

+15-15
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ fn main() {
3939
#[allow(clippy::expect_used)] // If this fails, I *want* to panic
4040
Universal
4141
.register(&mut registry)
42-
.expect("Couldn't register universal profile, fontspector bug");
42+
.expect("Couldn't register universal/opentype profile, fontspector bug");
4343
#[allow(clippy::expect_used)] // If this fails, I *want* to panic
4444
GoogleFonts
4545
.register(&mut registry)
@@ -116,17 +116,17 @@ fn main() {
116116
let count_of_files = testables.iter().filter(|x| x.is_single()).count();
117117
let count_of_families = testables.len() - count_of_files;
118118

119-
if !args.quiet {
120-
println!(
121-
"Running {:} check{} on {} file{} in {} famil{}",
122-
checkorder.len(),
123-
if checkorder.len() == 1 { "" } else { "s" },
124-
count_of_files,
125-
if count_of_files == 1 { "" } else { "s" },
126-
count_of_families,
127-
if count_of_families == 1 { "y" } else { "ies" }
128-
);
129-
}
119+
// if !args.quiet {
120+
println!(
121+
"Running {:} check{} on {} file{} in {} famil{}",
122+
checkorder.len(),
123+
if checkorder.len() == 1 { "" } else { "s" },
124+
count_of_files,
125+
if count_of_files == 1 { "" } else { "s" },
126+
count_of_families,
127+
if count_of_families == 1 { "y" } else { "ies" }
128+
);
129+
// }
130130

131131
let apply_fixes = |(testable, check, mut result): (&&TestableType, &&Check, CheckResult)| {
132132
if let TestableType::Single(testable) = testable {
@@ -189,9 +189,9 @@ fn main() {
189189
reporter.report(&results, &args, &registry);
190190
}
191191

192-
if !args.quiet {
193-
TerminalReporter::summary_report(results.summary());
194-
}
192+
// if !args.quiet {
193+
TerminalReporter::summary_report(results.summary());
194+
// }
195195
if worst_status >= args.error_code_on {
196196
std::process::exit(1);
197197
}

profile-universal/src/checks/hhea.rs

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
use fontspector_checkapi::{prelude::*, testfont, FileTypeConvert};
2+
use read_fonts::TableProvider;
3+
4+
fn maxadvancewidth(t: &Testable, _context: &Context) -> CheckFnResult {
5+
let f = testfont!(t);
6+
let hhea_advance_width_max = f.font().hhea()?.advance_width_max().to_u16();
7+
let hmtx_advance_width_max = f
8+
.font()
9+
.hmtx()?
10+
.h_metrics()
11+
.iter()
12+
.map(|m| m.advance.get())
13+
.max()
14+
.unwrap_or_default();
15+
Ok(if hmtx_advance_width_max != hhea_advance_width_max {
16+
Status::just_one_fail(
17+
"mismatch",
18+
&format!(
19+
"AdvanceWidthMax mismatch: expected {} from hmtx; got {} for hhea",
20+
hmtx_advance_width_max, hhea_advance_width_max
21+
),
22+
)
23+
} else {
24+
Status::just_one_pass()
25+
})
26+
}
27+
28+
pub const CHECK_MAXADVANCEWIDTH: Check = Check {
29+
id: "com.google.fonts/check/maxadvancewidth",
30+
title: "MaxAdvanceWidth is consistent with values in the Hmtx and Hhea tables?",
31+
rationale: "The 'hhea' table contains a field which specifies the maximum advance width. This value should be consistent with the maximum advance width of all glyphs specified in the 'hmtx' table.",
32+
proposal: "legacy:check/073",
33+
implementation: CheckImplementation::CheckOne(&maxadvancewidth),
34+
applies_to: "TTF",
35+
hotfix: None,
36+
fix_source: None,
37+
flags: CheckFlags::default(),
38+
};
39+
40+
fn caret_slope(t: &Testable, _context: &Context) -> CheckFnResult {
41+
let f = testfont!(t);
42+
let post_italic_angle = f.font().post()?.italic_angle().to_f32();
43+
let upem = f.font().head()?.units_per_em();
44+
let run = f.font().hhea()?.caret_slope_run();
45+
let rise = f.font().hhea()?.caret_slope_rise();
46+
if rise == 0 {
47+
return Ok(Status::just_one_fail(
48+
"zero-rise",
49+
"caretSlopeRise must not be zero. Set it to 1 for upright fonts.",
50+
));
51+
}
52+
let hhea_angle = (-run as f32 / rise as f32).atan().to_degrees();
53+
let expected_run = (-post_italic_angle.to_radians().tan() * upem as f32).round() as i16;
54+
let expected_rise = if expected_run == 0 { 1 } else { upem };
55+
if (post_italic_angle - hhea_angle).abs() > 0.1 {
56+
return Ok(Status::just_one_warn(
57+
"mismatch",
58+
&format!(
59+
"hhea.caretSlopeRise and hhea.caretSlopeRun do not match with post.italicAngle.
60+
Got caretSlopeRise: {}, caretSlopeRun: {}, expected caretSlopeRise: {}, caretSlopeRun: {}",
61+
rise, run, expected_rise, expected_run
62+
),
63+
));
64+
}
65+
Ok(Status::just_one_pass())
66+
}
67+
68+
pub const CHECK_CARET_SLOPE: Check = Check {
69+
id: "com.google.fonts/check/caret_slope",
70+
title: "Check hhea.caretSlopeRise and hhea.caretSlopeRun",
71+
rationale: r#"
72+
Checks whether hhea.caretSlopeRise and hhea.caretSlopeRun
73+
match with post.italicAngle.
74+
75+
For Upright fonts, you can set hhea.caretSlopeRise to 1
76+
and hhea.caretSlopeRun to 0.
77+
78+
For Italic fonts, you can set hhea.caretSlopeRise to head.unitsPerEm
79+
and calculate hhea.caretSlopeRun like this:
80+
round(math.tan(
81+
math.radians(-1 * font["post"].italicAngle)) * font["head"].unitsPerEm)
82+
83+
This check allows for a 0.1° rounding difference between the Italic angle
84+
as calculated by the caret slope and post.italicAngle
85+
"#,
86+
proposal: "https://github.com/fonttools/fontbakery/issues/3670",
87+
implementation: CheckImplementation::CheckOne(&caret_slope),
88+
applies_to: "TTF",
89+
hotfix: None,
90+
fix_source: None,
91+
flags: CheckFlags::default(),
92+
};

profile-universal/src/checks/mod.rs

+8-12
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
1-
mod bold_italic_unique;
2-
mod fvar;
3-
mod glyphnames;
4-
mod name_trailing_spaces;
5-
mod required_tables;
6-
mod unwanted_tables;
7-
pub use fvar::CHECK_REGULAR_COORDS_CORRECT;
8-
pub use glyphnames::CHECK_VALID_GLYPHNAMES;
9-
// pub use bold_italic_unique::CHECK_BOLD_ITALIC_UNIQUE;
10-
pub use name_trailing_spaces::CHECK_NAME_TRAILING_SPACES;
11-
pub use required_tables::CHECK_REQUIRED_TABLES;
12-
pub use unwanted_tables::CHECK_UNWANTED_TABLES;
1+
pub mod bold_italic_unique;
2+
pub mod fvar;
3+
pub mod glyphnames;
4+
pub mod hhea;
5+
pub mod name;
6+
pub mod name_trailing_spaces;
7+
pub mod required_tables;
8+
pub mod unwanted_tables;

profile-universal/src/checks/name.rs

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
use fontspector_checkapi::{prelude::*, testfont, FileTypeConvert};
2+
use read_fonts::TableProvider;
3+
4+
fn name_empty_records(t: &Testable, _context: &Context) -> CheckFnResult {
5+
let f = testfont!(t);
6+
let name = f.font().name()?;
7+
let mut problems: Vec<Status> = vec![];
8+
for record in name.name_record() {
9+
if record
10+
.string(name.string_data())?
11+
.to_string()
12+
.trim()
13+
.is_empty()
14+
{
15+
problems.push(Status::fail(
16+
"empty-record",
17+
&format!(
18+
"Empty name record found for name ID={} platform ID={} encoding ID={}",
19+
record.name_id(),
20+
record.platform_id(),
21+
record.encoding_id(),
22+
),
23+
));
24+
}
25+
}
26+
return_result(problems)
27+
}
28+
29+
pub const CHECK_NAME_EMPTY_RECORDS: Check = Check {
30+
id: "com.google.fonts/check/name/empty_records",
31+
title: "Check name table for empty records.",
32+
rationale: "Check the name table for empty records, as this can cause problems in Adobe apps.",
33+
proposal: "https://github.com/fonttools/fontbakery/pull/2369",
34+
implementation: CheckImplementation::CheckOne(&name_empty_records),
35+
applies_to: "TTF",
36+
hotfix: None,
37+
fix_source: None,
38+
flags: CheckFlags::default(),
39+
};

profile-universal/src/lib.rs

+140-12
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,152 @@ pub struct Universal;
66

77
impl fontspector_checkapi::Plugin for Universal {
88
fn register(&self, cr: &mut Registry) -> Result<(), String> {
9-
cr.register_check(checks::CHECK_NAME_TRAILING_SPACES);
10-
cr.register_check(checks::CHECK_UNWANTED_TABLES);
11-
cr.register_check(checks::CHECK_REQUIRED_TABLES);
12-
cr.register_check(checks::CHECK_REGULAR_COORDS_CORRECT);
13-
cr.register_check(checks::CHECK_VALID_GLYPHNAMES);
14-
let profile = Profile::from_toml(
9+
cr.register_check(checks::fvar::CHECK_AXIS_RANGES_CORRECT);
10+
cr.register_check(checks::fvar::CHECK_REGULAR_COORDS_CORRECT);
11+
cr.register_check(checks::glyphnames::CHECK_VALID_GLYPHNAMES);
12+
cr.register_check(checks::hhea::CHECK_CARET_SLOPE);
13+
cr.register_check(checks::hhea::CHECK_MAXADVANCEWIDTH);
14+
cr.register_check(checks::name_trailing_spaces::CHECK_NAME_TRAILING_SPACES);
15+
cr.register_check(checks::name::CHECK_NAME_EMPTY_RECORDS);
16+
cr.register_check(checks::required_tables::CHECK_REQUIRED_TABLES);
17+
cr.register_check(checks::unwanted_tables::CHECK_UNWANTED_TABLES);
18+
19+
let opentype_profile = Profile::from_toml(
20+
r#"
21+
[sections]
22+
"OpenType Specification Checks" = [
23+
"com.google.fonts/check/family/underline_thickness",
24+
"com.google.fonts/check/family/panose_familytype",
25+
"com.google.fonts/check/family/equal_font_versions",
26+
"com.adobe.fonts/check/family/bold_italic_unique_for_nameid1",
27+
"com.adobe.fonts/check/family/max_4_fonts_per_family_name",
28+
"com.adobe.fonts/check/family/consistent_family_name",
29+
"com.adobe.fonts/check/name/postscript_vs_cff",
30+
"com.adobe.fonts/check/name/postscript_name_consistency",
31+
"com.adobe.fonts/check/name/empty_records",
32+
"com.google.fonts/check/name/no_copyright_on_description",
33+
"com.google.fonts/check/name/match_familyname_fullfont",
34+
"com.google.fonts/check/fvar/regular_coords_correct",
35+
"com.google.fonts/check/fvar/axis_ranges_correct",
36+
"com.google.fonts/check/varfont/stat_axis_record_for_each_axis",
37+
"com.google.fonts/check/loca/maxp_num_glyphs",
38+
"com.adobe.fonts/check/cff_ascii_strings",
39+
"com.adobe.fonts/check/cff_call_depth",
40+
"com.adobe.fonts/check/cff_deprecated_operators",
41+
"com.adobe.fonts/check/cff2_call_depth",
42+
"com.google.fonts/check/font_version",
43+
"com.google.fonts/check/post_table_version",
44+
"com.google.fonts/check/monospace",
45+
"com.google.fonts/check/xavgcharwidth",
46+
"com.adobe.fonts/check/fsselection_matches_macstyle",
47+
"com.google.fonts/check/unitsperem",
48+
"com.google.fonts/check/gdef_spacing_marks",
49+
"com.google.fonts/check/gdef_mark_chars",
50+
"com.google.fonts/check/gdef_non_mark_chars",
51+
"com.google.fonts/check/gpos_kerning_info",
52+
"com.google.fonts/check/kern_table",
53+
"com.google.fonts/check/glyf_unused_data",
54+
"com.google.fonts/check/family_naming_recommendations",
55+
"com.google.fonts/check/maxadvancewidth",
56+
"com.adobe.fonts/check/postscript_name",
57+
"com.google.fonts/check/points_out_of_bounds",
58+
"com.google.fonts/check/glyf_non_transformed_duplicate_components",
59+
"com.google.fonts/check/code_pages",
60+
"com.google.fonts/check/layout_valid_feature_tags",
61+
"com.google.fonts/check/layout_valid_script_tags",
62+
"com.google.fonts/check/layout_valid_language_tags",
63+
"com.google.fonts/check/italic_angle",
64+
"com.google.fonts/check/mac_style",
65+
"com.google.fonts/check/fsselection",
66+
"com.google.fonts/check/name/italic_names",
67+
"com.adobe.fonts/check/varfont/valid_axis_nameid",
68+
"com.adobe.fonts/check/varfont/valid_subfamily_nameid",
69+
"com.adobe.fonts/check/varfont/valid_postscript_nameid",
70+
"com.adobe.fonts/check/varfont/valid_default_instance_nameids",
71+
"com.adobe.fonts/check/varfont/same_size_instance_records",
72+
"com.adobe.fonts/check/varfont/distinct_instance_records",
73+
"com.adobe.fonts/check/varfont/foundry_defined_tag_name",
74+
"com.google.fonts/check/varfont/family_axis_ranges",
75+
"com.adobe.fonts/check/stat_has_axis_value_tables",
76+
"com.thetypefounders/check/vendor_id",
77+
"com.google.fonts/check/caret_slope",
78+
"com.google.fonts/check/italic_axis_in_stat",
79+
"com.google.fonts/check/italic_axis_in_stat_is_boolean",
80+
"com.google.fonts/check/italic_axis_last",
81+
]
82+
"#,
83+
)
84+
.map_err(|_| "Couldn't parse profile")?;
85+
cr.register_profile("opentype", opentype_profile)?;
86+
87+
let universal_profile = Profile::from_toml(
1588
r#"
89+
include_profiles = ["opentype"]
1690
[sections]
91+
#"Superfamily Checks" = [
92+
# "com.google.fonts/check/superfamily/list",
93+
# "com.google.fonts/check/superfamily/vertical_metrics",
94+
#]
95+
#"UFO Sources" = [
96+
# # FIXME (orphan check): "com.daltonmaag/check/consistent_curve_type",
97+
# # https://github.com/fonttools/fontbakery/pull/4809
98+
# "com.google.fonts/check/designspace_has_sources",
99+
# "com.google.fonts/check/designspace_has_default_master",
100+
# "com.google.fonts/check/designspace_has_consistent_glyphset",
101+
# "com.google.fonts/check/designspace_has_consistent_codepoints",
102+
# "com.thetypefounders/check/features_default_languagesystem",
103+
# # FIXME (orphan check): "com.daltonmaag/check/no_open_corners",
104+
# "com.daltonmaag/check/ufolint",
105+
# "com.daltonmaag/check/ufo_required_fields",
106+
# "com.daltonmaag/check/ufo_recommended_fields",
107+
# "com.daltonmaag/check/ufo_unnecessary_fields",
108+
# "com.daltonmaag/check/designspace_has_consistent_groups",
109+
#]
17110
"Universal Profile Checks" = [
18-
"com.google.fonts/check/name/trailing_spaces",
19-
"com.google.fonts/check/unwanted_tables",
20-
"com.google.fonts/check/required_tables",
21-
"com.google.fonts/check/fvar/regular_coords_correct",
22-
"com.google.fonts/check/valid_glyphnames"
111+
"com.google.fonts/check/alt_caron",
112+
"com.google.fonts/check/arabic_high_hamza",
113+
"com.google.fonts/check/arabic_spacing_symbols",
114+
# "com.google.fonts/check/caps_vertically_centered", # Disabled: issue #4274
115+
"com.google.fonts/check/case_mapping",
116+
"com.google.fonts/check/cjk_chws_feature",
117+
"com.google.fonts/check/contour_count",
118+
"com.google.fonts/check/family/single_directory",
119+
"com.google.fonts/check/family/vertical_metrics",
120+
"com.google.fonts/check/family/win_ascent_and_descent",
121+
"com.google.fonts/check/fontbakery_version",
122+
"com.adobe.fonts/check/freetype_rasterizer",
123+
"com.google.fonts/check/gpos7",
124+
"com.google.fonts/check/gsub/smallcaps_before_ligatures",
125+
"com.google.fonts/check/interpolation_issues",
126+
"com.google.fonts/check/legacy_accents",
127+
"com.google.fonts/check/linegaps",
128+
"com.google.fonts/check/mandatory_glyphs",
129+
"com.google.fonts/check/math_signs_width",
130+
"com.google.fonts/check/name/trailing_spaces",
131+
"com.google.fonts/check/os2_metrics_match_hhea",
132+
"com.google.fonts/check/ots",
133+
"com.google.fonts/check/required_tables",
134+
"com.google.fonts/check/rupee",
135+
"com.adobe.fonts/check/sfnt_version",
136+
"com.google.fonts/check/soft_hyphen",
137+
"com.google.fonts/check/STAT_in_statics",
138+
"com.google.fonts/check/STAT_strings",
139+
"com.google.fonts/check/tabular_kerning",
140+
"com.google.fonts/check/transformed_components",
141+
"com.google.fonts/check/ttx_roundtrip",
142+
"com.arrowtype.fonts/check/typoascender_exceeds_Agrave",
143+
"com.google.fonts/check/unique_glyphnames",
144+
"com.google.fonts/check/unreachable_glyphs",
145+
"com.google.fonts/check/unwanted_tables",
146+
"com.google.fonts/check/valid_glyphnames",
147+
"com.google.fonts/check/whitespace_glyphnames",
148+
"com.google.fonts/check/whitespace_glyphs",
149+
"com.google.fonts/check/whitespace_ink",
150+
"com.google.fonts/check/whitespace_widths",
23151
]
24152
"#,
25153
)
26154
.map_err(|_| "Couldn't parse profile")?;
27-
cr.register_profile("universal", profile)
155+
cr.register_profile("universal", universal_profile)
28156
}
29157
}

0 commit comments

Comments
 (0)