Skip to content

Commit d150c59

Browse files
simoncozenssimoncozens
committed
tnum_horizontal_metrics
1 parent 3ba53ab commit d150c59

File tree

3 files changed

+81
-6
lines changed

3 files changed

+81
-6
lines changed

‎profile-googlefonts/src/checks/googlefonts/family/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@ mod has_license;
44
pub use has_license::has_license;
55
mod italics_have_roman_counterparts;
66
pub use italics_have_roman_counterparts::italics_have_roman_counterparts;
7+
mod tnum_horizontal_metrics;
8+
pub use tnum_horizontal_metrics::tnum_horizontal_metrics;

‎profile-googlefonts/src/checks/googlefonts/family/tnum_horizontal_metrics.rs

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
use fontspector_checkapi::{prelude::*, FileTypeConvert};
2+
use hashbrown::{HashMap, HashSet};
3+
use read_fonts::TableProvider;
4+
5+
#[check(
6+
id = "googlefonts/family/tnum_horizontal_metrics",
7+
rationale = "
8+
9+
Tabular figures need to have the same metrics in all styles in order to allow
10+
tables to be set with proper typographic control, but to maintain the placement
11+
of decimals and numeric columns between rows.
12+
13+
Here's a good explanation of this:
14+
https://www.typography.com/techniques/fonts-for-financials/#tabular-figs
15+
16+
",
17+
proposal = "https://github.com/fonttools/fontbakery/issues/2278",
18+
title = "All tabular figures must have the same width across the RIBBI-family.",
19+
implementation = "all"
20+
)]
21+
fn tnum_horizontal_metrics(c: &TestableCollection, context: &Context) -> CheckFnResult {
22+
let fonts = TTF.from_collection(c);
23+
if fonts.len() < 2 {
24+
return Err(CheckError::Skip {
25+
code: "no-siblings".to_string(),
26+
message: "No sibling fonts found".to_string(),
27+
});
28+
}
29+
let mut tnum_widths: HashMap<u16, HashSet<String>> = HashMap::new();
30+
for font in fonts {
31+
let hmtx = font.font().hmtx()?;
32+
for (tnum_glyph_id, tnum_glyph_name) in font
33+
.all_glyphs()
34+
.flat_map(|g| font.glyph_name_for_id(g).map(|name| (g, name)))
35+
.filter(|(_, s)| s.ends_with(".tnum"))
36+
{
37+
if let Some(width) = hmtx.advance(tnum_glyph_id) {
38+
tnum_widths
39+
.entry(width)
40+
.or_insert_with(HashSet::new)
41+
.insert(tnum_glyph_name.to_string());
42+
}
43+
}
44+
}
45+
if tnum_widths.len() > 1 {
46+
#[allow(clippy::unwrap_used)] // We checked the length above
47+
let most_common = tnum_widths
48+
.iter()
49+
.max_by_key(|(_, glyphs)| glyphs.len())
50+
.unwrap();
51+
Ok(Status::just_one_fail(
52+
"inconsistent-widths",
53+
&format!(
54+
"The most common tabular glyph width is {}. But there are other tabular glyphs with different widths such as the following ones:\n\t{}",
55+
most_common.0,
56+
bullet_list(context,
57+
tnum_widths
58+
.iter()
59+
.filter(|(width, _)| *width != most_common.0)
60+
.map(|(width, glyphs)| {
61+
format!(
62+
"Width: {} - Glyphs: {}",
63+
width,
64+
glyphs.iter().map(|s| format!("'{}'", s)).collect::<Vec<_>>().join(", ")
65+
)
66+
})
67+
)
68+
),
69+
))
70+
} else {
71+
Ok(Status::just_one_pass())
72+
}
73+
}

‎profile-googlefonts/src/lib.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ impl fontspector_checkapi::Plugin for GoogleFonts {
9393

9494
let builder = builder
9595
.add_and_register_check(checks::googlefonts::description::eof_linebreak)
96-
// .add_and_register_check(checks::googlefonts::description::family_update)
9796
.add_and_register_check(checks::googlefonts::description::git_url)
9897
.add_and_register_check(checks::googlefonts::description::has_article)
9998
.add_and_register_check(checks::googlefonts::description::has_unsupported_elements)
@@ -103,7 +102,7 @@ impl fontspector_checkapi::Plugin for GoogleFonts {
103102
.add_section("Family Checks")
104103
.add_and_register_check(checks::googlefonts::family::equal_codepoint_coverage)
105104
.add_and_register_check(checks::googlefonts::family::italics_have_roman_counterparts)
106-
// .add_and_register_check(checks::googlefonts::family::tnum_horizontal_metrics)
105+
.add_and_register_check(checks::googlefonts::family::tnum_horizontal_metrics)
107106
.add_section("Name table checks")
108107
.add_and_register_check(checks::googlefonts::name::family_name_compliance)
109108
.add_and_register_check(checks::googlefonts::name::line_breaks)
@@ -120,11 +119,11 @@ impl fontspector_checkapi::Plugin for GoogleFonts {
120119
.add_and_register_check(checks::googlefonts::name::rfn)
121120
.add_section("Repository Checks")
122121
// .add_and_register_check(checks::googlefonts::repo::dirname_matches_nameid_1)
123-
// .add_and_register_check(checks::googlefonts::repo::fb_report)
124-
// .add_and_register_check(checks::googlefonts::repo::sample_image)
125-
// checks::googlefonts::repo::upstream_yaml_has_required_fields // Redundant, no upstream.yaml any more
126122
// .add_and_register_check(checks::googlefonts::repo::vf_has_static_fonts)
127-
// .add_and_register_check(checks::googlefonts::repo::zip_files",
123+
// checks::googlefonts::repo::fb_report // Upstream repos should be checked separately
124+
// checks::googlefonts::repo::sample_image // Upstream repos should be checked separately
125+
// checks::googlefonts::repo::upstream_yaml_has_required_fields // Redundant, no upstream.yaml any more
126+
// checks::googlefonts::repo::zip_files // Upstream repos should be checked separately
128127
.add_section("Shaping Checks")
129128
.add_and_register_check(checks::dotted_circle);
130129
#[cfg(not(target_family = "wasm"))]
@@ -168,6 +167,7 @@ impl fontspector_checkapi::Plugin for GoogleFonts {
168167
.add_and_register_check(checks::googlefonts::old_ttfautohint)
169168
// checks::googlefonts::production_encoded_glyphs // DISABLED
170169
// checks::googlefonts::production_glyphs_similarity // Unlikely to be useful in the short term
170+
// checks::googlefonts::description::family_update // Unlikely to useful yet
171171
.add_and_register_check(checks::googlefonts::render_own_name)
172172
.add_and_register_check(checks::googlefonts::STAT::axis_order)
173173
.add_and_register_check(checks::googlefonts::STAT::axisregistry)

0 commit comments

Comments
 (0)