diff --git a/crates/compose/src/shape.rs b/crates/compose/src/shape.rs index 4ce7247d5..1de4b823c 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) struct ShapeInput<'a, 'font> { @@ -813,8 +827,19 @@ 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). + // 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(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), @@ -1144,8 +1169,11 @@ 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 selected = if face.covers(c) { face.clone() } else { db.fallback_for(c, face.id(), lang).unwrap_or_else(|| face.clone()) }; + 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; @@ -1495,3 +1523,23 @@ mod tests { assert!(t.elapsed().as_secs() < 5); } } + +#[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"); + } +} 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 e7fee7cbf..224098a6a 100644 --- a/crates/fonts/src/fontdb/db.rs +++ b/crates/fonts/src/fontdb/db.rs @@ -199,6 +199,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..73a2e0a25 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/crates/fonts/src/fontdb/fallback.rs @@ -92,7 +92,62 @@ 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)); + // 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{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. + if lower.replace("sans-serif", "sans").replace("sans serif", "sans").contains("serif") { + 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 +201,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 +227,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 f28e85455..358189186 100644 --- a/crates/fonts/src/fontdb/scoped.rs +++ b/crates/fonts/src/fontdb/scoped.rs @@ -402,7 +402,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..a80730754 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,70 @@ 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"); + } +} + +/// "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 (#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)); + // And the word outranks every sans indicator: Sweet Gothic Serif is a serif family + // 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); +} + +/// 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 1c1503dfa..3d772b305 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;