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.

5 files changed, 165 insertions, 79 deletions

diff --git a/font/src/ft/mod.rs b/font/src/ft/mod.rs
index 42134a88..ce5cea62 100644
--- a/font/src/ft/mod.rs
+++ b/font/src/ft/mod.rs
@@ -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)
                         }
                     }
diff --git a/font/src/lib.rs b/font/src/lib.rs
index 181fd88c..513dacdd 100644
--- a/font/src/lib.rs
+++ b/font/src/lib.rs
@@ -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,
diff --git a/src/display.rs b/src/display.rs
index 25318f0e..eb562e8f 100644
--- a/src/display.rs
+++ b/src/display.rs
@@ -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]
diff --git a/src/main.rs b/src/main.rs
index ca7a40c6..129c1618 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -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;
diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs
index 9e6036d4..3db9ccb6 100644
--- a/src/renderer/mod.rs
+++ b/src/renderer/mod.rs
@@ -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,36 +175,67 @@ 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();
+        let (regular, bold, italic) = Self::compute_font_keys(font, &mut rasterizer)?;
 
-        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)
+        // 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() })?;
+        let metrics = rasterizer.metrics(regular)?;
+
+        let mut cache = GlyphCache {
+            cache: HashMap::default(),
+            rasterizer: rasterizer,
+            font_size: font.size(),
+            font_key: regular,
+            bold_key: bold,
+            italic_key: italic,
+            glyph_offset: *font.glyph_offset(),
+            metrics: metrics
+        };
+
+        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 = make_desc(&font.normal, font::Slant::Normal, font::Weight::Normal);
+        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 load_or_regular = |desc:FontDesc, rasterizer: &mut Rasterizer| {
+        let mut load_or_regular = |desc:FontDesc| {
             if desc == regular_desc {
                 regular
             } else {
@@ -208,49 +244,29 @@ impl GlyphCache {
         };
 
         // Load bold font
-        let bold_desc = make_desc(&font.bold, font::Slant::Normal, font::Weight::Bold);
+        let bold_desc = Self::make_desc(&font.bold, font::Slant::Normal, font::Weight::Bold);
 
-        let bold = load_or_regular(bold_desc, &mut rasterizer);
+        let bold = load_or_regular(bold_desc);
 
         // Load italic font
-        let italic_desc = make_desc(&font.italic, font::Slant::Italic, font::Weight::Normal);
+        let italic_desc = Self::make_desc(&font.italic, font::Slant::Italic, font::Weight::Normal);
 
-        let italic = load_or_regular(italic_desc, &mut rasterizer);
+        let italic = load_or_regular(italic_desc);
 
-        // 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() })?;
-        let metrics = rasterizer.metrics(regular)?;
+        Ok((regular, bold, italic))
+    }
 
-        let mut cache = GlyphCache {
-            cache: HashMap::default(),
-            rasterizer: rasterizer,
-            font_size: font.size(),
-            font_key: regular,
-            bold_key: bold,
-            italic_key: italic,
-            glyph_offset: glyph_offset,
-            metrics: metrics
+    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}
         };
-
-        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);
-
-        Ok(cache)
+        FontDesc::new(&desc.family[..], style)
     }
 
     pub fn font_metrics(&self) -> font::Metrics {
@@ -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,