Merge #81: Text: kerning survives a colour change (#29)

crates/compose/src/shape.rs

@@ -92,6 +92,59 @@ pub struct ShapedPara {
     pub range: std::ops::Range<usize>,
 }
 
+/// A resolved piece of a paragraph: one character format (and overlay) over `range`.
+struct Piece {
+    range: std::ops::Range<usize>,
+    props: CharProps,
+    style: u32,
+    /// Shaping settings for the style (a missing font turns glyph fallback on).
+    env: TypeEnv,
+    /// Hidden conditional text or tracked deletions: laid out as zero-width place-holders.
+    hidden: bool,
+}
+
+/// The style of each byte of a shaped run: `(first byte, style)` in story order.
+struct StyleRuns<'a>(&'a [(usize, u32)]);
+
+impl StyleRuns<'_> {
+    fn at(&self, byte: usize) -> u32 {
+        let k = self.0.partition_point(|&(start, _)| start <= byte);
+        self.0.get(k.saturating_sub(1)).map_or(0, |&(_, s)| s)
+    }
+}
+
+/// The attributes that decide how text is shaped: `p` with everything that only paints the
+/// glyphs (colour, stroke, underline and strikethrough, skew, markers) reset, so two pieces with
+/// equal keys can be shaped as one. Attributes not listed here keep pieces apart.
+fn shaping_key(p: &CharProps) -> CharProps {
+    let d = CharProps::default();
+    CharProps {
+        skew: d.skew,
+        fill: d.fill,
+        fill_tint: d.fill_tint,
+        stroke: d.stroke,
+        stroke_tint: d.stroke_tint,
+        stroke_weight: d.stroke_weight,
+        underline: d.underline,
+        strikethrough: d.strikethrough,
+        underline_weight: d.underline_weight,
+        underline_offset: d.underline_offset,
+        underline_color: d.underline_color,
+        underline_tint: d.underline_tint,
+        strikethrough_weight: d.strikethrough_weight,
+        strikethrough_offset: d.strikethrough_offset,
+        strikethrough_color: d.strikethrough_color,
+        strikethrough_tint: d.strikethrough_tint,
+        ruby: d.ruby,
+        kenten: d.kenten,
+        kenten_character: d.kenten_character,
+        conditions: d.conditions,
+        change: d.change,
+        xml_tag: d.xml_tag,
+        ..p.clone()
+    }
+}
+
 /// Context for substitutions (page number markers).
 #[derive(Clone, Debug, Default)]
 pub struct SubstCtx {
@@ -236,6 +289,8 @@ pub(crate) fn shape_para(
             segments.push((w[0], w[1], top, fmt));
         }
     }
+    // Resolve every segment, then shape runs of segments that differ only in how they are
+    // painted as one piece, so kerning, ligatures and joining work across a colour change.
     let mut resolved = Vec::new();
     for (a, b, overlay, fmt) in segments {
         // A nested / GREP character style sits under the run's own style and overrides.
@@ -265,29 +320,19 @@ pub(crate) fn shape_para(
             resolved.push((a, b, props, fmt, para_chars.clone()));
         }
     }
+    let mut pieces: Vec<Piece> = Vec::with_capacity(resolved.len());
     for (a, b, props, fmt, resolved_base) in resolved {
         let style = table.intern(db, &props);
         // A missing font's substitute stands in for the whole font: fallback fonts help it.
         let auto_leading = table.env_for(auto_leading, style);
         let deleted = props.change == designcraft_doc::ChangeMark::Deleted;
         if deleted || (!props.conditions.is_empty() && props.conditions.iter().all(|c| sub.hidden_conditions.contains(c))) {
-            // Hidden conditional text: zero-width, unbreakable, undrawn place-holders keep every
-            // byte addressable (caret, selection) without taking space.
-            let face = db.face(&props.font_family, &props.font_style);
-            for (i, c) in story.text[a..b].char_indices() {
-                if c == '\n' {
-                    continue;
-                }
-                let mut g = control_glyph(&face, &props, auto_leading, style, a + i, c);
-                g.ch = HIDDEN;
-                (g.ascent, g.typo_ascent, g.descent, g.leading, g.cap, g.xh) = (0.0, 0.0, 0.0, 0.0, 0.0, 0.0);
-                glyphs.push(g);
-            }
+            pieces.push(Piece { range: a..b, props, style, env: auto_leading, hidden: true });
             continue;
         }
         let is_ref = |c: char| c == designcraft_doc::FOOTNOTE_REF || c == designcraft_doc::ENDNOTE_REF;
         if !story.text[a..b].contains(is_ref) {
-            shape_run(db, &story.text, a..b, &props, auto_leading, style, sub, &mut glyphs);
+            pieces.push(Piece { range: a..b, props, style, env: auto_leading, hidden: false });
             continue;
         }
         // Footnote references take the reference position / character style; endnote references
@@ -309,20 +354,67 @@ pub(crate) fn shape_para(
         for (i, m) in story.text[a..b].match_indices(is_ref) {
             let i = a + i;
             if k < i {
-                shape_run(db, &story.text, k..i, &props, auto_leading, style, sub, &mut glyphs);
+                pieces.push(Piece { range: k..i, props: props.clone(), style, env: auto_leading, hidden: false });
             }
             let e = i + m.len();
             if m.starts_with(designcraft_doc::ENDNOTE_REF) {
-                shape_run(db, &story.text, i..e, &eprops, eenv, estyle, sub, &mut glyphs);
+                pieces.push(Piece { range: i..e, props: eprops.clone(), style: estyle, env: eenv, hidden: false });
             } else {
-                shape_run(db, &story.text, i..e, &rprops, renv, rstyle, sub, &mut glyphs);
+                pieces.push(Piece { range: i..e, props: rprops.clone(), style: rstyle, env: renv, hidden: false });
             }
             k = e;
         }
         if k < b {
-            shape_run(db, &story.text, k..b, &props, auto_leading, style, sub, &mut glyphs);
+            pieces.push(Piece { range: k..b, props, style, env: auto_leading, hidden: false });
         }
     }
+    let mut i = 0;
+    while i < pieces.len() {
+        let first = &pieces[i];
+        if first.hidden {
+            // Hidden conditional text: zero-width, unbreakable, undrawn place-holders keep every
+            // byte addressable (caret, selection) without taking space.
+            let face = db.face(&first.props.font_family, &first.props.font_style);
+            for (k, c) in story.text[first.range.clone()].char_indices() {
+                if c == '\n' {
+                    continue;
+                }
+                let mut g = control_glyph(&face, &first.props, first.env, first.style, first.range.start + k, c);
+                g.ch = HIDDEN;
+                (g.ascent, g.typo_ascent, g.descent, g.leading, g.cap, g.xh) = (0.0, 0.0, 0.0, 0.0, 0.0, 0.0);
+                glyphs.push(g);
+            }
+            i += 1;
+            continue;
+        }
+        let key = shaping_key(&first.props);
+        let mut j = i + 1;
+        // Jidori fits its own run into whole ems: such pieces stay apart.
+        while j < pieces.len()
+            && first.props.jidori == 0
+            && !pieces[j].hidden
+            && pieces[j].range.start == pieces[j - 1].range.end
+            && pieces[j].env.glyph_fallback == first.env.glyph_fallback
+            && shaping_key(&pieces[j].props) == key
+        {
+            j += 1;
+        }
+        let group = &pieces[i..j];
+        let range = group[0].range.start..group[j - i - 1].range.end;
+        let runs: Vec<(usize, u32)> = group.iter().map(|p| (p.range.start, p.style)).collect();
+        let start = glyphs.len();
+        shape_run(db, &story.text, range, &first.props, first.env, &StyleRuns(&runs), sub, &mut glyphs);
+        // A cluster (a ligature, a letter with its marks) that spans two styles would paint one
+        // of them in the other's colour: shape those pieces apart instead.
+        let spans_cut = |g: &Glyph| runs[1..].iter().any(|&(cut, _)| g.byte < cut && cut < g.byte + g.len);
+        if group.len() > 1 && glyphs[start..].iter().any(spans_cut) {
+            glyphs.truncate(start);
+            for p in group {
+                shape_run(db, &story.text, p.range.clone(), &p.props, p.env, &StyleRuns(&[(p.range.start, p.style)]), sub, &mut glyphs);
+            }
+        }
+        i = j;
+    }
     collapse_tcy(&mut glyphs, sub.vertical);
     ShapedPara { glyphs, range }
 }
@@ -443,12 +535,12 @@ fn shape_run(
     range: std::ops::Range<usize>,
     p: &CharProps,
     env: TypeEnv,
-    style: u32,
+    styles: &StyleRuns<'_>,
     sub: &SubstCtx,
     out: &mut Vec<Glyph>,
 ) {
     let start = out.len();
-    shape_run_raw(db, text, range, p, env, style, sub, out);
+    shape_run_raw(db, text, range, p, env, styles, sub, out);
     let run = &mut out[start..];
     let em = p.size * p.h_scale;
     for g in run.iter_mut().filter(|g| g.adv > 0.0 && !g.ch.is_control()) {
@@ -498,7 +590,7 @@ fn shape_run_raw(
     range: std::ops::Range<usize>,
     p: &CharProps,
     auto_leading: TypeEnv,
-    style: u32,
+    styles: &StyleRuns<'_>,
     sub: &SubstCtx,
     out: &mut Vec<Glyph>,
 ) {
@@ -520,7 +612,7 @@ fn shape_run_raw(
     let mut seg_face = primary.clone();
     let flush = |a: usize, b: usize, face: &Arc<FontFace>, out: &mut Vec<Glyph>| {
         if a < b {
-            shape_segment(db, text, a..b, None, p, face, auto_leading, style, out, sub.vertical);
+            shape_segment(db, text, a..b, None, p, face, auto_leading, styles, out, sub.vertical);
         }
     };
     for (i, c) in text[range.clone()].char_indices() {
@@ -545,7 +637,7 @@ fn shape_run_raw(
                 flush(seg_start, i, &seg_face, out);
                 seg_start = i + c.len_utf8();
                 let face = if primary.covers(d) { primary.clone() } else { fallback(d) };
-                shape_segment(db, text, i..i + c.len_utf8(), Some(d.encode_utf8(&mut [0; 4])), p, &face, auto_leading, style, out, sub.vertical);
+                shape_segment(db, text, i..i + c.len_utf8(), Some(d.encode_utf8(&mut [0; 4])), p, &face, auto_leading, styles, out, sub.vertical);
                 continue;
             }
         }
@@ -575,9 +667,9 @@ fn shape_run_raw(
             seg_start = i + c.len_utf8();
             if let Some(vi) = designcraft_doc::vars::var_index(c) {
                 match sub.vars.get(vi).filter(|v| !v.is_empty()) {
-                    Some(v) => shape_segment(db, text, i..i + c.len_utf8(), Some(v), p, &primary, auto_leading, style, out, sub.vertical),
+                    Some(v) => shape_segment(db, text, i..i + c.len_utf8(), Some(v), p, &primary, auto_leading, styles, out, sub.vertical),
                     None => {
-                        let mut g = control_glyph(&primary, p, auto_leading, style, i, c);
+                        let mut g = control_glyph(&primary, p, auto_leading, styles.at(i), i, c);
                         g.adv = 0.0;
                         out.push(g);
                     }
@@ -591,10 +683,10 @@ fn shape_run_raw(
                     } else {
                         sub.page_name.clone().unwrap_or_else(|| "#".into())
                     };
-                    shape_segment(db, text, i..i + c.len_utf8(), Some(&s), p, &primary, auto_leading, style, out, sub.vertical);
+                    shape_segment(db, text, i..i + c.len_utf8(), Some(&s), p, &primary, auto_leading, styles, out, sub.vertical);
                 }
                 designcraft_doc::OBJECT_MARK => {
-                    let mut g = control_glyph(&primary, p, auto_leading, style, i, c);
+                    let mut g = control_glyph(&primary, p, auto_leading, styles.at(i), i, c);
                     if let Some(o) = sub.objects.get(&i).filter(|o| !o.custom) {
                         let auto = matches!(p.leading, Leading::Auto);
                         match o.y_offset {
@@ -622,20 +714,20 @@ fn shape_run_raw(
                     out.push(g);
                 }
                 designcraft_doc::XREF_MARK => match sub.xrefs.get(&i).filter(|t| !t.is_empty()) {
-                    Some(t) => shape_segment(db, text, i..i + c.len_utf8(), Some(t), p, &primary, auto_leading, style, out, sub.vertical),
+                    Some(t) => shape_segment(db, text, i..i + c.len_utf8(), Some(t), p, &primary, auto_leading, styles, out, sub.vertical),
                     None => {
-                        let mut g = control_glyph(&primary, p, auto_leading, style, i, c);
+                        let mut g = control_glyph(&primary, p, auto_leading, styles.at(i), i, c);
                         g.adv = 0.0;
                         out.push(g);
                     }
                 },
                 designcraft_doc::FOOTNOTE_REF | designcraft_doc::ENDNOTE_REF => {
                     let s = sub.notes.get(&i).map_or("#", String::as_str);
-                    shape_segment(db, text, i..i + c.len_utf8(), Some(s), p, &primary, auto_leading, style, out, sub.vertical);
+                    shape_segment(db, text, i..i + c.len_utf8(), Some(s), p, &primary, auto_leading, styles, out, sub.vertical);
                 }
                 _ => {
                     // Zero-width control glyph carrying metrics (tabs get their width at line layout).
-                    let mut g = control_glyph(&primary, p, auto_leading, style, i, c);
+                    let mut g = control_glyph(&primary, p, auto_leading, styles.at(i), i, c);
                     if c == '\t'
                         || c == story::RIGHT_INDENT_TAB
                         || c == story::TABLE_ANCHOR
@@ -823,7 +915,7 @@ fn shape_segment(
     p: &CharProps,
     face: &Arc<FontFace>,
     auto_leading: TypeEnv,
-    style: u32,
+    styles: &StyleRuns<'_>,
     out: &mut Vec<Glyph>,
     vertical: bool,
 ) {
@@ -910,7 +1002,7 @@ fn shape_segment(
             cap,
             xh,
             size,
-            style,
+            style: styles.at(byte),
             no_break: p.no_break,
             locked_advance: p.jidori > 0,
             tcy: p.tate_chu_yoko.then_some([0.0; 3]),

crates/compose/src/tests.rs

@@ -1960,6 +1960,66 @@ fn caret_follows_spaces_typed_at_the_end() {
     }
 }
 
+#[test]
+fn colour_change_keeps_kerning_and_ligatures_keep_colour() {
+    let mut d = Document::new(&NewDocument::default());
+    let lid = d.default_layer();
+    let (_, sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(100.0, 100.0, 400.0, 200.0), lid, "ayay", ParaFormat::default()).unwrap();
+    d.story_mut(sid).unwrap().format_chars(0..4, |f| f.over.font_style = Some("Bold".into()));
+    let line = |d: &Document| compose_story(d, sid, &ComposeOptions::default()).frames[0].lines[0].clone();
+    let xs = |l: &Line| l.glyphs.iter().map(|g| (g.x, g.adv)).collect::<Vec<_>>();
+    let kerned = line(&d);
+    // The font kerns "ay" (otherwise this test proves nothing).
+    d.story_mut(sid).unwrap().format_chars(0..4, |f| f.over.kerning = Some(designcraft_doc::Kerning::None));
+    assert_ne!(xs(&line(&d)), xs(&kerned), "Source Serif 4 Bold kerns a–y");
+    d.story_mut(sid).unwrap().format_chars(0..4, |f| f.over.kerning = Some(designcraft_doc::Kerning::Metrics));
+    // Colour the first "a": positions are unchanged, only its style differs.
+    d.story_mut(sid).unwrap().format_chars(0..1, |f| f.over.fill = Some("Cyan".into()));
+    let coloured = line(&d);
+    assert_eq!(xs(&coloured), xs(&kerned), "a colour change must not change spacing");
+    let cs = compose_story(&d, sid, &ComposeOptions::default());
+    let fill = |l: &Line, i: usize| cs.styles[l.glyphs[i].style as usize].fill.clone();
+    assert_eq!(fill(&coloured, 0), "Cyan");
+    assert_ne!(fill(&coloured, 1), "Cyan");
+
+    // A ligature never swallows a differently coloured letter: the "i" of "fi" keeps its colour.
+    let (_, sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(100.0, 300.0, 400.0, 400.0), lid, "fifi", ParaFormat::default()).unwrap();
+    d.story_mut(sid).unwrap().format_chars(1..2, |f| f.over.fill = Some("Cyan".into()));
+    let cs = compose_story(&d, sid, &ComposeOptions::default());
+    let l = &cs.frames[0].lines[0];
+    let at = |b: usize| l.glyphs.iter().find(|g| g.len > 0 && g.byte <= b && b < g.byte + g.len).map(|g| cs.styles[g.style as usize].fill.clone());
+    assert_eq!(at(1).as_deref(), Some("Cyan"));
+    assert_ne!(at(2).as_deref(), Some("Cyan"));
+}
+
+/// The emphasis mark drawn over the text only paints: a change of kenten character keeps the
+/// kerning across it.
+#[test]
+fn kenten_character_change_keeps_kerning() {
+    let mut d = Document::new(&NewDocument::default());
+    let lid = d.default_layer();
+    let (_, sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(100.0, 100.0, 400.0, 200.0), lid, "ayay", ParaFormat::default()).unwrap();
+    d.story_mut(sid).unwrap().format_chars(0..4, |f| {
+        f.over.font_style = Some("Bold".into());
+        f.over.kenten = Some(true);
+    });
+    let xs = |d: &Document| {
+        let cs = compose_story(d, sid, &ComposeOptions::default());
+        cs.frames[0].lines[0].glyphs.iter().filter(|g| g.len > 0).map(|g| (g.byte, g.x, g.adv)).collect::<Vec<_>>()
+    };
+    let kerned = xs(&d);
+    d.story_mut(sid).unwrap().format_chars(0..4, |f| f.over.kerning = Some(designcraft_doc::Kerning::None));
+    assert_ne!(xs(&d), kerned, "Source Serif 4 Bold kerns a–y");
+    d.story_mut(sid).unwrap().format_chars(0..4, |f| f.over.kerning = Some(designcraft_doc::Kerning::Metrics));
+    d.story_mut(sid).unwrap().format_chars(0..1, |f| f.over.kenten_character = Some("●".into()));
+    assert_eq!(xs(&d), kerned, "a kenten character change must not change spacing");
+    let cs = compose_story(&d, sid, &ComposeOptions::default());
+    let mark = |b: usize| {
+        cs.frames[0].lines[0].glyphs.iter().find(|g| g.len > 0 && g.byte == b).map(|g| cs.styles[g.style as usize].kenten_character.clone())
+    };
+    assert_eq!((mark(0).as_deref(), mark(1).as_deref()), (Some("●"), Some("")));
+}
+
 #[test]
 fn cjk_aki_adds_space_without_scaling_outlines() {
     let (mut d, sid, _) = doc_with("AB", Rect::new(0.0, 0.0, 300.0, 100.0), ParaAttrs::default());
@@ -2429,6 +2489,32 @@ fn missing_glyphs_are_the_fonts_box_unless_fallback_is_on() {
     assert_ne!(gid, 0, "a fallback font draws it");
 }
 
+/// A missing font's substitute draws what it lacks from fallback fonts whatever the setting, also
+/// in text shaped as one piece across a colour change.
+#[test]
+fn a_missing_fonts_substitute_takes_fallback_fonts() {
+    use designcraft_fonts::testing::font_with;
+    designcraft_fonts::FontDb::global().add_font(font_with("DC Test Missing Glyph Helper", &['語']).unwrap());
+    let text = "a語b語";
+    let (mut d, sid, _) = doc_with(text, Rect::new(36.0, 36.0, 300.0, 100.0), ParaAttrs::default());
+    {
+        let s = d.story_mut(sid).unwrap();
+        s.format_chars(0..text.len(), |f| {
+            f.over.language = Some("Japanese".into());
+            f.over.font_family = Some("DC Test No Such Family".into());
+        });
+        s.format_chars("a語".len()..text.len(), |f| f.over.fill = Some("Red".into()));
+    }
+    assert!(!d.settings.glyph_fallback);
+    let cs = compose_story(&d, sid, &ComposeOptions::default());
+    let glyphs: Vec<&PlacedGlyph> = all_lines(&cs).into_iter().flat_map(|l| l.glyphs.iter()).filter(|g| text[g.byte..].starts_with('語')).collect();
+    assert_eq!(glyphs.len(), 2);
+    for g in glyphs {
+        assert_ne!(g.face.family, designcraft_fonts::DEFAULT_FAMILY, "byte {}", g.byte);
+        assert_ne!(g.gid, 0, "byte {}: a fallback font draws it", g.byte);
+    }
+}
+
 #[test]
 fn kenten_missing_from_the_font_are_its_box_unless_fallback_is_on() {
     use designcraft_fonts::testing::font_with;