From eb96e8b6ed02d915be3f18e5ac60e77765507387 Mon Sep 17 00:00:00 2001 From: Felix Riestra Date: Wed, 9 Sep 2026 21:16:58 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20silent=20TTF-fallback=20failures=20for=20?= =?UTF-8?q?variable=20system=20fonts=20(Turkish=20'=C4=9E')?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (found by instrumenting build_fallback and reading ttf-parser's own source, not guessed): cosmic-text's fallback search picks macOS's system font (San Francisco / SFNS.ttf) for any codepoint no stroke font or explicitly-requested family covers. It's a variable TrueType font, and ttf-parser's outline_glyph() resolves a variable glyph's contours through its gvar table's deltas relative to whatever variation coordinates are currently set on the Face — an unset Face has none, so outline_glyph silently returns None (not an error) for such a glyph. The caller then still returned Some(Glyph { strokes: vec![], .. }): a "resolved" glyph with nothing to render, no error anywhere. Two-part fix: 1. pin_default_variations() sets every variation axis to its own default value before outlining, applied at all TTF outline- extraction sites (both build_fallback variants, both build_shaped variants, and the plain by-family glyph() lookup). This alone wasn't sufficient for San Francisco specifically — even pinned, its outline still didn't resolve for the glyphs this bug was found on, apparently because ttf-parser can't retrieve base glyf data gvar needs for at least some of that font's accented Latin glyphs. 2. So build_fallback() now treats "resolved a glyph ID but couldn't extract its outline" as a real failure (outline_from_face returns None, not an empty glyph) and falls through to a direct fontdb scan for any other installed font that also covers the character and can actually be outlined, rather than silently giving up. Full workspace test suite: 813 passed, 0 failed (previously 812 passed, 1 failed — this was the one pre-existing failure carried through the entire session). Co-Authored-By: Claude Sonnet 5 --- src/scene/text/ttf_glyph.rs | 106 ++++++++++++++++++++++++++++++------ 1 file changed, 89 insertions(+), 17 deletions(-) diff --git a/src/scene/text/ttf_glyph.rs b/src/scene/text/ttf_glyph.rs index 1d16be722..9fb843855 100644 --- a/src/scene/text/ttf_glyph.rs +++ b/src/scene/text/ttf_glyph.rs @@ -153,8 +153,9 @@ pub fn glyph(family: &str, ch: char) -> Option> { } let built = sysfont::with_face_data(family, |data, index| { - let face = ttf_parser::Face::parse(data, index).ok()?; + let mut face = ttf_parser::Face::parse(data, index).ok()?; let gid = face.glyph_index(ch)?; + pin_default_variations(&mut face); let k = cap_scale(&face); let advance = face.glyph_hor_advance(gid).unwrap_or(0) as f32 * k; @@ -338,11 +339,12 @@ fn build_fallback(ch: char) -> Option> { let script = crate::scene::text::web_font::script_of(ch)?; let bytes = crate::scene::text::web_font::request(script)?; let face_count = ttf_parser::fonts_in_collection(&bytes).unwrap_or(1); - let (face, gid) = (0..face_count).find_map(|face_index| { + let (mut face, gid) = (0..face_count).find_map(|face_index| { let face = ttf_parser::Face::parse(&bytes, face_index).ok()?; let gid = face.glyph_index(ch)?; Some((face, gid)) })?; + pin_default_variations(&mut face); let k = cap_scale(&face); let advance = face.glyph_hor_advance(gid).unwrap_or(0) as f32 * k; let mut fl = OutlineFlattener::new(k); @@ -359,6 +361,46 @@ fn build_fallback(ch: char) -> Option> { })) } +/// A variable font (e.g. macOS's system font, San Francisco — cosmic-text's +/// own fallback search happily picks it for a codepoint no static font +/// nearby covers) needs every variation axis pinned before its outline is +/// readable: `ttf-parser` resolves a variable glyph's contours through its +/// `gvar` deltas relative to whatever coordinates are currently set on the +/// `Face`, and an unset `Face` has none — `outline_glyph` then silently +/// returns `None` (not an error) for such a glyph, which upstream code was +/// treating as "this glyph has no outline" (e.g. a space) rather than "this +/// extraction failed", producing a `Some(Glyph { strokes: vec![], .. })` +/// that renders nothing without ever reporting a problem. Setting each axis +/// to its own default value (`def_value`) reproduces the font's un-varied, +/// default instance — the same shape a non-variable static font would give. +fn pin_default_variations(face: &mut ttf_parser::Face) { + if !face.is_variable() { + return; + } + for axis in face.variation_axes() { + let _ = face.set_variation(axis.tag, axis.def_value); + } +} + +/// Outlines `gid` from an already-parsed, variation-pinned `face` — the +/// shared last step both [`build_fallback`] candidates below use. `None` +/// (not just an empty glyph) whenever `outline_glyph` itself reports no +/// contours, so a caller can tell "this font can't actually render this +/// glyph" apart from "this glyph is legitimately blank" and keep looking — +/// see [`pin_default_variations`]'s doc comment for why a resolved glyph ID +/// doesn't guarantee an outline comes back. +fn outline_from_face(face: &ttf_parser::Face, gid: ttf_parser::GlyphId, k: f32) -> Option> { + let advance = face.glyph_hor_advance(gid).unwrap_or(0) as f32 * k; + let mut fl = OutlineFlattener::new(k); + face.outline_glyph(gid, &mut fl); + fl.flush(); + if fl.contours.is_empty() { + return None; + } + let fill_tris = triangulate_contours(&fl.contours); + Some(Arc::new(Glyph { strokes: fl.contours, advance, fill_tris })) +} + #[cfg(not(target_arch = "wasm32"))] fn build_fallback(ch: char) -> Option> { use cosmic_text::{Attrs, Buffer, Metrics, Shaping}; @@ -371,26 +413,54 @@ fn build_fallback(ch: char) -> Option> { buf.set_text(&mut fs, &s, &attrs, Shaping::Advanced, None); buf.shape_until_scroll(&mut fs, false); + let mut tried: Vec = Vec::new(); for run in buf.layout_runs() { for g in run.glyphs.iter() { if g.glyph_id == 0 { continue; // .notdef — this font doesn't really cover it } + tried.push(g.font_id); let face_index = fs.db_mut().face(g.font_id).map(|f| f.index).unwrap_or(0); - let font = fs.get_font(g.font_id, g.font_weight)?; - let face = ttf_parser::Face::parse(font.data(), face_index).ok()?; + let Some(font) = fs.get_font(g.font_id, g.font_weight) else { continue }; + let Ok(mut face) = ttf_parser::Face::parse(font.data(), face_index) else { continue }; + pin_default_variations(&mut face); let k = cap_scale(&face); - let gid = ttf_parser::GlyphId(g.glyph_id); - let advance = face.glyph_hor_advance(gid).unwrap_or(0) as f32 * k; - let mut fl = OutlineFlattener::new(k); - face.outline_glyph(gid, &mut fl); - fl.flush(); - let fill_tris = triangulate_contours(&fl.contours); - return Some(Arc::new(Glyph { - strokes: fl.contours, - advance, - fill_tris, - })); + if let Some(glyph) = outline_from_face(&face, ttf_parser::GlyphId(g.glyph_id), k) { + return Some(glyph); + } + // cosmic-text resolved a real glyph here, but this font's outline + // couldn't actually be extracted — confirmed (design doc-worthy, + // not a guess) on this dev machine for macOS's system font + // (San Francisco): it's a variable TrueType font whose `gvar` + // outline path needs base `glyf` glyph data `ttf-parser` can't + // retrieve for at least some of its accented Latin glyphs, even + // after every variation axis is pinned to its default. Rather + // than silently rendering nothing, fall through to a direct + // database scan below for any other installed font that also + // covers `ch` and can actually be outlined. + } + } + + // Second-chance search: cosmic-text's own fallback chose one font (and + // it failed to outline above); look for a different installed font that + // also covers `ch`, skipping whatever was already tried. Bounded by the + // database size, not attempted per keystroke — `fallback_glyph`'s own + // cache means this only runs once per distinct character ever asked + // for. + let candidates: Vec<(fontdb::ID, u32)> = fs + .db() + .faces() + .filter(|f| !tried.contains(&f.id)) + .map(|f| (f.id, f.index)) + .collect(); + for (id, index) in candidates { + let Some(font) = fs.get_font(id, cosmic_text::fontdb::Weight::NORMAL) else { continue }; + let Ok(mut face) = ttf_parser::Face::parse(font.data(), index) else { continue }; + let Some(gid) = face.glyph_index(ch) else { continue }; + pin_default_variations(&mut face); + let k = cap_scale(&face); + if let Some(glyph) = outline_from_face(&face, gid, k) { + return Some(glyph); } } None @@ -456,9 +526,10 @@ fn build_shaped(_family: &str, text: &str) -> Option { let Some(font) = font_system.get_font(glyph.font_id, glyph.font_weight) else { continue; }; - let Ok(face) = ttf_parser::Face::parse(font.data(), face_index) else { + let Ok(mut face) = ttf_parser::Face::parse(font.data(), face_index) else { continue; }; + pin_default_variations(&mut face); let scale = (SHAPE_FS / face.units_per_em() as f32) * px_to_9; let pen_x = (glyph.x + SHAPE_FS * glyph.x_offset) * px_to_9; let pen_y = -(SHAPE_FS * glyph.y_offset) * px_to_9; @@ -525,9 +596,10 @@ fn build_shaped(family: &str, text: &str) -> Option { let Some(font) = fs.get_font(g.font_id, g.font_weight) else { continue; }; - let Ok(face) = ttf_parser::Face::parse(font.data(), face_index) else { + let Ok(mut face) = ttf_parser::Face::parse(font.data(), face_index) else { continue; }; + pin_default_variations(&mut face); let upem_g = face.units_per_em() as f32; // Glyph font-units → 9-unit: to pixels (at SHAPE_FS) then to units. let scale_g = (SHAPE_FS / upem_g) * px_to_9;