Merge pull request #1244 from googlefonts/glyph-order-iter

Tweak GlyphOrder iteration

fontbe/src/cmap.rs

@@ -35,7 +35,7 @@ impl Work<Context, AnyWorkId, Error> for CmapWork {
         let glyph_order = context.ir.glyph_order.get();
 
         let mappings = glyph_order
-            .iter()
+            .names()
             .map(|glyph_name| context.ir.get_glyph(glyph_name.clone()))
             .enumerate()
             .flat_map(|(gid, glyph)| {

fontbe/src/features.rs

@@ -409,7 +409,7 @@ impl FeatureCompilationWork {
 
 fn write_debug_glyph_order(context: &Context, glyphs: &GlyphOrder) {
     let glyph_order_file = context.debug_dir().join("glyph_order.txt");
-    let glyph_order = glyphs.iter().map(|g| g.to_string()).collect::<Vec<_>>();
+    let glyph_order = glyphs.names().map(|g| g.as_str()).collect::<Vec<_>>();
     let glyph_order = glyph_order.join("\n");
     if let Err(e) = fs::write(glyph_order_file, glyph_order) {
         log::error!("failed to write glyph order to debug/glyph_order.txt: '{e}'");
@@ -449,7 +449,7 @@ impl Work<Context, AnyWorkId, Error> for FeatureParsingWork {
         let features = context.ir.features.get();
         let glyph_order = context.ir.glyph_order.get();
         let static_metadata = context.ir.static_metadata.get();
-        let glyph_map = glyph_order.iter().cloned().collect();
+        let glyph_map = glyph_order.names().cloned().collect();
 
         let result = self.parse(&features, &glyph_map);
 

fontbe/src/features/kern.rs

@@ -384,7 +384,7 @@ impl Work<Context, AnyWorkId, Error> for KerningGatherWork {
         let arc_fragments = context.kern_fragments.all();
         let ast = context.fea_ast.get();
         let glyph_order = context.ir.glyph_order.get();
-        let glyph_map = glyph_order.iter().cloned().collect();
+        let glyph_map = glyph_order.names().cloned().collect();
         let mut pairs: Vec<_> = arc_fragments
             .iter()
             .flat_map(|(_, fragment)| fragment.kerns.iter())
@@ -393,13 +393,7 @@ impl Work<Context, AnyWorkId, Error> for KerningGatherWork {
 
         let glyphs_and_gids = glyph_order
             .iter()
-            .enumerate()
-            .map(|(i, glyphname)| {
-                (
-                    context.ir.get_glyph(glyphname.clone()),
-                    GlyphId16::new(i as u16),
-                )
-            })
+            .map(|(i, glyphname)| (context.ir.get_glyph(glyphname.clone()), i))
             .collect::<Vec<_>>();
         let lookups = self.finalize_kerning(&pairs, &ast.ast, &glyph_map, glyphs_and_gids)?;
         context.fea_rs_kerns.set(lookups);

fontbe/src/features/marks.rs

@@ -237,7 +237,7 @@ impl<'a> MarkLookupBuilder<'a> {
         let mark_lig = self.make_lookups::<MarkToLigBuilder>(mark_lig_groups)?;
         let curs = self.make_cursive_lookups()?;
         Ok(FeaRsMarks {
-            glyphmap: self.glyph_order.iter().cloned().collect(),
+            glyphmap: self.glyph_order.names().cloned().collect(),
             mark_base,
             mark_mark,
             mark_lig,
@@ -539,7 +539,7 @@ fn get_gdef_classes(
     ast: &ParseTree,
     glyph_order: &GlyphOrder,
 ) -> BTreeMap<GlyphName, GlyphClassDef> {
-    let glyph_map = glyph_order.iter().cloned().collect();
+    let glyph_map = glyph_order.names().cloned().collect();
     // if we prefer classes defined in fea, compile the fea and see if we have any
     if meta.gdef_categories.prefer_gdef_categories_in_fea {
         if let Some(gdef_classes) =

fontbe/src/glyphs.rs

@@ -774,7 +774,7 @@ fn compute_composite_bboxes(context: &Context) -> Result<(), Error> {
     let glyph_order = context.ir.glyph_order.get();
 
     let glyphs: HashMap<_, _> = glyph_order
-        .iter()
+        .names()
         .map(|gn| {
             (
                 gn,
@@ -847,7 +847,7 @@ impl Work<Context, AnyWorkId, Error> for GlyfLocaWork {
         let glyph_order = context.ir.glyph_order.get();
         let mut builder = GlyfLocaBuilder::new();
 
-        for name in glyph_order.iter() {
+        for name in glyph_order.names() {
             let glyph = context
                 .glyphs
                 .get(&WorkId::GlyfFragment(name.clone()).into());

fontbe/src/gvar.rs

@@ -28,7 +28,7 @@ fn make_variations(
     get_deltas: impl Fn(&GlyphName) -> Vec<GlyphDeltas>,
 ) -> Vec<GlyphVariations> {
     glyph_order
-        .iter()
+        .names()
         .enumerate()
         .map(|(gid, gn)| GlyphVariations::new(GlyphId16::new(gid as u16).into(), get_deltas(gn)))
         .collect()

fontbe/src/hvar.rs

@@ -166,7 +166,7 @@ impl Work<Context, AnyWorkId, Error> for HvarWork {
         let glyph_order = context.ir.glyph_order.get();
         let axis_count = var_model.axes().count().try_into().unwrap();
         let glyphs: Vec<_> = glyph_order
-            .iter()
+            .names()
             .map(|name| context.ir.glyphs.get(&FeWorkId::Glyph(name.clone())))
             .collect();
         let glyph_locations: HashSet<_> = glyphs

fontbe/src/metrics_and_limits.rs

@@ -241,9 +241,7 @@ impl Work<Context, AnyWorkId, Error> for MetricAndLimitWork {
 
         let mut long_metrics: Vec<LongMetric> = glyph_order
             .iter()
-            .enumerate()
             .map(|(gid, gn)| {
-                let gid = GlyphId16::new(gid as u16);
                 let advance: u16 = context
                     .ir
                     .get_glyph(gn.clone())

fontbe/src/os2.rs

@@ -505,7 +505,7 @@ fn codepoints(context: &Context) -> HashSet<u32> {
     let glyph_order = context.ir.glyph_order.get();
 
     let mut codepoints = HashSet::new();
-    for glyph_name in glyph_order.iter() {
+    for glyph_name in glyph_order.names() {
         codepoints.extend(context.ir.get_glyph(glyph_name.clone()).codepoints.iter());
     }
     codepoints

fontbe/src/post.rs

@@ -48,7 +48,7 @@ impl Work<Context, AnyWorkId, Error> for PostWork {
         let glyph_order = context.ir.glyph_order.get();
         let mut post = Post::new_v2(
             glyph_order
-                .iter()
+                .names()
                 .map(|g| postscript_names.get(g).unwrap_or(g).as_str()),
         );
         post.is_fixed_pitch = static_metadata.misc.is_fixed_pitch.unwrap_or_default() as u32;

fontir/src/glyph.rs

@@ -384,7 +384,7 @@ fn apply_optional_transformations(
         .flags
         .contains(Flags::DECOMPOSE_TRANSFORMED_COMPONENTS)
     {
-        for glyph_name in glyph_order.iter() {
+        for glyph_name in glyph_order.names() {
             let glyph = context.get_glyph(glyph_name.clone());
             if glyph.has_nonidentity_2x2() {
                 convert_components_to_contours(context, &glyph)?;
@@ -393,7 +393,7 @@ fn apply_optional_transformations(
     }
 
     if context.flags.contains(Flags::FLATTEN_COMPONENTS) {
-        for glyph_name in glyph_order.iter() {
+        for glyph_name in glyph_order.names() {
             let glyph = context.get_glyph(glyph_name.clone());
             flatten_glyph(context, &glyph)?;
         }
@@ -461,13 +461,13 @@ impl Work<Context, WorkId, Error> for GlyphOrderWork {
         let arc_current = context.preliminary_glyph_order.get();
         let current_glyph_order = &*arc_current;
         let original_glyphs: HashMap<_, _> = current_glyph_order
-            .iter()
+            .names()
             .map(|gn| (gn, context.get_glyph(gn.clone())))
             .collect();
 
         // Anything the source specifically said not to retain shouldn't end up in the final font
         let mut new_glyph_order = current_glyph_order.clone();
-        for glyph_name in current_glyph_order.iter() {
+        for glyph_name in current_glyph_order.names() {
             let glyph = original_glyphs.get(glyph_name).unwrap();
             if !glyph.emit_to_binary {
                 new_glyph_order.remove(glyph_name);
@@ -481,7 +481,7 @@ impl Work<Context, WorkId, Error> for GlyphOrderWork {
         // 2) collapse such glyphs into a simple (contour-only) glyph
         //    fontmake (Python) prefers option 2.
         let mut todo = VecDeque::new();
-        for glyph_name in new_glyph_order.iter() {
+        for glyph_name in new_glyph_order.names() {
             let glyph = original_glyphs.get(glyph_name).unwrap();
             if !glyph.has_consistent_components() {
                 debug!(
@@ -519,7 +519,7 @@ impl Work<Context, WorkId, Error> for GlyphOrderWork {
         // Resolve component references to glyphs that are not retained by conversion to contours
         // Glyphs have to have consistent components at this point so it's safe to just check the default
         // See https://github.com/googlefonts/fontc/issues/532
-        for glyph_name in new_glyph_order.iter() {
+        for glyph_name in new_glyph_order.names() {
             // We are only int
             let glyph = context.get_glyph(glyph_name.clone());
             for component in glyph.default_instance().components.iter() {

fontir/src/ir.rs

@@ -263,7 +263,16 @@ impl GlyphOrder {
         self.0.contains(name)
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = &GlyphName> {
+    /// Iterate over ids and names
+    pub fn iter(&self) -> impl Iterator<Item = (GlyphId16, &GlyphName)> {
+        self.0
+            .iter()
+            .enumerate()
+            .map(|(i, name)| (GlyphId16::new(i as _), name))
+    }
+
+    /// Iterate glyph names, in order
+    pub fn names(&self) -> impl Iterator<Item = &GlyphName> {
         self.0.iter()
     }