Fix memory leak from font resizing

The source of the leak was loading up multiple copies of the FT_face
even when not necessary. Fonts are now appropriately cached for
FreeType when going through the `Rasterize::load_font` API.

Additionally, textures in the glyph cache are now reused.

The result of this is that resizing to already loaded fonts is free
from a memory consumption perspective.
This commit is contained in:
Joe Wilm 2017-10-14 18:58:19 -07:00
parent 8a0b1d9c3f
commit e8f5ab89ee
5 changed files with 170 additions and 84 deletions

View File

@ -105,10 +105,7 @@ impl ::Rasterize for FreeTypeRasterizer {
}
fn load_font(&mut self, desc: &FontDesc, size: Size) -> Result<FontKey, Error> {
let face = self.get_face(desc, size)?;
let key = face.key;
self.faces.insert(key, face);
Ok(key)
self.get_face(desc, size)
}
fn get_glyph(&mut self, glyph_key: &GlyphKey) -> Result<RasterizedGlyph, Error> {
@ -146,7 +143,7 @@ impl IntoFontconfigType for Weight {
impl FreeTypeRasterizer {
/// Load a font face accoring to `FontDesc`
fn get_face(&mut self, desc: &FontDesc, size: Size) -> Result<Face, Error> {
fn get_face(&mut self, desc: &FontDesc, size: Size) -> Result<FontKey, Error> {
// Adjust for DPI
let size = Size::new(size.as_f32_pts() * self.device_pixel_ratio * 96. / 72.);
@ -168,7 +165,7 @@ impl FreeTypeRasterizer {
slant: Slant,
weight: Weight,
size: Size,
) -> Result<Face, Error> {
) -> Result<FontKey, Error> {
let mut pattern = fc::Pattern::new();
pattern.add_family(&desc.name);
pattern.set_weight(weight.into_fontconfig_type());
@ -191,7 +188,7 @@ impl FreeTypeRasterizer {
desc: &FontDesc,
style: &str,
size: Size,
) -> Result<Face, Error> {
) -> Result<FontKey, Error> {
let mut pattern = fc::Pattern::new();
pattern.add_family(&desc.name);
pattern.add_style(style);
@ -207,10 +204,14 @@ impl FreeTypeRasterizer {
})
}
fn face_from_pattern(&self, pattern: &fc::Pattern) -> Result<Option<Face>, Error> {
fn face_from_pattern(&mut self, pattern: &fc::Pattern) -> Result<Option<FontKey>, Error> {
if let (Some(path), Some(index)) = (pattern.file(0), pattern.index().nth(0)) {
if let Some(key) = self.keys.get(&path) {
return Ok(Some(*key));
}
trace!("got font path={:?}", path);
let ft_face = self.library.new_face(path, index)?;
let ft_face = self.library.new_face(&path, index)?;
// Get available pixel sizes if font isn't scalable.
let non_scalable = if !pattern.scalable().next().unwrap_or(true) {
@ -234,7 +235,12 @@ impl FreeTypeRasterizer {
};
debug!("Loaded Face {:?}", face);
Ok(Some(face))
let key = face.key;
self.faces.insert(key, face);
self.keys.insert(path, key);
Ok(Some(key))
} else {
Ok(None)
}
@ -453,10 +459,7 @@ impl FreeTypeRasterizer {
debug!("Miss for font {:?}; loading now.", path);
// Safe to unwrap the option since we've already checked for the path
// and index above.
let face = self.face_from_pattern(&pattern)?.unwrap();
let key = face.key;
self.faces.insert(key, face);
self.keys.insert(path, key);
let key = self.face_from_pattern(&pattern)?.unwrap();
Ok(key)
}
}

View File

@ -172,6 +172,14 @@ impl Size {
}
}
impl ::std::ops::Add for Size {
type Output = Size;
fn add(self, other: Size) -> Size {
Size(self.0.saturating_add(other.0))
}
}
pub struct RasterizedGlyph {
pub c: char,
pub width: i32,

View File

@ -240,14 +240,15 @@ impl Display {
return Ok((glyph_cache, cell_width as f32, cell_height as f32));
}
pub fn update_glyph_cache(&mut self, config: &Config, font_size_delta: i8)
{
let (glyph_cache, cell_width, cell_height) =
Self::new_glyph_cache(&self.window,
&mut self.renderer, config, font_size_delta).unwrap();
self.glyph_cache = glyph_cache;
self.size_info.cell_width = cell_width;
self.size_info.cell_height = cell_height;
pub fn update_glyph_cache(&mut self, config: &Config, font_size_delta: i8) {
let cache = &mut self.glyph_cache;
self.renderer.with_loader(|mut api| {
let _ = cache.update_font_size(config.font(), font_size_delta, &mut api);
});
let metrics = cache.font_metrics();
self.size_info.cell_width = (metrics.average_advance + config.font().offset().x as f64) as f32;
self.size_info.cell_height = (metrics.line_height + config.font().offset().y as f64) as f32;
}
#[inline]

View File

@ -15,6 +15,7 @@
//! Alacritty - The GPU Enhanced Terminal
#![cfg_attr(feature = "clippy", plugin(clippy))]
#![cfg_attr(feature = "clippy", feature(plugin))]
#![feature(alloc_system)] extern crate alloc_system;
#[macro_use]
extern crate alacritty;

View File

@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::collections::HashMap;
use std::hash::BuildHasherDefault;
use std::fs::File;
use std::hash::BuildHasherDefault;
use std::io::{self, Read};
use std::mem::size_of;
use std::path::{PathBuf};
@ -29,7 +29,7 @@ use gl;
use index::{Line, Column, RangeInclusive};
use notify::{Watcher, watcher, RecursiveMode, DebouncedEvent};
use config::{self, Config, Font, Delta};
use config::{self, Config, Delta};
use term::{self, cell, RenderableCell};
use window::{Size, Pixels};
@ -51,6 +51,11 @@ static TEXT_SHADER_V: &'static str = include_str!(
pub trait LoadGlyph {
/// Load the rasterized glyph into GPU memory
fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph;
/// Clear any state accumulated from previous loaded glyphs
///
/// This can, for instance, be used to reset the texture Atlas.
fn clear(&mut self);
}
enum Msg {
@ -170,52 +175,12 @@ pub struct GlyphCache {
impl GlyphCache {
pub fn new<L>(
mut rasterizer: Rasterizer,
font: &Font,
font: &config::Font,
loader: &mut L
) -> Result<GlyphCache, font::Error>
where L: LoadGlyph
{
let size = font.size();
let glyph_offset = *font.glyph_offset();
fn make_desc(
desc: &config::FontDescription,
slant: font::Slant,
weight: font::Weight,
) -> FontDesc
{
let style = if let Some(ref spec) = desc.style {
font::Style::Specific(spec.to_owned())
} else {
font::Style::Description {slant:slant, weight:weight}
};
FontDesc::new(&desc.family[..], style)
}
// Load regular font
let regular_desc = make_desc(&font.normal, font::Slant::Normal, font::Weight::Normal);
let regular = rasterizer
.load_font(&regular_desc, size)?;
// helper to load a description if it is not the regular_desc
let load_or_regular = |desc:FontDesc, rasterizer: &mut Rasterizer| {
if desc == regular_desc {
regular
} else {
rasterizer.load_font(&desc, size).unwrap_or_else(|_| regular)
}
};
// Load bold font
let bold_desc = make_desc(&font.bold, font::Slant::Normal, font::Weight::Bold);
let bold = load_or_regular(bold_desc, &mut rasterizer);
// Load italic font
let italic_desc = make_desc(&font.italic, font::Slant::Italic, font::Weight::Normal);
let italic = load_or_regular(italic_desc, &mut rasterizer);
let (regular, bold, italic) = Self::compute_font_keys(font, &mut rasterizer)?;
// 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
@ -230,29 +195,80 @@ impl GlyphCache {
font_key: regular,
bold_key: bold,
italic_key: italic,
glyph_offset: glyph_offset,
glyph_offset: *font.glyph_offset(),
metrics: metrics
};
macro_rules! load_glyphs_for_font {
($font:expr) => {
for i in RangeInclusive::new(32u8, 128u8) {
cache.get(&GlyphKey {
font_key: $font,
c: i as char,
size: font.size()
}, loader);
}
}
}
load_glyphs_for_font!(regular);
load_glyphs_for_font!(bold);
load_glyphs_for_font!(italic);
cache.load_glyphs_for_font(regular, loader);
cache.load_glyphs_for_font(bold, loader);
cache.load_glyphs_for_font(italic, loader);
Ok(cache)
}
fn load_glyphs_for_font<L: LoadGlyph>(
&mut self,
font: FontKey,
loader: &mut L,
) {
let size = self.font_size;
for i in RangeInclusive::new(32u8, 128u8) {
self.get(&GlyphKey {
font_key: font,
c: i as char,
size: size
}, loader);
}
}
/// Computes font keys for (Regular, Bold, Italic)
fn compute_font_keys(
font: &config::Font,
rasterizer: &mut Rasterizer
) -> Result<(FontKey, FontKey, FontKey), font::Error> {
let size = font.size();
// Load regular font
let regular_desc = Self::make_desc(&font.normal, font::Slant::Normal, font::Weight::Normal);
let regular = rasterizer
.load_font(&regular_desc, size)?;
// helper to load a description if it is not the regular_desc
let mut load_or_regular = |desc:FontDesc| {
if desc == regular_desc {
regular
} else {
rasterizer.load_font(&desc, size).unwrap_or_else(|_| regular)
}
};
// Load bold font
let bold_desc = Self::make_desc(&font.bold, font::Slant::Normal, font::Weight::Bold);
let bold = load_or_regular(bold_desc);
// Load italic font
let italic_desc = Self::make_desc(&font.italic, font::Slant::Italic, font::Weight::Normal);
let italic = load_or_regular(italic_desc);
Ok((regular, bold, italic))
}
fn make_desc(
desc: &config::FontDescription,
slant: font::Slant,
weight: font::Weight,
) -> FontDesc {
let style = if let Some(ref spec) = desc.style {
font::Style::Specific(spec.to_owned())
} else {
font::Style::Description {slant:slant, weight:weight}
};
FontDesc::new(&desc.family[..], style)
}
pub fn font_metrics(&self) -> font::Metrics {
self.rasterizer
.metrics(self.font_key)
@ -278,6 +294,35 @@ impl GlyphCache {
loader.load_glyph(&rasterized)
})
}
pub fn update_font_size<L: LoadGlyph>(
&mut self,
font: &config::Font,
delta: i8,
loader: &mut L
) -> Result<(), font::Error> {
// Clear currently cached data in both GL and the registry
loader.clear();
self.cache = HashMap::default();
// Recompute font keys
let font = font.to_owned().with_size_delta(delta as _);
println!("{:?}", font.size);
let (regular, bold, italic) = Self::compute_font_keys(&font, &mut self.rasterizer)?;
self.rasterizer.get_glyph(&GlyphKey { font_key: regular, c: 'm', size: font.size() })?;
let metrics = self.rasterizer.metrics(regular)?;
self.font_size = font.size;
self.font_key = regular;
self.bold_key = bold;
self.italic_key = italic;
self.metrics = metrics;
self.load_glyphs_for_font(regular, loader);
self.load_glyphs_for_font(bold, loader);
self.load_glyphs_for_font(italic, loader);
Ok(())
}
}
#[derive(Debug)]
@ -316,6 +361,7 @@ pub struct QuadRenderer {
ebo: GLuint,
vbo_instance: GLuint,
atlas: Vec<Atlas>,
current_atlas: usize,
active_tex: GLuint,
batch: Batch,
rx: mpsc::Receiver<Msg>,
@ -326,6 +372,7 @@ pub struct RenderApi<'a> {
active_tex: &'a mut GLuint,
batch: &'a mut Batch,
atlas: &'a mut Vec<Atlas>,
current_atlas: &'a mut usize,
program: &'a mut ShaderProgram,
config: &'a Config,
visual_bell_intensity: f32
@ -335,6 +382,7 @@ pub struct RenderApi<'a> {
pub struct LoaderApi<'a> {
active_tex: &'a mut GLuint,
atlas: &'a mut Vec<Atlas>,
current_atlas: &'a mut usize,
}
#[derive(Debug)]
@ -564,6 +612,7 @@ impl QuadRenderer {
ebo: ebo,
vbo_instance: vbo_instance,
atlas: Vec::new(),
current_atlas: 0,
active_tex: 0,
batch: Batch::new(),
rx: msg_rx,
@ -611,6 +660,7 @@ impl QuadRenderer {
active_tex: &mut self.active_tex,
batch: &mut self.batch,
atlas: &mut self.atlas,
current_atlas: &mut self.current_atlas,
program: &mut self.program,
visual_bell_intensity: visual_bell_intensity as _,
config: config,
@ -637,6 +687,7 @@ impl QuadRenderer {
func(LoaderApi {
active_tex: &mut self.active_tex,
atlas: &mut self.atlas,
current_atlas: &mut self.current_atlas,
})
}
@ -814,16 +865,24 @@ impl<'a> LoadGlyph for LoaderApi<'a> {
fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph {
// At least one atlas is guaranteed to be in the `self.atlas` list; thus
// the unwrap should always be ok.
match self.atlas.last_mut().unwrap().insert(rasterized, &mut self.active_tex) {
match self.atlas[*self.current_atlas].insert(rasterized, &mut self.active_tex) {
Ok(glyph) => glyph,
Err(_) => {
let atlas = Atlas::new(ATLAS_SIZE);
*self.active_tex = 0; // Atlas::new binds a texture. Ugh this is sloppy.
*self.current_atlas = 0;
self.atlas.push(atlas);
self.load_glyph(rasterized)
}
}
}
fn clear(&mut self) {
for atlas in self.atlas.iter_mut() {
atlas.clear();
}
*self.current_atlas = 0;
}
}
impl<'a> LoadGlyph for RenderApi<'a> {
@ -833,16 +892,24 @@ impl<'a> LoadGlyph for RenderApi<'a> {
fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph {
// At least one atlas is guaranteed to be in the `self.atlas` list; thus
// the unwrap.
match self.atlas.last_mut().unwrap().insert(rasterized, &mut self.active_tex) {
match self.atlas[*self.current_atlas].insert(rasterized, &mut self.active_tex) {
Ok(glyph) => glyph,
Err(_) => {
let atlas = Atlas::new(ATLAS_SIZE);
*self.active_tex = 0; // Atlas::new binds a texture. Ugh this is sloppy.
*self.current_atlas = 0;
self.atlas.push(atlas);
self.load_glyph(rasterized)
}
}
}
fn clear(&mut self) {
for atlas in self.atlas.iter_mut() {
atlas.clear();
}
*self.current_atlas = 0;
}
}
impl<'a> Drop for RenderApi<'a> {
@ -1254,6 +1321,12 @@ impl Atlas {
}
}
pub fn clear(&mut self) {
self.row_extent = 0;
self.row_baseline = 0;
self.row_tallest = 0;
}
/// Insert a RasterizedGlyph into the texture atlas
pub fn insert(&mut self,
glyph: &RasterizedGlyph,