From 2dba80879f067536833ca7411cb0c7e993ca30df Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 5 Sep 2023 16:14:58 +0100 Subject: [PATCH] generated .notdef glyph should be left unmapped https://learn.microsoft.com/en-us/typography/opentype/spec/cmap#overview 'Regardless of the encoding scheme, character codes that do not correspond to any glyph in the font should be mapped to glyph index 0. The glyph at this location must be a special glyph representing a missing character, commonly known as .notdef.' --- fontc/src/lib.rs | 14 +++++++------- fontir/src/ir.rs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index 6b6570f5..9cb59b9d 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -1021,7 +1021,6 @@ mod tests { .collect(); assert_eq!( vec![ - (0x0000, 0), (0x002C, 2), (0x002E, 1), (0x0030, 3), @@ -1288,10 +1287,12 @@ mod tests { let buf = fs::read(font_file).unwrap(); let font = FontRef::new(&buf).unwrap(); - assert_eq!( - GlyphId::new(0), - font.cmap().unwrap().map_codepoint(0u32).unwrap() - ); + // Character 0x0000 (NULL) != '.notdef' glyph, and neither are any other + // characters actually, because '.notdef' (glyph index 0) means the absence + // of a character-to-glyph mapping: + // https://github.com/googlefonts/fontc/pull/423/files#r1309257127 + // https://learn.microsoft.com/en-us/typography/opentype/spec/cmap#overview + assert_eq!(None, font.cmap().unwrap().map_codepoint(0u32)); } #[test] @@ -1312,13 +1313,12 @@ mod tests { assert_eq!( vec![ - GlyphId::new(0), GlyphId::new(1), GlyphId::new(2), GlyphId::new(3), GlyphId::new(6), ], - [0x00, 0x20, 0x21, 0x2d, 0x3d] + [0x20, 0x21, 0x2d, 0x3d] .iter() .map(|cp| font.cmap().unwrap().map_codepoint(*cp as u32).unwrap()) .collect::>() diff --git a/fontir/src/ir.rs b/fontir/src/ir.rs index 4a6751c6..25270bf5 100644 --- a/fontir/src/ir.rs +++ b/fontir/src/ir.rs @@ -1028,7 +1028,7 @@ impl GlyphBuilder { Self { name: GlyphName::NOTDEF.clone(), - codepoints: HashSet::from([0]), + codepoints: HashSet::new(), sources: HashMap::from([( default_location, GlyphInstance {