`TextStyle::default()` ask system for known existing font family (#2542)

Julia created 2 years ago

Rather than assuming a specific family exists, try a set of specific
names and if they fail, just grab any old font that the system reports
as existing

Closes
https://linear.app/zed-industries/issue/Z-445/thread-main-panicked-at-called-resultunwrap-on-an-err-value-could-not

Release Notes:

* Fixed crash that could happen if system did not have a specific
fallback font

Change summary

crates/gpui/src/font_cache.rs         | 52 ++++++++++++++++++++++------
crates/gpui/src/fonts.rs              | 11 +++--
crates/gpui/src/platform.rs           |  1 
crates/gpui/src/platform/mac/fonts.rs |  8 ++++
4 files changed, 56 insertions(+), 16 deletions(-)

Detailed changes

crates/gpui/src/font_cache.rs

@@ -25,8 +25,9 @@ struct Family {
 pub struct FontCache(RwLock<FontCacheState>);
 
 pub struct FontCacheState {
-    fonts: Arc<dyn platform::FontSystem>,
+    font_system: Arc<dyn platform::FontSystem>,
     families: Vec<Family>,
+    default_family: Option<FamilyId>,
     font_selections: HashMap<FamilyId, HashMap<Properties, FontId>>,
     metrics: HashMap<FontId, Metrics>,
     wrapper_pool: HashMap<(FontId, OrderedFloat<f32>), Vec<LineWrapper>>,
@@ -42,8 +43,9 @@ unsafe impl Send for FontCache {}
 impl FontCache {
     pub fn new(fonts: Arc<dyn platform::FontSystem>) -> Self {
         Self(RwLock::new(FontCacheState {
-            fonts,
+            font_system: fonts,
             families: Default::default(),
+            default_family: None,
             font_selections: Default::default(),
             metrics: Default::default(),
             wrapper_pool: Default::default(),
@@ -73,14 +75,14 @@ impl FontCache {
 
             let mut state = RwLockUpgradableReadGuard::upgrade(state);
 
-            if let Ok(font_ids) = state.fonts.load_family(name, features) {
+            if let Ok(font_ids) = state.font_system.load_family(name, features) {
                 if font_ids.is_empty() {
                     continue;
                 }
 
                 let family_id = FamilyId(state.families.len());
                 for font_id in &font_ids {
-                    if state.fonts.glyph_for_char(*font_id, 'm').is_none() {
+                    if state.font_system.glyph_for_char(*font_id, 'm').is_none() {
                         return Err(anyhow!("font must contain a glyph for the 'm' character"));
                     }
                 }
@@ -99,6 +101,31 @@ impl FontCache {
         ))
     }
 
+    /// Returns an arbitrary font family that is available on the system.
+    pub fn known_existing_family(&self) -> FamilyId {
+        if let Some(family_id) = self.0.read().default_family {
+            return family_id;
+        }
+
+        let default_family = self
+            .load_family(
+                &["Courier", "Helvetica", "Arial", "Verdana"],
+                &Default::default(),
+            )
+            .unwrap_or_else(|_| {
+                let all_family_names = self.0.read().font_system.all_families();
+                let all_family_names: Vec<_> = all_family_names
+                    .iter()
+                    .map(|string| string.as_str())
+                    .collect();
+                self.load_family(&all_family_names, &Default::default())
+                    .expect("could not load any default font family")
+            });
+
+        self.0.write().default_family = Some(default_family);
+        default_family
+    }
+
     pub fn default_font(&self, family_id: FamilyId) -> FontId {
         self.select_font(family_id, &Properties::default()).unwrap()
     }
@@ -115,7 +142,7 @@ impl FontCache {
             let mut inner = RwLockUpgradableReadGuard::upgrade(inner);
             let family = &inner.families[family_id.0];
             let font_id = inner
-                .fonts
+                .font_system
                 .select_font(&family.font_ids, properties)
                 .unwrap_or(family.font_ids[0]);
 
@@ -137,7 +164,7 @@ impl FontCache {
         if let Some(metrics) = state.metrics.get(&font_id) {
             f(metrics)
         } else {
-            let metrics = state.fonts.font_metrics(font_id);
+            let metrics = state.font_system.font_metrics(font_id);
             let metric = f(&metrics);
             let mut state = RwLockUpgradableReadGuard::upgrade(state);
             state.metrics.insert(font_id, metrics);
@@ -157,8 +184,11 @@ impl FontCache {
         let bounds;
         {
             let state = self.0.read();
-            glyph_id = state.fonts.glyph_for_char(font_id, 'm').unwrap();
-            bounds = state.fonts.typographic_bounds(font_id, glyph_id).unwrap();
+            glyph_id = state.font_system.glyph_for_char(font_id, 'm').unwrap();
+            bounds = state
+                .font_system
+                .typographic_bounds(font_id, glyph_id)
+                .unwrap();
         }
         bounds.width() * self.em_scale(font_id, font_size)
     }
@@ -168,8 +198,8 @@ impl FontCache {
         let advance;
         {
             let state = self.0.read();
-            glyph_id = state.fonts.glyph_for_char(font_id, 'm').unwrap();
-            advance = state.fonts.advance(font_id, glyph_id).unwrap();
+            glyph_id = state.font_system.glyph_for_char(font_id, 'm').unwrap();
+            advance = state.font_system.advance(font_id, glyph_id).unwrap();
         }
         advance.x() * self.em_scale(font_id, font_size)
     }
@@ -214,7 +244,7 @@ impl FontCache {
             .or_default();
         let wrapper = wrappers
             .pop()
-            .unwrap_or_else(|| LineWrapper::new(font_id, font_size, state.fonts.clone()));
+            .unwrap_or_else(|| LineWrapper::new(font_id, font_size, state.font_system.clone()));
         LineWrapperHandle {
             wrapper: Some(wrapper),
             font_cache: self.clone(),

crates/gpui/src/fonts.rs

@@ -295,13 +295,14 @@ impl Default for TextStyle {
                 .as_ref()
                 .expect("TextStyle::default can only be called within a call to with_font_cache");
 
-            let font_family_name = Arc::from("Courier");
-            let font_family_id = font_cache
-                .load_family(&[&font_family_name], &Default::default())
-                .unwrap();
+            let font_family_id = font_cache.known_existing_family();
             let font_id = font_cache
                 .select_font(font_family_id, &Default::default())
-                .unwrap();
+                .expect("did not have any font in system-provided family");
+            let font_family_name = font_cache
+                .family_name(font_family_id)
+                .expect("we loaded this family from the font cache, so this should work");
+
             Self {
                 color: Default::default(),
                 font_family_name,

crates/gpui/src/platform.rs

@@ -343,6 +343,7 @@ pub enum RasterizationOptions {
 
 pub trait FontSystem: Send + Sync {
     fn add_fonts(&self, fonts: &[Arc<Vec<u8>>]) -> anyhow::Result<()>;
+    fn all_families(&self) -> Vec<String>;
     fn load_family(&self, name: &str, features: &FontFeatures) -> anyhow::Result<Vec<FontId>>;
     fn select_font(
         &self,

crates/gpui/src/platform/mac/fonts.rs

@@ -66,6 +66,14 @@ impl platform::FontSystem for FontSystem {
         self.0.write().add_fonts(fonts)
     }
 
+    fn all_families(&self) -> Vec<String> {
+        self.0
+            .read()
+            .system_source
+            .all_families()
+            .expect("core text should never return an error")
+    }
+
     fn load_family(&self, name: &str, features: &Features) -> anyhow::Result<Vec<FontId>> {
         self.0.write().load_family(name, features)
     }