From 3025a269ec47240226cdd83f7138c4e61ca379f0 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 2 Apr 2018 21:49:19 +1000 Subject: [PATCH] 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. --- components/gfx/font.rs | 2 +- components/gfx/font_cache_thread.rs | 2 +- components/gfx/font_context.rs | 45 ++++++++++++++++++++++++---- components/gfx/tests/font_context.rs | 37 ++++++++++++++++++++++- 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/components/gfx/font.rs b/components/gfx/font.rs index f4864448a373..b45707330f20 100644 --- a/components/gfx/font.rs +++ b/components/gfx/font.rs @@ -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 } } diff --git a/components/gfx/font_cache_thread.rs b/components/gfx/font_cache_thread.rs index 523837c78fb2..df04519e90b6 100644 --- a/components/gfx/font_cache_thread.rs +++ b/components/gfx/font_cache_thread.rs @@ -32,7 +32,7 @@ pub struct FontTemplates { templates: Vec, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct FontTemplateInfo { pub font_template: Arc, pub font_key: webrender_api::FontKey, diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs index 6567e223e867..a09e6cf7b57d 100644 --- a/components/gfx/font_context.rs +++ b/components/gfx/font_context.rs @@ -50,6 +50,7 @@ pub struct FontContext { // 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>, + font_template_cache: HashMap>, font_group_cache: HashMap>, BuildHasherDefault>, @@ -64,6 +65,7 @@ impl FontContext { platform_handle: handle, font_source, font_cache: HashMap::new(), + font_template_cache: HashMap::new(), font_group_cache: HashMap::with_hasher(Default::default()), epoch: 0, } @@ -76,6 +78,7 @@ impl FontContext { } self.font_cache.clear(); + self.font_template_cache.clear(); self.font_group_cache.clear(); self.epoch = current_epoch } @@ -120,18 +123,42 @@ impl FontContext { ); 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 { + 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( @@ -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, diff --git a/components/gfx/tests/font_context.rs b/components/gfx/tests/font_context.rs index a16d38229976..dc9d1ac2df50 100644 --- a/components/gfx/tests/font_context.rs +++ b/components/gfx/tests/font_context.rs @@ -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; @@ -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"); +}