shaping: make mark-to-mark positioning more selective

Reported in #107 The selection of glyph pairs for mark-to-mark positioning was too eager. For the text in question the ligature component position was not being considered which was resulting in mark-to-mark positioning being applied to marks 2 and 3 when it should not have. Considering the ligature component position corrects this. However, requiring the ligature component position to always match broke some marks in Vietnamese. The solution was the introduction of a ligature flag on RawGlyph so that a mark glyph can be tracked as a ligature. This is used during GPOS to allow mark-to-mark positioning between a pair of marks if one of them is a ligature despite having differing ligature component positions. This behaviour matches Harfbuzz: https://github.com/harfbuzz/harfbuzz/blob/09a17a086c54e5c1b1aa674bba5b5449286e1480/src/OT/Layout/GPOS/MarkMarkPosFormat1.hh#L134-L138
yeslogic · May 29, 2024 · 1bf2db8 · 1bf2db8
1 parent f343287
commit 1bf2db8
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 1 deletion.
diff --git a/src/font.rs b/src/font.rs
@@ -407,6 +407,7 @@ impl<T: FontTableProvider> Font<T> {
                         small_caps: false,
                         multi_subst_dup: false,
                         is_vert_alt: false,
+                        ligature: false,
                         fake_bold: false,
                         fake_italic: false,
                         extra_data: (),

diff --git a/src/gpos.rs b/src/gpos.rs
@@ -616,12 +616,19 @@ fn forall_mark_mark_glyph_pairs(
         let mut i = start;
         while i + 1 < infos.len() {
             if infos[i].is_mark {
+                // infos[i] is the base mark. Scan forward looking for attaching marks
                 for j in i + 1..infos.len() {
-                    f(i, j, infos)?;
                     if !infos[j].is_mark {
                         start = i + 1;
                         continue 'outer;
                     }
+
+                    // infos[j] is a candidate attaching mark
+                    if infos[i].glyph.liga_component_pos == infos[j].glyph.liga_component_pos {
+                        f(i, j, infos)?;
+                    } else if infos[i].glyph.ligature || infos[j].glyph.ligature {
+                        f(i, j, infos)?;
+                    }
                 }
             }
             i += 1;

diff --git a/src/gsub.rs b/src/gsub.rs
@@ -97,6 +97,7 @@ impl Ligature {
                     glyphs[i].unicodes.append(&mut matched_glyph.unicodes);
                     glyphs[i].extra_data =
                         GlyphData::merge(glyphs[i].extra_data.clone(), matched_glyph.extra_data);
+                    glyphs[i].ligature = true;
                 } else {
                     glyphs[index].liga_component_pos = matched as u16;
                     skip += 1;
@@ -127,6 +128,7 @@ pub struct RawGlyph<T> {
     pub small_caps: bool,
     pub multi_subst_dup: bool,
     pub is_vert_alt: bool,
+    pub ligature: bool,
     pub fake_bold: bool,
     pub fake_italic: bool,
     pub variation: Option<VariationSelector>,
@@ -440,6 +442,7 @@ fn multiplesubst<T: GlyphData>(
                         small_caps: glyphs[i].small_caps,
                         multi_subst_dup: true,
                         is_vert_alt: glyphs[i].is_vert_alt,
+                        ligature: false,
                         fake_bold: glyphs[i].fake_bold,
                         fake_italic: glyphs[i].fake_italic,
                         extra_data: glyphs[i].extra_data.clone(),
@@ -981,6 +984,7 @@ fn find_alternate(features_list: &[FeatureInfo], feature_tag: u32) -> Option<usi
 ///                     small_caps: false,
 ///                     multi_subst_dup: false,
 ///                     is_vert_alt: false,
+///                     ligature: false,
 ///                     fake_bold: false,
 ///                     fake_italic: false,
 ///                     extra_data: (),

diff --git a/src/scripts/arabic.rs b/src/scripts/arabic.rs
@@ -73,6 +73,7 @@ impl From<&RawGlyph<()>> for ArabicGlyph {
             small_caps: raw_glyph.small_caps,
             multi_subst_dup: raw_glyph.multi_subst_dup,
             is_vert_alt: raw_glyph.is_vert_alt,
+            ligature: raw_glyph.ligature,
             fake_bold: raw_glyph.fake_bold,
             fake_italic: raw_glyph.fake_italic,
             variation: raw_glyph.variation,
@@ -96,6 +97,7 @@ impl From<&ArabicGlyph> for RawGlyph<()> {
             small_caps: arabic_glyph.small_caps,
             multi_subst_dup: arabic_glyph.multi_subst_dup,
             is_vert_alt: arabic_glyph.is_vert_alt,
+            ligature: arabic_glyph.ligature,
             fake_bold: arabic_glyph.fake_bold,
             variation: arabic_glyph.variation,
             fake_italic: arabic_glyph.fake_italic,

diff --git a/src/scripts/indic.rs b/src/scripts/indic.rs
@@ -1240,6 +1240,7 @@ fn insert_dotted_circle(
         small_caps: false,
         multi_subst_dup: false,
         is_vert_alt: false,
+        ligature: false,
         fake_bold: false,
         fake_italic: false,
         variation: None,
@@ -2264,6 +2265,7 @@ fn to_raw_glyph_indic(glyph: &RawGlyph<()>) -> RawGlyphIndic {
         small_caps: glyph.small_caps,
         multi_subst_dup: glyph.multi_subst_dup,
         is_vert_alt: glyph.is_vert_alt,
+        ligature: glyph.ligature,
         fake_bold: glyph.fake_bold,
         fake_italic: glyph.fake_italic,
         variation: glyph.variation,
@@ -2283,6 +2285,7 @@ fn from_raw_glyph_indic(glyph: RawGlyphIndic) -> RawGlyph<()> {
         small_caps: glyph.small_caps,
         multi_subst_dup: glyph.multi_subst_dup,
         is_vert_alt: glyph.is_vert_alt,
+        ligature: glyph.ligature,
         fake_bold: glyph.fake_bold,
         fake_italic: glyph.fake_italic,
         variation: glyph.variation,

diff --git a/src/scripts/khmer.rs b/src/scripts/khmer.rs
@@ -405,6 +405,7 @@ fn insert_dotted_circle(dotted_circle_index: u16, glyphs: &mut Vec<RawGlyphKhmer
         small_caps: false,
         multi_subst_dup: false,
         is_vert_alt: false,
+        ligature: false,
         fake_bold: false,
         fake_italic: false,
         variation: None,
@@ -606,6 +607,7 @@ fn to_raw_glyph_khmer(g: &RawGlyph<()>) -> RawGlyphKhmer {
         small_caps: g.small_caps,
         multi_subst_dup: g.multi_subst_dup,
         is_vert_alt: g.is_vert_alt,
+        ligature: g.ligature,
         fake_bold: g.fake_bold,
         fake_italic: g.fake_italic,
         variation: g.variation,
@@ -624,6 +626,7 @@ fn from_raw_glyph_khmer(g: RawGlyphKhmer) -> RawGlyph<()> {
         small_caps: g.small_caps,
         multi_subst_dup: g.multi_subst_dup,
         is_vert_alt: g.is_vert_alt,
+        ligature: g.ligature,
         fake_bold: g.fake_bold,
         fake_italic: g.fake_italic,
         variation: g.variation,

diff --git a/src/scripts/syriac.rs b/src/scripts/syriac.rs
@@ -89,6 +89,7 @@ impl From<&RawGlyph<()>> for SyriacGlyph {
             small_caps: raw_glyph.small_caps,
             multi_subst_dup: raw_glyph.multi_subst_dup,
             is_vert_alt: raw_glyph.is_vert_alt,
+            ligature: raw_glyph.ligature,
             fake_bold: raw_glyph.fake_bold,
             fake_italic: raw_glyph.fake_italic,
             variation: raw_glyph.variation,
@@ -113,6 +114,7 @@ impl From<&SyriacGlyph> for RawGlyph<()> {
             small_caps: syriac_glyph.small_caps,
             multi_subst_dup: syriac_glyph.multi_subst_dup,
             is_vert_alt: syriac_glyph.is_vert_alt,
+            ligature: syriac_glyph.ligature,
             fake_bold: syriac_glyph.fake_bold,
             variation: syriac_glyph.variation,
             fake_italic: syriac_glyph.fake_italic,

diff --git a/tests/aots.rs b/tests/aots.rs
@@ -333,6 +333,7 @@ fn make_direct_glyph(glyph_index: u16) -> RawGlyph<()> {
         small_caps: false,
         multi_subst_dup: false,
         is_vert_alt: false,
+        ligature: false,
         fake_bold: false,
         fake_italic: false,
         extra_data: (),

diff --git a/tests/shape.rs b/tests/shape.rs
@@ -36,6 +36,7 @@ pub fn make_glyph(ch: char, glyph_index: u16) -> RawGlyph<()> {
         small_caps: false,
         multi_subst_dup: false,
         is_vert_alt: false,
+        ligature: false,
         fake_bold: false,
         fake_italic: false,
         extra_data: (),