Merge pull request #5185 from dotCooCoo/fix/rtl-manual-kerning-moves-neighbor

Type: manual kerning in right-to-left text spaces the pair it is set on

crates/text/src/justify.rs

@@ -91,6 +91,7 @@ pub(crate) fn apply_desired(sg: &mut [SGlyph], j: &JustificationOptions, scale_g
             g.adv *= g_des;
             g.sx *= g_des;
             g.dx *= g_des;
+            g.kern_shift *= g_des;
         }
         f.width = g.adv;
         f.scale = g_des;

crates/text/src/layout.rs

@@ -181,6 +181,7 @@ impl Ctx<'_> {
         tab.gid = 0;
         tab.dx = 0.0;
         tab.dy = 0.0;
+        tab.kern_shift = 0.0;
         tab.tcy = None;
         tab.vpal = None;
         let pen = para.left_indent + para.first_line_indent + marker.iter().map(|g| g.adv).sum::<f64>();
@@ -229,8 +230,9 @@ impl Ctx<'_> {
             Some(ib) => Affine::translate((-lead, -g.bshift)) * ib.xf,
             // Baseline shift moves the glyph across its line, and Character Rotation then turns it
             // about its shifted pen position, so the shift is across the line at any rotation.
+            // Right-to-left manual kerning moves that pen position along the line.
             None => {
-                Affine::translate((0.0, -g.bshift))
+                Affine::translate((g.kern_shift, -g.bshift))
                     * Affine::rotate(-g.rotation.to_radians())
                     * Affine::translate((g.dx - lead, g.dy))
                     * Affine::scale_non_uniform(g.sx, g.sy)
@@ -238,7 +240,7 @@ impl Ctx<'_> {
         };
         // The turning point in font units, which later moves of the glyph's transform keep.
         let pivot = match local.determinant() {
-            det if det != 0.0 && det.is_finite() => local.inverse() * Point::new(0.0, -g.bshift),
+            det if det != 0.0 && det.is_finite() => local.inverse() * Point::new(g.kern_shift, -g.bshift),
             _ => Point::ZERO,
         };
         let unturned = if g.inline.is_none() && g.rotation != 0.0 && g.sx != 0.0 && g.sy != 0.0 {
@@ -332,6 +334,7 @@ impl Ctx<'_> {
         k.adv = t;
         k.dx = 0.0;
         k.dy = 0.0;
+        k.kern_shift = 0.0;
         k.len = 0;
         k.lead = 0.0;
         k.ch = kashida::TATWEEL;

crates/text/src/layout/wrapping_tests.rs

@@ -18,6 +18,7 @@ fn synthetic_glyphs(specs: &[(usize, usize, f64, char)]) -> Vec<SGlyph> {
             adv,
             dx: 0.0,
             dy: 0.0,
+            kern_shift: 0.0,
             sx: 1.0,
             sy: 1.0,
             bshift: 0.0,

crates/text/src/shape.rs

@@ -33,6 +33,9 @@ pub(crate) struct SGlyph {
     /// Offset from the pen position, y down.
     pub dx: f64,
     pub dy: f64,
+    /// Right to left: the run's manual kerning, which moves the glyph right along the baseline,
+    /// outside its Character Rotation. 0 otherwise.
+    pub kern_shift: f64,
     /// Outline scale (font units -> points).
     pub sx: f64,
     pub sy: f64,
@@ -169,6 +172,7 @@ pub(crate) fn hyphen_glyph(g: &SGlyph) -> SGlyph {
     h.adv = g.face.advance(gid) * g.sx;
     h.dx = 0.0;
     h.dy = 0.0;
+    h.kern_shift = 0.0;
     h.byte = g.byte + g.len;
     h.len = 0;
     h.ch = '-';
@@ -616,13 +620,17 @@ fn optical_pairs(db: &FontDb, text: &str, glyphs: &mut [SGlyph], runs: &[(Range<
         {
             continue;
         }
+        // Neither glyph is rotated, so a right-to-left manual kerning shift adds to the offset.
+        let left_x = left.dx + left.kern_shift;
+        let right_x = right.dx + right.kern_shift;
         let left_y = left.dy - left.bshift;
         let right_y = right.dy - right.bshift;
         let Some(left_style) = runs.get(left.run).map(|(_, s)| *s) else { continue };
         let em = left.sx * left.face.upem / (left_style.h_scale / 100.0);
         let tracking = left_style.tracking / 1000.0 * em;
-        let manual = left_style.kerning.unwrap_or(0.0) / 1000.0 * em;
-        let advance = glyphs[cluster_start..=cluster_end].iter().map(|g| g.adv).sum::<f64>() - tracking - manual;
+        // A manual kerning value on the left glyph is part of its advance and, right to left, of
+        // its offset too (it spaces the left glyph from its left neighbor): it is not taken off.
+        let advance = glyphs[cluster_start..=cluster_end].iter().map(|g| g.adv).sum::<f64>() - tracking;
         let key = [
             u64::from(left.face.id()),
             u64::from(left.gid),
@@ -630,11 +638,11 @@ fn optical_pairs(db: &FontDb, text: &str, glyphs: &mut [SGlyph], runs: &[(Range<
             u64::from(right.gid),
             left.sx.to_bits(),
             left.sy.to_bits(),
-            left.dx.to_bits(),
+            left_x.to_bits(),
             left_y.to_bits(),
             right.sx.to_bits(),
             right.sy.to_bits(),
-            right.dx.to_bits(),
+            right_x.to_bits(),
             right_y.to_bits(),
             advance.to_bits(),
             em.to_bits(),
@@ -647,8 +655,8 @@ fn optical_pairs(db: &FontDb, text: &str, glyphs: &mut [SGlyph], runs: &[(Range<
             profile_correction(
                 &a.0,
                 &b.0,
-                (left.sx, left.sy, left.dx, left_y),
-                (right.sx, right.sy, right.dx, right_y),
+                (left.sx, left.sy, left_x, left_y),
+                (right.sx, right.sy, right_x, right_y),
                 advance,
                 em,
                 left.face.upem,
@@ -811,6 +819,7 @@ fn inline_glyph(face: &Arc<FontFace>, st: &CharStyle, art: &InlineArt, run: usiz
         adv: if adv.is_finite() { adv } else { 0.0 },
         dx: 0.0,
         dy: 0.0,
+        kern_shift: 0.0,
         // Not used to draw (the art has its own transform), but they make the glyph's em box
         // its run's em: Character Alignment and Top-to-Top leading treat an inline graphic as a
         // character of its run's size, whatever its scale (see `layout::glyph_em`).
@@ -1110,12 +1119,21 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
         let last_in_cluster = gi + 1 == n || raw[gi + 1].1 as usize != cl;
         let ch = first_char(cl);
         let mut adv = xa as f64 * k * hs;
+        let mut kern_shift = 0.0;
         if ch == SOFT_HYPHEN || ch == '\u{2028}' {
             adv = 0.0;
-        } else if last_in_cluster {
-            let tracking = gst.tracking / 1000.0 * size;
+        } else {
             let manual_kern = gst.kerning.map(|v| v / 1000.0 * size).unwrap_or(0.0);
-            adv += tracking + manual_kern;
+            // Manual kerning spaces a character from the next one in reading order. Right to left,
+            // that one is drawn to its left: the cluster's glyphs move right along the baseline by
+            // the value, so that the space its last glyph's advance gains is at the cluster's left.
+            if level.is_rtl() && !feats.vertical {
+                kern_shift = manual_kern;
+            }
+            if last_in_cluster {
+                let tracking = gst.tracking / 1000.0 * size;
+                adv += tracking + manual_kern;
+            }
         }
         out.push(SGlyph {
             face: face.clone(),
@@ -1126,6 +1144,7 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
             adv,
             dx: xo as f64 * k * hs,
             dy: -(yo as f64) * k * vs,
+            kern_shift,
             sx: k * hs,
             sy: k * vs,
             bshift: gst.baseline_shift + pos_shift,

crates/text/src/tests/typo.rs

@@ -66,6 +66,64 @@ fn optical_pairs_cross_paint_runs_and_manual_and_tracking_are_independent() {
     assert!((pair_advance(&rtl_manual) - pair_advance(&rtl_pair_zero) - 20.0).abs() < 0.01, "manual spacing changes the adjacent right-to-left pair");
 }
 
+#[test]
+fn rtl_manual_kerning_spaces_the_character_from_the_next_in_reading_order() {
+    for optical_kerning in [false, true] {
+        // Reading order: alef (bytes 0..2), bet (2..4), gimel (4..6); bet carries the kerning.
+        let base = CharStyle { size: 100.0, optical_kerning, ..CharStyle::default() };
+        let kerned = |value: f64| {
+            let mut t = TextObject::point(Point::ZERO, "אבג", base.clone());
+            edit::style_range(&mut t.runs, 2, 4, |s| s.kerning = Some(value));
+            layout(db(), &t)
+        };
+        let ink = |lay: &TextLayout, byte: usize| {
+            let g = lay.glyphs.iter().find(|g| g.byte == byte).unwrap();
+            assert!(g.rtl && g.gid != 0, "byte {byte} is set right to left with a glyph of its own");
+            (g.xf * Point::ZERO).x
+        };
+        let (plain, spaced) = (kerned(0.0), kerned(200.0));
+        let gap = |lay: &TextLayout, right: usize, left: usize| ink(lay, right) - ink(lay, left);
+        assert!(
+            (gap(&spaced, 2, 4) - gap(&plain, 2, 4) - 20.0).abs() < 0.01,
+            "kerning on bet spaces it from gimel, after it in reading order (Optical: {optical_kerning})"
+        );
+        assert!(
+            (gap(&spaced, 0, 2) - gap(&plain, 0, 2)).abs() < 0.01,
+            "alef, before bet in reading order, keeps its place (Optical: {optical_kerning})"
+        );
+        // The caret between bet and gimel stays at gimel: the new space is on bet's side of it.
+        let caret = |lay: &TextLayout| caret_position(lay, 4).0.x;
+        assert!((caret(&spaced) - ink(&spaced, 4) - (caret(&plain) - ink(&plain, 4))).abs() < 0.01);
+        assert!((ink(&spaced, 2) - caret(&spaced) - (ink(&plain, 2) - caret(&plain)) - 20.0).abs() < 0.01);
+    }
+}
+
+#[test]
+fn rtl_manual_kerning_on_a_rotated_character_moves_it_along_the_baseline() {
+    // Reading order: alef (bytes 0..2), bet (2..4), gimel (4..6); bet is turned 90 degrees and
+    // carries the kerning.
+    let kerned = |value: f64| {
+        let mut t = TextObject::point(Point::ZERO, "אבג", CharStyle { size: 100.0, ..CharStyle::default() });
+        edit::style_range(&mut t.runs, 2, 4, |s| {
+            s.kerning = Some(value);
+            s.rotation = 90.0;
+        });
+        layout(db(), &t)
+    };
+    let origin = |lay: &TextLayout, byte: usize| {
+        let g = lay.glyphs.iter().find(|g| g.byte == byte).unwrap();
+        assert!(g.rtl && g.gid != 0, "byte {byte} is set right to left with a glyph of its own");
+        g.xf * Point::ZERO
+    };
+    let (plain, spaced) = (kerned(0.0), kerned(200.0));
+    let moved = origin(&spaced, 2) - origin(&plain, 2);
+    let gimel = origin(&spaced, 4) - origin(&plain, 4);
+    assert!((moved.x - gimel.x - 20.0).abs() < 0.01, "kerning on the turned bet spaces it from gimel along the line: {moved:?}, {gimel:?}");
+    assert!(moved.y.abs() < 0.01, "the turned bet stays on its baseline: {moved:?}");
+    let alef = origin(&spaced, 0) - origin(&plain, 0);
+    assert!((alef.x - moved.x).abs() < 0.01 && alef.y.abs() < 0.01, "alef keeps its place next to bet: {alef:?}");
+}
+
 #[test]
 fn optical_pair_skips_an_invisible_soft_hyphen_with_an_outline() {
     use crate::test_fonts::{SOFT_HYPHEN_OUTLINE_FAMILY, soft_hyphen_outline_font};