Don't copy Glyph on GlyphCache hit

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);
  }
  }
  }