Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't copy Glyph on GlyphCache hit #6954

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 70 additions & 42 deletions alacritty/src/renderer/text/glyph_cache.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::hash::BuildHasherDefault;

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -192,72 +200,92 @@ impl GlyphCache {
&mut self,
glyph_key: GlyphKey,
loader: &mut L,
show_missing: bool,
) -> Glyph
) -> Result<&Glyph, Box<RasterizerError>>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error happens once per glyph key, so to not enlarge the return value we simply box the value.

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.
Expand Down
25 changes: 20 additions & 5 deletions alacritty/src/renderer/text/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bitflags::bitflags;
use crossfont::{GlyphKey, RasterizedGlyph};
use crossfont::{Error as RasterizerError, GlyphKey, RasterizedGlyph};

use alacritty_terminal::term::cell::Flags;

Expand Down Expand Up @@ -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);
}
}
}
Expand Down