Skip to content

Commit bda0f73

Browse files
committed
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 <CE.Mohammad.AlSaleh@gmail.com>
1 parent 4fe90bb commit bda0f73

File tree

3 files changed

+107
-20
lines changed

3 files changed

+107
-20
lines changed

Cargo.toml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ smol_str = { version = "0.2.2", default-features = false }
2525
swash = { version = "0.1.17", optional = true }
2626
syntect = { version = "5.1.0", optional = true }
2727
sys-locale = { version = "0.3.1", optional = true }
28-
ttf-parser = { version = "0.21", default-features = false }
28+
ttf-parser = { version = "0.25", default-features = false, features = [ "opentype-layout" ] }
2929
unicode-linebreak = "0.1.5"
3030
unicode-script = "0.5.5"
3131
unicode-segmentation = "1.10.1"
@@ -40,7 +40,7 @@ features = ["hardcoded-data"]
4040
default = ["std", "swash", "fontconfig"]
4141
fontconfig = ["fontdb/fontconfig", "std"]
4242
monospace_fallback = []
43-
no_std = ["rustybuzz/libm", "hashbrown", "dep:libm"]
43+
no_std = ["rustybuzz/libm", "ttf-parser/no-std-float", "hashbrown", "dep:libm"]
4444
shape-run-cache = []
4545
std = [
4646
"fontdb/memmap",
@@ -73,3 +73,6 @@ opt-level = 1
7373

7474
[package.metadata.docs.rs]
7575
features = ["vi"]
76+
77+
[patch.crates-io]
78+
ttf-parser = { git = "https://github.com/MoSal/ttf-parser", branch = "codepoints_iter" }

src/font/mod.rs

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,78 @@ impl Font {
9090
}
9191

9292
impl Font {
93-
pub fn new(db: &fontdb::Database, id: fontdb::ID) -> Option<Self> {
93+
#[cfg(feature = "monospace_fallback")]
94+
fn proportional_monospaced(face: &ttf_parser::Face) -> Option<bool> {
95+
use ttf_parser::cmap::{Format, Subtable};
96+
use ttf_parser::Face;
97+
98+
// Pick a unicode cmap subtable to check against its glyphs
99+
let cmap = face.tables().cmap.as_ref()?;
100+
let subtable12 = cmap.subtables.into_iter().find(|subtable| {
101+
subtable.is_unicode() && matches!(subtable.format, Format::SegmentedCoverage(_))
102+
});
103+
let subtable4_fn = || {
104+
cmap.subtables.into_iter().find(|subtable| {
105+
subtable.is_unicode()
106+
&& matches!(subtable.format, Format::SegmentMappingToDeltaValues(_))
107+
})
108+
};
109+
let unicode_subtable = subtable12.or_else(subtable4_fn)?;
110+
111+
fn is_proportional(
112+
face: &Face,
113+
unicode_subtable: Subtable,
114+
code_point_iter: impl Iterator<Item = u32>,
115+
) -> Option<bool> {
116+
// Fonts like "Noto Sans Mono" have single, double, AND triple width glyphs.
117+
// So we check proportionality up to 3x width, and assume non-proportionality
118+
// once a forth non-zero advance value is encountered.
119+
const MAX_ADVANCES: usize = 3;
120+
121+
let mut advances = Vec::with_capacity(MAX_ADVANCES);
122+
123+
for code_point in code_point_iter {
124+
if let Some(glyph_id) = unicode_subtable.glyph_index(code_point) {
125+
match face.glyph_hor_advance(glyph_id) {
126+
Some(advance) if advance != 0 => match advances.binary_search(&advance) {
127+
Err(_) if advances.len() == MAX_ADVANCES => return Some(false),
128+
Err(i) => advances.insert(i, advance),
129+
Ok(_) => (),
130+
},
131+
_ => (),
132+
}
133+
}
134+
}
135+
136+
let mut advances = advances.into_iter();
137+
let smallest = advances.next()?;
138+
Some(advances.find(|advance| advance % smallest > 0).is_none())
139+
}
140+
141+
match unicode_subtable.format {
142+
Format::SegmentedCoverage(subtable12) => {
143+
is_proportional(face, unicode_subtable, subtable12.codepoints_iter())
144+
}
145+
Format::SegmentMappingToDeltaValues(subtable4) => {
146+
is_proportional(face, unicode_subtable, subtable4.codepoints_iter())
147+
}
148+
_ => unreachable!(),
149+
}
150+
}
151+
152+
pub fn new(db: &fontdb::Database, id: fontdb::ID, is_monospace: bool) -> Option<Self> {
94153
let info = db.face(id)?;
95154

96-
let monospace_fallback = if cfg!(feature = "monospace_fallback") {
155+
let monospace_fallback = if cfg!(feature = "monospace_fallback") && is_monospace {
97156
db.with_face_data(id, |font_data, face_index| {
98157
let face = ttf_parser::Face::parse(font_data, face_index).ok()?;
99-
let monospace_em_width = info
100-
.monospaced
101-
.then(|| {
158+
let monospace_em_width = {
159+
|| {
102160
let hor_advance = face.glyph_hor_advance(face.glyph_index(' ')?)? as f32;
103161
let upem = face.units_per_em() as f32;
104162
Some(hor_advance / upem)
105-
})
106-
.flatten();
107-
108-
if info.monospaced && monospace_em_width.is_none() {
109-
None?;
110-
}
163+
}
164+
}();
111165

112166
let scripts = face
113167
.tables()

src/font/system.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,25 @@ impl FontSystem {
158158

159159
/// Create a new [`FontSystem`] with a pre-specified locale and font database.
160160
pub fn new_with_locale_and_db(locale: String, db: fontdb::Database) -> Self {
161-
let mut monospace_font_ids = db
162-
.faces()
163-
.filter(|face_info| {
164-
face_info.monospaced && !face_info.post_script_name.contains("Emoji")
165-
})
161+
#[cfg(feature = "std")]
162+
use rayon::iter::{IntoParallelIterator, ParallelIterator};
163+
164+
let faces = db.faces();
165+
#[cfg(feature = "std")]
166+
let faces = faces.collect::<Vec<_>>();
167+
#[cfg(feature = "std")]
168+
let faces = faces.into_par_iter();
169+
170+
let mono_filter_fn = |face_info: &&crate::fontdb::FaceInfo| {
171+
let monospaced = face_info.monospaced;
172+
let proportional_monospaced =
173+
|| Self::proportional_monospaced(&db, face_info.id).unwrap_or(false);
174+
(monospaced || proportional_monospaced())
175+
&& !face_info.post_script_name.contains("Emoji")
176+
};
177+
178+
let mut monospace_font_ids = faces
179+
.filter(mono_filter_fn)
166180
.map(|face_info| face_info.id)
167181
.collect::<Vec<_>>();
168182
monospace_font_ids.sort();
@@ -197,6 +211,21 @@ impl FontSystem {
197211
ret
198212
}
199213

214+
fn proportional_monospaced(db: &fontdb::Database, id: fontdb::ID) -> Option<bool> {
215+
#[cfg(feature = "monospace_fallback")]
216+
{
217+
db.with_face_data(id, |font_data, face_index| {
218+
let face = ttf_parser::Face::parse(font_data, face_index).ok()?;
219+
Font::proportional_monospaced(&face)
220+
})?
221+
}
222+
#[cfg(not(feature = "monospace_fallback"))]
223+
{
224+
let (_, _) = (db, id);
225+
None
226+
}
227+
}
228+
200229
/// Get the locale.
201230
pub fn locale(&self) -> &str {
202231
&self.locale
@@ -244,7 +273,7 @@ impl FontSystem {
244273
let fonts = ids.iter();
245274

246275
fonts
247-
.map(|id| match Font::new(&self.db, *id) {
276+
.map(|id| match Font::new(&self.db, *id, self.is_monospace(*id)) {
248277
Some(font) => Some(Arc::new(font)),
249278
None => {
250279
log::warn!(
@@ -264,14 +293,15 @@ impl FontSystem {
264293

265294
/// Get a font by its ID.
266295
pub fn get_font(&mut self, id: fontdb::ID) -> Option<Arc<Font>> {
296+
let is_monospace = self.is_monospace(id);
267297
self.font_cache
268298
.entry(id)
269299
.or_insert_with(|| {
270300
#[cfg(feature = "std")]
271301
unsafe {
272302
self.db.make_shared_face_data(id);
273303
}
274-
match Font::new(&self.db, id) {
304+
match Font::new(&self.db, id, is_monospace) {
275305
Some(font) => Some(Arc::new(font)),
276306
None => {
277307
log::warn!(

0 commit comments

Comments
 (0)