Merge pull request #4549 from storytold/fix/vietnamese-stacked-marks

Text: keep a letter and its marks in one font when falling back (#4491)

crates/text/src/shape.rs

@@ -5,12 +5,14 @@ use std::sync::Arc;
 
 use unicode_bidi::Level;
 use unicode_script::{Script, UnicodeScript};
+use unicode_segmentation::UnicodeSegmentation;
 
 use harfrust::{Direction, Feature, ShapeOptions, Tag, UnicodeBuffer};
 use skrifa::MetadataProvider;
+use skrifa::charmap::Charmap;
 use skrifa::instance::Size;
 use vectorcraft_doc::text::INLINE_CHAR;
-use vectorcraft_doc::{CharStyle, InlineArt};
+use vectorcraft_doc::{CharStyle, Digits, InlineArt};
 
 use crate::features::{OtFeatures, ligatures_suppressed_by};
 use crate::fontdb::{FontDb, FontFace};
@@ -393,24 +395,23 @@ pub(crate) fn shape_range(
         let small_caps = st.small_caps.is_some() && !st.all_caps;
         // Split into segments by font coverage (and case, for Small Caps).
         let mut seg = Segment { range: a..a, runs, source_runs, st, face: primary.clone(), small: false, level: level_at(a) };
-        let mut cache: std::collections::HashMap<char, Arc<FontFace>> = std::collections::HashMap::new();
+        let mut cache: std::collections::HashMap<&str, Arc<FontFace>> = std::collections::HashMap::new();
         let mut script = Script::Common;
-        for (i, c) in text[a..b].char_indices() {
+        // A grapheme cluster (a letter and its marks, an emoji sequence) is set in one face.
+        for (i, cluster) in text[a..b].grapheme_indices(true) {
             let i = a + i;
             // Digits are drawn in the style's numeral system: the font must cover that digit.
-            let c = st.digits.map(c);
+            let Some(c) = cluster.chars().next().map(|c| st.digits.map(c)) else { continue };
             let level = level_at(i);
             // Whitespace the primary font doesn't map goes to the fallback face like any other
             // character (#1844): an ideographic space set at zero width would remove an indent.
-            // What the layout handles itself (tab, line and paragraph breaks) stays.
-            let covered = c.is_control() || pmap.as_ref().is_none_or(|m| m.map(c).is_some());
-            let face = if covered {
+            let face = if drawn_chars(cluster, st.digits).all(|c| in_face(pmap.as_ref(), c)) {
                 primary.clone()
-            } else if let Some(f) = cache.get(&c) {
+            } else if let Some(f) = cache.get(cluster) {
                 f.clone()
             } else {
-                let f = db.fallback_for_style(c, primary.id(), primary.weight, primary.italic).unwrap_or_else(|| primary.clone());
-                cache.insert(c, f.clone());
+                let f = fallback_face(db, &primary, pmap.as_ref(), cluster, st.digits);
+                cache.insert(cluster, f.clone());
                 f
             };
             let small = small_caps && c.is_lowercase();
@@ -516,6 +517,32 @@ fn shapes_alike(a: &CharStyle, b: &CharStyle) -> bool {
         && ligatures_suppressed_by(*tracking) == ligatures_suppressed_by(b.tracking)
 }
 
+/// The characters of a grapheme cluster that need a glyph of their own, in the numeral system
+/// `digits`: not what the layout handles itself (tab, line and paragraph breaks), joiners or
+/// variation selectors.
+fn drawn_chars(cluster: &str, digits: Digits) -> impl Iterator<Item = char> + '_ {
+    cluster
+        .chars()
+        .map(move |c| digits.map(c))
+        .filter(|&c| !(c.is_control() || matches!(c, '\u{200C}' | '\u{200D}' | '\u{FE00}'..='\u{FE0F}' | '\u{E0100}'..='\u{E01EF}')))
+}
+
+/// Does the face with charmap `map` have a glyph for `c` (any face does when it has no charmap)?
+fn in_face(map: Option<&Charmap<'_>>, c: char) -> bool {
+    map.is_none_or(|m| m.map(c).is_some())
+}
+
+/// The face of a grapheme cluster the primary face (charmap `pmap`) lacks a character of: the
+/// first fallback face with all of them, so that a letter's marks are shaped with it and attach to
+/// it (#4491); when there is none, the face its first character takes on its own.
+fn fallback_face(db: &FontDb, primary: &Arc<FontFace>, pmap: Option<&Charmap<'_>>, cluster: &str, digits: Digits) -> Arc<FontFace> {
+    let fallback = |c| db.fallback_for_style(c, primary.id(), primary.weight, primary.italic);
+    drawn_chars(cluster, digits)
+        .find_map(|c| fallback(c).filter(|f| drawn_chars(cluster, digits).all(|c| f.covers(c))))
+        .or_else(|| drawn_chars(cluster, digits).next().filter(|&c| !in_face(pmap, c)).and_then(fallback))
+        .unwrap_or_else(|| primary.clone())
+}
+
 /// The script `c` is shaped in: Japanese and Chinese text mixes Han, Hiragana, Katakana and
 /// Bopomofo, shaped together (splitting them would cost a shaper call per change of script).
 fn shaping_script(c: char) -> Script {

crates/text/src/tests.rs

@@ -25,6 +25,7 @@ mod typo;
 mod variable;
 mod versions;
 mod vertical;
+mod vietnamese;
 
 /// The bundled fonts only: what these tests measure doesn't change with the fonts installed.
 fn db() -> &'static FontDb {

crates/text/src/tests/vietnamese.rs

@@ -0,0 +1,123 @@
+//! Vietnamese letters with two diacritics (Ể, Ứ, Ự, Ẩ…) draw both marks on their base letter
+//! (#4491), whichever form the text comes in: precomposed (NFC), decomposed (NFD), or as
+//! Vietnamese keyboards type it (a precomposed vowel and a combining tone mark), in a font with
+//! the precomposed letters, one that builds them from its marks (GPOS mark-to-base and
+//! mark-to-mark), and one that has to borrow the marks from a fallback font.
+
+use kurbo::{Point, Rect, Shape};
+use skrifa::MetadataProvider;
+use unicode_segmentation::UnicodeSegmentation;
+use vectorcraft_doc::{CharStyle, TextObject};
+use write_fonts::FontBuilder;
+use write_fonts::types::Tag;
+
+use crate::test_fonts::name_table;
+use crate::{FontDb, TextLayout, layout};
+
+/// Letters as (precomposed, decomposed, keyboard) text: the words "HỌC ĐỂ ỨNG DỤNG" and more
+/// letters with two marks, capital and small.
+const LETTERS: [[&str; 3]; 20] = [
+    ["H", "H", "H"],
+    ["\u{1ECC}", "O\u{323}", "O\u{323}"],
+    ["C", "C", "C"],
+    ["\u{110}", "\u{110}", "\u{110}"],
+    ["\u{1EC2}", "E\u{302}\u{309}", "\u{CA}\u{309}"],
+    ["\u{1EE8}", "U\u{31B}\u{301}", "\u{1AF}\u{301}"],
+    ["N", "N", "N"],
+    ["G", "G", "G"],
+    ["D", "D", "D"],
+    ["\u{1EE4}", "U\u{323}", "U\u{323}"],
+    ["\u{1EF0}", "U\u{31B}\u{323}", "\u{1AF}\u{323}"],
+    ["\u{1EA8}", "A\u{302}\u{309}", "\u{C2}\u{309}"],
+    ["\u{1EBE}", "E\u{302}\u{301}", "\u{CA}\u{301}"],
+    ["\u{1ED8}", "O\u{302}\u{323}", "\u{D4}\u{323}"],
+    ["\u{1EDC}", "O\u{31B}\u{300}", "\u{1A0}\u{300}"],
+    ["\u{1EC3}", "e\u{302}\u{309}", "\u{EA}\u{309}"],
+    ["\u{1EE9}", "u\u{31B}\u{301}", "\u{1B0}\u{301}"],
+    ["\u{1EF1}", "u\u{31B}\u{323}", "\u{1B0}\u{323}"],
+    ["\u{1EA9}", "a\u{302}\u{309}", "\u{E2}\u{309}"],
+    ["\u{1EAB}", "a\u{306}\u{303}", "\u{103}\u{303}"],
+];
+
+const SIZE: f64 = 100.0;
+const SOURCE_SANS: &[u8] = include_bytes!("../../../../assets/fonts/SourceSans3-Regular.ttf");
+
+fn text(form: usize) -> String {
+    LETTERS.iter().filter_map(|l| l.get(form).copied()).collect::<Vec<_>>().join(" ")
+}
+
+/// Each letter's ink, relative to the pen position of its first glyph.
+fn letter_inks(db: &FontDb, family: &str, text: &str) -> Vec<Rect> {
+    let st = CharStyle { font_family: family.into(), font_style: "Regular".into(), size: SIZE, ..CharStyle::default() };
+    let l: TextLayout = layout(db, &TextObject::point(Point::ZERO, text, st));
+    text.grapheme_indices(true)
+        .filter(|(_, g)| *g != " ")
+        .map(|(start, g)| {
+            let glyphs: Vec<_> = l.glyphs.iter().filter(|gl| (start..start + g.len()).contains(&gl.byte)).collect();
+            assert!(!glyphs.is_empty(), "{g:?} has glyphs");
+            let pen = glyphs[0].origin.x;
+            let ink = glyphs.iter().map(|gl| gl.outline.bounding_box()).reduce(|a, b| a.union(b)).unwrap();
+            assert!(ink.area() > 0.0, "{g:?} draws");
+            assert!(glyphs.iter().all(|gl| gl.gid != 0), "{g:?} has no missing glyph");
+            ink - kurbo::Vec2::new(pen, 0.0)
+        })
+        .collect()
+}
+
+/// Every letter of `form` (drawn in `family` of `db`) has the ink of the precomposed letter in
+/// Source Sans 3: its marks are on the letter, not beside it.
+fn assert_attached(db: &FontDb, family: &str, form: usize) {
+    let want = letter_inks(FontDb::global(), "Source Sans 3", &text(0));
+    let got = letter_inks(db, family, &text(form));
+    assert_eq!(want.len(), got.len());
+    for ((w, g), letter) in want.iter().zip(&got).zip(LETTERS) {
+        let off = [w.x0 - g.x0, w.x1 - g.x1, w.y0 - g.y0, w.y1 - g.y1].into_iter().fold(0.0_f64, |m, d| m.max(d.abs()));
+        assert!(off < 0.08 * SIZE, "{family}, form {form}: {:?} drawn {g:?}, precomposed {w:?}", letter[form]);
+    }
+}
+
+#[test]
+fn stacked_marks_sit_on_their_letter_in_every_form() {
+    for form in 0..3 {
+        assert_attached(FontDb::global(), "Source Sans 3", form);
+    }
+}
+
+/// Source Sans 3 renamed `family`, with only the characters `keep` takes in its cmap.
+fn source_sans_with(family: &str, keep: impl Fn(u32) -> bool) -> Vec<u8> {
+    let font = skrifa::FontRef::new(SOURCE_SANS).unwrap();
+    let map: Vec<(u32, u16)> = font.charmap().mappings().filter(|(c, _)| keep(*c)).map(|(c, g)| (c, u16::try_from(g.to_u32()).unwrap())).collect();
+    let mut builder = FontBuilder::new();
+    builder.add_raw(Tag::new(b"cmap"), crate::embed::cmap(&map).unwrap());
+    builder.add_raw(Tag::new(b"name"), name_table(&[(1, family), (2, "Regular"), (4, family), (6, "Viettest-Regular")]).unwrap());
+    for r in font.table_directory.table_records() {
+        let tag = Tag::new(&r.tag().to_be_bytes());
+        if !builder.contains(tag) {
+            builder.add_raw(tag, font.table_data(r.tag()).unwrap().as_bytes());
+        }
+    }
+    builder.build()
+}
+
+/// A font without the precomposed letters builds them from its base letters and marks.
+#[test]
+fn stacked_marks_are_positioned_mark_to_mark() {
+    const FAMILY: &str = "Viettest Marks";
+    let db = FontDb::with_font_dirs(vec![]);
+    assert_eq!(db.add_font(source_sans_with(FAMILY, |c| c < 0x1E00 && !(0xC0..0x250).contains(&c))), 1);
+    for form in 0..3 {
+        assert_attached(&db, FAMILY, form);
+    }
+}
+
+/// A font with the base letters but none of the marks takes the whole letter, marks included,
+/// from a font that has them.
+#[test]
+fn letters_whose_marks_a_font_lacks_fall_back_whole() {
+    const FAMILY: &str = "Viettest Bare";
+    let db = FontDb::with_font_dirs(vec![]);
+    assert_eq!(db.add_font(source_sans_with(FAMILY, |c| c < 0x80)), 1);
+    for form in 0..3 {
+        assert_attached(&db, FAMILY, form);
+    }
+}

docs/localization-parity.md

@@ -1,6 +1,6 @@
 # Localization parity
 
-> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-28 · **Change:** minor (Catalan (`ca`) catalog added, complete under the tests)
+> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (Vietnamese text: stacked diacritics tested and attach in every form, #4491; previously: Catalan (`ca`) catalog added, complete under the tests)
 
 Per-language status of VectorCraft's interface. How the catalogs work and how to add a language:
 [`development.md` › Localisation](development.md#localisation). Part of
@@ -37,7 +37,7 @@ Chinese by `imachus`). Only Georgian is recorded as reviewed by a native speaker
 | Japanese | `ja` | 4,004 / 4,020 (99.6%) | all | CJK UI font as above; Japanese IME; vertical type partial; Japanese crop marks and composition settings | no | full | — (review) |
 | German | `de` | 3,952 / 4,020 (98%; menus, literals and messages complete when counted) | all (602 of 604 message rows; landed 2026-10-10) | Latin | contributor-written, not reviewed | full | — (review) |
 | Korean | `ko` | 4,870 rows (counted 2026-10-21): menus, `tl!` literals and messages complete when counted; one plural form | all | Hangul from an installed system font on the desktop (Apple SD Gothic Neo, Malgun Gothic, Noto Sans CJK KR…); the web build has no Korean UI font yet (craft-fonts); IME path exists | no (AI-drafted) | full | — (review; web font) |
-| Vietnamese | `vi` | 4,886 rows (counted 2026-10-21): menus, `tl!` literals and messages complete when counted; one plural form; 4,046 plain + 18 `@plural` + 28 contextual + 794 `@msg` | all | Latin with Vietnamese diacritics (precomposed forms; glyph tests pass) | contributor is a Vietnamese speaker; AI-assisted draft, further native review welcome | full | — (review) |
+| Vietnamese | `vi` | 4,886 rows (counted 2026-10-21): menus, `tl!` literals and messages complete when counted; one plural form; 4,046 plain + 18 `@plural` + 28 contextual + 794 `@msg` | all | Latin with Vietnamese diacritics (stacked marks attach in precomposed, decomposed and keyboard forms, #4491; glyph tests pass) | contributor is a Vietnamese speaker; AI-assisted draft, further native review welcome | full | — (review) |
 
 **12-language score: ~82%** (equal weights: 100 + 85 + 100 + 0 + 0 + 100 + 100 + 100 + 100 + 98 + 100 + 100, over 12).
 **To `full` for all twelve: ~50–85 h**, most of it Arabic (right-to-left interface) and Hindi (interface
@@ -95,6 +95,7 @@ font for the web build.
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-10-29 | minor | Vietnamese text: letters with two marks (Ể, Ứ, Ự) attach in precomposed, decomposed and keyboard forms, and a font lacking the marks falls back for the whole letter (#4491); scores unchanged |
 | 2026-10-28 | minor | Catalan (`ca`): complete catalog under the tests (menus, literals, messages; one/other plurals); every `ca-*` locale resolves here; AI-drafted, native review welcome. Added the Turkish and Indonesian rows the table was missing. Scores and estimates unchanged |
 | 2026-10-23 | patch | A missing translation never fails a test or a build; one untranslated report, printed by `cargo xtask ci` and `cargo xtask i18n-report`, lists the missing strings per language; scores unchanged |
 | 2026-10-16 | minor | Bulgarian (`bg`, #1909) and Greek (`el`, #2076) catalogs added, complete under the tests; Swedish (#2139) and Finnish (#1994) rows for strings added since; eighteen non-English catalogs |