Fonts: a missing Korean sans face falls back to a sans one (#1692)

Rebuilt on the split `fontdb/`: `KOREAN_SANS_FALLBACKS` and `sans_by_name` sit
in `fontdb/fallback.rs` beside `KOREAN_FALLBACKS`, `cjk_chain` takes the style,
`fallback_for_styled` is on both `ScopedFonts` and `FontDb`, and the tests live
in `fontdb/tests/fallback.rs` where that file's other fallback tests are.

`KOREAN_FALLBACKS` is serif-first (AppleMyungjo, Apple SD Gothic Neo, Batang,
Malgun Gothic...), so on Windows the first installed entry is Batang. A document
whose Korean text asks for faces that are not installed -- KoPub Dotum, KHNP
Han Dotum, BM Dohyeon -- came out in a serif face throughout: the report
measured 5,298 characters of one school document turning from sans to serif.

The font is not installed, so its name is all there is to go on. `sans_by_name`
reads the style out of the family name: Dotum (돋움), Gulim (굴림), Gothic
(고딕), Sans, Headline, Graphic and Dohyeon are sans; Batang (바탕),
Myungjo/Myeongjo (명조), Gungsuh (궁서) and Serif are serif. Serif is tested
first, so "Noto Serif KR" is not caught by the `sans` rule.

A run whose requested face is sans draws its fallback from the sans chain, which
leads with the system faces. If the requested name says neither, the
substitute's own name decides, which leaves today's chain in place.

The tests do not depend on which fonts the machine has: they pin the name
classification and the chain order -- the first sans entry comes before Batang,
and the serif chain still leads with a serif face, so a missing Myungjo is not
turned into a sans one.

`fallback_for` keeps its signature and delegates, so the ruby, kenten and image
callers are unchanged.

crates/compose/src/shape.rs

@@ -784,8 +784,16 @@ fn shape_run_raw(
     // The language picks the fallback for CJK characters the primary font lacks. Without
     // fallback, the primary font sets them: the shaper gives its .notdef (glyph 0).
     let lang = designcraft_doc::language_tag(&p.language);
+    // A missing Korean face is replaced by one of its own style: the requested name says whether
+    // it was Dotum/Gothic or Batang/Myungjo, and if it does not, the substitute's own name does,
+    // which keeps today's chain (#1692).
+    let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| designcraft_fonts::sans_by_name(&primary.family));
     let fallback = |c: char| {
-        if auto_leading.glyph_fallback { db.fallback_for(c, primary.id(), lang).unwrap_or_else(|| primary.clone()) } else { primary.clone() }
+        if auto_leading.glyph_fallback {
+            db.fallback_for_styled(c, primary.id(), lang, prefer_sans).unwrap_or_else(|| primary.clone())
+        } else {
+            primary.clone()
+        }
     };
     let strong = |c| match unicode_bidi::bidi_class(c) {
         unicode_bidi::BidiClass::AL => Some(true),
@@ -1116,7 +1124,9 @@ fn shape_segment(
         let lang = designcraft_doc::language_tag(&p.language);
         let mut segments: Vec<(usize, usize, Arc<FontFace>)> = Vec::new();
         for (at, c) in label.char_indices() {
-            let selected = if face.covers(c) { face.clone() } else { db.fallback_for(c, face.id(), lang).unwrap_or_else(|| face.clone()) };
+            let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| designcraft_fonts::sans_by_name(&face.family));
+            let selected =
+                if face.covers(c) { face.clone() } else { db.fallback_for_styled(c, face.id(), lang, prefer_sans).unwrap_or_else(|| face.clone()) };
             if let Some((_, end, previous)) = segments.last_mut().filter(|(_, _, previous)| previous.id() == selected.id()) {
                 *end = at + c.len_utf8();
                 let _ = previous;

crates/fonts/src/fontdb.rs

@@ -138,6 +138,7 @@ pub(crate) use face::{measured_top, plausible};
 use fallback::FallbackSnapshotHook;
 #[cfg(not(target_arch = "wasm32"))]
 use fallback::SysFallback;
+pub use fallback::sans_by_name;
 pub use names::{base_style, same_style};
 use outline::FlipPen;
 pub use scoped::ScopedFonts;

crates/fonts/src/fontdb/db.rs

@@ -197,6 +197,11 @@ impl FontDb {
         self.scoped(0).fallback_for(c, exclude, language)
     }
 
+    /// [`ScopedFonts::fallback_for_styled`] on the shared scope.
+    pub fn fallback_for_styled(&self, c: char, exclude: u32, language: Option<&str>, prefer_sans: Option<bool>) -> Option<Arc<FontFace>> {
+        self.scoped(0).fallback_for_styled(c, exclude, language, prefer_sans)
+    }
+
     /// The box drawn for `face`'s missing glyph (.notdef) when the font's own has no outline, in
     /// font units, y-down: a hollow rectangle across the glyph's advance, cap height tall. `None`
     /// when the font draws its own.

crates/fonts/src/fontdb/fallback.rs

@@ -92,7 +92,56 @@ const TRADITIONAL_CHINESE_FALLBACKS: &[&str] = &[
     "Source Han Sans TC",
 ];
 
-const KOREAN_FALLBACKS: &[&str] = &[
+/// Korean fallbacks for a missing **sans** (Dotum/Gothic) face, system faces first.
+///
+/// [`KOREAN_FALLBACKS`] is serif-first, so on Windows the first installed entry is Batang and a
+/// document whose Korean text asked for KoPub Dotum or BM Dohyeon came out in a serif face --
+/// 5,298 characters of one school document, turning it from sans to serif (#1692).
+pub(super) const KOREAN_SANS_FALLBACKS: &[&str] = &[
+    "Malgun Gothic",
+    "Apple SD Gothic Neo",
+    "Dotum",
+    "Gulim",
+    "Noto Sans CJK KR",
+    "Source Han Sans K",
+    "Noto Sans KR",
+    "Noto Serif CJK KR",
+    "Batang",
+];
+
+/// Whether a Korean family name says the face is sans (`Some(true)`) or serif (`Some(false)`).
+/// `None` when the name does not say, so the caller decides.
+///
+/// Korean family names carry the style: Dotum (돋움), Gulim (굴림) and Gothic (고딕) are sans;
+/// Batang (바탕), Myungjo/Myeongjo (명조) and Gungsuh (궁서) are serif. A missing font's name is
+/// the only thing left to go on, since the font itself is not there to classify (#1692).
+pub fn sans_by_name(family: &str) -> Option<bool> {
+    let lower = family.to_lowercase();
+    let has = |needles: &[&str]| needles.iter().any(|n| lower.contains(n));
+    // Serif first: "Myungjo Gothic" does not exist, but a name holding both reads as the serif
+    // it names, and "Sans" inside "Noto Sans Myungjo" would otherwise win.
+    if has(&["batang", "myungjo", "myeongjo", "gungsuh", "serif", "\u{BC14}\u{D5D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) {
+        return Some(false);
+    }
+    if has(&[
+        "dotum",
+        "gothic",
+        "gulim",
+        "sans",
+        "headline",
+        "graphic",
+        "dohyeon",
+        "\u{B3CB}\u{C6C0}",
+        "\u{ACE0}\u{B515}",
+        "\u{AD74}\u{B9BC}",
+        "\u{ADF8}\u{B798}\u{D53D}",
+    ]) {
+        return Some(true);
+    }
+    None
+}
+
+pub(super) const KOREAN_FALLBACKS: &[&str] = &[
     "AppleMyungjo",
     "Apple SD Gothic Neo",
     "Batang",
@@ -146,9 +195,10 @@ fn japanese_chain() -> &'static [&'static str] {
 /// The CJK fallback chain for `c` in `language` (a BCP 47 tag). The script decides where it is
 /// unambiguous (Hangul is Korean, kana Japanese, Bopomofo Traditional Chinese); ideographs and CJK
 /// punctuation follow the language, and have no chain without a CJK one.
-pub(super) fn cjk_chain(c: char, language: Option<&str>) -> Option<&'static [&'static str]> {
+pub(super) fn cjk_chain(c: char, language: Option<&str>, prefer_sans: Option<bool>) -> Option<&'static [&'static str]> {
+    let korean = if prefer_sans == Some(true) { KOREAN_SANS_FALLBACKS } else { KOREAN_FALLBACKS };
     match c as u32 {
-        0x1100..=0x11FF | 0x3130..=0x318F | 0xA960..=0xA97F | 0xAC00..=0xD7FF | 0xFFA0..=0xFFDC => return Some(KOREAN_FALLBACKS),
+        0x1100..=0x11FF | 0x3130..=0x318F | 0xA960..=0xA97F | 0xAC00..=0xD7FF | 0xFFA0..=0xFFDC => return Some(korean),
         0x3040..=0x30FF | 0x31F0..=0x31FF | 0xFF66..=0xFF9F | 0x1B000..=0x1B16F => return Some(japanese_chain()),
         0x3100..=0x312F | 0x31A0..=0x31BF => return Some(TRADITIONAL_CHINESE_FALLBACKS),
         // Radicals, CJK symbols and punctuation, kanbun, strokes, enclosed and compatibility
@@ -171,7 +221,7 @@ pub(super) fn cjk_chain(c: char, language: Option<&str>) -> Option<&'static [&'s
     let language = language?;
     match language.split('-').next()? {
         "ja" => Some(japanese_chain()),
-        "ko" => Some(KOREAN_FALLBACKS),
+        "ko" => Some(korean),
         "zh" if language.starts_with("zh-Hant") => Some(TRADITIONAL_CHINESE_FALLBACKS),
         "zh" => Some(SIMPLIFIED_CHINESE_FALLBACKS),
         _ => None,

crates/fonts/src/fontdb/scoped.rs

@@ -390,7 +390,14 @@ impl ScopedFonts<'_> {
     /// then load order, the document's fonts last) that covers `c`. On native, system fonts are
     /// cataloged and loaded lazily the first time no loaded face covers a character.
     pub fn fallback_for(&self, c: char, exclude: u32, language: Option<&str>) -> Option<Arc<FontFace>> {
-        if let Some(chain) = cjk_chain(c, language).or_else(|| script_chain(c))
+        self.fallback_for_styled(c, exclude, language, None)
+    }
+
+    /// [`Self::fallback_for`] following the style of the font that was asked for: `Some(true)`
+    /// takes the sans chain, so a missing Dotum or Gothic face is replaced by a sans one instead
+    /// of by the first installed serif (#1692). `None` keeps the chain as it was.
+    pub fn fallback_for_styled(&self, c: char, exclude: u32, language: Option<&str>, prefer_sans: Option<bool>) -> Option<Arc<FontFace>> {
+        if let Some(chain) = cjk_chain(c, language, prefer_sans).or_else(|| script_chain(c))
             && let Some(f) = chain.iter().find_map(|family| self.chain_face(family, c, exclude))
         {
             return Some(f);

crates/fonts/src/fontdb/tests/fallback.rs

@@ -1,6 +1,7 @@
 //! CJK fallback follows the character's language (and the script where it is unambiguous).
 
 use crate::fontdb::FontDb;
+use crate::fontdb::fallback::{KOREAN_FALLBACKS, KOREAN_SANS_FALLBACKS, cjk_chain, sans_by_name};
 use crate::testing::font_with;
 
 const HAN: char = '直';
@@ -157,3 +158,40 @@ fn malayalam_fonts_load_from_the_font_folders_when_first_needed() {
     assert!(!db.is_loaded("Noto Sans Malayalam"));
     assert_eq!(family(&db, KA, None).as_deref(), Some("Noto Sans Malayalam"));
 }
+
+/// A missing Korean face is replaced by one of its own style. The name is all there is to go
+/// on, since the font itself is not installed to classify (#1692).
+#[test]
+fn a_korean_family_name_says_sans_or_serif() {
+    for sans in ["KoPub Dotum Bold", "KHNP Han Dotum", "BM Dohyeon", "Malgun Gothic", "\u{B3CB}\u{C6C0}\u{CCB4}", "\u{ACE0}\u{B515}", "Noto Sans KR"]
+    {
+        assert_eq!(sans_by_name(sans), Some(true), "{sans} is sans");
+    }
+    for serif in ["Batang", "HCR Batang", "KoPub Myungjo", "\u{BA85}\u{C870}", "Gungsuh", "Noto Serif KR"] {
+        assert_eq!(sans_by_name(serif), Some(false), "{serif} is serif");
+    }
+    // A name that says neither leaves the choice to the caller.
+    for unknown in ["Times New Roman", "Helvetica", "Arial Unicode MS", ""] {
+        assert_eq!(sans_by_name(unknown), None, "{unknown} says neither");
+    }
+}
+
+/// The reported bug is the order of the chain: on Windows the first installed entry of the
+/// serif-first chain is Batang, so every missing Dotum face came out serif (#1692).
+#[test]
+fn the_sans_chain_puts_a_sans_face_before_batang() {
+    let hangul = '\u{D55C}';
+    let sans = cjk_chain(hangul, None, Some(true)).expect("Hangul has a chain");
+    let serif = cjk_chain(hangul, None, None).expect("Hangul has a chain");
+    assert_eq!(serif, KOREAN_FALLBACKS, "without a style the chain is unchanged");
+    assert_eq!(sans, KOREAN_SANS_FALLBACKS);
+
+    let batang = |chain: &[&str]| chain.iter().position(|f| *f == "Batang");
+    let first_sans = |chain: &[&str]| chain.iter().position(|f| sans_by_name(f) == Some(true));
+    assert!(first_sans(sans) < batang(sans), "the sans chain reaches a sans face first: {sans:?}");
+    // And the Windows and macOS system faces are the first two entries, so an installed
+    // system font is preferred to a downloaded one.
+    assert_eq!(&sans[..2], &["Malgun Gothic", "Apple SD Gothic Neo"]);
+    // The serif chain still leads with serif faces, so a missing Myungjo is not made sans.
+    assert_eq!(sans_by_name(serif[0]), Some(false), "the serif chain still leads serif: {:?}", serif[0]);
+}

crates/fonts/src/lib.rs

@@ -20,7 +20,7 @@ mod vertical;
 pub use features::{FontFeatures, font_features};
 pub use fontdb::{
     DOCUMENT_FONTS_FOLDER, DocumentFonts, FALLBACK_FAMILY, FaceRef, FontDb, FontFace, FontSource, MAX_DOCUMENT_FONT_BYTES, MAX_DOCUMENT_FONT_FILES,
-    MAX_DOCUMENT_FONTS_TOTAL, ReloadedFonts, ScopedFonts, base_style, bundled, same_style, system_font_dirs,
+    MAX_DOCUMENT_FONTS_TOTAL, ReloadedFonts, ScopedFonts, base_style, bundled, same_style, sans_by_name, system_font_dirs,
 };
 pub use group::{FamilyInfo, FontGroup, sort_for_menu};
 pub use harfrust::Feature;