From 17c82898ccc629e2a5b3f704f8b3d65af52c5e83 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Mon, 15 Apr 2024 16:26:26 -0400 Subject: [PATCH] [glyphs] Store codepoints in glyph struct Previously we were generating a separate cmap-like thing in the glyphs module, which was then used to set the codepoints in the ir::Glyph. This just keeps the codepoints in the glyphs Glyph, which will be nice because it means that all the things that we use to determine the glyph category will be in one place. --- glyphs-reader/src/font.rs | 64 ++++++++++++++++--------------------- glyphs2fontir/src/source.rs | 21 ++++++------ 2 files changed, 38 insertions(+), 47 deletions(-) diff --git a/glyphs-reader/src/font.rs b/glyphs-reader/src/font.rs index c8b46dab..9e89d79f 100644 --- a/glyphs-reader/src/font.rs +++ b/glyphs-reader/src/font.rs @@ -51,7 +51,6 @@ pub struct Font { pub default_master_idx: usize, pub glyphs: BTreeMap, pub glyph_order: Vec, - pub glyph_to_codepoints: BTreeMap>, // tag => (user:design) tuples pub axis_mappings: RawUserToDesignMapping, pub virtual_masters: Vec>>, @@ -174,6 +173,7 @@ pub struct Glyph { pub glyphname: SmolStr, pub export: bool, pub layers: Vec, + pub unicode: BTreeSet, /// The left kerning group pub left_kern: Option, /// The right kerning group @@ -1334,21 +1334,11 @@ fn parse_glyph_order(raw_font: &RawFont) -> Vec { glyph_order } -/// Returns a map from glyph name to codepoint(s). -fn parse_codepoints(raw_font: &mut RawFont, radix: u32) -> BTreeMap> { - let mut name_to_cp: BTreeMap> = BTreeMap::new(); - raw_font - .glyphs - .iter() - .filter_map(|g| g.unicode.as_ref().map(|u| (&g.glyphname, u))) - .flat_map(|(g, u)| { - u.split(',') - .map(|cp| (g.clone(), i64::from_str_radix(cp, radix).unwrap() as u32)) - }) - .for_each(|(glyph_name, codepoint)| { - name_to_cp.entry(glyph_name).or_default().insert(codepoint); - }); - name_to_cp +// glyphs2 uses hex, glyphs3 uses base10 +fn parse_codepoint_str(s: &str, radix: u32) -> BTreeSet { + s.split(',') + .map(|cp| u32::from_str_radix(cp, radix).unwrap()) + .collect() } /// @@ -1630,23 +1620,26 @@ impl TryFrom for Layer { } } -impl TryFrom for Glyph { - type Error = Error; - - fn try_from(from: RawGlyph) -> Result { +impl RawGlyph { + // we pass in the radix because it depends on the version, stored in the font struct + fn build(self, codepoint_radix: u32) -> Result { let mut instances = Vec::new(); - for layer in from.layers { + for layer in self.layers { if layer.is_draft() { continue; } instances.push(layer.try_into()?); } Ok(Glyph { - glyphname: from.glyphname, - export: from.export.unwrap_or(true), + glyphname: self.glyphname, + export: self.export.unwrap_or(true), layers: instances, - left_kern: from.kern_left, - right_kern: from.kern_right, + left_kern: self.kern_left, + right_kern: self.kern_right, + unicode: self + .unicode + .map(|s| parse_codepoint_str(&s, codepoint_radix)) + .unwrap_or_default(), }) } } @@ -1833,7 +1826,6 @@ impl TryFrom for Font { let radix = if from.is_v2() { 16 } else { 10 }; let glyph_order = parse_glyph_order(&from); - let glyph_to_codepoints = parse_codepoints(&mut from, radix); let use_typo_metrics = from.custom_parameters.bool("Use Typo Metrics"); let has_wws_names = from.custom_parameters.bool("Has WWS Names"); @@ -1850,7 +1842,7 @@ impl TryFrom for Font { let mut glyphs = BTreeMap::new(); for raw_glyph in from.glyphs.into_iter() { - glyphs.insert(raw_glyph.glyphname.clone(), raw_glyph.try_into()?); + glyphs.insert(raw_glyph.glyphname.clone(), raw_glyph.build(radix)?); } let mut features = Vec::new(); @@ -1962,7 +1954,7 @@ impl TryFrom for Font { default_master_idx, glyphs, glyph_order, - glyph_to_codepoints, + //glyph_to_codepoints, axis_mappings, virtual_masters, features, @@ -2351,8 +2343,8 @@ mod tests { fn understand_v2_style_unquoted_hex_unicode() { let font = Font::load(&glyphs2_dir().join("Unicode-UnquotedHex.glyphs")).unwrap(); assert_eq!( - &BTreeSet::from([0x1234]), - font.glyph_to_codepoints.get("name").unwrap(), + BTreeSet::from([0x1234]), + font.glyphs.get("name").unwrap().unicode, ); assert_eq!(1, font.glyphs.len()); } @@ -2361,8 +2353,8 @@ mod tests { fn understand_v2_style_quoted_hex_unicode_sequence() { let font = Font::load(&glyphs2_dir().join("Unicode-QuotedHexSequence.glyphs")).unwrap(); assert_eq!( - &BTreeSet::from([0x2044, 0x200D, 0x2215]), - font.glyph_to_codepoints.get("name").unwrap(), + BTreeSet::from([0x2044, 0x200D, 0x2215]), + font.glyphs.get("name").unwrap().unicode, ); assert_eq!(1, font.glyphs.len()); } @@ -2371,8 +2363,8 @@ mod tests { fn understand_v3_style_unquoted_decimal_unicode() { let font = Font::load(&glyphs3_dir().join("Unicode-UnquotedDec.glyphs")).unwrap(); assert_eq!( - &BTreeSet::from([182]), - font.glyph_to_codepoints.get("name").unwrap() + BTreeSet::from([182]), + font.glyphs.get("name").unwrap().unicode ); assert_eq!(1, font.glyphs.len()); } @@ -2381,8 +2373,8 @@ mod tests { fn understand_v3_style_unquoted_decimal_unicode_sequence() { let font = Font::load(&glyphs3_dir().join("Unicode-UnquotedDecSequence.glyphs")).unwrap(); assert_eq!( - &BTreeSet::from([1619, 1764]), - font.glyph_to_codepoints.get("name").unwrap(), + BTreeSet::from([1619, 1764]), + font.glyphs.get("name").unwrap().unicode, ); assert_eq!(1, font.glyphs.len()); } diff --git a/glyphs2fontir/src/source.rs b/glyphs2fontir/src/source.rs index 12503f90..69564fc4 100644 --- a/glyphs2fontir/src/source.rs +++ b/glyphs2fontir/src/source.rs @@ -93,7 +93,6 @@ impl GlyphsIrSource { default_master_idx: font.default_master_idx, glyphs: Default::default(), glyph_order: Default::default(), - glyph_to_codepoints: Default::default(), axis_mappings: font.axis_mappings.clone(), virtual_masters: Default::default(), features: Default::default(), @@ -123,7 +122,6 @@ impl GlyphsIrSource { default_master_idx: font.default_master_idx, glyphs: Default::default(), glyph_order: Default::default(), - glyph_to_codepoints: Default::default(), axis_mappings: Default::default(), virtual_masters: Default::default(), features: Default::default(), @@ -837,14 +835,10 @@ impl Work for GlyphIrWork { let mut ir_glyph = ir::GlyphBuilder::new(self.glyph_name.clone()); ir_glyph.emit_to_binary = glyph.export; - if let Some(codepoints) = font.glyph_to_codepoints.get(self.glyph_name.as_str()) { - codepoints.iter().for_each(|cp| { - ir_glyph.codepoints.insert(*cp); - }); - } + ir_glyph.codepoints.extend(glyph.unicode.iter().copied()); // https://github.com/googlefonts/fontmake-rs/issues/285 glyphs non-spacing marks are 0-width - let zero_width = is_nonspacing_mark(&ir_glyph.codepoints, &ir_glyph.name); + let zero_width = is_nonspacing_mark(glyph); let mut ir_anchors = AnchorBuilder::new(self.glyph_name.clone()); @@ -937,11 +931,16 @@ impl Work for GlyphIrWork { // This will eventually need to be replaced with something that can handle // custom GlyphData.xml files, as well as handle overrides that are part of the // glyph source. -fn is_nonspacing_mark(codepoints: &HashSet, name: &GlyphName) -> bool { +fn is_nonspacing_mark(glyph: &glyphs_reader::Glyph) -> bool { static GLYPH_DATA: OnceLock = OnceLock::new(); let data = GLYPH_DATA.get_or_init(|| GlyphData::new(None).unwrap()); - data.get_by_name(name) - .or_else(|| codepoints.iter().find_map(|cp| data.get_by_codepoint(*cp))) + data.get_by_name(&glyph.glyphname) + .or_else(|| { + glyph + .unicode + .iter() + .find_map(|cp| data.get_by_codepoint(*cp)) + }) .map(|info| (info.is_nonspacing_mark())) .unwrap_or(false) }