Several more checks

Author: simoncozens

profile-universal/Cargo.toml

@@ -14,4 +14,5 @@ read-fonts = {workspace = true}
 write-fonts = {workspace = true}
 font-types = {workspace = true}
 skrifa = {workspace = true}
-itertools = "0.13.0"
+itertools = "0.13.0"
+unicode-properties = "0.1.3"

profile-universal/src/checks/gdef.rs

@@ -0,0 +1,88 @@
+use fontspector_checkapi::{constants::GlyphClass, prelude::*, testfont, FileTypeConvert};
+use read_fonts::TableProvider;
+use skrifa::MetadataProvider;
+use unicode_properties::{GeneralCategory, UnicodeGeneralCategory};
+
+fn is_nonspacing_mark(c: char) -> bool {
+    matches!(
+        c.general_category(),
+        GeneralCategory::NonspacingMark | GeneralCategory::EnclosingMark
+    )
+}
+
+#[check(
+    id = "opentype/gdef_spacing_marks",
+    rationale = "
+        Glyphs in the GDEF mark glyph class should be non-spacing.
+
+        Spacing glyphs in the GDEF mark glyph class may have incorrect anchor
+        positioning that was only intended for building composite glyphs during design.
+    ",
+    proposal = "https://github.com/fonttools/fontbakery/issues/2877",
+    title = "Check glyphs in mark glyph class are non-spacing."
+)]
+fn gdef_spacing_marks(f: &Testable, context: &Context) -> CheckFnResult {
+    let font = testfont!(f);
+    let hmtx = font.font().hmtx()?;
+    let gdef = font
+        .font()
+        .gdef()
+        .map_err(|_| CheckError::skip("no-gdef", "GDEF table unreadable or not present"))?;
+    let glyph_classdef = gdef.glyph_class_def().ok_or_else(|| {
+        CheckError::skip("no-glyph-class-def", "GDEF table has no GlyphClassDef")
+    })??;
+    let nonspacing_mark_glyphs = bullet_list(
+        context,
+        glyph_classdef
+            .iter()
+            .filter(|(glyph, class)| *class == 3 && hmtx.advance(*glyph).unwrap_or(0) > 0)
+            .map(|(glyph, _)| {
+                #[allow(clippy::unwrap_used)] // synthesis=true means this is infallible
+                font.glyph_name_for_id(glyph, true).unwrap()
+            }),
+    );
+    if !nonspacing_mark_glyphs.is_empty() {
+        return Ok(Status::just_one_fail("spacing-mark-glyphs", &format!(
+            "The following glyphs seem to be spacing (because they have width > 0 on the hmtx table) so they may be in the GDEF mark glyph class by mistake, or they should have zero width instead:\n{}",
+                nonspacing_mark_glyphs
+        )));
+    }
+
+    Ok(Status::just_one_pass())
+}
+
+#[check(
+    id = "opentype/gdef_mark_chars",
+    rationale = "Mark characters should be in the GDEF mark glyph class.",
+    proposal = "https://github.com/fonttools/fontbakery/issues/2877",
+    title = "Check mark characters are in GDEF mark glyph class."
+)]
+fn gdef_mark_chars(f: &Testable, context: &Context) -> CheckFnResult {
+    let font = testfont!(f);
+    let mark_chars_not_in_gdef_mark = bullet_list(
+        context,
+        font.font()
+            .charmap()
+            .mappings()
+            .filter(|(u, gid)| {
+                char::from_u32(*u).map_or(false, is_nonspacing_mark)
+                    && font.gdef_class(*gid) != Some(GlyphClass::Mark)
+            })
+            .map(|(u, gid)| {
+                #[allow(clippy::unwrap_used)] // synthesis=true means this is infallible
+                let name = font.glyph_name_for_id(gid, true).unwrap();
+                format!("U+{:04X} ({})", u, name)
+            }),
+    );
+    if !mark_chars_not_in_gdef_mark.is_empty() {
+        return Ok(Status::just_one_fail(
+            "mark-chars",
+            &format!(
+                "The following mark characters should be in the GDEF mark glyph class:\n{}",
+                mark_chars_not_in_gdef_mark
+            ),
+        ));
+    }
+
+    Ok(Status::just_one_pass())
+}

profile-universal/src/checks/kern.rs

@@ -0,0 +1,38 @@
+// Fontations doesn't yet support kern table
+
+// use fontspector_checkapi::{prelude::*, skip, testfont, FileTypeConvert};
+// use read_fonts::TableProvider;
+
+// #[check(
+//     id = "opentype/kern_table",
+//     rationale = "
+//         Even though all fonts should have their kerning implemented in the GPOS table,
+//         there may be kerning info at the kern table as well.
+
+//         Some applications such as MS PowerPoint require kerning info on the kern table.
+//         More specifically, they require a format 0 kern subtable from a kern table
+//         version 0 with only glyphs defined in the cmap table, which is the only one
+//         that Windows understands (and which is also the simplest and more limited
+//         of all the kern subtables).
+
+//         Google Fonts ingests fonts made for download and use on desktops, and does
+//         all web font optimizations in the serving pipeline (using libre libraries
+//         that anyone can replicate.)
+
+//         Ideally, TTFs intended for desktop users (and thus the ones intended for
+//         Google Fonts) should have both KERN and GPOS tables.
+
+//         Given all of the above, we currently treat kerning on a v0 kern table
+//         as a good-to-have (but optional) feature.
+//     ",
+//     proposal = "https://github.com/fonttools/fontbakery/issues/1675",
+//     title = "Is there a usable 'kern' table declared in the font?"
+// )]
+// fn kern_table(f: &Testable, _context: &Context) -> CheckFnResult {
+//     let font = testfont!(f);
+//     if !font.has_table(b"kern") {
+//         return Ok(Status::just_one_pass());
+//     }
+//     let kern = font.font().kern().ok();
+//     Ok(Status::just_one_pass())
+// }

profile-universal/src/checks/mod.rs

@@ -2,10 +2,12 @@ pub mod arabic_spacing_symbols;
 pub mod bold_italic_unique;
 pub mod code_pages;
 pub mod fvar;
+pub mod gdef;
 pub mod glyf;
 pub mod glyphnames;
 pub mod head;
 pub mod hhea;
+pub mod kern;
 pub mod layout;
 pub mod name;
 pub mod name_trailing_spaces;

profile-universal/src/checks/name.rs

@@ -3,6 +3,7 @@ use std::collections::HashMap;
 use font_types::NameId;
 use fontspector_checkapi::{prelude::*, skip, testfont, FileTypeConvert};
 use read_fonts::TableProvider;
+use skrifa::MetadataProvider;
 
 #[check(
     id = "opentype/name/empty_records",
@@ -283,6 +284,82 @@ fn name_italic_names(t: &Testable, _context: &Context) -> CheckFnResult {
     return_result(problems)
 }
 
+#[check(
+    id = "opentype/family/consistent_family_name",
+    rationale = r#"
+        Per the OpenType spec:
+
+            * "...many existing applications that use this pair of names assume that a
+              Font Family name is shared by at most four fonts that form a font
+              style-linking group"
+
+            * "For extended typographic families that includes fonts other than the
+              four basic styles(regular, italic, bold, bold italic), it is strongly
+              recommended that name IDs 16 and 17 be used in fonts to create an
+              extended, typographic grouping."
+
+            * "If name ID 16 is absent, then name ID 1 is considered to be the
+              typographic family name."
+
+        https://learn.microsoft.com/en-us/typography/opentype/spec/name
+
+        Fonts within a font family all must have consistent names
+        in the Typographic Family name (nameID 16)
+        or Font Family name (nameID 1), depending on which it uses.
+
+        Inconsistent font/typographic family names across fonts in a family
+        can result in unexpected behaviors, such as broken style linking.
+    "#,
+    proposal = "https://github.com/fonttools/fontbakery/issues/4112",
+    title = "Verify that family names in the name table are consistent across all fonts in the family. Checks Typographic Family name (nameID 16) if present, otherwise uses Font Family name (nameID 1)",
+    implementation = "all"
+)]
+fn consistent_family_name(c: &TestableCollection, context: &Context) -> CheckFnResult {
+    let fonts = TTF.from_collection(c);
+    let mut problems = vec![];
+    let mut family_names = HashMap::new();
+    for font in fonts {
+        let family_name = font
+            .font()
+            .localized_strings(NameId::TYPOGRAPHIC_FAMILY_NAME)
+            .english_or_first()
+            .or_else(|| {
+                font.font()
+                    .localized_strings(NameId::FAMILY_NAME)
+                    .english_or_first()
+            })
+            .ok_or_else(|| {
+                CheckError::Error(format!(
+                    "Font {} is missing a Family Name entry",
+                    font.filename.to_string_lossy()
+                ))
+            })?
+            .chars()
+            .collect::<String>();
+        family_names
+            .entry(family_name)
+            .or_insert_with(Vec::new)
+            .push(font.filename.to_string_lossy().to_string());
+    }
+    if family_names.len() > 1 {
+        let report = bullet_list(
+            context,
+            family_names
+                .iter()
+                .map(|(name, fonts)| format!("{} (found in fonts {})", name, fonts.join(", "))),
+        );
+        problems.push(Status::fail(
+            "inconsistent-family-name",
+            &format!(
+                "{} different family names were found:\n\n{}",
+                family_names.len(),
+                report
+            ),
+        ));
+    }
+    return_result(problems)
+}
+
 #[cfg(test)]
 #[allow(clippy::unwrap_used)]
 mod tests {