diff --git a/fontbe/src/glyphs.rs b/fontbe/src/glyphs.rs index 48b6e4c6f..996cb937a 100644 --- a/fontbe/src/glyphs.rs +++ b/fontbe/src/glyphs.rs @@ -22,7 +22,9 @@ use read_fonts::{ }; use write_fonts::{ tables::{ - glyf::{Bbox, Component, ComponentFlags, CompositeGlyph, Glyph, SimpleGlyph}, + glyf::{ + Bbox, Component, ComponentFlags, CompositeGlyph, GlyfLocaBuilder, Glyph, SimpleGlyph, + }, variations::iup_delta_optimize, }, OtRound, @@ -30,7 +32,7 @@ use write_fonts::{ use crate::{ error::{Error, GlyphProblem}, - orchestration::{AnyWorkId, BeWork, Context, GvarFragment, LocaFormat, NamedGlyph, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, GvarFragment, NamedGlyph, WorkId}, }; #[derive(Debug)] @@ -779,36 +781,21 @@ impl Work for GlyfLocaWork { compute_composite_bboxes(context)?; let glyph_order = context.ir.glyph_order.get(); + let mut builder = GlyfLocaBuilder::new(); - // Glue together glyf and loca - // Build loca as u32 first, then shrink if it'll fit - // This isn't overly memory efficient but ... fonts aren't *that* big (yet?) - let mut loca = vec![0]; - let mut glyf: Vec = Vec::new(); - glyf.reserve(1024 * 1024); // initial size, will grow as needed - glyph_order - .iter() - .map(|gn| context.glyphs.get(&WorkId::GlyfFragment(gn.clone()).into())) - .for_each(|g| { - let bytes = g.to_bytes(); - loca.push(loca.last().unwrap() + bytes.len() as u32); - glyf.extend(bytes); - }); + for name in glyph_order.iter() { + let glyph = context + .glyphs + .get(&WorkId::GlyfFragment(name.clone()).into()); + builder.add_glyph(&glyph.glyph).unwrap(); + } - let loca_format = LocaFormat::new(&loca); - let loca: Vec = match loca_format { - LocaFormat::Short => loca - .iter() - .flat_map(|offset| ((offset >> 1) as u16).to_be_bytes()) - .collect(), - LocaFormat::Long => loca - .iter() - .flat_map(|offset| offset.to_be_bytes()) - .collect(), - }; - context.loca_format.set(loca_format); - context.glyf.set(glyf.into()); - context.loca.set(loca.into()); + let (glyf, loca, loca_format) = builder.build(); + let raw_loca = write_fonts::dump_table(&loca).unwrap(); + let raw_glyf = write_fonts::dump_table(&glyf).unwrap(); + context.loca_format.set(loca_format.into()); + context.glyf.set(raw_glyf.into()); + context.loca.set(raw_loca.into()); Ok(()) } diff --git a/fontbe/src/head.rs b/fontbe/src/head.rs index 3d78f5688..f4124e8c3 100644 --- a/fontbe/src/head.rs +++ b/fontbe/src/head.rs @@ -7,11 +7,11 @@ use font_types::{Fixed, LongDateTime}; use fontdrasil::orchestration::{Access, Work}; use fontir::orchestration::WorkId as FeWorkId; use log::warn; -use write_fonts::tables::head::Head; +use write_fonts::tables::{head::Head, loca::LocaFormat}; use crate::{ error::Error, - orchestration::{AnyWorkId, BeWork, Context, LocaFormat, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, WorkId}, }; #[derive(Debug)] @@ -104,10 +104,10 @@ impl Work for HeadWork { /// Generate [head](https://learn.microsoft.com/en-us/typography/opentype/spec/head) fn exec(&self, context: &Context) -> Result<(), Error> { let static_metadata = context.ir.static_metadata.get(); - let loca_format = context.loca_format.get(); + let loca_format = (*context.loca_format.get().as_ref()).into(); let mut head = init_head( static_metadata.units_per_em, - *loca_format, + loca_format, static_metadata.misc.head_flags, static_metadata.misc.lowest_rec_ppm, ); @@ -130,8 +130,9 @@ mod tests { use chrono::{TimeZone, Utc}; use more_asserts::assert_ge; use temp_env; + use write_fonts::tables::loca::LocaFormat; - use crate::{head::apply_created_modified, orchestration::LocaFormat}; + use crate::head::apply_created_modified; use super::{init_head, seconds_since_mac_epoch}; diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index 976c3180d..968d21076 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -28,8 +28,8 @@ use write_fonts::{ dump_table, tables::{ 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, + head::Head, hhea::Hhea, loca::LocaFormat, maxp::Maxp, name::Name, os2::Os2, post::Post, + stat::Stat, variations::Tuple, }, validate::Validate, FontWrite, OtRound, @@ -248,19 +248,25 @@ impl TupleBuilder { } } -#[repr(u8)] -#[derive(Copy, Clone, Debug, PartialEq)] -pub enum LocaFormat { - Short = 0, - Long = 1, +// work around orphan rules. +// +// FIXME: Clarify if there's a good reason not to treat glyf/loca as a single +// entity, for the purpose of persistence? like a struct that contains both +// tables, and from which the format can be retrieved +// +// this whole thing needs a rethink, but this gets us working +#[derive(Clone, Copy, Serialize, Deserialize, PartialEq)] +pub struct LocaFormatWrapper(u8); + +impl From for LocaFormatWrapper { + fn from(value: LocaFormat) -> Self { + LocaFormatWrapper(value as _) + } } -impl LocaFormat { - pub fn new(loca: &[u32]) -> LocaFormat { - // https://github.com/fonttools/fonttools/blob/1c283756a5e39d69459eea80ed12792adc4922dd/Lib/fontTools/ttLib/tables/_l_o_c_a.py#L37 - if loca.last().copied().unwrap_or_default() < 0x20000 - && loca.iter().all(|offset| offset % 2 == 0) - { +impl From for LocaFormat { + fn from(value: LocaFormatWrapper) -> Self { + if value.0 == 0 { LocaFormat::Short } else { LocaFormat::Long @@ -268,21 +274,13 @@ impl LocaFormat { } } -impl Persistable for LocaFormat { +impl Persistable for LocaFormatWrapper { fn read(from: &mut dyn Read) -> Self { - let mut buf = Vec::new(); - from.read_to_end(&mut buf).unwrap(); - match buf.first() { - Some(0) => LocaFormat::Short, - Some(1) => LocaFormat::Long, - _ => { - panic!("serialized LocaFormat is invalid") - } - } + bincode::deserialize_from(from).unwrap() } fn write(&self, to: &mut dyn io::Write) { - to.write_all(&[*self as u8]).unwrap(); + bincode::serialize_into(to, self).unwrap() } } @@ -377,7 +375,7 @@ pub struct Context { pub gvar: BeContextItem, pub post: BeContextItem>, pub loca: BeContextItem, - pub loca_format: BeContextItem, + pub loca_format: BeContextItem, pub maxp: BeContextItem>, pub name: BeContextItem>, pub os2: BeContextItem>, @@ -515,7 +513,6 @@ where #[cfg(test)] mod tests { - use crate::orchestration::LocaFormat; use font_types::Tag; use fontir::{ coords::NormalizedCoord, @@ -524,29 +521,6 @@ mod tests { use super::GvarFragment; - #[test] - fn no_glyphs_is_short() { - assert_eq!(LocaFormat::Short, LocaFormat::new(&Vec::new())); - } - - #[test] - fn some_glyphs_is_short() { - assert_eq!(LocaFormat::Short, LocaFormat::new(&[24, 48, 112])); - } - - #[test] - fn unpadded_glyphs_is_long() { - assert_eq!(LocaFormat::Long, LocaFormat::new(&[24, 7, 112])); - } - - #[test] - fn big_glyphs_is_long() { - assert_eq!( - LocaFormat::Long, - LocaFormat::new(&(0..=32).map(|i| i * 0x1000).collect::>()) - ); - } - fn non_default_region() -> VariationRegion { let mut region = VariationRegion::default(); region.insert( diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index 383189d93..a449677a7 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, LocaFormat, NamedGlyph, WorkId as BeWorkIdentifier, + AnyWorkId, Context as BeContext, LocaFormatWrapper, NamedGlyph, WorkId as BeWorkIdentifier, }; use fontdrasil::types::GlyphName; use fontir::{ @@ -344,7 +344,10 @@ mod tests { use tempfile::{tempdir, TempDir}; use write_fonts::{ dump_table, - tables::glyf::{Bbox, Glyph}, + tables::{ + glyf::{Bbox, Glyph}, + loca::LocaFormat, + }, }; use super::*; @@ -397,9 +400,10 @@ mod tests { impl Glyphs { fn new(build_dir: &Path) -> Self { Glyphs { - loca_format: LocaFormat::read( + loca_format: LocaFormatWrapper::read( &mut File::open(build_dir.join("loca.format")).unwrap(), - ), + ) + .into(), raw_glyf: read_file(&build_dir.join("glyf.table")), raw_loca: read_file(&build_dir.join("loca.table")), }