From bb49e566277d083db153aef11d9c81cbb0a818a1 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 27 Oct 2026 16:30:57 +0500 Subject: [PATCH 1/5] Fonts: a missing Korean sans face falls back to a sans one (#1692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 14 +++++- crates/fonts/src/fontdb.rs | 1 + crates/fonts/src/fontdb/db.rs | 5 ++ crates/fonts/src/fontdb/fallback.rs | 58 +++++++++++++++++++++-- crates/fonts/src/fontdb/scoped.rs | 9 +++- crates/fonts/src/fontdb/tests/fallback.rs | 38 +++++++++++++++ crates/fonts/src/lib.rs | 2 +- 7 files changed, 119 insertions(+), 8 deletions(-) diff --git a/crates/compose/src/shape.rs b/crates/compose/src/shape.rs index 0f739e16e..6c9aca4d3 100644 --- a/crates/compose/src/shape.rs +++ b/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)> = 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; diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs index d0f0054f8..6e145ade0 100644 --- a/crates/fonts/src/fontdb.rs +++ b/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; diff --git a/crates/fonts/src/fontdb/db.rs b/crates/fonts/src/fontdb/db.rs index 30b2b4b28..c97a06dd7 100644 --- a/crates/fonts/src/fontdb/db.rs +++ b/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) -> Option> { + 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. diff --git a/crates/fonts/src/fontdb/fallback.rs b/crates/fonts/src/fontdb/fallback.rs index 7e29fa4b3..ad8acc538 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/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 { + 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) -> 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, diff --git a/crates/fonts/src/fontdb/scoped.rs b/crates/fonts/src/fontdb/scoped.rs index 8fba9ff6e..e0881ff05 100644 --- a/crates/fonts/src/fontdb/scoped.rs +++ b/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> { - 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) -> Option> { + 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); diff --git a/crates/fonts/src/fontdb/tests/fallback.rs b/crates/fonts/src/fontdb/tests/fallback.rs index 19eb30cec..d385a8fd9 100644 --- a/crates/fonts/src/fontdb/tests/fallback.rs +++ b/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]); +} diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index aca293d12..30b54e206 100644 --- a/crates/fonts/src/lib.rs +++ b/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; From 0c9e59c0090a82437936f8fa3f98b43e18d3bc7f Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 28 Oct 2026 15:15:15 +0500 Subject: [PATCH 2/5] Fonts: do not read a requested style off the face that stands in for it Review of #1755 caught a real over-reach in my own patch. When a family cannot be resolved, db.face hands back a bundled face, and I was reading "Sans" out of its name: FALLBACK_FAMILY is "Source Sans 3", so every unclassified missing font took the sans chain -- picking Malgun Gothic on Windows over the Batang fallback that was already there. DEFAULT_FAMILY, "Source Serif 4", would have forced the other way. A stand-in face says nothing about what the document asked for, so style_of_substitute returns None for both and only a face that really names itself gets a vote. The test pins both families by their constants; with the old body it fails with Some(true) against None. --- crates/compose/src/shape.rs | 41 +++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/compose/src/shape.rs b/crates/compose/src/shape.rs index 6c9aca4d3..87f697e56 100644 --- a/crates/compose/src/shape.rs +++ b/crates/compose/src/shape.rs @@ -317,6 +317,20 @@ impl StyleTable<'_> { } } +/// What a substitute face's own name says about its style, for choosing a CJK fallback chain. +/// +/// `None` for the bundled default and last-resort families: they stand in for a font that could +/// not be resolved at all, so their names say nothing about what the document asked for. Without +/// this, "Source Sans 3" made every unresolved family look like a sans one (#1692, noted in +/// review). +fn style_of_substitute(family: &str) -> Option { + let standin = [designcraft_fonts::DEFAULT_FAMILY, designcraft_fonts::FALLBACK_FAMILY]; + if standin.iter().any(|s| s.eq_ignore_ascii_case(family)) { + return None; + } + designcraft_fonts::sans_by_name(family) +} + /// Resolve and shape one paragraph. Its drop cap ends at byte `drop.0` (`range.start` without /// one) and takes the character style `drop.1` ("" = none). pub(crate) fn shape_para( @@ -787,7 +801,10 @@ fn shape_run_raw( // 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)); + // The substitute's own name is consulted only when it is a real substitute: a requested + // family that resolves to nothing comes back as the bundled default, whose name holds "Sans", + // and every unclassified missing font would then have taken the sans chain (noted in review). + let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| style_of_substitute(&primary.family)); let fallback = |c: char| { if auto_leading.glyph_fallback { db.fallback_for_styled(c, primary.id(), lang, prefer_sans).unwrap_or_else(|| primary.clone()) @@ -1124,7 +1141,7 @@ fn shape_segment( let lang = designcraft_doc::language_tag(&p.language); let mut segments: Vec<(usize, usize, Arc)> = Vec::new(); for (at, c) in label.char_indices() { - let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| designcraft_fonts::sans_by_name(&face.family)); + let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| style_of_substitute(&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()) { @@ -1405,3 +1422,23 @@ fn digit_mode(p: &designcraft_doc::CharProps, arabic_context: Option) -> d _ => Digits::Native, } } + +#[cfg(test)] +mod style_of_substitute_tests { + use super::style_of_substitute; + + /// A face that stands in for a font that could not be resolved says nothing about the style + /// the document asked for. `FALLBACK_FAMILY` is "Source Sans 3" and `DEFAULT_FAMILY` is + /// "Source Serif 4", so reading their names would have forced the sans chain on every + /// unresolved family -- and the serif one on the other (#1692, noted in review). + #[test] + fn a_stand_in_face_says_nothing_about_the_requested_style() { + assert_eq!(style_of_substitute(designcraft_fonts::FALLBACK_FAMILY), None); + assert_eq!(style_of_substitute(designcraft_fonts::DEFAULT_FAMILY), None); + assert_eq!(style_of_substitute("source sans 3"), None, "however it is spelled"); + // A real substitute still speaks for itself. + assert_eq!(style_of_substitute("Malgun Gothic"), Some(true)); + assert_eq!(style_of_substitute("Batang"), Some(false)); + assert_eq!(style_of_substitute("Helvetica"), None, "a name that says neither"); + } +} From 8444abfbca2d6d666b1629e4a9255fd5ddb56220 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 28 Oct 2026 15:18:58 +0500 Subject: [PATCH 3/5] Fonts: "sans serif" is a sans name, not a serif one Review pointed out that the CSS generic "sans-serif" matched the bare "serif" needle first and classified as serif. The same ordering hit real family names: "Microsoft Sans Serif", "MS Sans Serif" and "Lucida Sans Serif" are sans faces that a Word import can name, and epub's own generic_family already maps Microsoft Sans Serif to sans-serif. So rather than special-casing the one hyphenated spelling, the bare "serif" word now sorts after the sans needles. The Korean serif names stay ahead of everything, which is what keeps "Noto Sans Myungjo" and "Myungjo Gothic" serif; the test pins those two along with both "sans serif" spellings. --- crates/fonts/src/fontdb/fallback.rs | 11 ++++++++--- crates/fonts/src/fontdb/tests/fallback.rs | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/fonts/src/fontdb/fallback.rs b/crates/fonts/src/fontdb/fallback.rs index ad8acc538..13afffd07 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/crates/fonts/src/fontdb/fallback.rs @@ -118,9 +118,9 @@ pub(super) const KOREAN_SANS_FALLBACKS: &[&str] = &[ pub fn sans_by_name(family: &str) -> Option { 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}"]) { + // A Korean serif name comes 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", "\u{BC14}\u{D5D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) { return Some(false); } if has(&[ @@ -138,6 +138,11 @@ pub fn sans_by_name(family: &str) -> Option { ]) { return Some(true); } + // Only now the bare word, which "sans serif" and "sans-serif" end with and do not mean + // (noted in review): "Microsoft Sans Serif" is a sans face, and so is the CSS generic. + if lower.contains("serif") { + return Some(false); + } None } diff --git a/crates/fonts/src/fontdb/tests/fallback.rs b/crates/fonts/src/fontdb/tests/fallback.rs index d385a8fd9..d9a5ee71c 100644 --- a/crates/fonts/src/fontdb/tests/fallback.rs +++ b/crates/fonts/src/fontdb/tests/fallback.rs @@ -176,6 +176,26 @@ fn a_korean_family_name_says_sans_or_serif() { } } +/// "sans serif" ends with the word it is not. The bare "serif" needle matched first, so +/// "Microsoft Sans Serif" -- and the CSS generic -- classified as serif and a missing Hangul +/// glyph took the serif-first chain (noted in review of #1692). +#[test] +fn a_sans_serif_name_is_not_read_as_serif() { + for sans in ["sans-serif", "Microsoft Sans Serif", "MS Sans Serif", "Lucida Sans Serif"] { + assert_eq!(sans_by_name(sans), Some(true), "{sans} is sans"); + } + // The bare word still decides a name that holds nothing else. + for serif in ["serif", "Source Serif 4", "DejaVu Serif", "Noto Serif KR"] { + assert_eq!(sans_by_name(serif), Some(false), "{serif} is serif"); + } + // And a Korean serif name still outranks "Sans", which is why it is tested first of all. + assert_eq!(sans_by_name("Noto Sans Myungjo"), Some(false)); + assert_eq!(sans_by_name("Myungjo Gothic"), Some(false)); + // Neither word present, so the caller still decides. + assert_eq!(sans_by_name("monospace"), None); + assert_eq!(sans_by_name("system-ui"), None); +} + /// 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] From 0d16a040388d5b5ec1d062eeec0b07021fc31055 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 28 Oct 2026 15:53:44 +0500 Subject: [PATCH 4/5] Fonts: the word "serif" outranks every sans indicator My own previous commit introduced this: moving the bare "serif" needle after the sans list to let "Microsoft Sans Serif" read as sans also let "gothic" win in "Sweet Gothic Serif", a real serif family, which review caught. The rule is narrower than either ordering. A name holding "serif" is serif, and that outranks the sans indicators; the only exception is the phrase it ends: "sans serif" and "sans-serif" are the name of a sans face. So the phrase is removed before the word is looked for, which keeps both spellings sans and gives every explicitly serif name back. Korean serif names stay ahead of all of it, so "Noto Sans Myungjo" and "Myungjo Gothic" are unchanged. --- crates/fonts/src/fontdb/fallback.rs | 11 ++++++----- crates/fonts/src/fontdb/tests/fallback.rs | 5 +++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/fonts/src/fontdb/fallback.rs b/crates/fonts/src/fontdb/fallback.rs index 13afffd07..0b68f9c7c 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/crates/fonts/src/fontdb/fallback.rs @@ -123,6 +123,12 @@ pub fn sans_by_name(family: &str) -> Option { if has(&["batang", "myungjo", "myeongjo", "gungsuh", "\u{BC14}\u{D5D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) { return Some(false); } + // Then the word itself, which outranks every sans indicator: "Sweet Gothic Serif" is a serif + // family whatever "Gothic" suggests. "Sans serif" and "sans-serif" end with the word and do + // not mean it, so the phrase goes before the word is looked for (both noted in review). + if lower.replace("sans-serif", "sans").replace("sans serif", "sans").contains("serif") { + return Some(false); + } if has(&[ "dotum", "gothic", @@ -138,11 +144,6 @@ pub fn sans_by_name(family: &str) -> Option { ]) { return Some(true); } - // Only now the bare word, which "sans serif" and "sans-serif" end with and do not mean - // (noted in review): "Microsoft Sans Serif" is a sans face, and so is the CSS generic. - if lower.contains("serif") { - return Some(false); - } None } diff --git a/crates/fonts/src/fontdb/tests/fallback.rs b/crates/fonts/src/fontdb/tests/fallback.rs index d9a5ee71c..798443524 100644 --- a/crates/fonts/src/fontdb/tests/fallback.rs +++ b/crates/fonts/src/fontdb/tests/fallback.rs @@ -191,6 +191,11 @@ fn a_sans_serif_name_is_not_read_as_serif() { // And a Korean serif name still outranks "Sans", which is why it is tested first of all. assert_eq!(sans_by_name("Noto Sans Myungjo"), Some(false)); assert_eq!(sans_by_name("Myungjo Gothic"), Some(false)); + // And the word outranks every sans indicator: Sweet Gothic Serif is a serif family + // whatever "Gothic" suggests (noted in review). + for serif in ["Sweet Gothic Serif", "Gothic Serif", "Sans Forgetica Serif"] { + assert_eq!(sans_by_name(serif), Some(false), "{serif} names itself serif"); + } // Neither word present, so the caller still decides. assert_eq!(sans_by_name("monospace"), None); assert_eq!(sans_by_name("system-ui"), None); From 39ea3513a4abe9f91b8bded4ed321e392e2a614d Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 4 Nov 2026 12:55:57 +0500 Subject: [PATCH 5/5] =?UTF-8?q?Korean=20fallback:=20spell=20=EB=B0=94?= =?UTF-8?q?=ED=83=95=20right,=20and=20choose=20the=20label's=20fallback=20?= =?UTF-8?q?style=20once?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/compose/src/shape.rs | 3 ++- crates/fonts/src/fontdb/fallback.rs | 4 ++-- crates/fonts/src/fontdb/tests/fallback.rs | 9 +++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/compose/src/shape.rs b/crates/compose/src/shape.rs index 3c78a1392..1de4b823c 100644 --- a/crates/compose/src/shape.rs +++ b/crates/compose/src/shape.rs @@ -1169,8 +1169,9 @@ fn shape_segment( if let Some(label) = replacement.filter(|_| auto_leading.glyph_fallback) { let lang = designcraft_doc::language_tag(&p.language); let mut segments: Vec<(usize, usize, Arc)> = Vec::new(); + // The same for every character of the label: worked out once, not per character. + let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| style_of_substitute(&face.family)); for (at, c) in label.char_indices() { - let prefer_sans = designcraft_fonts::sans_by_name(&p.font_family).or_else(|| style_of_substitute(&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()) { diff --git a/crates/fonts/src/fontdb/fallback.rs b/crates/fonts/src/fontdb/fallback.rs index 0b68f9c7c..73a2e0a25 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/crates/fonts/src/fontdb/fallback.rs @@ -120,12 +120,12 @@ pub fn sans_by_name(family: &str) -> Option { let has = |needles: &[&str]| needles.iter().any(|n| lower.contains(n)); // A Korean serif name comes 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", "\u{BC14}\u{D5D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) { + if has(&["batang", "myungjo", "myeongjo", "gungsuh", "\u{BC14}\u{D0D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) { return Some(false); } // Then the word itself, which outranks every sans indicator: "Sweet Gothic Serif" is a serif // family whatever "Gothic" suggests. "Sans serif" and "sans-serif" end with the word and do - // not mean it, so the phrase goes before the word is looked for (both noted in review). + // not mean it, so the phrase goes before the word is looked for. if lower.replace("sans-serif", "sans").replace("sans serif", "sans").contains("serif") { return Some(false); } diff --git a/crates/fonts/src/fontdb/tests/fallback.rs b/crates/fonts/src/fontdb/tests/fallback.rs index 798443524..a80730754 100644 --- a/crates/fonts/src/fontdb/tests/fallback.rs +++ b/crates/fonts/src/fontdb/tests/fallback.rs @@ -178,7 +178,7 @@ fn a_korean_family_name_says_sans_or_serif() { /// "sans serif" ends with the word it is not. The bare "serif" needle matched first, so /// "Microsoft Sans Serif" -- and the CSS generic -- classified as serif and a missing Hangul -/// glyph took the serif-first chain (noted in review of #1692). +/// glyph took the serif-first chain (#1692). #[test] fn a_sans_serif_name_is_not_read_as_serif() { for sans in ["sans-serif", "Microsoft Sans Serif", "MS Sans Serif", "Lucida Sans Serif"] { @@ -192,10 +192,15 @@ fn a_sans_serif_name_is_not_read_as_serif() { assert_eq!(sans_by_name("Noto Sans Myungjo"), Some(false)); assert_eq!(sans_by_name("Myungjo Gothic"), Some(false)); // And the word outranks every sans indicator: Sweet Gothic Serif is a serif family - // whatever "Gothic" suggests (noted in review). + // whatever "Gothic" suggests. for serif in ["Sweet Gothic Serif", "Gothic Serif", "Sans Forgetica Serif"] { assert_eq!(sans_by_name(serif), Some(false), "{serif} names itself serif"); } + // Korean serif names written in Hangul: 바탕 (Batang), 명조 (Myungjo), 궁서 (Gungsuh). 바탕 was + // spelled with the wrong second syllable, so a Hangul Batang name was not recognised. + for serif in ["\u{BC14}\u{D0D5}\u{CCB4}", "HCR \u{BC14}\u{D0D5}", "\u{D55C}\u{CEF4} \u{BA85}\u{C870}", "\u{AD81}\u{C11C}\u{CCB4}"] { + assert_eq!(sans_by_name(serif), Some(false), "{serif} is serif"); + } // Neither word present, so the caller still decides. assert_eq!(sans_by_name("monospace"), None); assert_eq!(sans_by_name("system-ui"), None);