Bug 1351200 - Part 5: stylo: Use font metrics provider as a cache for font size results; r?emilio draft
authorManish Goregaokar <manishearth@gmail.com>
Tue, 04 Apr 2017 11:11:27 -0700
changeset 559265 f04f9ac72cb87cd7e86cbcfc7933a9060bc88b81
parent 559264 cf8a6b1137cfb4db96984008f209a5e09f14fe1f
child 623336 6453aef8d8a12acb2a16f9a3ce4af103ce0bf3ff
push id53030
push userbmo:manishearth@gmail.com
push dateSun, 09 Apr 2017 09:34:27 +0000
reviewersemilio
bugs1351200
milestone55.0a1
Bug 1351200 - Part 5: stylo: Use font metrics provider as a cache for font size results; r?emilio MozReview-Commit-ID: KGR6Tbyu3sT
servo/components/style/build_gecko.rs
servo/components/style/font_metrics.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/gecko_bindings/bindings.rs
servo/components/style/gecko_bindings/structs_debug.rs
servo/components/style/gecko_bindings/structs_release.rs
servo/components/style/gecko_string_cache/mod.rs
servo/components/style/properties/longhand/font.mako.rs
--- a/servo/components/style/build_gecko.rs
+++ b/servo/components/style/build_gecko.rs
@@ -300,16 +300,17 @@ mod bindings {
             "NS_FONT_.*",
             "NS_STYLE_.*",
             "NS_RADIUS_.*",
             "BORDER_COLOR_.*",
             "BORDER_STYLE_.*",
             "mozilla::SERVO_PREF_.*",
             "kNameSpaceID_.*",
             "kGenericFont_.*",
+            "kPresContext_.*",
         ];
         let whitelist = [
             "RawGecko.*",
             "mozilla::ServoStyleSheet",
             "mozilla::ServoElementSnapshot.*",
             "mozilla::CSSPseudoClassType",
             "mozilla::css::SheetParsingMode",
             "mozilla::HalfCorner",
--- a/servo/components/style/font_metrics.rs
+++ b/servo/components/style/font_metrics.rs
@@ -41,42 +41,48 @@ pub enum FontMetricsQueryResult {
 /// any metrics.
 pub struct ServoMetricsProvider;
 
 #[cfg(feature = "servo")]
 impl FontMetricsProvider for ServoMetricsProvider {
     fn create_from(_: &SharedStyleContext) -> Self {
         ServoMetricsProvider
     }
+
+    fn get_size(&self, _font_name: &Atom, _font_family: u8) -> Au {
+        unreachable!("Dummy provider should never be used to compute font size")
+    }
 }
 
+// Servo's font metrics provider will probably not live in this crate, so this will
+// have to be replaced with something else (perhaps a trait method on TElement)
+// when we get there
+
 #[cfg(feature = "gecko")]
 /// Construct a font metrics provider for the current product
 pub fn get_metrics_provider_for_product() -> ::gecko::wrapper::GeckoFontMetricsProvider {
     ::gecko::wrapper::GeckoFontMetricsProvider::new()
 }
 
 #[cfg(feature = "servo")]
 /// Construct a font metrics provider for the current product
 pub fn get_metrics_provider_for_product() -> ServoMetricsProvider {
     ServoMetricsProvider
 }
 
 /// A trait used to represent something capable of providing us font metrics.
-pub trait FontMetricsProvider: Send + fmt::Debug {
+pub trait FontMetricsProvider: fmt::Debug {
     /// Obtain the metrics for given font family.
     ///
     /// TODO: We could make this take the full list, I guess, and save a few
     /// virtual calls in the case we are repeatedly unable to find font metrics?
     /// That is not too common in practice though.
     fn query(&self, _font_name: &Atom) -> FontMetricsQueryResult {
         FontMetricsQueryResult::NotAvailable
     }
 
     /// Get default size of a given language and generic family
-    fn get_size(&self, _font_name: &Atom, _font_family: u8) -> Au {
-        unimplemented!()
-    }
+    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au;
 
     /// Construct from a shared style context
     fn create_from(context: &SharedStyleContext) -> Self where Self: Sized;
 }
 
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -9,23 +9,25 @@
 //!
 //! This really follows the Servo pattern in
 //! `components/script/layout_wrapper.rs`.
 //!
 //! This theoretically should live in its own crate, but now it lives in the
 //! style system it's kind of pointless in the Stylo case, and only Servo forces
 //! the separation between the style system implementation and everything else.
 
+use app_units::Au;
 use atomic_refcell::AtomicRefCell;
 use context::{SharedStyleContext, UpdateAnimationsTasks};
 use data::ElementData;
 use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthetizer};
 use element_state::ElementState;
 use error_reporting::StdoutErrorReporter;
+use font_metrics::FontMetricsProvider;
 use gecko::global_style_data::GLOBAL_STYLE_DATA;
 use gecko::selector_parser::{SelectorImpl, NonTSPseudoClass, PseudoElement};
 use gecko::snapshot_helpers;
 use gecko_bindings::bindings;
 use gecko_bindings::bindings::{Gecko_DropStyleChildrenIterator, Gecko_MaybeCreateStyleChildrenIterator};
 use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetLastChild, Gecko_GetNextStyleChild};
 use gecko_bindings::bindings::{Gecko_IsRootElement, Gecko_MatchesElement, Gecko_Namespace};
 use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags};
@@ -427,34 +429,65 @@ fn get_animation_rule(element: &GeckoEle
     }
 }
 
 #[derive(Debug)]
 /// Gecko font metrics provider
 pub struct GeckoFontMetricsProvider {
     /// Cache of base font sizes for each language
     ///
-    /// Should have at most 31 elements (the number of language groups). Usually
-    /// will have 1.
+    /// Usually will have 1 element.
+    ///
+    // This may be slow on pages using more languages, might be worth optimizing
+    // by caching lang->group mapping separately and/or using a hashmap on larger
+    // loads.
     pub font_size_cache: RefCell<Vec<(Atom, ::gecko_bindings::structs::FontSizePrefs)>>,
 }
 
 impl GeckoFontMetricsProvider {
     /// Construct
     pub fn new() -> Self {
         GeckoFontMetricsProvider {
             font_size_cache: RefCell::new(Vec::new()),
         }
     }
 }
 
-impl ::font_metrics::FontMetricsProvider for GeckoFontMetricsProvider {
-    fn create_from(_: &SharedStyleContext) -> Self {
+impl FontMetricsProvider for GeckoFontMetricsProvider {
+    fn create_from(_: &SharedStyleContext) -> GeckoFontMetricsProvider {
         GeckoFontMetricsProvider::new()
     }
+
+    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au {
+        use gecko_bindings::bindings::Gecko_GetBaseSize;
+        let mut cache = self.font_size_cache.borrow_mut();
+        if let Some(sizes) = cache.iter().find(|el| el.0 == *font_name) {
+            return sizes.1.size_for_generic(font_family);
+        }
+        let sizes = unsafe {
+            Gecko_GetBaseSize(font_name.as_ptr())
+        };
+        cache.push((font_name.clone(), sizes));
+        sizes.size_for_generic(font_family)
+    }
+}
+
+impl structs::FontSizePrefs {
+    fn size_for_generic(&self, font_family: u8) -> Au {
+        Au(match font_family {
+            structs::kPresContext_DefaultVariableFont_ID => self.mDefaultVariableSize,
+            structs::kPresContext_DefaultFixedFont_ID => self.mDefaultFixedSize,
+            structs::kGenericFont_serif => self.mDefaultSerifSize,
+            structs::kGenericFont_sans_serif => self.mDefaultSansSerifSize,
+            structs::kGenericFont_monospace => self.mDefaultMonospaceSize,
+            structs::kGenericFont_cursive => self.mDefaultCursiveSize,
+            structs::kGenericFont_fantasy => self.mDefaultFantasySize,
+            x => unreachable!("Unknown generic ID {}", x),
+        })
+    }
 }
 
 impl<'le> TElement for GeckoElement<'le> {
     type ConcreteNode = GeckoNode<'le>;
     type FontMetricsProvider = GeckoFontMetricsProvider;
 
     fn as_node(&self) -> Self::ConcreteNode {
         unsafe { GeckoNode(&*(self.0 as *const _ as *const RawGeckoNode)) }
--- a/servo/components/style/gecko_bindings/bindings.rs
+++ b/servo/components/style/gecko_bindings/bindings.rs
@@ -1017,19 +1017,17 @@ extern "C" {
     pub fn Gecko_nsStyleFont_SetLang(font: *mut nsStyleFont,
                                      atom: *mut nsIAtom);
 }
 extern "C" {
     pub fn Gecko_nsStyleFont_CopyLangFrom(aFont: *mut nsStyleFont,
                                           aSource: *const nsStyleFont);
 }
 extern "C" {
-    pub fn Gecko_GetBaseSize(lang: *mut nsIAtom,
-                             pres_context: RawGeckoPresContextBorrowed)
-     -> FontSizePrefs;
+    pub fn Gecko_GetBaseSize(lang: *mut nsIAtom) -> FontSizePrefs;
 }
 extern "C" {
     pub fn Gecko_GetMediaFeatures() -> *const nsMediaFeature;
 }
 extern "C" {
     pub fn Gecko_CSSFontFaceRule_Create() -> *mut nsCSSFontFaceRule;
 }
 extern "C" {
--- a/servo/components/style/gecko_bindings/structs_debug.rs
+++ b/servo/components/style/gecko_bindings/structs_debug.rs
@@ -22614,16 +22614,18 @@ pub mod root {
     #[derive(Debug, Copy, Clone)]
     pub struct nsAnimationManager([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct nsDeviceContext([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct gfxMissingFontRecorder([u8; 0]);
+    pub const kPresContext_DefaultVariableFont_ID: u8 = 0;
+    pub const kPresContext_DefaultFixedFont_ID: u8 = 1;
     #[repr(C)]
     #[derive(Debug)]
     pub struct nsRootPresContext {
         pub _base: root::nsPresContext,
         pub mNotifyDidPaintTimers: [u64; 10usize],
         pub mApplyPluginGeometryTimer: root::nsCOMPtr<root::nsITimer>,
         pub mRegisteredPlugins: [u64; 6usize],
         pub mWillPaintObservers: root::nsTArray<root::nsCOMPtr<root::nsIRunnable>>,
--- a/servo/components/style/gecko_bindings/structs_release.rs
+++ b/servo/components/style/gecko_bindings/structs_release.rs
@@ -22034,16 +22034,18 @@ pub mod root {
     #[derive(Debug, Copy, Clone)]
     pub struct nsAnimationManager([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct nsDeviceContext([u8; 0]);
     #[repr(C)]
     #[derive(Debug, Copy, Clone)]
     pub struct gfxMissingFontRecorder([u8; 0]);
+    pub const kPresContext_DefaultVariableFont_ID: u8 = 0;
+    pub const kPresContext_DefaultFixedFont_ID: u8 = 1;
     #[repr(C)]
     #[derive(Debug)]
     pub struct nsRootPresContext {
         pub _base: root::nsPresContext,
         pub mNotifyDidPaintTimers: [u64; 10usize],
         pub mApplyPluginGeometryTimer: root::nsCOMPtr<root::nsITimer>,
         pub mRegisteredPlugins: [u64; 5usize],
         pub mWillPaintObservers: root::nsTArray<root::nsCOMPtr<root::nsIRunnable>>,
--- a/servo/components/style/gecko_string_cache/mod.rs
+++ b/servo/components/style/gecko_string_cache/mod.rs
@@ -174,20 +174,21 @@ impl fmt::Display for WeakAtom {
             try!(w.write_char(c.unwrap_or(char::REPLACEMENT_CHARACTER)))
         }
         Ok(())
     }
 }
 
 impl Atom {
     /// Execute a callback with the atom represented by `ptr`.
-    pub unsafe fn with<F>(ptr: *mut nsIAtom, callback: &mut F) where F: FnMut(&Atom) {
+    pub unsafe fn with<F, R: 'static>(ptr: *mut nsIAtom, callback: &mut F) -> R where F: FnMut(&Atom) -> R {
         let atom = Atom(WeakAtom::new(ptr));
-        callback(&atom);
+        let ret = callback(&atom);
         mem::forget(atom);
+        ret
     }
 
     /// Creates an atom from an static atom pointer without checking in release
     /// builds.
     ///
     /// Right now it's only used by the atom macro, and ideally it should keep
     /// that way, now we have sugar for is_static, creating atoms using
     /// Atom::from should involve almost no overhead.
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -557,27 +557,20 @@
                     [9,   10,    13,    15,    18,    23,    30,    45],
                     [9,   10,    13,    16,    18,    24,    32,    48]
                 ];
 
                 static FONT_SIZE_FACTORS: [i32; 8] = [60, 75, 89, 100, 120, 150, 200, 300];
 
                 // XXXManishearth handle quirks mode
 
-                let base_sizes = unsafe {
-                    Gecko_GetBaseSize(cx.style().get_font().gecko().mLanguage.raw())
-                };
-                let base_size = match cx.style().get_font().gecko().mGenericID {
-                    structs::kGenericFont_serif => base_sizes.mDefaultSerifSize,
-                    structs::kGenericFont_sans_serif => base_sizes.mDefaultSansSerifSize,
-                    structs::kGenericFont_monospace => base_sizes.mDefaultMonospaceSize,
-                    structs::kGenericFont_cursive => base_sizes.mDefaultCursiveSize,
-                    structs::kGenericFont_fantasy => base_sizes.mDefaultFantasySize,
-                    x => unreachable!("Unknown generic ID {}", x),
-                };
+                let ref gecko_font = cx.style().get_font().gecko();
+                let base_size = unsafe { Atom::with(gecko_font.mLanguage.raw(), &mut |atom| {
+                    cx.font_metrics_provider.get_size(atom, gecko_font.mGenericID).0
+                }) };
 
                 let base_size_px = au_to_int_px(base_size as f32);
                 let html_size = *self as usize;
                 if base_size_px >= 9 && base_size_px <= 16 {
                     Au::from_px(FONT_SIZE_MAPPING[(base_size_px - 9) as usize][html_size])
                 } else {
                     Au(FONT_SIZE_FACTORS[html_size] * base_size / 100)
                 }