Font fallback (#334)

* thanks nganhkhoa; should solve issue-327

* save work

* intended to address issue-332 among other font problems; added some tests for caching_shaper; clippy

* remove nightly feature

* choose random font instead

* add droid font to workflow linux

* switch to sans mono

* switch font

* cleaner random font implementation

Cargo.toml

@@ -7,7 +7,7 @@ build = "build.rs"
 description = "A simple GUI for Neovim."
 
 [features]
-default = ["embed-fonts"]
+default = []
 embed-fonts = []
 
 [dependencies]
@@ -32,10 +32,10 @@ parking_lot="0.10.0"
 cfg-if = "0.1.10"
 which = "4"
 dirs = "2"
+rand = "0.7"
 
 [dev-dependencies]
 mockall = "0.7.0"
-rand = "0.7"
 
 [dev-dependencies.cargo-husky]
 version = "1"

src/bridge/ui_commands.rs

@@ -83,9 +83,6 @@ impl UiCommand {
     }
 
     pub fn is_resize(&self) -> bool {
-        match self {
+        matches!(self, UiCommand::Resize { .. })
-            UiCommand::Resize { .. } => true,
-            _ => false,
-        }
     }
 }

src/renderer/caching_shaper.rs

@@ -1,7 +1,7 @@
 use cfg_if::cfg_if as define;
 use font_kit::{
+    family_handle::FamilyHandle,
     font::Font,
-    handle::Handle,
     metrics::Metrics,
     properties::{Properties, Stretch, Style, Weight},
     source::SystemSource,
@@ -14,6 +14,8 @@ use skulpin::skia_safe::{Data, Font as SkiaFont, TextBlob, TextBlobBuilder, Type
 use std::collections::HashMap;
 use std::iter;
 
+use rand::Rng;
+
 use super::font_options::FontOptions;
 
 const STANDARD_CHARACTER_STRING: &str =
@@ -25,7 +27,7 @@ define! {
         const SYSTEM_SYMBOL_FONT: &str = "Segoe UI Symbol";
         const SYSTEM_EMOJI_FONT: &str = "Segoe UI Emoji";
     } else if #[cfg(target_os = "linux")] {
-        const SYSTEM_DEFAULT_FONT: &str = "Droid Sans Mono";
+        const SYSTEM_DEFAULT_FONT: &str = "Noto Sans Mono";
         const SYSTEM_SYMBOL_FONT: &str = "Noto Sans Mono";
         const SYSTEM_EMOJI_FONT: &str = "Noto Color Emoji";
     } else if #[cfg(target_os = "macos")] {
@@ -38,7 +40,7 @@ define! {
 const EXTRA_SYMBOL_FONT: &str = "Extra Symbols.otf";
 const MISSING_GLYPH_FONT: &str = "Missing Glyphs.otf";
 
-#[cfg(feature = "embed-fonts")]
+#[cfg(any(feature = "embed-fonts", test))]
 #[derive(RustEmbed)]
 #[folder = "assets/fonts/"]
 struct Asset;
@@ -50,6 +52,12 @@ pub struct ExtendedFontFamily {
     pub fonts: Vec<SkriboFont>,
 }
 
+impl Default for ExtendedFontFamily {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl ExtendedFontFamily {
     pub fn new() -> ExtendedFontFamily {
         ExtendedFontFamily { fonts: Vec::new() }
@@ -60,6 +68,7 @@ impl ExtendedFontFamily {
     }
 
     pub fn get(&self, props: Properties) -> Option<&Font> {
+        if let Some(first_handle) = &self.fonts.first() {
             for handle in &self.fonts {
                 let font = &handle.font;
                 let properties = font.properties();
@@ -69,39 +78,43 @@ impl ExtendedFontFamily {
                 }
             }
 
-        if let Some(handle) = &self.fonts.first() {
+            return Some(&first_handle.font);
-            return Some(&handle.font);
         }
 
         None
     }
+}
 
-    pub fn from_normal_font_family(fonts: &[Handle]) -> ExtendedFontFamily {
+impl From<FamilyHandle> for ExtendedFontFamily {
-        let mut family = ExtendedFontFamily::new();
+    fn from(handle: FamilyHandle) -> Self {
-
+        handle
-        for font in fonts.iter() {
+            .fonts()
+            .iter()
+            .fold(ExtendedFontFamily::new(), |mut family, font| {
                 if let Ok(font) = font.load() {
                     family.add_font(SkriboFont::new(font));
                 }
-        }
-
                 family
+            })
     }
-
-    pub fn to_normal_font_family(&self) -> FontFamily {
-        let mut new_family = FontFamily::new();
-
-        for font in &self.fonts {
-            new_family.add_font(font.clone());
 }
 
+impl From<ExtendedFontFamily> for FontFamily {
+    fn from(extended_font_family: ExtendedFontFamily) -> Self {
+        extended_font_family
+            .fonts
+            .iter()
+            .fold(FontFamily::new(), |mut new_family, font| {
+                new_family.add_font(font.clone());
                 new_family
+            })
     }
 }
 
 pub struct FontLoader {
     cache: LruCache<String, ExtendedFontFamily>,
     source: SystemSource,
+    random_font_name: Option<String>,
 }
 
 impl FontLoader {
@@ -109,6 +122,7 @@ impl FontLoader {
         FontLoader {
             cache: LruCache::new(10),
             source: SystemSource::new(),
+            random_font_name: None,
         }
     }
 
@@ -116,20 +130,22 @@ impl FontLoader {
         self.cache.get(&String::from(font_name)).cloned()
     }
 
-    #[cfg(feature = "embed-fonts")]
+    #[cfg(any(feature = "embed-fonts", test))]
     fn load_from_asset(&mut self, font_name: &str) -> Option<ExtendedFontFamily> {
         let mut family = ExtendedFontFamily::new();
 
         if let Some(font) = Asset::get(font_name)
             .and_then(|font_data| Font::from_bytes(font_data.to_vec().into(), 0).ok())
         {
-            family.add_font(SkriboFont::new(font))
+            family.add_font(SkriboFont::new(font));
-        }
             self.cache.put(String::from(font_name), family);
             self.get(font_name)
+        } else {
+            None
+        }
     }
 
-    #[cfg(not(feature = "embed-fonts"))]
+    #[cfg(not(any(feature = "embed-fonts", test)))]
     fn load_from_asset(&self, font_name: &str) -> Option<ExtendedFontFamily> {
         warn!(
             "Tried to load {} from assets but build didn't include embed-fonts feature",
@@ -144,8 +160,8 @@ impl FontLoader {
             _ => return None,
         };
 
-        let family = ExtendedFontFamily::from_normal_font_family(handle.fonts());
+        if !handle.is_empty() {
-        if !family.fonts.is_empty() {
+            let family = ExtendedFontFamily::from(handle);
             self.cache.put(String::from(font_name), family);
             self.get(font_name)
         } else {
@@ -153,6 +169,18 @@ impl FontLoader {
         }
     }
 
+    fn get_random_system_font_family(&mut self) -> Option<ExtendedFontFamily> {
+        if let Some(font) = self.random_font_name.clone() {
+            self.get(&font)
+        } else {
+            let font_names = self.source.all_families().expect("fonts exist");
+            let n = rand::thread_rng().gen::<usize>() % font_names.len();
+            let font_name = &font_names[n];
+            self.random_font_name = Some(font_name.clone());
+            self.load(&font_name)
+        }
+    }
+
     pub fn get_or_load(&mut self, font_name: &str) -> Option<ExtendedFontFamily> {
         if let Some(cached) = self.get(font_name) {
             Some(cached)
@@ -162,41 +190,23 @@ impl FontLoader {
             self.load_from_asset(font_name)
         }
     }
-}
-
-#[derive(new, Clone, Hash, PartialEq, Eq, Debug)]
-struct ShapeKey {
-    pub text: String,
-    pub bold: bool,
-    pub italic: bool,
-}
 
     pub fn build_collection_by_font_name(
-    loader: &mut FontLoader,
+        &mut self,
         fallback_list: &[String],
-    bold: bool,
+        properties: Properties,
-    italic: bool,
     ) -> FontCollection {
         let mut collection = FontCollection::new();
 
-    let weight = if bold { Weight::BOLD } else { Weight::NORMAL };
-    let style = if italic { Style::Italic } else { Style::Normal };
-    let properties = Properties {
-        weight,
-        style,
-        stretch: Stretch::NORMAL,
-    };
-
         let gui_fonts = fallback_list
             .iter()
             .map(|fallback_item| fallback_item.as_ref())
             .chain(iter::once(SYSTEM_DEFAULT_FONT));
 
         for font_name in gui_fonts {
-        if let Some(family) = loader.get_or_load(font_name) {
+            if let Some(family) = self.get_or_load(font_name) {
                 if let Some(font) = family.get(properties) {
                     collection.add_family(FontFamily::new_from_font(font.clone()));
-                break;
                 }
             }
         }
@@ -207,13 +217,35 @@ pub fn build_collection_by_font_name(
             EXTRA_SYMBOL_FONT,
             MISSING_GLYPH_FONT,
         ] {
-        if let Some(family) = loader.get_or_load(font) {
+            if let Some(family) = self.get_or_load(font) {
-            collection.add_family(family.to_normal_font_family());
+                collection.add_family(FontFamily::from(family));
             }
         }
 
+        if self.cache.is_empty() {
+            let font_family = self.get_random_system_font_family();
+            collection.add_family(FontFamily::from(font_family.expect("font family loaded")));
+        }
         collection
     }
+}
+
+#[derive(new, Clone, Hash, PartialEq, Eq, Debug)]
+struct ShapeKey {
+    pub text: String,
+    pub bold: bool,
+    pub italic: bool,
+}
+
+pub fn build_properties(bold: bool, italic: bool) -> Properties {
+    let weight = if bold { Weight::BOLD } else { Weight::NORMAL };
+    let style = if italic { Style::Italic } else { Style::Normal };
+    Properties {
+        weight,
+        style,
+        stretch: Stretch::NORMAL,
+    }
+}
 
 struct FontSet {
     normal: FontCollection,
@@ -222,11 +254,14 @@ struct FontSet {
 }
 
 impl FontSet {
-    fn new(fallback_list: &[String], mut loader: &mut FontLoader) -> FontSet {
+    fn new(fallback_list: &[String], loader: &mut FontLoader) -> FontSet {
         FontSet {
-            normal: build_collection_by_font_name(&mut loader, fallback_list, false, false),
+            normal: loader
-            bold: build_collection_by_font_name(&mut loader, fallback_list, true, false),
+                .build_collection_by_font_name(fallback_list, build_properties(false, false)),
-            italic: build_collection_by_font_name(&mut loader, fallback_list, false, true),
+            bold: loader
+                .build_collection_by_font_name(fallback_list, build_properties(true, false)),
+            italic: loader
+                .build_collection_by_font_name(fallback_list, build_properties(false, true)),
         }
     }
 
@@ -272,7 +307,6 @@ impl CachingShaper {
 
     fn get_skia_font(&mut self, skribo_font: &SkriboFont) -> Option<&SkiaFont> {
         let font_name = skribo_font.font.postscript_name()?;
-
         if !self.font_cache.contains(&font_name) {
             let font = build_skia_font_from_skribo_font(skribo_font, self.options.size)?;
             self.font_cache.put(font_name.clone(), font);
@@ -296,7 +330,6 @@ impl CachingShaper {
         let style = TextStyle {
             size: self.options.size,
         };
-
         let session = LayoutSession::create(text, &style, &self.font_set.get(bold, italic));
         let metrics = self.metrics();
         let ascent = metrics.ascent * self.options.size / metrics.units_per_em as f32;
@@ -383,3 +416,155 @@ impl CachingShaper {
         -metrics.underline_position * self.options.size / metrics.units_per_em as f32
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    const PROPERTIES1: Properties = Properties {
+        weight: Weight::NORMAL,
+        style: Style::Normal,
+        stretch: Stretch::NORMAL,
+    };
+
+    const PROPERTIES2: Properties = Properties {
+        weight: Weight::BOLD,
+        style: Style::Normal,
+        stretch: Stretch::NORMAL,
+    };
+
+    const PROPERTIES3: Properties = Properties {
+        weight: Weight::NORMAL,
+        style: Style::Italic,
+        stretch: Stretch::NORMAL,
+    };
+
+    const PROPERTIES4: Properties = Properties {
+        weight: Weight::BOLD,
+        style: Style::Italic,
+        stretch: Stretch::NORMAL,
+    };
+
+    fn dummy_font() -> SkriboFont {
+        SkriboFont::new(
+            Asset::get(EXTRA_SYMBOL_FONT)
+                .and_then(|font_data| Font::from_bytes(font_data.to_vec().into(), 0).ok())
+                .unwrap(),
+        )
+    }
+
+    #[test]
+    fn test_build_properties() {
+        assert_eq!(build_properties(false, false), PROPERTIES1);
+        assert_eq!(build_properties(true, false), PROPERTIES2);
+        assert_eq!(build_properties(false, true), PROPERTIES3);
+        assert_eq!(build_properties(true, true), PROPERTIES4);
+    }
+
+    mod extended_font_family {
+        use super::*;
+
+        #[test]
+        fn test_add_font() {
+            let mut eft = ExtendedFontFamily::new();
+            let font = dummy_font();
+            eft.add_font(font.clone());
+            assert_eq!(
+                eft.fonts.first().unwrap().font.full_name(),
+                font.font.full_name()
+            );
+        }
+
+        #[test]
+        fn test_get() {
+            let mut eft = ExtendedFontFamily::new();
+            assert!(eft.get(PROPERTIES1).is_none());
+
+            let font = dummy_font();
+            eft.fonts.push(font.clone());
+            assert_eq!(
+                eft.get(font.font.properties()).unwrap().full_name(),
+                font.font.full_name()
+            );
+        }
+    }
+
+    mod font_loader {
+        use super::*;
+
+        #[test]
+        fn test_load_from_asset() {
+            let mut loader = FontLoader::new();
+
+            let font_family = loader.load_from_asset("");
+            assert!(font_family.is_none());
+
+            let font = dummy_font();
+            let mut eft = ExtendedFontFamily::new();
+            eft.add_font(font.clone());
+            let font_family = loader.load_from_asset(EXTRA_SYMBOL_FONT);
+            let result = font_family.unwrap().fonts.first().unwrap().font.full_name();
+            assert_eq!(&result, &eft.fonts.first().unwrap().font.full_name());
+
+            assert_eq!(
+                &result,
+                &loader
+                    .cache
+                    .get(&EXTRA_SYMBOL_FONT.to_string())
+                    .unwrap()
+                    .fonts
+                    .first()
+                    .unwrap()
+                    .font
+                    .full_name()
+            );
+        }
+
+        #[test]
+        fn test_load() {
+            let mut loader = FontLoader::new();
+            let junk_text = "uhasiudhaiudshiaushd";
+            let font_family = loader.load(junk_text);
+            assert!(font_family.is_none());
+
+            #[cfg(target_os = "linux")]
+            const SYSTEM_DEFAULT_FONT: &str = "DejaVu Serif";
+
+            let font_family = loader.load(SYSTEM_DEFAULT_FONT);
+            let result = font_family.unwrap().fonts.first().unwrap().font.full_name();
+            assert_eq!(
+                &result,
+                &loader
+                    .cache
+                    .get(&SYSTEM_DEFAULT_FONT.to_string())
+                    .unwrap()
+                    .fonts
+                    .first()
+                    .unwrap()
+                    .font
+                    .full_name()
+            );
+        }
+
+        #[test]
+        fn test_get_random_system_font() {
+            let mut loader = FontLoader::new();
+
+            let font_family = loader.get_random_system_font_family();
+            let font_name = loader.random_font_name.unwrap();
+            let result = font_family.unwrap().fonts.first().unwrap().font.full_name();
+            assert_eq!(
+                &result,
+                &loader
+                    .cache
+                    .get(&font_name)
+                    .unwrap()
+                    .fonts
+                    .first()
+                    .unwrap()
+                    .font
+                    .full_name()
+            );
+        }
+    }
+}

src/renderer/cursor_renderer/mod.rs

@@ -293,10 +293,7 @@ impl CursorRenderer {
                 _ => font_width,
             };
 
-            let in_insert_mode = match editor.current_mode {
+            let in_insert_mode = matches!(editor.current_mode, EditorMode::Insert);
-                EditorMode::Insert => true,
-                _ => false,
-            };
 
             (character, (font_width, font_height).into(), in_insert_mode)
         };

src/window.rs

@@ -77,7 +77,7 @@ pub fn window_geometry() -> Result<(u64, u64), String> {
                 .map(|dimension| {
                     dimension
                         .parse::<u64>()
-                        .or_else(|_| Err(invalid_parse_err.as_str()))
+                        .map_err(|_| invalid_parse_err.as_str())
                         .and_then(|dimension| {
                             if dimension > 0 {
                                 Ok(dimension)