Lazy load fonts in a FontGroup

This is a step towards fixing #17267. To fix that, we need to be able to
try various different fallback fonts in turn, which would become
unweildy with the prior eager-loading strategy.

Prior to this change, FontGroup loaded up all Font instances, including
the fallback font, before any of them were checked for the presence of
the glyphs we're trying to render.

So for the following CSS:

    font-family: Helvetica, Arial;

The FontGroup would contain a Font instance for Helvetica, and a Font
instance for Arial, and a Font instance for the fallback font.

It may be that Helvetica contains glyphs for every character in the
document, and therefore Arial and the fallback font are not needed at
all.

This change makes the strategy lazy, so that we'll only create a Font
for Arial if we cannot find a glyph within Helvetica. I've also
substantially refactored the existing code in the process and added
some documentation along the way.

components/gfx/font.rs

@@ -4,11 +4,13 @@
 
 use app_units::Au;
 use euclid::{Point2D, Rect, Size2D};
+use font_context::FontContext;
 use font_template::FontTemplateDescriptor;
 use ordered_float::NotNaN;
 use platform::font::{FontHandle, FontTable};
 use platform::font_context::FontContextHandle;
 use platform::font_template::FontTemplateData;
+use servo_atoms::Atom;
 use smallvec::SmallVec;
 use std::borrow::ToOwned;
 use std::cell::RefCell;
@@ -18,6 +20,8 @@ use std::str;
 use std::sync::Arc;
 use std::sync::atomic::{ATOMIC_USIZE_INIT, AtomicUsize, Ordering};
 use style::computed_values::{font_stretch, font_variant_caps, font_weight};
+use style::properties::style_structs::Font as FontStyleStruct;
+use style::values::computed::font::SingleFontFamily;
 use text::Shaper;
 use text::glyph::{ByteIndex, GlyphData, GlyphId, GlyphStore};
 use text::shaping::ShaperMethods;
@@ -59,6 +63,9 @@ pub trait FontHandleMethods: Sized {
     fn can_do_fast_shaping(&self) -> bool;
     fn metrics(&self) -> FontMetrics;
     fn table_for_tag(&self, FontTableTag) -> Option<FontTable>;
+
+    /// A unique identifier for the font, allowing comparison.
+    fn identifier(&self) -> Atom;
 }
 
 // Used to abstract over the shaper's choice of fixed int representation.
@@ -100,13 +107,32 @@ pub struct FontMetrics {
     pub line_gap:         Au,
 }
 
+/// `FontDescriptor` describes the parameters of a `Font`. It represents rendering a given font
+/// template at a particular size, with a particular font-variant-caps applied, etc. This contrasts
+/// with `FontTemplateDescriptor` in that the latter represents only the parameters inherent in the
+/// font data (weight, stretch, etc.).
+#[derive(Clone, Debug, PartialEq)]
+pub struct FontDescriptor {
+    pub template_descriptor: FontTemplateDescriptor,
+    pub variant: font_variant_caps::T,
+    pub pt_size: Au,
+}
+
+impl<'a> From<&'a FontStyleStruct> for FontDescriptor {
+    fn from(style: &'a FontStyleStruct) -> Self {
+        FontDescriptor {
+            template_descriptor: FontTemplateDescriptor::from(style),
+            variant: style.font_variant_caps,
+            pt_size: style.font_size.size(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct Font {
     pub handle: FontHandle,
     pub metrics: FontMetrics,
-    pub variant: font_variant_caps::T,
+    pub descriptor: FontDescriptor,
-    pub descriptor: FontTemplateDescriptor,
-    pub requested_pt_size: Au,
     pub actual_pt_size: Au,
     shaper: Option<Shaper>,
     shape_cache: RefCell<HashMap<ShapeCacheEntry, Arc<GlyphStore>>>,
@@ -116,25 +142,27 @@ pub struct Font {
 
 impl Font {
     pub fn new(handle: FontHandle,
-               variant: font_variant_caps::T,
+               descriptor: FontDescriptor,
-               descriptor: FontTemplateDescriptor,
-               requested_pt_size: Au,
                actual_pt_size: Au,
                font_key: webrender_api::FontInstanceKey) -> Font {
         let metrics = handle.metrics();
+
         Font {
             handle: handle,
             shaper: None,
-            variant: variant,
+            descriptor,
-            descriptor: descriptor,
+            actual_pt_size,
-            requested_pt_size: requested_pt_size,
+            metrics,
-            actual_pt_size: actual_pt_size,
-            metrics: metrics,
             shape_cache: RefCell::new(HashMap::new()),
             glyph_advance_cache: RefCell::new(HashMap::new()),
-            font_key: font_key,
+            font_key,
         }
     }
+
+    /// A unique identifier for the font, allowing comparison.
+    pub fn identifier(&self) -> Atom {
+        self.handle.identifier()
+    }
 }
 
 bitflags! {
@@ -260,13 +288,17 @@ impl Font {
 
     #[inline]
     pub fn glyph_index(&self, codepoint: char) -> Option<GlyphId> {
-        let codepoint = match self.variant {
+        let codepoint = match self.descriptor.variant {
             font_variant_caps::T::SmallCaps => codepoint.to_uppercase().next().unwrap(), //FIXME: #5938
             font_variant_caps::T::Normal => codepoint,
         };
         self.handle.glyph_index(codepoint)
     }
 
+    pub fn has_glyph_for(&self, codepoint: char) -> bool {
+        self.glyph_index(codepoint).is_some()
+    }
+
     pub fn glyph_h_kerning(&self, first_glyph: GlyphId, second_glyph: GlyphId)
                            -> FractionalPixel {
         self.handle.glyph_h_kerning(first_glyph, second_glyph)
@@ -282,17 +314,92 @@ impl Font {
     }
 }
 
+pub type FontRef = Rc<RefCell<Font>>;
+
+/// A `FontGroup` is a prioritised list of fonts for a given set of font styles. It is used by
+/// `TextRun` to decide which font to render a character with. If none of the fonts listed in the
+/// styles are suitable, a fallback font may be used.
 #[derive(Debug)]
 pub struct FontGroup {
-    pub fonts: SmallVec<[Rc<RefCell<Font>>; 8]>,
+    descriptor: FontDescriptor,
+    families: SmallVec<[FontGroupFamily; 8]>,
 }
 
 impl FontGroup {
-    pub fn new(fonts: SmallVec<[Rc<RefCell<Font>>; 8]>) -> FontGroup {
+    pub fn new(style: &FontStyleStruct) -> FontGroup {
-        FontGroup {
+        let descriptor = FontDescriptor::from(style);
-            fonts: fonts,
+
+        let families =
+            style.font_family.0.iter()
+                .map(|family| FontGroupFamily::new(descriptor.clone(), family.clone()))
+                .collect();
+
+        FontGroup { descriptor, families }
+    }
+
+    /// Finds the first font, or else the first fallback font, which contains a glyph for
+    /// `codepoint`. If no such font is found, returns the first available font or fallback font
+    /// (which will cause a "glyph not found" character to be rendered). If no font at all can be
+    /// found, returns None.
+    pub fn find_by_codepoint(&mut self, mut font_context: &mut FontContext, codepoint: char) -> Option<FontRef> {
+        self.find(&mut font_context, |font| font.borrow().has_glyph_for(codepoint))
+            .or_else(|| self.first(&mut font_context))
+    }
+
+    pub fn first(&mut self, mut font_context: &mut FontContext) -> Option<FontRef> {
+        self.find(&mut font_context, |_| true)
+    }
+
+    /// Find a font which returns true for `predicate`. This method mutates because we may need to
+    /// load new font data in the process of finding a suitable font.
+    fn find<P>(
+        &mut self,
+        mut font_context: &mut FontContext,
+        mut predicate: P
+    ) -> Option<FontRef>
+    where P: FnMut(&FontRef) -> bool {
+        self.families.iter_mut()
+            .filter_map(|family| family.font(&mut font_context))
+            .find(|f| predicate(f))
+            .or_else(|| {
+                font_context.fallback_font(&self.descriptor)
+                    .into_iter().find(predicate)
+            })
+    }
+}
+
+/// A `FontGroupFamily` is a single font family in a `FontGroup`. It corresponds to one of the
+/// families listed in the `font-family` CSS property. The corresponding font data is lazy-loaded,
+/// only if actually needed.
+#[derive(Debug)]
+struct FontGroupFamily {
+    descriptor: FontDescriptor,
+    family: SingleFontFamily,
+    loaded: bool,
+    font: Option<FontRef>,
+}
+
+impl FontGroupFamily {
+    fn new(descriptor: FontDescriptor, family: SingleFontFamily) -> FontGroupFamily {
+        FontGroupFamily {
+            descriptor,
+            family,
+            loaded: false,
+            font: None,
         }
     }
+
+    /// Returns the font within this family which matches the style. We'll fetch the data from the
+    /// `FontContext` the first time this method is called, and return a cached reference on
+    /// subsequent calls.
+    fn font(&mut self, font_context: &mut FontContext) -> Option<FontRef> {
+        if !self.loaded {
+            self.font = font_context.font(&self.descriptor, &self.family);
+            self.loaded = true;
+        }
+
+        self.font.clone()
+    }
 }
 
 pub struct RunMetrics {

components/gfx/font_context.rs

@@ -4,38 +4,54 @@
 
 use app_units::Au;
 use fnv::FnvHasher;
-use font::{Font, FontGroup, FontHandleMethods};
+use font::{Font, FontDescriptor, FontGroup, FontHandleMethods, FontRef};
-use font_cache_thread::FontCacheThread;
+use font_cache_thread::{FontCacheThread, FontTemplateInfo};
-use font_template::FontTemplateDescriptor;
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use platform::font::FontHandle;
 pub use platform::font_context::FontContextHandle;
-use platform::font_template::FontTemplateData;
+use servo_arc::Arc;
-use servo_arc::Arc as ServoArc;
+use servo_atoms::Atom;
-use smallvec::SmallVec;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::default::Default;
 use std::hash::{BuildHasherDefault, Hash, Hasher};
 use std::rc::Rc;
-use std::sync::Arc;
 use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
-use style::computed_values::font_style::T as FontStyle;
 use style::computed_values::font_variant_caps::T as FontVariantCaps;
-use style::properties::style_structs;
+use style::properties::style_structs::Font as FontStyleStruct;
-use webrender_api;
+use style::values::computed::font::SingleFontFamily;
 
 static SMALL_CAPS_SCALE_FACTOR: f32 = 0.8;      // Matches FireFox (see gfxFont.h)
 
 #[derive(Debug)]
-struct LayoutFontCacheEntry {
+struct FontCacheEntry {
-    family: String,
+    family: Atom,
-    font: Option<Rc<RefCell<Font>>>,
+    font: Option<FontRef>,
+}
+
+impl FontCacheEntry {
+    fn matches(&self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> bool {
+        if self.family != *family.atom() {
+            return false
+        }
+
+        if let Some(ref font) = self.font {
+            (*font).borrow().descriptor == *descriptor
+        } else {
+            true
+        }
+    }
 }
 
 #[derive(Debug)]
 struct FallbackFontCacheEntry {
-    font: Rc<RefCell<Font>>,
+    font: FontRef,
+}
+
+impl FallbackFontCacheEntry {
+    fn matches(&self, descriptor: &FontDescriptor) -> bool {
+        self.font.borrow().descriptor == *descriptor
+    }
 }
 
 /// An epoch for the font context cache. The cache is flushed if the current epoch does not match
@@ -51,12 +67,16 @@ pub struct FontContext {
     platform_handle: FontContextHandle,
     font_cache_thread: FontCacheThread,
 
-    /// TODO: See bug https://github.com/servo/servo/issues/3300.
+    // TODO: The font context holds a strong ref to the cached fonts
-    layout_font_cache: Vec<LayoutFontCacheEntry>,
+    // so they will never be released. Find out a good time to drop them.
+    // See bug https://github.com/servo/servo/issues/3300
+    //
+    // GWTODO: Check on real pages if this is faster as Vec() or HashMap().
+    font_cache: Vec<FontCacheEntry>,
     fallback_font_cache: Vec<FallbackFontCacheEntry>,
 
-    layout_font_group_cache:
+    font_group_cache:
-        HashMap<LayoutFontGroupCacheKey, Rc<FontGroup>, BuildHasherDefault<FnvHasher>>,
+        HashMap<FontGroupCacheKey, Rc<RefCell<FontGroup>>, BuildHasherDefault<FnvHasher>>,
 
     epoch: usize,
 }
@@ -67,35 +87,32 @@ impl FontContext {
         FontContext {
             platform_handle: handle,
             font_cache_thread: font_cache_thread,
-            layout_font_cache: vec!(),
+            font_cache: vec!(),
             fallback_font_cache: vec!(),
-            layout_font_group_cache: HashMap::with_hasher(Default::default()),
+            font_group_cache: HashMap::with_hasher(Default::default()),
             epoch: 0,
         }
     }
 
-    /// Create a font for use in layout calculations.
+    /// Create a `Font` for use in layout calculations, from a `FontTemplateInfo` returned by the
-    fn create_layout_font(&self,
+    /// cache thread (which contains the underlying font data) and a `FontDescriptor` which
-                          template: Arc<FontTemplateData>,
+    /// contains the styling parameters.
-                          descriptor: FontTemplateDescriptor,
+    fn create_font(&self, info: FontTemplateInfo, descriptor: FontDescriptor) -> Result<Font, ()> {
-                          pt_size: Au,
-                          variant: FontVariantCaps,
-                          font_key: webrender_api::FontKey) -> Result<Font, ()> {
         // TODO: (Bug #3463): Currently we only support fake small-caps
         // painting. We should also support true small-caps (where the
         // font supports it) in the future.
-        let actual_pt_size = match variant {
+        let actual_pt_size = match descriptor.variant {
-            FontVariantCaps::SmallCaps => pt_size.scale_by(SMALL_CAPS_SCALE_FACTOR),
+            FontVariantCaps::SmallCaps => descriptor.pt_size.scale_by(SMALL_CAPS_SCALE_FACTOR),
-            FontVariantCaps::Normal => pt_size,
+            FontVariantCaps::Normal => descriptor.pt_size,
         };
 
         let handle = FontHandle::new_from_template(&self.platform_handle,
-                                                        template,
+                                                        info.font_template,
                                                         Some(actual_pt_size))?;
 
         let font_instance_key = self.font_cache_thread
-                                    .get_font_instance(font_key, actual_pt_size);
+                                    .get_font_instance(info.font_key, actual_pt_size);
-        Ok(Font::new(handle, variant, descriptor, pt_size, actual_pt_size, font_instance_key))
+        Ok(Font::new(handle, descriptor.to_owned(), actual_pt_size, font_instance_key))
     }
 
     fn expire_font_caches_if_necessary(&mut self) {
@@ -104,133 +121,103 @@ impl FontContext {
             return
         }
 
-        self.layout_font_cache.clear();
+        self.font_cache.clear();
         self.fallback_font_cache.clear();
-        self.layout_font_group_cache.clear();
+        self.font_group_cache.clear();
         self.epoch = current_epoch
     }
 
-    /// Create a group of fonts for use in layout calculations. May return
+    /// Returns a `FontGroup` representing fonts which can be used for layout, given the `style`.
-    /// a cached font if this font instance has already been used by
+    /// Font groups are cached, so subsequent calls with the same `style` will return a reference
-    /// this context.
+    /// to an existing `FontGroup`.
-    pub fn layout_font_group_for_style(&mut self, style: ServoArc<style_structs::Font>)
+    pub fn font_group(&mut self, style: Arc<FontStyleStruct>) -> Rc<RefCell<FontGroup>> {
-                                       -> Rc<FontGroup> {
         self.expire_font_caches_if_necessary();
 
-        let layout_font_group_cache_key = LayoutFontGroupCacheKey {
+        let cache_key = FontGroupCacheKey {
-            pointer: style.clone(),
             size: style.font_size.size(),
+            style,
         };
-        if let Some(ref cached_font_group) = self.layout_font_group_cache.get(
+
-                &layout_font_group_cache_key) {
+        if let Some(ref font_group) = self.font_group_cache.get(&cache_key) {
-            return (*cached_font_group).clone()
+            return (*font_group).clone()
         }
 
-        // TODO: The font context holds a strong ref to the cached fonts
+        let font_group = Rc::new(RefCell::new(FontGroup::new(&cache_key.style)));
-        // so they will never be released. Find out a good time to drop them.
+        self.font_group_cache.insert(cache_key, font_group.clone());
-
-        let desc = FontTemplateDescriptor::new(style.font_weight,
-                                               style.font_stretch,
-                                               style.font_style == FontStyle::Italic ||
-                                                style.font_style == FontStyle::Oblique);
-
-        let mut fonts: SmallVec<[Rc<RefCell<Font>>; 8]> = SmallVec::new();
-
-        for family in style.font_family.0.iter() {
-            // GWTODO: Check on real pages if this is faster as Vec() or HashMap().
-            let mut cache_hit = false;
-            for cached_font_entry in &self.layout_font_cache {
-                if cached_font_entry.family == family.name() {
-                    match cached_font_entry.font {
-                        None => {
-                            cache_hit = true;
-                            break;
-                        }
-                        Some(ref cached_font_ref) => {
-                            let cached_font = (*cached_font_ref).borrow();
-                            if cached_font.descriptor == desc &&
-                               cached_font.requested_pt_size == style.font_size.size() &&
-                               cached_font.variant == style.font_variant_caps {
-                                fonts.push((*cached_font_ref).clone());
-                                cache_hit = true;
-                                break;
-                            }
-                        }
-                    }
-                }
-            }
-
-            if !cache_hit {
-                let template_info = self.font_cache_thread.find_font_template(family.clone(),
-                                                                             desc.clone());
-                match template_info {
-                    Some(template_info) => {
-                        let layout_font = self.create_layout_font(template_info.font_template,
-                                                                  desc.clone(),
-                                                                  style.font_size.size(),
-                                                                  style.font_variant_caps,
-                                                                  template_info.font_key);
-                        let font = match layout_font {
-                            Ok(layout_font) => {
-                                let layout_font = Rc::new(RefCell::new(layout_font));
-                                fonts.push(layout_font.clone());
-
-                                Some(layout_font)
-                            }
-                            Err(_) => None
-                        };
-
-                        self.layout_font_cache.push(LayoutFontCacheEntry {
-                            family: family.name().to_owned(),
-                            font: font
-                        });
-                    }
-                    None => {
-                        self.layout_font_cache.push(LayoutFontCacheEntry {
-                            family: family.name().to_owned(),
-                            font: None,
-                        });
-                    }
-                }
-            }
-        }
-
-        // Add a last resort font as a fallback option.
-        let mut cache_hit = false;
-        for cached_font_entry in &self.fallback_font_cache {
-            let cached_font = cached_font_entry.font.borrow();
-            if cached_font.descriptor == desc &&
-                        cached_font.requested_pt_size == style.font_size.size() &&
-                        cached_font.variant == style.font_variant_caps {
-                fonts.push(cached_font_entry.font.clone());
-                cache_hit = true;
-                break;
-            }
-        }
-
-        if !cache_hit {
-            let template_info = self.font_cache_thread.last_resort_font_template(desc.clone());
-            let layout_font = self.create_layout_font(template_info.font_template,
-                                                      desc.clone(),
-                                                      style.font_size.size(),
-                                                      style.font_variant_caps,
-                                                      template_info.font_key);
-            match layout_font {
-                Ok(layout_font) => {
-                    let layout_font = Rc::new(RefCell::new(layout_font));
-                    self.fallback_font_cache.push(FallbackFontCacheEntry {
-                        font: layout_font.clone(),
-                    });
-                    fonts.push(layout_font);
-                }
-                Err(_) => debug!("Failed to create fallback layout font!")
-            }
-        }
-
-        let font_group = Rc::new(FontGroup::new(fonts));
-        self.layout_font_group_cache.insert(layout_font_group_cache_key, font_group.clone());
         font_group
     }
+
+    /// Returns a reference to an existing font cache entry matching `descriptor` and `family`, if
+    /// there is one.
+    fn font_cache_entry(&self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> Option<&FontCacheEntry> {
+        self.font_cache.iter()
+            .find(|cache_entry| cache_entry.matches(&descriptor, &family))
+    }
+
+    /// Creates a new font cache entry matching `descriptor` and `family`.
+    fn create_font_cache_entry(&self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> FontCacheEntry {
+        let font =
+            self.font_cache_thread.find_font_template(family.clone(), descriptor.template_descriptor.clone())
+                .and_then(|template_info|
+                    self.create_font(template_info, descriptor.to_owned()).ok()
+                )
+                .map(|font| Rc::new(RefCell::new(font)));
+
+        FontCacheEntry { family: family.atom().to_owned(), font }
+    }
+
+    /// Returns a font from `family` matching the `descriptor`. Fonts are cached, so repeated calls
+    /// will return a reference to the same underlying `Font`.
+    pub fn font(&mut self, descriptor: &FontDescriptor, family: &SingleFontFamily) -> Option<FontRef> {
+        if let Some(entry) = self.font_cache_entry(descriptor, family) {
+            return entry.font.clone()
+        }
+
+        let entry = self.create_font_cache_entry(descriptor, family);
+        let font = entry.font.clone();
+        self.font_cache.push(entry);
+        font
+    }
+
+    /// Returns a reference to an existing fallback font cache entry matching `descriptor`, if
+    /// there is one.
+    fn fallback_font_cache_entry(&self, descriptor: &FontDescriptor) -> Option<&FallbackFontCacheEntry> {
+        self.fallback_font_cache.iter()
+            .find(|cache_entry| cache_entry.matches(descriptor))
+    }
+
+    /// Creates a new fallback font cache entry matching `descriptor`.
+    fn create_fallback_font_cache_entry(&self, descriptor: &FontDescriptor) -> Option<FallbackFontCacheEntry> {
+        let template_info = self.font_cache_thread.last_resort_font_template(descriptor.template_descriptor.clone());
+
+        match self.create_font(template_info, descriptor.to_owned()) {
+            Ok(font) =>
+                Some(FallbackFontCacheEntry {
+                    font: Rc::new(RefCell::new(font))
+                }),
+
+            Err(_) => {
+                debug!("Failed to create fallback font!");
+                None
+            }
+        }
+    }
+
+    /// Returns a fallback font matching the `descriptor`. Fonts are cached, so repeated calls will
+    /// return a reference to the same underlying `Font`.
+    pub fn fallback_font(&mut self, descriptor: &FontDescriptor) -> Option<FontRef> {
+        if let Some(cached_entry) = self.fallback_font_cache_entry(descriptor) {
+            return Some(cached_entry.font.clone())
+        };
+
+        if let Some(entry) = self.create_fallback_font_cache_entry(descriptor) {
+            let font = entry.font.clone();
+            self.fallback_font_cache.push(entry);
+            Some(font)
+        } else {
+            None
+        }
+    }
 }
 
 impl MallocSizeOf for FontContext {
@@ -241,22 +228,22 @@ impl MallocSizeOf for FontContext {
 }
 
 #[derive(Debug)]
-struct LayoutFontGroupCacheKey {
+struct FontGroupCacheKey {
-    pointer: ServoArc<style_structs::Font>,
+    style: Arc<FontStyleStruct>,
     size: Au,
 }
 
-impl PartialEq for LayoutFontGroupCacheKey {
+impl PartialEq for FontGroupCacheKey {
-    fn eq(&self, other: &LayoutFontGroupCacheKey) -> bool {
+    fn eq(&self, other: &FontGroupCacheKey) -> bool {
-        self.pointer == other.pointer && self.size == other.size
+        self.style == other.style && self.size == other.size
     }
 }
 
-impl Eq for LayoutFontGroupCacheKey {}
+impl Eq for FontGroupCacheKey {}
 
-impl Hash for LayoutFontGroupCacheKey {
+impl Hash for FontGroupCacheKey {
     fn hash<H>(&self, hasher: &mut H) where H: Hasher {
-        self.pointer.hash.hash(hasher)
+        self.style.hash.hash(hasher)
     }
 }
 

components/gfx/font_template.rs

@@ -11,7 +11,10 @@ use std::fmt::{Debug, Error, Formatter};
 use std::io::Error as IoError;
 use std::sync::{Arc, Weak};
 use std::u32;
-use style::computed_values::{font_stretch, font_weight};
+use style::computed_values::font_stretch::T as FontStretch;
+use style::computed_values::font_style::T as FontStyle;
+use style::properties::style_structs::Font as FontStyleStruct;
+use style::values::computed::font::FontWeight;
 
 /// Describes how to select a font from a given family. This is very basic at the moment and needs
 /// to be expanded or refactored when we support more of the font styling parameters.
@@ -19,14 +22,14 @@ use style::computed_values::{font_stretch, font_weight};
 /// NB: If you change this, you will need to update `style::properties::compute_font_hash()`.
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Serialize)]
 pub struct FontTemplateDescriptor {
-    pub weight: font_weight::T,
+    pub weight: FontWeight,
-    pub stretch: font_stretch::T,
+    pub stretch: FontStretch,
     pub italic: bool,
 }
 
 impl FontTemplateDescriptor {
     #[inline]
-    pub fn new(weight: font_weight::T, stretch: font_stretch::T, italic: bool)
+    pub fn new(weight: FontWeight, stretch: FontStretch, italic: bool)
                -> FontTemplateDescriptor {
         FontTemplateDescriptor {
             weight: weight,
@@ -57,15 +60,25 @@ impl FontTemplateDescriptor {
     #[inline]
     fn stretch_number(&self) -> i32 {
         match self.stretch {
-            font_stretch::T::UltraCondensed => 1,
+            FontStretch::UltraCondensed => 1,
-            font_stretch::T::ExtraCondensed => 2,
+            FontStretch::ExtraCondensed => 2,
-            font_stretch::T::Condensed      => 3,
+            FontStretch::Condensed      => 3,
-            font_stretch::T::SemiCondensed  => 4,
+            FontStretch::SemiCondensed  => 4,
-            font_stretch::T::Normal         => 5,
+            FontStretch::Normal         => 5,
-            font_stretch::T::SemiExpanded   => 6,
+            FontStretch::SemiExpanded   => 6,
-            font_stretch::T::Expanded       => 7,
+            FontStretch::Expanded       => 7,
-            font_stretch::T::ExtraExpanded  => 8,
+            FontStretch::ExtraExpanded  => 8,
-            font_stretch::T::UltraExpanded  => 9,
+            FontStretch::UltraExpanded  => 9,
+        }
+    }
+}
+
+impl<'a> From<&'a FontStyleStruct> for FontTemplateDescriptor {
+    fn from(style: &'a FontStyleStruct) -> Self {
+        FontTemplateDescriptor {
+            weight: style.font_weight,
+            stretch: style.font_stretch,
+            italic: style.font_style == FontStyle::Italic || style.font_style == FontStyle::Oblique,
         }
     }
 }

components/gfx/platform/freetype/font.rs

@@ -17,6 +17,7 @@ use freetype::freetype::FT_Sfnt_Tag;
 use freetype::tt_os2::TT_OS2;
 use platform::font_context::FontContextHandle;
 use platform::font_template::FontTemplateData;
+use servo_atoms::Atom;
 use std::{mem, ptr};
 use std::os::raw::{c_char, c_long};
 use std::sync::Arc;
@@ -306,6 +307,10 @@ impl FontHandleMethods for FontHandle {
             Some(FontTable { buffer: buf })
         }
     }
+
+    fn identifier(&self) -> Atom {
+        self.font_data.identifier.clone()
+    }
 }
 
 impl<'a> FontHandle {

components/gfx/platform/macos/font.rs

@@ -18,6 +18,7 @@ use font::{FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, Fract
 use font::{GPOS, GSUB, KERN};
 use platform::font_template::FontTemplateData;
 use platform::macos::font_context::FontContextHandle;
+use servo_atoms::Atom;
 use std::{fmt, ptr};
 use std::ops::Range;
 use std::sync::Arc;
@@ -318,4 +319,8 @@ impl FontHandleMethods for FontHandle {
             Some(FontTable::wrap(data))
         })
     }
+
+    fn identifier(&self) -> Atom {
+        self.font_data.identifier.clone()
+    }
 }

components/gfx/platform/windows/font.rs

@@ -15,6 +15,7 @@ use font::{FontTableTag, FractionalPixel};
 use platform::font_template::FontTemplateData;
 use platform::windows::font_context::FontContextHandle;
 use platform::windows::font_list::font_from_atom;
+use servo_atoms::Atom;
 use std::sync::Arc;
 use style::computed_values::font_stretch::T as StyleFontStretch;
 use style::computed_values::font_weight::T as StyleFontWeight;
@@ -374,4 +375,8 @@ impl FontHandleMethods for FontHandle {
     fn table_for_tag(&self, tag: FontTableTag) -> Option<FontTable> {
         self.face.get_font_table(tag).map(|bytes| FontTable { data: bytes })
     }
+
+    fn identifier(&self) -> Atom {
+        self.font_data.identifier.clone()
+    }
 }

components/layout/text.rs

@@ -9,7 +9,7 @@
 use app_units::Au;
 use fragment::{Fragment, ScannedTextFlags};
 use fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo, UnscannedTextFragmentInfo};
-use gfx::font::{FontMetrics, RunMetrics, ShapingFlags, ShapingOptions};
+use gfx::font::{FontRef, FontMetrics, RunMetrics, ShapingFlags, ShapingOptions};
 use gfx::font_context::FontContext;
 use gfx::text::glyph::ByteIndex;
 use gfx::text::text_run::TextRun;
@@ -18,6 +18,7 @@ use inline::{InlineFragmentNodeFlags, InlineFragments};
 use linked_list::split_off_head;
 use ordered_float::NotNaN;
 use range::Range;
+use servo_atoms::Atom;
 use std::borrow::ToOwned;
 use std::collections::LinkedList;
 use std::mem;
@@ -28,7 +29,7 @@ use style::computed_values::white_space::T as WhiteSpace;
 use style::computed_values::word_break::T as WordBreak;
 use style::logical_geometry::{LogicalSize, WritingMode};
 use style::properties::ComputedValues;
-use style::properties::style_structs;
+use style::properties::style_structs::Font as FontStyleStruct;
 use style::values::generics::text::LineHeight;
 use unicode_bidi as bidi;
 use unicode_script::{Script, get_script};
@@ -136,7 +137,7 @@ impl TextRunScanner {
     /// for correct painting order. Since we compress several leaf fragments here, the mapping must
     /// be adjusted.
     fn flush_clump_to_list(&mut self,
-                           font_context: &mut FontContext,
+                           mut font_context: &mut FontContext,
                            out_fragments: &mut Vec<Fragment>,
                            paragraph_bytes_processed: &mut usize,
                            bidi_levels: Option<&[bidi::Level]>,
@@ -159,7 +160,7 @@ impl TextRunScanner {
         // Concatenate all of the transformed strings together, saving the new character indices.
         let mut mappings: Vec<RunMapping> = Vec::new();
         let runs = {
-            let fontgroup;
+            let font_group;
             let compression;
             let text_transform;
             let letter_spacing;
@@ -170,7 +171,7 @@ impl TextRunScanner {
                 let in_fragment = self.clump.front().unwrap();
                 let font_style = in_fragment.style().clone_font();
                 let inherited_text_style = in_fragment.style().get_inheritedtext();
-                fontgroup = font_context.layout_font_group_for_style(font_style);
+                font_group = font_context.font_group(font_style);
                 compression = match in_fragment.white_space() {
                     WhiteSpace::Normal |
                     WhiteSpace::Nowrap => CompressionMode::CompressWhitespaceNewline,
@@ -214,14 +215,7 @@ impl TextRunScanner {
 
                 let (mut start_position, mut end_position) = (0, 0);
                 for (byte_index, character) in text.char_indices() {
-                    // Search for the first font in this font group that contains a glyph for this
+                    let font = font_group.borrow_mut().find_by_codepoint(&mut font_context, character);
-                    // character.
-                    let font_index = fontgroup.fonts.iter().position(|font| {
-                        font.borrow().glyph_index(character).is_some()
-                    }).unwrap_or(0);
-
-                    // The following code panics one way or another if this condition isn't met.
-                    assert!(fontgroup.fonts.len() > 0);
 
                     let bidi_level = match bidi_levels {
                         Some(levels) => levels[*paragraph_bytes_processed],
@@ -245,7 +239,7 @@ impl TextRunScanner {
                     };
 
                     // Now, if necessary, flush the mapping we were building up.
-                    let flush_run = run_info.font_index != font_index ||
+                    let flush_run = !run_info.has_font(&font) ||
                                     run_info.bidi_level != bidi_level ||
                                     !compatible_script;
                     let new_mapping_needed = flush_run || mapping.selected != selected;
@@ -272,7 +266,7 @@ impl TextRunScanner {
                             mapping = RunMapping::new(&run_info_list[..],
                                                       fragment_index);
                         }
-                        run_info.font_index = font_index;
+                        run_info.font = font;
                         run_info.bidi_level = bidi_level;
                         run_info.script = script;
                         mapping.selected = selected;
@@ -328,9 +322,14 @@ impl TextRunScanner {
                 if run_info.bidi_level.is_rtl() {
                     options.flags.insert(ShapingFlags::RTL_FLAG);
                 }
-                let mut font = fontgroup.fonts.get(run_info.font_index).unwrap().borrow_mut();
 
-                let (run, break_at_zero) = TextRun::new(&mut *font,
+                // If no font is found (including fallbacks), there's no way we can render.
+                let font =
+                    run_info.font
+                        .or_else(|| font_group.borrow_mut().first(&mut font_context))
+                        .expect("No font found for text run!");
+
+                let (run, break_at_zero) = TextRun::new(&mut *font.borrow_mut(),
                                                         run_info.text,
                                                         &options,
                                                         run_info.bidi_level,
@@ -456,15 +455,20 @@ fn bounding_box_for_run_metrics(metrics: &RunMetrics, writing_mode: WritingMode)
         metrics.bounding_box.size.height)
 }
 
-/// Returns the metrics of the font represented by the given `style_structs::Font`, respectively.
+/// Returns the metrics of the font represented by the given `FontStyleStruct`.
 ///
 /// `#[inline]` because often the caller only needs a few fields from the font metrics.
+///
+/// # Panics
+///
+/// Panics if no font can be found for the given font style.
 #[inline]
-pub fn font_metrics_for_style(font_context: &mut FontContext, font_style: ::ServoArc<style_structs::Font>)
+pub fn font_metrics_for_style(mut font_context: &mut FontContext, style: ::ServoArc<FontStyleStruct>)
                               -> FontMetrics {
-    let fontgroup = font_context.layout_font_group_for_style(font_style);
+    let font_group = font_context.font_group(style);
-    // FIXME(https://github.com/rust-lang/rust/issues/23338)
+    let font = font_group.borrow_mut().first(&mut font_context);
-    let font = fontgroup.fonts[0].borrow();
+    let font = font.as_ref().unwrap().borrow();
+
     font.metrics.clone()
 }
 
@@ -546,8 +550,8 @@ struct RunInfo {
     text: String,
     /// The insertion point in this text run, if applicable.
     insertion_point: Option<ByteIndex>,
-    /// The index of the applicable font in the font group.
+    /// The font that the text should be rendered with.
-    font_index: usize,
+    font: Option<FontRef>,
     /// The bidirection embedding level of this text run.
     bidi_level: bidi::Level,
     /// The Unicode script property of this text run.
@@ -559,7 +563,7 @@ impl RunInfo {
         RunInfo {
             text: String::new(),
             insertion_point: None,
-            font_index: 0,
+            font: None,
             bidi_level: bidi::Level::ltr(),
             script: Script::Common,
         }
@@ -584,6 +588,14 @@ impl RunInfo {
         }
         list.push(self);
     }
+
+    fn has_font(&self, font: &Option<FontRef>) -> bool {
+        fn identifier(font: &Option<FontRef>) -> Option<Atom> {
+            font.as_ref().map(|f| f.borrow().identifier())
+        }
+
+        identifier(&self.font) == identifier(font)
+    }
 }
 
 /// A mapping from a portion of an unscanned text fragment to the text run we're going to create