From bda0f733389d2731756ad91251147bc5b397c02e Mon Sep 17 00:00:00 2001 From: Mohammad AlSaleh Date: Mon, 30 Sep 2024 05:21:34 +0300 Subject: [PATCH] Deep-check for monospacity Before this change, a font is considered monospace if `fontdb` flags it as such. `fontdb` checks the `post` table for this property. But some fonts don't set that property there. Most notably, "Noto Sans Mono" is among these fonts. Monospace as a property is said to be communicated in other places like `OS/2`'s `panose`, but that's not set in the Noto font either. Loosely based on a `fontconfig` function called `FcFreeTypeSpacing()`, this commit adds an additional check against fonts that are not set as `monospaced` by `fontdb`. The horizontal advances of all glyphs of a cmap unicode table are checked to see if they are monospace. Proportionality with double-width and treble-width advances is taken into consideration. Treble width advances exist in the aforementioned Noto font. The checks should be efficient, but the overhead is not in the noise. So these extra checks are only run if the "monospace_fallback" crate feature is enabled. This change also requires library users to check monospacity with `FontSystem::is_monospace()` instead of `FaceInfo::monospaced` from `fontdb` to be in-sync with cosmic-text's view. This requirement was probably coming in the future anyway for when cosmic-text adds support for variable fonts. Signed-off-by: Mohammad AlSaleh --- Cargo.toml | 7 +++-- src/font/mod.rs | 76 +++++++++++++++++++++++++++++++++++++++------- src/font/system.rs | 44 ++++++++++++++++++++++----- 3 files changed, 107 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cca5840bb0..d3a7ee41ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ smol_str = { version = "0.2.2", default-features = false } swash = { version = "0.1.17", optional = true } syntect = { version = "5.1.0", optional = true } sys-locale = { version = "0.3.1", optional = true } -ttf-parser = { version = "0.21", default-features = false } +ttf-parser = { version = "0.25", default-features = false, features = [ "opentype-layout" ] } unicode-linebreak = "0.1.5" unicode-script = "0.5.5" unicode-segmentation = "1.10.1" @@ -40,7 +40,7 @@ features = ["hardcoded-data"] default = ["std", "swash", "fontconfig"] fontconfig = ["fontdb/fontconfig", "std"] monospace_fallback = [] -no_std = ["rustybuzz/libm", "hashbrown", "dep:libm"] +no_std = ["rustybuzz/libm", "ttf-parser/no-std-float", "hashbrown", "dep:libm"] shape-run-cache = [] std = [ "fontdb/memmap", @@ -73,3 +73,6 @@ opt-level = 1 [package.metadata.docs.rs] features = ["vi"] + +[patch.crates-io] +ttf-parser = { git = "https://github.com/MoSal/ttf-parser", branch = "codepoints_iter" } diff --git a/src/font/mod.rs b/src/font/mod.rs index 252a2a0744..9153b48ada 100644 --- a/src/font/mod.rs +++ b/src/font/mod.rs @@ -90,24 +90,78 @@ impl Font { } impl Font { - pub fn new(db: &fontdb::Database, id: fontdb::ID) -> Option { + #[cfg(feature = "monospace_fallback")] + fn proportional_monospaced(face: &ttf_parser::Face) -> Option { + use ttf_parser::cmap::{Format, Subtable}; + use ttf_parser::Face; + + // Pick a unicode cmap subtable to check against its glyphs + let cmap = face.tables().cmap.as_ref()?; + let subtable12 = cmap.subtables.into_iter().find(|subtable| { + subtable.is_unicode() && matches!(subtable.format, Format::SegmentedCoverage(_)) + }); + let subtable4_fn = || { + cmap.subtables.into_iter().find(|subtable| { + subtable.is_unicode() + && matches!(subtable.format, Format::SegmentMappingToDeltaValues(_)) + }) + }; + let unicode_subtable = subtable12.or_else(subtable4_fn)?; + + fn is_proportional( + face: &Face, + unicode_subtable: Subtable, + code_point_iter: impl Iterator, + ) -> Option { + // Fonts like "Noto Sans Mono" have single, double, AND triple width glyphs. + // So we check proportionality up to 3x width, and assume non-proportionality + // once a forth non-zero advance value is encountered. + const MAX_ADVANCES: usize = 3; + + let mut advances = Vec::with_capacity(MAX_ADVANCES); + + for code_point in code_point_iter { + if let Some(glyph_id) = unicode_subtable.glyph_index(code_point) { + match face.glyph_hor_advance(glyph_id) { + Some(advance) if advance != 0 => match advances.binary_search(&advance) { + Err(_) if advances.len() == MAX_ADVANCES => return Some(false), + Err(i) => advances.insert(i, advance), + Ok(_) => (), + }, + _ => (), + } + } + } + + let mut advances = advances.into_iter(); + let smallest = advances.next()?; + Some(advances.find(|advance| advance % smallest > 0).is_none()) + } + + match unicode_subtable.format { + Format::SegmentedCoverage(subtable12) => { + is_proportional(face, unicode_subtable, subtable12.codepoints_iter()) + } + Format::SegmentMappingToDeltaValues(subtable4) => { + is_proportional(face, unicode_subtable, subtable4.codepoints_iter()) + } + _ => unreachable!(), + } + } + + pub fn new(db: &fontdb::Database, id: fontdb::ID, is_monospace: bool) -> Option { let info = db.face(id)?; - let monospace_fallback = if cfg!(feature = "monospace_fallback") { + let monospace_fallback = if cfg!(feature = "monospace_fallback") && is_monospace { db.with_face_data(id, |font_data, face_index| { let face = ttf_parser::Face::parse(font_data, face_index).ok()?; - let monospace_em_width = info - .monospaced - .then(|| { + let monospace_em_width = { + || { let hor_advance = face.glyph_hor_advance(face.glyph_index(' ')?)? as f32; let upem = face.units_per_em() as f32; Some(hor_advance / upem) - }) - .flatten(); - - if info.monospaced && monospace_em_width.is_none() { - None?; - } + } + }(); let scripts = face .tables() diff --git a/src/font/system.rs b/src/font/system.rs index ef689863e4..efd7ff0e22 100644 --- a/src/font/system.rs +++ b/src/font/system.rs @@ -158,11 +158,25 @@ impl FontSystem { /// Create a new [`FontSystem`] with a pre-specified locale and font database. pub fn new_with_locale_and_db(locale: String, db: fontdb::Database) -> Self { - let mut monospace_font_ids = db - .faces() - .filter(|face_info| { - face_info.monospaced && !face_info.post_script_name.contains("Emoji") - }) + #[cfg(feature = "std")] + use rayon::iter::{IntoParallelIterator, ParallelIterator}; + + let faces = db.faces(); + #[cfg(feature = "std")] + let faces = faces.collect::>(); + #[cfg(feature = "std")] + let faces = faces.into_par_iter(); + + let mono_filter_fn = |face_info: &&crate::fontdb::FaceInfo| { + let monospaced = face_info.monospaced; + let proportional_monospaced = + || Self::proportional_monospaced(&db, face_info.id).unwrap_or(false); + (monospaced || proportional_monospaced()) + && !face_info.post_script_name.contains("Emoji") + }; + + let mut monospace_font_ids = faces + .filter(mono_filter_fn) .map(|face_info| face_info.id) .collect::>(); monospace_font_ids.sort(); @@ -197,6 +211,21 @@ impl FontSystem { ret } + fn proportional_monospaced(db: &fontdb::Database, id: fontdb::ID) -> Option { + #[cfg(feature = "monospace_fallback")] + { + db.with_face_data(id, |font_data, face_index| { + let face = ttf_parser::Face::parse(font_data, face_index).ok()?; + Font::proportional_monospaced(&face) + })? + } + #[cfg(not(feature = "monospace_fallback"))] + { + let (_, _) = (db, id); + None + } + } + /// Get the locale. pub fn locale(&self) -> &str { &self.locale @@ -244,7 +273,7 @@ impl FontSystem { let fonts = ids.iter(); fonts - .map(|id| match Font::new(&self.db, *id) { + .map(|id| match Font::new(&self.db, *id, self.is_monospace(*id)) { Some(font) => Some(Arc::new(font)), None => { log::warn!( @@ -264,6 +293,7 @@ impl FontSystem { /// Get a font by its ID. pub fn get_font(&mut self, id: fontdb::ID) -> Option> { + let is_monospace = self.is_monospace(id); self.font_cache .entry(id) .or_insert_with(|| { @@ -271,7 +301,7 @@ impl FontSystem { unsafe { self.db.make_shared_face_data(id); } - match Font::new(&self.db, id) { + match Font::new(&self.db, id, is_monospace) { Some(font) => Some(Arc::new(font)), None => { log::warn!(