Don't copy Glyph on GlyphCache hit

The Glyph is being passed around by reference, so doing the copy is
not desired.

alacritty/src/renderer/text/glyph_cache.rs

@@ -1,3 +1,4 @@
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::hash::BuildHasherDefault;
 
@@ -110,7 +111,14 @@ impl GlyphCache {
 
         // Cache all ascii characters.
         for i in 32u8..=126u8 {
-            self.get(GlyphKey { font_key: font, character: i as char, size }, loader, true);
+            let glyph_key = GlyphKey { font_key: font, character: i as char, size };
+            let missing_glyph = match self.get(glyph_key, loader).map_err(|err| *err) {
+                Ok(_) => continue,
+                Err(RasterizerError::MissingGlyph(glyph)) => Some(glyph),
+                _ => None,
+            };
+
+            let _ = self.insert_missing_or_default(glyph_key, loader, missing_glyph);
         }
     }
 
@@ -192,72 +200,92 @@ impl GlyphCache {
         &mut self,
         glyph_key: GlyphKey,
         loader: &mut L,
-        show_missing: bool,
-    ) -> Glyph
+    ) -> Result<&Glyph, Box<RasterizerError>>
     where
         L: LoadGlyph,
     {
         // Try to load glyph from cache.
-        if let Some(glyph) = self.cache.get(&glyph_key) {
-            return *glyph;
-        };
+        match self.cache.entry(glyph_key) {
+            Entry::Occupied(glyph) => Ok(glyph.into_mut()),
+            Entry::Vacant(vacant) => {
+                // // Rasterize the glyph using the built-in font for special characters or the
+                // user's font // for everything else.
+                let rasterized = self
+                    .builtin_box_drawing
+                    .then(|| {
+                        builtin_font::builtin_glyph(
+                            glyph_key.character,
+                            &self.metrics,
+                            &self.font_offset,
+                            &self.glyph_offset,
+                        )
+                    })
+                    .flatten()
+                    .map_or_else(|| self.rasterizer.get_glyph(glyph_key), Ok)?;
+
+                let glyph = Self::load_glyph(loader, rasterized, &self.glyph_offset, &self.metrics);
+
+                // Cache rasterized glyph.
+                Ok(vacant.insert(glyph))
+            },
+        }
+    }
 
-        // Rasterize the glyph using the built-in font for special characters or the user's font
-        // for everything else.
-        let rasterized = self
-            .builtin_box_drawing
-            .then(|| {
-                builtin_font::builtin_glyph(
-                    glyph_key.character,
-                    &self.metrics,
-                    &self.font_offset,
-                    &self.glyph_offset,
-                )
-            })
-            .flatten()
-            .map_or_else(|| self.rasterizer.get_glyph(glyph_key), Ok);
-
-        let glyph = match rasterized {
-            Ok(rasterized) => self.load_glyph(loader, rasterized),
-            // Load fallback glyph.
-            Err(RasterizerError::MissingGlyph(rasterized)) if show_missing => {
-                // Use `\0` as "missing" glyph to cache it only once.
+    /// Insert the `missing_glyph` for the given `glyph_key` or the default glyph.
+    pub fn insert_missing_or_default<L: ?Sized>(
+        &mut self,
+        glyph_key: GlyphKey,
+        loader: &mut L,
+        missing_glyph: Option<RasterizedGlyph>,
+    ) -> &Glyph
+    where
+        L: LoadGlyph,
+    {
+        let to_insert = match missing_glyph {
+            Some(missing_glyph) => {
                 let missing_key = GlyphKey { character: '\0', ..glyph_key };
-                if let Some(glyph) = self.cache.get(&missing_key) {
-                    *glyph
-                } else {
-                    // If no missing glyph was loaded yet, insert it as `\0`.
-                    let glyph = self.load_glyph(loader, rasterized);
-                    self.cache.insert(missing_key, glyph);
-
-                    glyph
+                *match self.cache.entry(missing_key) {
+                    Entry::Occupied(glyph) => glyph.into_mut(),
+                    Entry::Vacant(glyph) => glyph.insert(Self::load_glyph(
+                        loader,
+                        missing_glyph,
+                        &self.glyph_offset,
+                        &self.metrics,
+                    )),
                 }
             },
-            Err(_) => self.load_glyph(loader, Default::default()),
+            None => Self::load_glyph(loader, Default::default(), &self.glyph_offset, &self.metrics),
         };
 
-        // Cache rasterized glyph.
-        *self.cache.entry(glyph_key).or_insert(glyph)
+        match self.cache.entry(glyph_key) {
+            Entry::Occupied(glyph) => glyph.into_mut(),
+            Entry::Vacant(glyph) => glyph.insert(to_insert),
+        }
     }
 
     /// Load glyph into the atlas.
     ///
     /// This will apply all transforms defined for the glyph cache to the rasterized glyph before
-    pub fn load_glyph<L: ?Sized>(&self, loader: &mut L, mut glyph: RasterizedGlyph) -> Glyph
+    pub fn load_glyph<L: ?Sized>(
+        loader: &mut L,
+        mut glyph: RasterizedGlyph,
+        glyph_offset: &Delta<i8>,
+        metrics: &Metrics,
+    ) -> Glyph
     where
         L: LoadGlyph,
     {
-        glyph.left += i32::from(self.glyph_offset.x);
-        glyph.top += i32::from(self.glyph_offset.y);
-        glyph.top -= self.metrics.descent as i32;
+        glyph.left += i32::from(glyph_offset.x);
+        glyph.top += i32::from(glyph_offset.y);
+        glyph.top -= metrics.descent as i32;
 
         // The metrics of zero-width characters are based on rendering
         // the character after the current cell, with the anchor at the
         // right side of the preceding character. Since we render the
         // zero-width characters inside the preceding character, the
         // anchor has been moved to the right by one cell.
         if glyph.character.width() == Some(0) {
-            glyph.left += self.metrics.average_advance as i32;
+            glyph.left += metrics.average_advance as i32;
         }
 
         // Add glyph to cache.

alacritty/src/renderer/text/mod.rs

@@ -1,5 +1,5 @@
 use bitflags::bitflags;
-use crossfont::{GlyphKey, RasterizedGlyph};
+use crossfont::{Error as RasterizerError, GlyphKey, RasterizedGlyph};
 
 use alacritty_terminal::term::cell::Flags;
 
@@ -156,17 +156,32 @@ pub trait TextRenderApi<T: TextRenderBatch>: LoadGlyph {
             GlyphKey { font_key, size: glyph_cache.font_size, character: cell.character };
 
         // Add cell to batch.
-        let glyph = glyph_cache.get(glyph_key, self, true);
-        self.add_render_item(&cell, &glyph, size_info);
+        let glyph = match glyph_cache.get(glyph_key, self) {
+            Ok(glyph) => glyph,
+            Err(error) => {
+                let missing_glyph = if let RasterizerError::MissingGlyph(missing_glyph) = *error {
+                    Some(missing_glyph)
+                } else {
+                    None
+                };
+                glyph_cache.insert_missing_or_default(glyph_key, self, missing_glyph)
+            },
+        };
+
+        self.add_render_item(&cell, glyph, size_info);
 
         // Render visible zero-width characters.
         if let Some(zerowidth) =
             cell.extra.as_mut().and_then(|extra| extra.zerowidth.take().filter(|_| !hidden))
         {
             for character in zerowidth {
                 glyph_key.character = character;
-                let glyph = glyph_cache.get(glyph_key, self, false);
-                self.add_render_item(&cell, &glyph, size_info);
+                // Ignore the rendering errors for zerowidth to not obscure content.
+                let glyph = match glyph_cache.get(glyph_key, self) {
+                    Ok(glyph) => glyph,
+                    Err(_) => glyph_cache.insert_missing_or_default(glyph_key, self, None),
+                };
+                self.add_render_item(&cell, glyph, size_info);
             }
         }
     }