Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stylo: Make font base size computation threadsafe #16316

Merged
merged 3 commits into from Apr 9, 2017
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev

stylo: Use font metrics provider as a cache for font size results

  • Loading branch information
Manishearth committed Apr 9, 2017
commit 2febe7ccde1c40826800927cbfeed29545091831
@@ -308,6 +308,7 @@ mod bindings {
"mozilla::SERVO_PREF_.*",
"kNameSpaceID_.*",
"kGenericFont_.*",
"kPresContext_.*",
];
let whitelist = [
"RawGecko.*",
@@ -46,8 +46,16 @@ 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 {
@@ -61,7 +69,7 @@ pub fn get_metrics_provider_for_product() -> 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
@@ -72,9 +80,7 @@ pub trait FontMetricsProvider: Send + fmt::Debug {
}

/// 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;
@@ -14,13 +14,15 @@
//! 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;
@@ -432,8 +434,11 @@ fn get_animation_rule(element: &GeckoElement,
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)>>,
}

@@ -446,10 +451,38 @@ impl GeckoFontMetricsProvider {
}
}

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> {
@@ -1063,9 +1063,7 @@ extern "C" {
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;
#[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 {
#[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 {
@@ -186,10 +186,11 @@ impl fmt::Display for WeakAtom {

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
@@ -562,17 +562,10 @@ ${helpers.single_keyword("font-variant-caps",

// 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;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.