From 2adc9492791d55b5e08880e86490d7bab3a5266d Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Tue, 8 Aug 2023 10:55:55 -0400 Subject: [PATCH] Use fontations Glyph enum This renames the backend 'Glyph' type to 'NamedGlyph', and turns it into a simple struct, containing a `write-fonts` Glyph and the associated name. This also lets us delete the GlyphPersistable type, since we don't need to store a flag indicating whether the glyph is simple or composite. --- fontbe/src/glyphs.rs | 22 +++--- fontbe/src/metrics_and_limits.rs | 22 +++--- fontbe/src/orchestration.rs | 123 +++++++------------------------ fontc/src/lib.rs | 46 +++++++----- 4 files changed, 75 insertions(+), 138 deletions(-) diff --git a/fontbe/src/glyphs.rs b/fontbe/src/glyphs.rs index c3d4570b..48b6e4c6 100644 --- a/fontbe/src/glyphs.rs +++ b/fontbe/src/glyphs.rs @@ -22,7 +22,7 @@ use read_fonts::{ }; use write_fonts::{ tables::{ - glyf::{Bbox, Component, ComponentFlags, CompositeGlyph, SimpleGlyph}, + glyf::{Bbox, Component, ComponentFlags, CompositeGlyph, Glyph, SimpleGlyph}, variations::iup_delta_optimize, }, OtRound, @@ -30,7 +30,7 @@ use write_fonts::{ use crate::{ error::{Error, GlyphProblem}, - orchestration::{AnyWorkId, BeWork, Context, Glyph, GvarFragment, LocaFormat, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, GvarFragment, LocaFormat, NamedGlyph, WorkId}, }; #[derive(Debug)] @@ -281,7 +281,7 @@ impl Work for GlyphWork { let composite = create_composite(context, ir_glyph, default_location, &components)?; context .glyphs - .set_unconditionally(Glyph::new_composite(name.clone(), composite)); + .set_unconditionally(NamedGlyph::new(name.clone(), composite)); let point_seqs = point_seqs_for_composite_glyph(ir_glyph); (name, point_seqs, Vec::new()) } @@ -309,7 +309,7 @@ impl Work for GlyphWork { }; context .glyphs - .set_unconditionally(Glyph::new_simple(name.clone(), base_glyph.clone())); + .set_unconditionally(NamedGlyph::new(name.clone(), base_glyph.clone())); let mut contour_end = 0; let mut contour_ends = Vec::with_capacity(base_glyph.contours().len()); @@ -661,7 +661,7 @@ fn compute_composite_bboxes(context: &Context) -> Result<(), Error> { let mut bbox_acquired: HashMap = HashMap::new(); let mut composites = glyphs .values() - .filter(|glyph| matches!(glyph.as_ref(), Glyph::Composite(..))) + .filter(|glyph| !glyph.is_simple()) .collect::>(); trace!("Resolve bbox for {} composites", composites.len()); @@ -670,9 +670,11 @@ fn compute_composite_bboxes(context: &Context) -> Result<(), Error> { // Hopefully we can figure out some of those bboxes! for composite in composites.iter() { - let Glyph::Composite(glyph_name, composite) = composite.as_ref() else { + let glyph_name = &composite.name; + let Glyph::Composite(composite) = &composite.glyph else { panic!("Only composites should be in our vector of composites!!"); }; + let mut missing_boxes = false; let boxes: Vec<_> = composite .components() @@ -686,9 +688,9 @@ fn compute_composite_bboxes(context: &Context) -> Result<(), Error> { glyphs .get(ref_glyph_name) .map(|g| g.as_ref().clone()) - .and_then(|g| match g { + .and_then(|g| match &g.glyph { Glyph::Composite(..) => None, - Glyph::Simple(_, simple_glyph) => Some(bbox2rect(simple_glyph.bbox)), + Glyph::Simple(simple_glyph) => Some(bbox2rect(simple_glyph.bbox)), }) }); if bbox.is_none() { @@ -715,7 +717,7 @@ fn compute_composite_bboxes(context: &Context) -> Result<(), Error> { } // Kerplode if we didn't make any progress this spin - composites.retain(|composite| !bbox_acquired.contains_key(composite.glyph_name())); + composites.retain(|composite| !bbox_acquired.contains_key(&composite.name)); if pending == composites.len() { panic!("Unable to make progress on composite bbox, stuck at\n{composites:?}"); } @@ -727,7 +729,7 @@ fn compute_composite_bboxes(context: &Context) -> Result<(), Error> { .glyphs .get(&WorkId::GlyfFragment(glyph_name.clone()).into())) .clone(); - let Glyph::Composite(_, composite) = &mut glyph else { + let Glyph::Composite(composite) = &mut glyph.glyph else { panic!("{glyph_name} is not a composite; we shouldn't be trying to update it"); }; composite.bbox = bbox.into(); // delay conversion to Bbox to avoid accumulating rounding error diff --git a/fontbe/src/metrics_and_limits.rs b/fontbe/src/metrics_and_limits.rs index c74ed310..321d2772 100644 --- a/fontbe/src/metrics_and_limits.rs +++ b/fontbe/src/metrics_and_limits.rs @@ -14,7 +14,7 @@ use read_fonts::types::FWord; use write_fonts::{ dump_table, tables::{ - glyf::{Bbox, Contour}, + glyf::{Bbox, Contour, Glyph}, hhea::Hhea, hmtx::Hmtx, maxp::Maxp, @@ -25,7 +25,7 @@ use write_fonts::{ use crate::{ error::Error, - orchestration::{AnyWorkId, BeWork, Context, Glyph, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, NamedGlyph, WorkId}, }; #[derive(Debug)] @@ -81,11 +81,11 @@ impl GlyphLimits { } impl FontLimits { - fn update(&mut self, id: GlyphId, advance: u16, glyph: &Glyph) { + fn update(&mut self, id: GlyphId, advance: u16, glyph: &NamedGlyph) { // min side bearings are only for non-empty glyphs // we will presume only simple glyphs with no contours are empty - if !glyph.is_empty() { - let bbox = glyph.bbox(); + if !glyph.glyph.is_empty() { + let bbox = glyph.glyph.bbox(); let left_side_bearing = bbox.x_min; // aw - (lsb + xMax - xMin) ... but if lsb == xMin then just advance - xMax? let right_side_bearing: i16 = match advance as i32 - bbox.x_max as i32 { @@ -108,11 +108,11 @@ impl FontLimits { self.advance_width_max = max(self.advance_width_max, advance); } - let bbox = glyph.bbox(); + let bbox = glyph.glyph.bbox(); self.bbox = self.bbox.map(|b| b.union(bbox)).or(Some(bbox)); - match glyph { - Glyph::Simple(_, simple) => { + match &glyph.glyph { + Glyph::Simple(simple) => { let num_points = simple.contours().iter().map(Contour::len).sum::() as u16; let num_contours = simple.contours().len() as u16; self.max_points = max(self.max_points, num_points); @@ -129,7 +129,7 @@ impl FontLimits { }, ) } - Glyph::Composite(_, composite) => { + Glyph::Composite(composite) => { let num_components = composite.components().len() as u16; self.max_component_elements = max(self.max_component_elements, num_components); let components = Some(composite.components().iter().map(|c| c.glyph).collect()); @@ -262,7 +262,7 @@ impl Work for MetricAndLimitWork { glyph_limits.update(gid, advance, &glyph); LongMetric { advance, - side_bearing: glyph.bbox().x_min, + side_bearing: glyph.glyph.bbox().x_min, } }) .collect(); @@ -379,7 +379,7 @@ mod tests { glyph_limits.update( GlyphId::new(0), 0, - &crate::orchestration::Glyph::Simple( + &crate::orchestration::NamedGlyph::new( "don't care".into(), SimpleGlyph::from_bezpath( &BezPath::from_svg("M-437,611 L-334,715 L-334,611 Z").unwrap(), diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index ab7b791e..976c3180 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -27,26 +27,13 @@ use serde::{Deserialize, Serialize}; use write_fonts::{ dump_table, tables::{ - avar::Avar, - cmap::Cmap, - fvar::Fvar, - glyf::{Bbox, SimpleGlyph}, - gpos::Gpos, - gsub::Gsub, - gvar::GlyphDeltas, - head::Head, - hhea::Hhea, - maxp::Maxp, - name::Name, - os2::Os2, - post::Post, - stat::Stat, + avar::Avar, cmap::Cmap, fvar::Fvar, glyf::Glyph, gpos::Gpos, gsub::Gsub, gvar::GlyphDeltas, + head::Head, hhea::Hhea, maxp::Maxp, name::Name, os2::Os2, post::Post, stat::Stat, variations::Tuple, }, validate::Validate, FontWrite, OtRound, }; -use write_fonts::{from_obj::FromTableRef, tables::glyf::CompositeGlyph}; use crate::{error::Error, paths::Paths}; @@ -127,107 +114,47 @@ impl From for AnyWorkId { } } -/// +/// A glyph and its associated name #[derive(Debug, Clone)] -pub enum Glyph { - Simple(GlyphName, SimpleGlyph), - Composite(GlyphName, CompositeGlyph), +pub struct NamedGlyph { + pub name: GlyphName, + pub glyph: Glyph, } -impl Glyph { - pub(crate) fn new_simple(glyph_name: GlyphName, simple: SimpleGlyph) -> Glyph { - Glyph::Simple(glyph_name, simple) - } - - pub(crate) fn new_composite(glyph_name: GlyphName, composite: CompositeGlyph) -> Glyph { - Glyph::Composite(glyph_name, composite) - } - - pub(crate) fn glyph_name(&self) -> &GlyphName { - match self { - Glyph::Simple(name, _) | Glyph::Composite(name, _) => name, - } - } - - pub fn to_bytes(&self) -> Vec { - match self { - Glyph::Simple(_, table) => dump_table(table), - Glyph::Composite(_, table) => dump_table(table), +impl NamedGlyph { + pub(crate) fn new(name: GlyphName, glyph: impl Into) -> Self { + Self { + name, + glyph: glyph.into(), } - .unwrap() } - pub fn bbox(&self) -> Bbox { - match self { - Glyph::Simple(_, table) => table.bbox, - Glyph::Composite(_, table) => table.bbox, - } + pub fn is_simple(&self) -> bool { + matches!(&self.glyph, Glyph::Simple(_)) } - pub fn is_empty(&self) -> bool { - match self { - Glyph::Simple(_, table) => table.contours().is_empty(), - Glyph::Composite(_, table) => table.components().is_empty(), - } + pub fn to_bytes(&self) -> Vec { + dump_table(&self.glyph).unwrap() } } -impl IdAware for Glyph { +impl IdAware for NamedGlyph { fn id(&self) -> AnyWorkId { - AnyWorkId::Be(WorkId::GlyfFragment(self.glyph_name().clone())) - } -} - -#[derive(Serialize, Deserialize)] -struct GlyphPersistable { - name: GlyphName, - simple: bool, - glyph: Vec, -} - -impl From for Glyph { - fn from(value: GlyphPersistable) -> Self { - match value.simple { - true => { - let glyph = - read_fonts::tables::glyf::SimpleGlyph::read(FontData::new(&value.glyph)) - .unwrap(); - let glyph = SimpleGlyph::from_table_ref(&glyph); - Glyph::Simple(value.name, glyph) - } - false => { - let glyph = - read_fonts::tables::glyf::CompositeGlyph::read(FontData::new(&value.glyph)) - .unwrap(); - let glyph = CompositeGlyph::from_table_ref(&glyph); - Glyph::Composite(value.name, glyph) - } - } + AnyWorkId::Be(WorkId::GlyfFragment(self.name.clone())) } } -impl Persistable for Glyph { +impl Persistable for NamedGlyph { fn read(from: &mut dyn Read) -> Self { - bincode::deserialize_from::<&mut dyn Read, GlyphPersistable>(from) - .unwrap() - .into() + let (name, bytes): (GlyphName, Vec) = bincode::deserialize_from(from).unwrap(); + let glyph = Glyph::read(bytes.as_slice().into()).unwrap(); + NamedGlyph { name, glyph } } fn write(&self, to: &mut dyn Write) { - let obj = match self { - Glyph::Simple(name, table) => GlyphPersistable { - name: name.clone(), - simple: true, - glyph: dump_table(table).unwrap(), - }, - Glyph::Composite(name, table) => GlyphPersistable { - name: name.clone(), - simple: false, - glyph: dump_table(table).unwrap(), - }, - }; - - bincode::serialize_into::<&mut dyn Write, GlyphPersistable>(to, &obj).unwrap(); + let glyph_bytes = dump_table(&self.glyph).unwrap(); + let to_write = (&self.name, glyph_bytes); + bincode::serialize_into(to, &to_write).unwrap(); } } @@ -439,7 +366,7 @@ pub struct Context { // work results we've completed or restored from disk pub gvar_fragments: BeContextMap, - pub glyphs: BeContextMap, + pub glyphs: BeContextMap, pub avar: BeContextItem>, pub cmap: BeContextItem>, diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index 43ef30ea..383189d9 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -310,7 +310,7 @@ mod tests { use chrono::{Duration, TimeZone, Utc}; use fontbe::orchestration::{ - AnyWorkId, Context as BeContext, Glyph as BeGlyph, LocaFormat, WorkId as BeWorkIdentifier, + AnyWorkId, Context as BeContext, LocaFormat, NamedGlyph, WorkId as BeWorkIdentifier, }; use fontdrasil::types::GlyphName; use fontir::{ @@ -342,7 +342,10 @@ mod tests { GlyphId, Tag, }; use tempfile::{tempdir, TempDir}; - use write_fonts::{dump_table, tables::glyf::Bbox}; + use write_fonts::{ + dump_table, + tables::glyf::{Bbox, Glyph}, + }; use super::*; @@ -402,16 +405,16 @@ mod tests { } } - fn read(&self) -> Vec { + fn read(&self) -> Vec> { let glyf = Glyf::read(FontData::new(&self.raw_glyf)).unwrap(); - let loca = Loca::read_with_args( + let loca = Loca::read( FontData::new(&self.raw_loca), - &(self.loca_format == LocaFormat::Long), + self.loca_format == LocaFormat::Long, ) .unwrap(); (0..loca.len()) .map(|gid| loca.get_glyf(GlyphId::new(gid as u16), &glyf)) - .map(|r| r.unwrap().unwrap()) + .map(|r| r.unwrap()) .collect() } } @@ -751,10 +754,10 @@ mod tests { buf } - fn read_be_glyph(build_dir: &Path, name: &str) -> BeGlyph { + fn read_be_glyph(build_dir: &Path, name: &str) -> Glyph { let raw_glyph = read_file(&build_dir.join(format!("glyphs/{name}.glyf"))); let read: &mut dyn Read = &mut raw_glyph.as_slice(); - BeGlyph::read(read) + NamedGlyph::read(read).glyph } #[test] @@ -764,7 +767,7 @@ mod tests { assert!(glyph.default_instance().contours.is_empty(), "{glyph:?}"); assert_eq!(2, glyph.default_instance().components.len(), "{glyph:?}"); - let BeGlyph::Composite(_, glyph) = read_be_glyph(temp_dir.path(), glyph.name.as_str()) else { + let Glyph::Composite(glyph) = read_be_glyph(temp_dir.path(), glyph.name.as_str()) else { panic!("Expected a simple glyph"); }; let raw_glyph = dump_table(&glyph).unwrap(); @@ -780,7 +783,7 @@ mod tests { assert!(glyph.default_instance().components.is_empty(), "{glyph:?}"); assert_eq!(2, glyph.default_instance().contours.len(), "{glyph:?}"); - let BeGlyph::Simple(_, glyph) = read_be_glyph(temp_dir.path(), glyph.name.as_str()) else { + let Glyph::Simple(glyph) = read_be_glyph(temp_dir.path(), glyph.name.as_str()) else { panic!("Expected a simple glyph"); }; assert_eq!(2, glyph.contours().len()); @@ -792,7 +795,7 @@ mod tests { let build_dir = temp_dir.path(); compile(Args::for_test(build_dir, "static.designspace")); - let BeGlyph::Simple(_, glyph) = read_be_glyph(build_dir, "bar") else { + let Glyph::Simple( glyph) = read_be_glyph(build_dir, "bar") else { panic!("Expected a simple glyph"); }; @@ -830,8 +833,10 @@ mod tests { .read() .iter() .map(|g| match g { - glyf::Glyph::Simple(glyph) => (glyph.num_points(), glyph.number_of_contours()), - glyf::Glyph::Composite(glyph) => (0, glyph.number_of_contours()), + None => (0, 0), + Some(glyf::Glyph::Simple(glyph)) => + (glyph.num_points(), glyph.number_of_contours()), + Some(glyf::Glyph::Composite(glyph)) => (0, glyph.number_of_contours()), }) .collect::>() ); @@ -847,7 +852,7 @@ mod tests { let glyphs = glyph_data.read(); // the glyph 'O' contains several quad splines let uppercase_o = &glyphs[result.get_glyph_index("O") as usize]; - let glyf::Glyph::Simple(glyph) = uppercase_o else { + let Some(glyf::Glyph::Simple(glyph)) = uppercase_o else { panic!("Expected 'O' to be a simple glyph, got {:?}", uppercase_o); }; assert_eq!(2, glyph.number_of_contours()); @@ -882,16 +887,19 @@ mod tests { let glyphs = glyph_data.read(); assert!(glyphs.len() > 1, "{glyphs:#?}"); let period_idx = result.get_glyph_index("period"); - assert!(matches!(glyphs[0], glyf::Glyph::Simple(..)), "{glyphs:#?}"); + assert!( + matches!(glyphs[0], Some(glyf::Glyph::Simple(..))), + "{glyphs:#?}" + ); for (idx, glyph) in glyphs.iter().enumerate() { if idx == period_idx.try_into().unwrap() { assert!( - matches!(glyphs[idx], glyf::Glyph::Simple(..)), + matches!(glyphs[idx], Some(glyf::Glyph::Simple(..))), "glyphs[{idx}] should be simple\n{glyph:#?}\nAll:\n{glyphs:#?}" ); } else { assert!( - matches!(glyphs[idx], glyf::Glyph::Composite(..)), + matches!(glyphs[idx], Some(glyf::Glyph::Composite(..))), "glyphs[{idx}] should be composite\n{glyph:#?}\nAll:\n{glyphs:#?}" ); } @@ -908,7 +916,7 @@ mod tests { let non_uniform_scale_idx = result.get_glyph_index("non_uniform_scale"); let glyph_data = result.glyphs(); let glyphs = glyph_data.read(); - let glyf::Glyph::Composite(glyph) = &glyphs[non_uniform_scale_idx as usize] else { + let Some(glyf::Glyph::Composite(glyph)) = &glyphs[non_uniform_scale_idx as usize] else { panic!("Expected a composite\n{glyphs:#?}"); }; let component = glyph.components().next().unwrap(); @@ -950,7 +958,7 @@ mod tests { let gid = result.get_glyph_index("simple_transform_again"); let glyph_data = result.glyphs(); let glyphs = glyph_data.read(); - let glyf::Glyph::Composite(glyph) = &glyphs[gid as usize] else { + let Some(glyf::Glyph::Composite(glyph)) = &glyphs[gid as usize] else { panic!("Expected a composite\n{glyphs:#?}"); };