Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
FontContext: Cache data fetched from the cache thread
Before this change, if we needed to create a Font which we've already
created, but at a new size, then we'd fetch the FontTemplateInfo again.
If the bytes of the font are held in memory, then this could be
expensive as we need to pass those bytes over IPC.
  • Loading branch information
jonleighton committed May 19, 2018
1 parent dc683a1 commit 3025a26
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 9 deletions.
2 changes: 1 addition & 1 deletion components/gfx/font.rs
Expand Up @@ -562,7 +562,7 @@ pub struct FontFamilyDescriptor {
}

impl FontFamilyDescriptor {
fn new(name: FontFamilyName, scope: FontSearchScope) -> FontFamilyDescriptor {
pub fn new(name: FontFamilyName, scope: FontSearchScope) -> FontFamilyDescriptor {
FontFamilyDescriptor { name, scope }
}

Expand Down
2 changes: 1 addition & 1 deletion components/gfx/font_cache_thread.rs
Expand Up @@ -32,7 +32,7 @@ pub struct FontTemplates {
templates: Vec<FontTemplate>,
}

#[derive(Debug, Deserialize, Serialize)]
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct FontTemplateInfo {
pub font_template: Arc<FontTemplateData>,
pub font_key: webrender_api::FontKey,
Expand Down
45 changes: 39 additions & 6 deletions components/gfx/font_context.rs
Expand Up @@ -50,6 +50,7 @@ pub struct FontContext<S: FontSource> {
// so they will never be released. Find out a good time to drop them.
// See bug https://github.com/servo/servo/issues/3300
font_cache: HashMap<FontCacheKey, Option<FontRef>>,
font_template_cache: HashMap<FontTemplateCacheKey, Option<FontTemplateInfo>>,

font_group_cache:
HashMap<FontGroupCacheKey, Rc<RefCell<FontGroup>>, BuildHasherDefault<FnvHasher>>,
Expand All @@ -64,6 +65,7 @@ impl<S: FontSource> FontContext<S> {
platform_handle: handle,
font_source,
font_cache: HashMap::new(),
font_template_cache: HashMap::new(),
font_group_cache: HashMap::with_hasher(Default::default()),
epoch: 0,
}
Expand All @@ -76,6 +78,7 @@ impl<S: FontSource> FontContext<S> {
}

self.font_cache.clear();
self.font_template_cache.clear();
self.font_group_cache.clear();
self.epoch = current_epoch
}
Expand Down Expand Up @@ -120,18 +123,42 @@ impl<S: FontSource> FontContext<S> {
);

let font =
self.font_source.font_template(
font_descriptor.template_descriptor.clone(),
family_descriptor.clone(),
)
.and_then(|template_info| self.create_font(template_info, font_descriptor.to_owned()).ok())
.map(|font| Rc::new(RefCell::new(font)));
self.font_template(&font_descriptor.template_descriptor, family_descriptor)
.and_then(|template_info| self.create_font(template_info, font_descriptor.to_owned()).ok())
.map(|font| Rc::new(RefCell::new(font)));

self.font_cache.insert(cache_key, font.clone());
font
})
}

fn font_template(
&mut self,
template_descriptor: &FontTemplateDescriptor,
family_descriptor: &FontFamilyDescriptor
) -> Option<FontTemplateInfo> {
let cache_key = FontTemplateCacheKey {
template_descriptor: template_descriptor.clone(),
family_descriptor: family_descriptor.clone(),
};

self.font_template_cache.get(&cache_key).map(|v| v.clone()).unwrap_or_else(|| {
debug!(
"FontContext::font_template cache miss for template_descriptor={:?} family_descriptor={:?}",
template_descriptor,
family_descriptor
);

let template_info = self.font_source.font_template(
template_descriptor.clone(),
family_descriptor.clone(),
);

self.font_template_cache.insert(cache_key, template_info.clone());
template_info
})
}

/// Create a `Font` for use in layout calculations, from a `FontTemplateData` returned by the
/// cache thread and a `FontDescriptor` which contains the styling parameters.
fn create_font(
Expand Down Expand Up @@ -171,6 +198,12 @@ struct FontCacheKey {
family_descriptor: FontFamilyDescriptor,
}

#[derive(Debug, Eq, Hash, PartialEq)]
struct FontTemplateCacheKey {
template_descriptor: FontTemplateDescriptor,
family_descriptor: FontFamilyDescriptor,
}

#[derive(Debug)]
struct FontGroupCacheKey {
style: Arc<FontStyleStruct>,
Expand Down
37 changes: 36 additions & 1 deletion components/gfx/tests/font_context.rs
Expand Up @@ -10,7 +10,7 @@ extern crate style;
extern crate webrender_api;

use app_units::Au;
use gfx::font::{fallback_font_families, FontFamilyDescriptor, FontHandleMethods};
use gfx::font::{fallback_font_families, FontDescriptor, FontFamilyDescriptor, FontFamilyName, FontSearchScope};
use gfx::font_cache_thread::{FontTemplates, FontTemplateInfo};
use gfx::font_context::{FontContext, FontContextHandle, FontSource};
use gfx::font_template::FontTemplateDescriptor;
Expand Down Expand Up @@ -195,3 +195,38 @@ fn test_font_fallback() {
"a fallback font should be used if there is no matching glyph in the group"
);
}

#[test]
fn test_font_template_is_cached() {
let source = TestFontSource::new();
let count = source.find_font_count.clone();
let mut context = FontContext::new(source);

let mut font_descriptor = FontDescriptor {
template_descriptor: FontTemplateDescriptor {
weight: FontWeight::normal(),
stretch: FontStretch::hundred(),
style: FontStyle::Normal,
},
variant: FontVariantCaps::Normal,
pt_size: Au(10),
};

let family_descriptor = FontFamilyDescriptor::new(
FontFamilyName::from("CSSTest Basic"),
FontSearchScope::Any,
);

let font1 = context.font(&font_descriptor, &family_descriptor).unwrap();

font_descriptor.pt_size = Au(20);
let font2 = context.font(&font_descriptor, &family_descriptor).unwrap();

assert_ne!(
font1.borrow().actual_pt_size,
font2.borrow().actual_pt_size,
"the same font should not have been returned"
);

assert_eq!(count.get(), 1, "we should only have fetched the template data from the cache thread once");
}

0 comments on commit 3025a26

Please sign in to comment.