Use fontations GlyfLocaBuilder

This also uses the LocaFormat type in fontations (which was copied from
here) which then forces us to hack around orphan rules.

fontbe/src/glyphs.rs

@@ -22,15 +22,17 @@ 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,
 };
 
 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<Context, AnyWorkId, Error> 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<u8> = 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<u8> = 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(())
     }

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<Context, AnyWorkId, Error> 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};
 

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,41 +248,39 @@ 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<LocaFormat> 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<LocaFormatWrapper> for LocaFormat {
+    fn from(value: LocaFormatWrapper) -> Self {
+        if value.0 == 0 {
             LocaFormat::Short
         } else {
             LocaFormat::Long
         }
     }
 }
 
-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<Bytes>,
     pub post: BeContextItem<BeValue<Post>>,
     pub loca: BeContextItem<Bytes>,
-    pub loca_format: BeContextItem<LocaFormat>,
+    pub loca_format: BeContextItem<LocaFormatWrapper>,
     pub maxp: BeContextItem<BeValue<Maxp>>,
     pub name: BeContextItem<BeValue<Name>>,
     pub os2: BeContextItem<BeValue<Os2>>,
@@ -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::<Vec<_>>())
-        );
-    }
-
     fn non_default_region() -> VariationRegion {
         let mut region = VariationRegion::default();
         region.insert(

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")),
             }