Skip to content

Commit 05b309e

Browse files
committed
Add fontbakery bridge (proof of concept)
1 parent b682152 commit 05b309e

File tree

6 files changed

+226
-0
lines changed

6 files changed

+226
-0
lines changed

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ members = [
77
"fontspector-checkhelper",
88
"profile-universal", "profile-testplugin", "profile-googlefonts",
99
"fontspector-web",
10+
"fontbakery-bridge",
1011
]
1112

1213
default-members = [ "fontspector-cli" ]

fontbakery-bridge/Cargo.toml

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
[package]
2+
name = "fontbakery-bridge"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
[lib]
7+
crate-type=["cdylib"]
8+
9+
[dependencies]
10+
fontspector-checkapi = { path = "../fontspector-checkapi" }
11+
pyo3 = "0.22"
12+
13+
[target.'cfg(not(target_family = "wasm"))'.dependencies]
14+
# Plugin architecture
15+
pluginator = {workspace = true}

fontbakery-bridge/src/checks.rs

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use crate::run_a_python_test;
2+
use fontspector_checkapi::prelude::*;
3+
4+
pub(crate) const hinting_impact: Check = Check {
5+
id: "hinting_impact",
6+
title: "Show hinting filesize impact",
7+
rationale: r#"""
8+
This check is merely informative, displaying an useful comparison of filesizes
9+
of hinted versus unhinted font files.
10+
"""#,
11+
proposal: "https://github.com/fonttools/fontbakery/issues/4829",
12+
hotfix: None,
13+
fix_source: None,
14+
applies_to: "TTF",
15+
flags: CheckFlags::default(),
16+
implementation: CheckImplementation::CheckOne(&run_a_python_test),
17+
_metadata: Some(
18+
r#" { "module": "fontbakery.checks.hinting", "function": "check_hinting_impact" } "#,
19+
),
20+
};
21+
22+
pub(crate) const opentype_name_empty_records: Check = Check {
23+
id: "opentype/name/empty_records",
24+
title: "Check name table for empty records",
25+
rationale: "Check the name table for empty records, as this can cause problems in Adobe apps.",
26+
proposal: "https://github.com/fonttools/fontbakery/pull/2369",
27+
hotfix: None,
28+
fix_source: None,
29+
applies_to: "TTF",
30+
flags: CheckFlags::default(),
31+
implementation: CheckImplementation::CheckOne(&run_a_python_test),
32+
_metadata: Some(
33+
r#" { "module": "fontbakery.checks.opentype.name", "function": "check_name_empty_records" } "#,
34+
),
35+
};
36+
37+
pub(crate) const monospace: Check = Check {
38+
id: "opentype/monospace",
39+
title: "Check name table for empty records",
40+
rationale: r#"""
41+
There are various metadata in the OpenType spec to specify if a font is
42+
monospaced or not. If the font is not truly monospaced, then no monospaced
43+
metadata should be set (as sometimes they mistakenly are...)
44+
45+
Requirements for monospace fonts:
46+
47+
* post.isFixedPitch - "Set to 0 if the font is proportionally spaced,
48+
non-zero if the font is not proportionally spaced (monospaced)"
49+
(https://www.microsoft.com/typography/otspec/post.htm)
50+
51+
* hhea.advanceWidthMax must be correct, meaning no glyph's width value
52+
is greater. (https://www.microsoft.com/typography/otspec/hhea.htm)
53+
54+
* OS/2.panose.bProportion must be set to 9 (monospace) on latin text fonts.
55+
56+
* OS/2.panose.bSpacing must be set to 3 (monospace) on latin hand written
57+
or latin symbol fonts.
58+
59+
* Spec says: "The PANOSE definition contains ten digits each of which currently
60+
describes up to sixteen variations. Windows uses bFamilyType, bSerifStyle
61+
and bProportion in the font mapper to determine family type. It also uses
62+
bProportion to determine if the font is monospaced."
63+
(https://www.microsoft.com/typography/otspec/os2.htm#pan
64+
https://monotypecom-test.monotype.de/services/pan2)
65+
66+
* OS/2.xAvgCharWidth must be set accurately.
67+
"OS/2.xAvgCharWidth is used when rendering monospaced fonts,
68+
at least by Windows GDI"
69+
(http://typedrawers.com/discussion/comment/15397/#Comment_15397)
70+
71+
Also we should report an error for glyphs not of average width.
72+
73+
74+
Please also note:
75+
76+
Thomas Phinney told us that a few years ago (as of December 2019), if you gave
77+
a font a monospace flag in Panose, Microsoft Word would ignore the actual
78+
advance widths and treat it as monospaced.
79+
80+
Source: https://typedrawers.com/discussion/comment/45140/#Comment_45140
81+
"""#,
82+
proposal: "https://github.com/fonttools/fontbakery/issues/4829",
83+
hotfix: None,
84+
fix_source: None,
85+
applies_to: "TTF",
86+
flags: CheckFlags::default(),
87+
implementation: CheckImplementation::CheckOne(&run_a_python_test),
88+
_metadata: Some(
89+
r#" { "module": "fontbakery.checks.opentype.name", "function": "check_monospace" } "#,
90+
),
91+
};

fontbakery-bridge/src/lib.rs

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
#![allow(non_upper_case_globals)]
2+
#![deny(clippy::unwrap_used, clippy::expect_used)]
3+
use fontspector_checkapi::{prelude::*, StatusCode};
4+
use pyo3::prelude::*;
5+
mod checks;
6+
struct FontbakeryBridge;
7+
8+
// We isolate the Python part to avoid type/result madness.
9+
fn call_python(module: &str, function: &str, testable: &Testable) -> PyResult<CheckFnResult> {
10+
let filename = testable.filename.to_string_lossy();
11+
Python::with_gil(|py| {
12+
let module = PyModule::import_bound(py, module)?;
13+
let check = module.getattr(function)?;
14+
15+
// Let's check this check's mandatory arguments
16+
let args = check.getattr("mandatoryArgs")?.extract::<Vec<String>>()?;
17+
if args.len() != 1 {
18+
return Err(PyErr::new::<pyo3::exceptions::PyValueError, _>(
19+
"Expected exactly one mandatory argument".to_string(),
20+
));
21+
}
22+
let arg = if args[0] == "font" {
23+
// Convert the Testable to a Python Font object
24+
let testable = PyModule::import_bound(py, "fontbakery.testable")?;
25+
let font = testable.getattr("Font")?;
26+
font.call1((filename,))?
27+
} else if args[0] == "ttFont" {
28+
// Convert the Testable to a Python TTFont object
29+
let ttlib = PyModule::import_bound(py, "fontTools.ttLib")?;
30+
let ttfont = ttlib.getattr("TTFont")?;
31+
ttfont.call1((filename,))?
32+
} else {
33+
return Err(PyErr::new::<pyo3::exceptions::PyValueError, _>(
34+
"Unknown mandatory argument".to_string(),
35+
));
36+
};
37+
38+
let checkresult = check.call1((arg,))?;
39+
let mut messages: Vec<Status> = vec![];
40+
41+
// Now convert the Fontbakery status to our StatusList
42+
while let Ok(value) = checkresult.getattr("__next__")?.call0() {
43+
// Value is a tuple of status and message
44+
let status_str = value.get_item(0)?.getattr("name")?.extract::<String>()?;
45+
let status = StatusCode::from_string(&status_str).ok_or_else(|| {
46+
PyErr::new::<pyo3::exceptions::PyValueError, _>(
47+
"Fontbakery returned unknown status code".to_string(),
48+
)
49+
})?;
50+
let code = value.get_item(1)?.getattr("code")?.extract::<String>()?;
51+
let message = value.get_item(1)?.getattr("message")?.extract::<String>()?;
52+
messages.push(Status {
53+
message: Some(message),
54+
severity: status,
55+
code: Some(code),
56+
});
57+
}
58+
Ok(return_result(messages))
59+
})
60+
}
61+
62+
// This wrapper will work for any fontbakery check that takes a single
63+
// Font or ttFont object as an argument.
64+
fn run_a_python_test(c: &Testable, context: &Context) -> CheckFnResult {
65+
let module = context
66+
.check_metadata
67+
.get("module")
68+
.ok_or_else(|| CheckError::Error("No module specified".to_string()))?
69+
.as_str()
70+
.ok_or_else(|| CheckError::Error("module in metadata was not a string!".to_string()))?;
71+
let function = context
72+
.check_metadata
73+
.get("function")
74+
.ok_or_else(|| CheckError::Error("No function specified".to_string()))?
75+
.as_str()
76+
.ok_or_else(|| CheckError::Error("function in metadata was not a string!".to_string()))?;
77+
call_python(module, function, c)
78+
.unwrap_or_else(|e| Err(CheckError::Error(format!("Python error: {}", e))))
79+
}
80+
81+
impl fontspector_checkapi::Plugin for FontbakeryBridge {
82+
fn register(&self, cr: &mut Registry) -> Result<(), String> {
83+
cr.register_check(checks::hinting_impact);
84+
cr.register_check(checks::opentype_name_empty_records);
85+
cr.register_check(checks::monospace);
86+
pyo3::prepare_freethreaded_python();
87+
cr.register_profile(
88+
"fontbakery",
89+
Profile::from_toml(
90+
r#"
91+
[sections]
92+
"Test profile" = [
93+
"hinting_impact",
94+
"opentype/name/empty_records",
95+
"opentype/monospace",
96+
]
97+
"#,
98+
)
99+
.map_err(|_| "Couldn't parse profile")?,
100+
)
101+
}
102+
}
103+
104+
#[cfg(not(target_family = "wasm"))]
105+
pluginator::plugin_implementation!(fontspector_checkapi::Plugin, FontbakeryBridge);

fontspector-checkapi/src/status.rs

+13
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@ impl StatusCode {
3131
]
3232
.into_iter()
3333
}
34+
35+
pub fn from_string(s: &str) -> Option<StatusCode> {
36+
match s {
37+
"SKIP" => Some(StatusCode::Skip),
38+
"INFO" => Some(StatusCode::Info),
39+
"PASS" => Some(StatusCode::Pass),
40+
"WARN" => Some(StatusCode::Warn),
41+
"FAIL" => Some(StatusCode::Fail),
42+
"ERROR" => Some(StatusCode::Error),
43+
_ => None,
44+
}
45+
}
3446
}
3547
impl std::fmt::Display for StatusCode {
3648
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -130,6 +142,7 @@ impl Status {
130142
/// Reflects the result of some kind of early return from a check function
131143
///
132144
/// This may be because there was an error, or because the check was skipped.
145+
#[derive(Debug)]
133146
pub enum CheckError {
134147
Error(String),
135148
Skip { message: String, code: String },

profile-testplugin/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ crate-type=["cdylib"]
99
[dependencies]
1010
fontspector-checkapi = { path = "../fontspector-checkapi" }
1111
toml = "0.8.14"
12+
serde_json = "1.0.68"
1213

1314
[target.'cfg(not(target_family = "wasm"))'.dependencies]
1415
# Plugin architecture

0 commit comments

Comments
 (0)