Skip to content

Commit 0ef8110

Browse files
committed
glyf table checks
1 parent 2d10caf commit 0ef8110

File tree

3 files changed

+164
-7
lines changed

3 files changed

+164
-7
lines changed

profile-universal/src/checks/glyf.rs

+153
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
use fontspector_checkapi::{prelude::*, testfont, FileTypeConvert};
2+
use read_fonts::{
3+
tables::glyf::{Anchor, Glyph},
4+
TableProvider,
5+
};
6+
use skrifa::{GlyphId, Tag};
7+
use std::{cmp::Ordering, collections::HashSet};
8+
9+
#[check(
10+
id="opentype/glyf_unused_data",
11+
rationale="
12+
This check validates the structural integrity of the glyf table,
13+
by checking that all glyphs referenced in the loca table are
14+
actually present in the glyf table and that there is no unused
15+
data at the end of the glyf table. A failure here indicates a
16+
problem with the font compiler.
17+
",
18+
proposal="https://github.com/fonttools/fontbakery/issues/4829", // legacy check
19+
title="Is there any unused data at the end of the glyf table?"
20+
)]
21+
fn glyf_unused_data(t: &Testable, _context: &Context) -> CheckFnResult {
22+
let ttf = testfont!(t);
23+
let glyf = ttf
24+
.font()
25+
.table_data(Tag::new(b"glyf"))
26+
.ok_or(CheckError::skip("no-glyf", "No glyf table"))?;
27+
let loca = ttf
28+
.font()
29+
.loca(None)
30+
.map_err(|_| CheckError::Error("No loca table".to_string()))?;
31+
if let Some(last_index) = loca.get_raw(loca.len() + 1) {
32+
Ok(match glyf.len().cmp(&(last_index as usize)) {
33+
Ordering::Greater => Status::just_one_fail(
34+
"unreachable-data",
35+
"Unused data at the end of the glyf table",
36+
),
37+
Ordering::Less => {
38+
Status::just_one_fail("missing-data", "Missing data at the end of the glyf table")
39+
}
40+
Ordering::Equal => Status::just_one_pass(),
41+
})
42+
} else {
43+
Err(CheckError::Error("Invalid loca table".to_string()))
44+
}
45+
}
46+
47+
#[check(
48+
id = "opentype/points_out_of_bounds",
49+
rationale = "
50+
The glyf table specifies a bounding box for each glyph. This check
51+
ensures that all points in all glyph paths are within the bounding
52+
box. Glyphs with out-of-bounds points can cause rendering issues in
53+
some software, and should be corrected.
54+
",
55+
proposal = "https://github.com/fonttools/fontbakery/issues/735",
56+
title = "Check for points out of bounds"
57+
)]
58+
fn check_point_out_of_bounds(t: &Testable, context: &Context) -> CheckFnResult {
59+
let ttf = testfont!(t);
60+
let font = ttf.font();
61+
let glyf = font.glyf()?;
62+
let loca = font.loca(None)?;
63+
let mut messages = vec![];
64+
for gid in 0..font.maxp()?.num_glyphs() {
65+
let gid = GlyphId::new(gid);
66+
if let Some(Glyph::Simple(glyph)) = loca.get_glyf(gid, &glyf)? {
67+
for point in glyph.points() {
68+
if point.x < glyph.x_min() || point.x > glyph.x_max() {
69+
#[allow(clippy::unwrap_used)] // Synthesise is true so this will never fail
70+
messages.push(format!(
71+
"{} (x={}, bounds are {}<->{})",
72+
ttf.glyph_name_for_id(gid, true).unwrap(),
73+
point.x,
74+
glyph.x_min(),
75+
glyph.x_max()
76+
));
77+
}
78+
if point.y < glyph.y_min() || point.y > glyph.y_max() {
79+
#[allow(clippy::unwrap_used)] // Synthesise is true so this will never fail
80+
messages.push(format!(
81+
"{} (y={}, bounds are {}<->{})",
82+
ttf.glyph_name_for_id(gid, true).unwrap(),
83+
point.y,
84+
glyph.y_min(),
85+
glyph.y_max()
86+
));
87+
}
88+
}
89+
}
90+
}
91+
if messages.is_empty() {
92+
Ok(Status::just_one_pass())
93+
} else {
94+
Ok(Status::just_one_warn("points-out-of-bounds",
95+
&format!("The following glyphs have coordinates which are out of bounds:\n\n{}\n\nThis happens a lot when points are not extremes, which is usually bad. However, fixing this alert by adding points on extremes may do more harm than good, especially with italics, calligraphic-script, handwriting, rounded and other fonts. So it is common to ignore this message.",
96+
bullet_list(context, messages)))
97+
)
98+
}
99+
}
100+
101+
#[check(
102+
id = "opentype/glyf_non_transformed_duplicate_components",
103+
rationale = "
104+
There have been cases in which fonts had faulty double quote marks, with each
105+
of them containing two single quote marks as components with the same
106+
x, y coordinates which makes them visually look like single quote marks.
107+
108+
This check ensures that glyphs do not contain duplicate components
109+
which have the same x,y coordinates.
110+
",
111+
proposal = "https://github.com/fonttools/fontbakery/pull/2709",
112+
title = "Check glyphs do not have duplicate components which have the same x,y coordinates."
113+
)]
114+
fn check_glyf_non_transformed_duplicate_components(
115+
t: &Testable,
116+
context: &Context,
117+
) -> CheckFnResult {
118+
let ttf = testfont!(t);
119+
let font = ttf.font();
120+
let glyf = font.glyf()?;
121+
let loca = font.loca(None)?;
122+
let mut messages = vec![];
123+
for gid in 0..font.maxp()?.num_glyphs() {
124+
let gid = GlyphId::new(gid);
125+
if let Some(Glyph::Composite(glyph)) = loca.get_glyf(gid, &glyf)? {
126+
let mut components = HashSet::new();
127+
for component in glyph.components() {
128+
if let Anchor::Offset { x, y } = component.anchor {
129+
if !components.insert((component.glyph, x, y)) {
130+
#[allow(clippy::unwrap_used)] // Synthesise is true so this will never fail
131+
messages.push(format!(
132+
"{}: duplicate component {} at {},{}",
133+
ttf.glyph_name_for_id(gid, true).unwrap(),
134+
ttf.glyph_name_for_id(component.glyph, true).unwrap(),
135+
x,
136+
y
137+
));
138+
}
139+
}
140+
}
141+
}
142+
}
143+
if messages.is_empty() {
144+
Ok(Status::just_one_pass())
145+
} else {
146+
Ok(
147+
Status::just_one_warn("found-duplicates",
148+
&format!("The following glyphs have duplicate components which have the same x,y coordinates.\n\n{}",
149+
bullet_list(context, messages))
150+
)
151+
)
152+
}
153+
}

profile-universal/src/checks/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub mod arabic_spacing_symbols;
22
pub mod bold_italic_unique;
33
pub mod code_pages;
44
pub mod fvar;
5+
pub mod glyf;
56
pub mod glyphnames;
67
pub mod head;
78
pub mod hhea;

profile-universal/src/lib.rs

+10-7
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ impl fontspector_checkapi::Plugin for Universal {
1111
cr.register_check(checks::code_pages::code_pages);
1212
cr.register_check(checks::fvar::axis_ranges_correct);
1313
cr.register_check(checks::fvar::regular_coords_correct);
14+
cr.register_check(checks::glyf::glyf_unused_data);
15+
cr.register_check(checks::glyf::check_point_out_of_bounds);
16+
cr.register_check(checks::glyf::check_glyf_non_transformed_duplicate_components);
1417
cr.register_check(checks::hhea::caret_slope);
1518
cr.register_check(checks::hhea::maxadvancewidth);
1619
cr.register_check(checks::post::post_table_version);
@@ -38,6 +41,10 @@ impl fontspector_checkapi::Plugin for Universal {
3841
"opentype/unitsperem",
3942
"opentype/fsselection",
4043
"opentype/family/panose_familytype",
44+
"opentype/fvar/axis_ranges_correct",
45+
"opentype/glyf_unused_data",
46+
"opentype/points_out_of_bounds",
47+
"opentype/glyf_non_transformed_duplicate_components",
4148
4249
# Checks left to port
4350
"opentype/cff2_call_depth",
@@ -52,7 +59,9 @@ impl fontspector_checkapi::Plugin for Universal {
5259
# Checks we don't need because they have been integrated into other checks
5360
# "opentype/dsig", (unwanted_tables)
5461
# "opentype/varfont/ital_range", (opentype/fvar/axis_ranges_correct)
55-
# "opentype/varfont/slnt_range",
62+
# "opentype/varfont/wdth_valid_range", (above)
63+
# "opentype/varfont/wght_valid_range", (above)
64+
# "opentype/varfont/slnt_range", (above)
5665
# "opentype/varfont/regular_ital_coord", (opentype/fvar/regular_coords_correct)
5766
# "opentype/varfont/regular_opsz_coord",
5867
# "opentype/varfont/regular_slnt_coord",
@@ -61,12 +70,9 @@ impl fontspector_checkapi::Plugin for Universal {
6170
# "opentype/fsselection_matches_macstyle", (merged into opentype/fsselection)
6271
6372
# Checks I haven't got around to classifying yet
64-
"opentype/fvar/axis_ranges_correct",
6573
"opentype/gdef_mark_chars",
6674
"opentype/gdef_non_mark_chars",
6775
"opentype/gdef_spacing_marks",
68-
"opentype/glyf_non_transformed_duplicate_components",
69-
"opentype/glyf_unused_data",
7076
"opentype/gpos_kerning_info",
7177
"opentype/italic_angle",
7278
"opentype/italic_axis_in_stat",
@@ -83,7 +89,6 @@ impl fontspector_checkapi::Plugin for Universal {
8389
"opentype/name/no_copyright_on_description",
8490
"opentype/name/postscript_name_consistency",
8591
"opentype/name/postscript_vs_cff",
86-
"opentype/points_out_of_bounds",
8792
"opentype/postscript_name",
8893
"opentype/slant_direction",
8994
"opentype/stat_has_axis_value_tables",
@@ -95,8 +100,6 @@ impl fontspector_checkapi::Plugin for Universal {
95100
"opentype/varfont/valid_default_instance_nameids",
96101
"opentype/varfont/valid_postscript_nameid",
97102
"opentype/varfont/valid_subfamily_nameid",
98-
"opentype/varfont/wdth_valid_range",
99-
"opentype/varfont/wght_valid_range",
100103
"opentype/vendor_id",
101104
"opentype/weight_class_fvar",
102105
"opentype/xavgcharwidth",

0 commit comments

Comments
 (0)