diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 49dc252e6..424b3167c 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -358,27 +358,28 @@ impl<'a> FeatureWriter<'a> { for (group_name, group) in mark_groups.iter() { // if we have bases *and* marks produce mark to base if !group.bases.is_empty() && !group.marks.is_empty() { + let mut mark_base = MarkToBaseBuilder::default(); + + for (mark_name, mark_anchor) in group.marks.iter() { + let mark_gid = self.glyph_id(mark_name)?; + let mark_anchor_table = self.create_anchor_table(mark_anchor, builder)?; + mark_base + .insert_mark(mark_gid, group_name.0.into(), mark_anchor_table.clone()) + .map_err(Error::PreviouslyAssignedClass)?; + } + for (base_name, base_anchor) in group.bases.iter() { let base_gid = self.glyph_id(base_name)?; - let mut mark_base = MarkToBaseBuilder::default(); - for (mark_name, mark_anchor) in group.marks.iter() { - let mark_gid = self.glyph_id(mark_name)?; - let mark_anchor_table = self.create_anchor_table(mark_anchor, builder)?; - mark_base - .insert_mark(mark_gid, group_name.0.into(), mark_anchor_table.clone()) - .map_err(Error::PreviouslyAssignedClass)?; - } - let base_anchor_table = self.create_anchor_table(base_anchor, builder)?; mark_base.insert_base(base_gid, &group_name.0.into(), base_anchor_table); - - // each in it's own lookup, whch differs from fontmake - mark_base_lookups.push(builder.add_lookup( - LookupFlag::default(), - None, - vec![mark_base], - )); } + + // each mark to base it's own lookup, whch differs from fontmake + mark_base_lookups.push(builder.add_lookup( + LookupFlag::default(), + None, + vec![mark_base], + )); } // If a mark has anchors that are themselves marks what we got here is a mark to mark diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index 8a5eeb282..20c0acf36 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -362,12 +362,13 @@ mod tests { use read_fonts::{ tables::{ - gpos::{AnchorTable, PositionLookup}, + gpos::{AnchorTable, Gpos, MarkBasePosFormat1Marker, PositionLookup}, name::Name, os2::SelectionFlags, variations::{DeltaSetIndexMap, ItemVariationData}, }, types::NameId, + TableRef, }; use skrifa::{ charmap::Charmap, @@ -2310,6 +2311,19 @@ mod tests { } } + fn mark_base_lookups<'a>(gpos: &'a Gpos) -> Vec> { + // If only we had more indirections + gpos.lookup_list() + .iter() + .flat_map(|l| l.lookups().iter().map(|l| l.unwrap())) + .filter_map(|l| match l { + PositionLookup::MarkToBase(mark_base) => Some(mark_base), + _ => None, + }) + .flat_map(|mb| mb.subtables().iter().map(|s| s.unwrap())) + .collect() + } + #[test] fn compile_basic_gpos_mark_base() { let temp_dir = tempdir().unwrap(); @@ -2328,16 +2342,7 @@ mod tests { let brevecomb_gid = GlyphId::new(result.get_glyph_index("brevecomb").unwrap() as u16); // If only we had more indirections - let mark_base_lookups: Vec<_> = gpos - .lookup_list() - .iter() - .flat_map(|l| l.lookups().iter().map(|l| l.unwrap())) - .filter_map(|l| match l { - PositionLookup::MarkToBase(mark_base) => Some(mark_base), - _ => None, - }) - .flat_map(|mb| mb.subtables().iter().map(|s| s.unwrap())) - .collect(); + let mark_base_lookups = mark_base_lookups(&gpos); let bases = mark_base_lookups .iter() @@ -2455,4 +2460,19 @@ mod tests { fn default_fs_type_designspace() { assert_fs_type("designspace_from_glyphs/WghtVar.designspace", 1 << 2); } + + #[test] + fn one_lookup_per_group() { + let temp_dir = tempdir().unwrap(); + let build_dir = temp_dir.path(); + compile(Args::for_test(build_dir, "glyphs3/Oswald-AE-comb.glyphs")); + + let font_file = build_dir.join("font.ttf"); + let buf = fs::read(font_file).unwrap(); + let font = FontRef::new(&buf).unwrap(); + let gpos = font.gpos().unwrap(); + + // We had a bug where it was 2 + assert_eq!(1, mark_base_lookups(&gpos).len()); + } }