Hide "missing" glyp for zerowidth character

This patch prevents missing zerowidth glyphs from obscuring the rendered
glyph of a cell.

The missing glyph itself is also consistently loaded and displayed on
all platforms. It is initialized once together with the ascii symbols
and then written to the atlas only once for every cached missing glyph.

Co-authored-by: Christian Duerr <contact@christianduerr.com>

5 files changed, 125 insertions, 82 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d9701053..63c7c8ba 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - Nonexistent config imports are ignored instead of raising an error
 - Value for disabling logging with `config.log_level` is `Off` instead of `None`
+- Missing glyph symbols are no longer drawn for zerowidth characters
 
 ### Fixed
 
@@ -43,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Alacritty not discarding invalid escape sequences starting with ESC
 - Crash due to clipboard not being properly released on Wayland
 - Shadow artifacts when resizing transparent windows on macOS
+- Missing glyph symbols not being rendered for missing glyphs on macOS and Windows
 
 ### Removed
 
diff --git a/Cargo.lock b/Cargo.lock
index a520832a..3f6b40a7 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -519,9 +519,9 @@ dependencies = [
 
 [[package]]
 name = "crossfont"
-version = "0.1.1"
+version = "0.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6f4b217779694a81b60a4174f121697c9bd7550cb9a6d7e970051bea1b13dbc0"
+checksum = "108946cf5228893bf470564410639b87a53f959d937f1c57bbc2a6603d98e812"
 dependencies = [
  "cocoa 0.24.0",
  "core-foundation 0.9.1",
@@ -529,7 +529,6 @@ dependencies = [
  "core-graphics 0.22.1",
  "core-text",
  "dwrote",
- "euclid",
  "foreign-types 0.5.0",
  "freetype-rs",
  "libc",
@@ -671,15 +670,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "euclid"
-version = "0.22.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5337024b8293bdce5265dc9570ef6e608a34bfacbbc87fe1a5dcb5f1dac2f4e2"
-dependencies = [
- "num-traits",
-]
-
-[[package]]
 name = "expat-sys"
 version = "2.1.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1328,15 +1318,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "num-traits"
-version = "0.2.12"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ac267bcc07f48ee5f8935ab0d24f316fb722d7a1292e2913f0cc196b29ffd611"
-dependencies = [
- "autocfg",
-]
-
-[[package]]
 name = "num_enum"
 version = "0.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/alacritty/Cargo.toml b/alacritty/Cargo.toml
index 4810ced4..f80e96c2 100644
--- a/alacritty/Cargo.toml
+++ b/alacritty/Cargo.toml
@@ -28,7 +28,7 @@ serde_json = "1"
 glutin = { version = "0.26.0", default-features = false, features = ["serde"] }
 notify = "4"
 parking_lot = "0.11.0"
-crossfont = { version = "0.1.0", features = ["force_system_fontconfig"] }
+crossfont = { version = "0.2.0", features = ["force_system_fontconfig"] }
 urlocator = "0.1.3"
 copypasta = { version = "0.7.0", default-features = false }
 libc = "0.2"
diff --git a/alacritty/src/cursor.rs b/alacritty/src/cursor.rs
index 94808029..806f6ff8 100644
--- a/alacritty/src/cursor.rs
+++ b/alacritty/src/cursor.rs
@@ -37,39 +37,39 @@ pub fn get_cursor_glyph(
 /// Return a custom underline cursor character.
 pub fn get_underline_cursor_glyph(width: usize, line_width: usize) -> RasterizedGlyph {
     // Create a new rectangle, the height is relative to the font width.
-    let buf = vec![255u8; width * line_width * 3];
+    let buffer = BitmapBuffer::RGB(vec![255u8; width * line_width * 3]);
 
     // Create a custom glyph with the rectangle data attached to it.
     RasterizedGlyph {
-        c: ' ',
+        character: ' ',
         top: line_width as i32,
         left: 0,
         height: line_width as i32,
         width: width as i32,
-        buf: BitmapBuffer::RGB(buf),
+        buffer,
     }
 }
 
 /// Return a custom beam cursor character.
 pub fn get_beam_cursor_glyph(height: usize, line_width: usize) -> RasterizedGlyph {
     // Create a new rectangle that is at least one pixel wide
-    let buf = vec![255u8; line_width * height * 3];
+    let buffer = BitmapBuffer::RGB(vec![255u8; line_width * height * 3]);
 
     // Create a custom glyph with the rectangle data attached to it
     RasterizedGlyph {
-        c: ' ',
+        character: ' ',
         top: height as i32,
         left: 0,
         height: height as i32,
         width: line_width as i32,
-        buf: BitmapBuffer::RGB(buf),
+        buffer,
     }
 }
 
 /// Returns a custom box cursor character.
 pub fn get_box_cursor_glyph(height: usize, width: usize, line_width: usize) -> RasterizedGlyph {
     // Create a new box outline rectangle.
-    let mut buf = Vec::with_capacity(width * height * 3);
+    let mut buffer = Vec::with_capacity(width * height * 3);
     for y in 0..height {
         for x in 0..width {
             if y < line_width
@@ -77,36 +77,36 @@ pub fn get_box_cursor_glyph(height: usize, width: usize, line_width: usize) -> R
                 || x < line_width
                 || x >= width - line_width
             {
-                buf.append(&mut vec![255u8; 3]);
+                buffer.append(&mut vec![255u8; 3]);
             } else {
-                buf.append(&mut vec![0u8; 3]);
+                buffer.append(&mut vec![0u8; 3]);
             }
         }
     }
 
     // Create a custom glyph with the rectangle data attached to it.
     RasterizedGlyph {
-        c: ' ',
+        character: ' ',
         top: height as i32,
         left: 0,
         height: height as i32,
         width: width as i32,
-        buf: BitmapBuffer::RGB(buf),
+        buffer: BitmapBuffer::RGB(buffer),
     }
 }
 
 /// Return a custom block cursor character.
 pub fn get_block_cursor_glyph(height: usize, width: usize) -> RasterizedGlyph {
     // Create a completely filled glyph.
-    let buf = vec![255u8; width * height * 3];
+    let buffer = BitmapBuffer::RGB(vec![255u8; width * height * 3]);
 
     // Create a custom glyph with the rectangle data attached to it.
     RasterizedGlyph {
-        c: ' ',
+        character: ' ',
         top: height as i32,
         left: 0,
         height: height as i32,
         width: width as i32,
-        buf: BitmapBuffer::RGB(buf),
+        buffer,
     }
 }
diff --git a/alacritty/src/renderer/mod.rs b/alacritty/src/renderer/mod.rs
index cc2515be..ca3553dc 100644
--- a/alacritty/src/renderer/mod.rs
+++ b/alacritty/src/renderer/mod.rs
@@ -1,3 +1,4 @@
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::fmt::{self, Display, Formatter};
 use std::hash::BuildHasherDefault;
@@ -7,11 +8,12 @@ use std::ptr;
 
 use bitflags::bitflags;
 use crossfont::{
-    BitmapBuffer, FontDesc, FontKey, GlyphKey, Rasterize, RasterizedGlyph, Rasterizer, Size, Slant,
-    Style, Weight,
+    BitmapBuffer, Error as RasterizerError, FontDesc, FontKey, GlyphKey, Rasterize,
+    RasterizedGlyph, Rasterizer, Size, Slant, Style, Weight,
 };
 use fnv::FnvHasher;
-use log::{error, info};
+use log::{debug, error, info};
+use unicode_width::UnicodeWidthChar;
 
 use alacritty_terminal::config::Cursor;
 use alacritty_terminal::index::{Column, Line};
@@ -92,7 +94,7 @@ pub struct TextShaderProgram {
     u_background: GLint,
 }
 
-#[derive(Copy, Debug, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub struct Glyph {
     tex_id: GLuint,
     multicolor: bool,
@@ -156,7 +158,7 @@ impl GlyphCache {
         // Need to load at least one glyph for the face before calling metrics.
         // The glyph requested here ('m' at the time of writing) has no special
         // meaning.
-        rasterizer.get_glyph(GlyphKey { font_key: regular, c: 'm', size: font.size() })?;
+        rasterizer.get_glyph(GlyphKey { font_key: regular, character: 'm', size: font.size() })?;
 
         let metrics = rasterizer.metrics(regular, font.size())?;
 
@@ -180,8 +182,13 @@ impl GlyphCache {
 
     fn load_glyphs_for_font<L: LoadGlyph>(&mut self, font: FontKey, loader: &mut L) {
         let size = self.font_size;
+
+        // Cache the "missing glyph" character.
+        self.cache_glyph(GlyphKey { font_key: font, character: '\0', size }, loader);
+
+        // Cache all ascii characters.
         for i in 32u8..=126u8 {
-            self.get(GlyphKey { font_key: font, c: i as char, size }, loader);
+            self.cache_glyph(GlyphKey { font_key: font, character: i as char, size }, loader);
         }
     }
 
@@ -250,23 +257,71 @@ impl GlyphCache {
         FontDesc::new(desc.family.clone(), style)
     }
 
-    pub fn get<L>(&mut self, glyph_key: GlyphKey, loader: &mut L) -> &Glyph
+    /// Get a glyph from the font.
+    ///
+    /// If the glyph has never been loaded before, it will be rasterized and inserted into the
+    /// cache.
+    ///
+    /// # Errors
+    ///
+    /// This will fail when the glyph could not be rasterized. Usually this is due to the glyph
+    /// not being present in any font.
+    fn get<L>(&mut self, glyph_key: GlyphKey, loader: &mut L) -> Result<&Glyph, RasterizerError>
     where
         L: LoadGlyph,
     {
-        let glyph_offset = self.glyph_offset;
-        let rasterizer = &mut self.rasterizer;
-        let metrics = &self.metrics;
-        self.cache.entry(glyph_key).or_insert_with(|| {
-            let mut rasterized =
-                rasterizer.get_glyph(glyph_key).unwrap_or_else(|_| Default::default());
-
-            rasterized.left += i32::from(glyph_offset.x);
-            rasterized.top += i32::from(glyph_offset.y);
-            rasterized.top -= metrics.descent as i32;
-
-            loader.load_glyph(&rasterized)
-        })
+        match self.cache.entry(glyph_key) {
+            // Glyph was loaded from cache.
+            Entry::Occupied(entry) => Ok(entry.into_mut()),
+            // Try to rasterize the glyph if it's uncached.
+            Entry::Vacant(entry) => match self.rasterizer.get_glyph(glyph_key) {
+                // Add rasterized glyph to the cache.
+                Ok(mut rasterized) => {
+                    rasterized.left += i32::from(self.glyph_offset.x);
+                    rasterized.top += i32::from(self.glyph_offset.y);
+                    rasterized.top -= self.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_key.character.width() == Some(0) {
+                        rasterized.left += self.metrics.average_advance as i32;
+                    }
+
+                    Ok(entry.insert(loader.load_glyph(&rasterized)))
+                },
+                // Propagate rasterization failure.
+                Err(err) => Err(err),
+            },
+        }
+    }
+
+    /// Load a glyph into the cache.
+    ///
+    /// This will always insert a new glyph in the cache, even if the rasterization returned a
+    /// "missing" glyph or failed completely.
+    fn cache_glyph<L>(&mut self, glyph_key: GlyphKey, loader: &mut L)
+    where
+        L: LoadGlyph,
+    {
+        let rasterized = match self.rasterizer.get_glyph(glyph_key) {
+            Ok(mut rasterized) | Err(RasterizerError::MissingGlyph(mut rasterized)) => {
+                rasterized.left += i32::from(self.glyph_offset.x);
+                rasterized.top += i32::from(self.glyph_offset.y);
+                rasterized.top -= self.metrics.descent as i32;
+                rasterized
+            },
+            // Use empty glyph as fallback.
+            Err(err) => {
+                debug!("{}", err);
+                Default::default()
+            },
+        };
+
+        // Cache rasterized glyph.
+        self.cache.insert(glyph_key, loader.load_glyph(&rasterized));
     }
 
     /// Clear currently cached data in both GL and the registry.
@@ -291,7 +346,11 @@ impl GlyphCache {
         let (regular, bold, italic, bold_italic) =
             Self::compute_font_keys(font, &mut self.rasterizer)?;
 
-        self.rasterizer.get_glyph(GlyphKey { font_key: regular, c: 'm', size: font.size() })?;
+        self.rasterizer.get_glyph(GlyphKey {
+            font_key: regular,
+            character: 'm',
+            size: font.size(),
+        })?;
         let metrics = self.rasterizer.metrics(regular, font.size())?;
 
         info!("Font size changed to {:?} with DPR of {}", font.size(), dpr);
@@ -325,7 +384,7 @@ impl GlyphCache {
         let mut rasterizer = crossfont::Rasterizer::new(dpr as f32, font.use_thin_strokes)?;
         let regular_desc = GlyphCache::make_desc(&font.normal(), Slant::Normal, Weight::Normal);
         let regular = Self::load_regular_font(&mut rasterizer, &regular_desc, font.size())?;
-        rasterizer.get_glyph(GlyphKey { font_key: regular, c: 'm', size: font.size() })?;
+        rasterizer.get_glyph(GlyphKey { font_key: regular, character: 'm', size: font.size() })?;
 
         rasterizer.metrics(regular, font.size())
     }
@@ -822,7 +881,7 @@ impl<'a> RenderApi<'a> {
     }
 
     pub fn render_cell(&mut self, mut cell: RenderableCell, glyph_cache: &mut GlyphCache) {
-        let (mut c, zerowidth) = match cell.inner {
+        let (mut character, zerowidth) = match cell.inner {
             RenderableCellContent::Cursor(cursor_key) => {
                 // Raw cell pixel buffers like cursors don't need to go through font lookup.
                 let metrics = glyph_cache.metrics;
@@ -852,30 +911,31 @@ impl<'a> RenderApi<'a> {
 
         // Ignore hidden cells and render tabs as spaces to prevent font issues.
         let hidden = cell.flags.contains(Flags::HIDDEN);
-        if c == '\t' || hidden {
-            c = ' ';
+        if character == '\t' || hidden {
+            character = ' ';
         }
 
-        let mut glyph_key = GlyphKey { font_key, size: glyph_cache.font_size, c };
+        let mut glyph_key = GlyphKey { font_key, size: glyph_cache.font_size, character };
 
         // Add cell to batch.
-        let glyph = glyph_cache.get(glyph_key, self);
-        self.add_render_item(&cell, glyph);
+        match glyph_cache.get(glyph_key, self) {
+            Ok(glyph) => self.add_render_item(&cell, glyph),
+            // Insert the "missing" glyph for this key into the cache on error.
+            Err(_) => {
+                let missing_key = GlyphKey { character: '\0', ..glyph_key };
+                let glyph = *glyph_cache.get(missing_key, self).expect("no 'missing' glyph");
+                glyph_cache.cache.insert(glyph_key, glyph);
+                self.add_render_item(&cell, &glyph)
+            },
+        }
 
         // Render visible zero-width characters.
         if let Some(zerowidth) = zerowidth.filter(|_| !hidden) {
-            for c in zerowidth {
-                glyph_key.c = c;
-                let mut glyph = *glyph_cache.get(glyph_key, self);
-
-                // 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.
-                glyph.left += glyph_cache.metrics.average_advance as i16;
-
-                self.add_render_item(&cell, &glyph);
+            for character in zerowidth.iter() {
+                glyph_key.character = *character;
+                if let Ok(glyph) = glyph_cache.get(glyph_key, self) {
+                    self.add_render_item(&cell, &glyph);
+                }
             }
         }
     }
@@ -1314,14 +1374,14 @@ impl Atlas {
             gl::BindTexture(gl::TEXTURE_2D, self.id);
 
             // Load data into OpenGL.
-            let (format, buf) = match &glyph.buf {
-                BitmapBuffer::RGB(buf) => {
+            let (format, buffer) = match &glyph.buffer {
+                BitmapBuffer::RGB(buffer) => {
                     multicolor = false;
-                    (gl::RGB, buf)
+                    (gl::RGB, buffer)
                 },
-                BitmapBuffer::RGBA(buf) => {
+                BitmapBuffer::RGBA(buffer) => {
                     multicolor = true;
-                    (gl::RGBA, buf)
+                    (gl::RGBA, buffer)
                 },
             };
 
@@ -1334,7 +1394,7 @@ impl Atlas {
                 height,
                 format,
                 gl::UNSIGNED_BYTE,
-                buf.as_ptr() as *const _,
+                buffer.as_ptr() as *const _,
             );
 
             gl::BindTexture(gl::TEXTURE_2D, 0);