diff options
author | Martin Robinson <mrobinson@igalia.com> | 2024-05-02 12:34:10 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-02 10:34:10 +0000 |
commit | 556bfb7dff48f64e9e02872dba29fbdabc8c6ad0 (patch) | |
tree | 0c9e1e80582fee2a64aa5904df3230e8a3d2befd /components/layout | |
parent | 8ec5344f70dd1d556cacd72d778924048b0b1154 (diff) | |
download | servo-556bfb7dff48f64e9e02872dba29fbdabc8c6ad0.tar.gz servo-556bfb7dff48f64e9e02872dba29fbdabc8c6ad0.zip |
fonts: Make `FontContext` thread-safe and share it per-Layout (#32205)
This allows sharing font templates, fonts, and platform fonts across
layout threads. It's the first step toward storing web fonts in the
layout versus the shared `FontCacheThread`. Now fonts and font groups
have some locking (especially on FreeType), which will probably affect
performance. On the other hand, we measured memory usage and this saves
roughly 40 megabytes of memory when loading servo.org based on data from
the memory profiler.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Diffstat (limited to 'components/layout')
-rw-r--r-- | components/layout/construct.rs | 29 | ||||
-rw-r--r-- | components/layout/context.rs | 32 | ||||
-rw-r--r-- | components/layout/fragment.rs | 22 | ||||
-rw-r--r-- | components/layout/generated_content.rs | 6 | ||||
-rw-r--r-- | components/layout/inline.rs | 4 | ||||
-rw-r--r-- | components/layout/list_item.rs | 14 | ||||
-rw-r--r-- | components/layout/text.rs | 27 |
7 files changed, 47 insertions, 87 deletions
diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 522635543fd..1c4eff55898 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -42,7 +42,7 @@ use style::values::generics::counters::ContentItem; use style::LocalName; use crate::block::BlockFlow; -use crate::context::{with_thread_local_font_context, LayoutContext}; +use crate::context::LayoutContext; use crate::data::{InnerLayoutData, LayoutDataFlags}; use crate::display_list::items::OpaqueNode; use crate::flex::FlexFlow; @@ -517,11 +517,10 @@ where // We must scan for runs before computing minimum ascent and descent because scanning // for runs might collapse so much whitespace away that only hypothetical fragments // remain. In that case the inline flow will compute its ascent and descent to be zero. - let scanned_fragments = - with_thread_local_font_context(self.layout_context, |font_context| { - TextRunScanner::new() - .scan_for_runs(font_context, mem::take(&mut fragments.fragments)) - }); + let scanned_fragments = TextRunScanner::new().scan_for_runs( + &self.layout_context.font_context, + mem::take(&mut fragments.fragments), + ); let mut inline_flow_ref = FlowRef::new(Arc::new(InlineFlow::from_fragments( scanned_fragments, node.style(self.style_context()).writing_mode, @@ -550,11 +549,10 @@ where { // FIXME(#6503): Use Arc::get_mut().unwrap() here. let inline_flow = FlowRef::deref_mut(&mut inline_flow_ref).as_mut_inline(); - inline_flow.minimum_line_metrics = - with_thread_local_font_context(self.layout_context, |font_context| { - inline_flow - .minimum_line_metrics(font_context, &node.style(self.style_context())) - }); + inline_flow.minimum_line_metrics = inline_flow.minimum_line_metrics( + &self.layout_context.font_context, + &node.style(self.style_context()), + ); } inline_flow_ref.finish(); @@ -1545,11 +1543,10 @@ where )), self.layout_context, )); - let marker_fragments = - with_thread_local_font_context(self.layout_context, |font_context| { - TextRunScanner::new() - .scan_for_runs(font_context, unscanned_marker_fragments) - }); + let marker_fragments = TextRunScanner::new().scan_for_runs( + &self.layout_context.font_context, + unscanned_marker_fragments, + ); marker_fragments.fragments }, ListStyleTypeContent::GeneratedContent(info) => vec![Fragment::new( diff --git a/components/layout/context.rs b/components/layout/context.rs index d43210c141b..5cedf5d38aa 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -4,7 +4,6 @@ //! Data needed by layout. -use std::cell::{RefCell, RefMut}; use std::collections::HashMap; use std::hash::BuildHasherDefault; use std::sync::{Arc, Mutex}; @@ -13,7 +12,6 @@ use std::thread; use fnv::FnvHasher; use gfx::font_cache_thread::FontCacheThread; use gfx::font_context::FontContext; -use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use msg::constellation_msg::PipelineId; use net_traits::image_cache::{ ImageCache, ImageCacheResult, ImageOrMetadataAvailable, UsePlaceholder, @@ -29,32 +27,6 @@ use crate::display_list::items::{OpaqueNode, WebRenderImageInfo}; pub type LayoutFontContext = FontContext<FontCacheThread>; -thread_local!(static FONT_CONTEXT_KEY: RefCell<Option<LayoutFontContext>> = RefCell::new(None)); - -pub fn with_thread_local_font_context<F, R>(layout_context: &LayoutContext, f: F) -> R -where - F: FnOnce(&mut LayoutFontContext) -> R, -{ - FONT_CONTEXT_KEY.with(|k| { - let mut font_context = k.borrow_mut(); - if font_context.is_none() { - let font_cache_thread = layout_context.font_cache_thread.lock().unwrap().clone(); - *font_context = Some(FontContext::new(font_cache_thread)); - } - f(&mut RefMut::map(font_context, |x| x.as_mut().unwrap())) - }) -} - -pub fn malloc_size_of_persistent_local_context(ops: &mut MallocSizeOfOps) -> usize { - FONT_CONTEXT_KEY.with(|r| { - if let Some(ref context) = *r.borrow() { - context.size_of(ops) - } else { - 0 - } - }) -} - type WebrenderImageCache = HashMap<(ServoUrl, UsePlaceholder), WebRenderImageInfo, BuildHasherDefault<FnvHasher>>; @@ -72,8 +44,8 @@ pub struct LayoutContext<'a> { /// Reference to the script thread image cache. pub image_cache: Arc<dyn ImageCache>, - /// Interface to the font cache thread. - pub font_cache_thread: Mutex<FontCacheThread>, + /// A FontContext to be used during layout. + pub font_context: Arc<FontContext<FontCacheThread>>, /// A cache of WebRender image info. pub webrender_image_cache: Arc<RwLock<WebrenderImageCache>>, diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 8e50d1c499e..45843627493 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -57,7 +57,7 @@ use style::values::generics::transform; use webrender_api::units::LayoutTransform; use webrender_api::{self, ImageKey}; -use crate::context::{with_thread_local_font_context, LayoutContext}; +use crate::context::LayoutContext; use crate::display_list::items::{ClipScrollNodeIndex, OpaqueNode, BLUR_INFLATION_FACTOR}; use crate::display_list::ToLayout; use crate::floats::ClearType; @@ -852,9 +852,8 @@ impl Fragment { ))), ); unscanned_ellipsis_fragments.push_back(ellipsis_fragment); - let ellipsis_fragments = with_thread_local_font_context(layout_context, |font_context| { - TextRunScanner::new().scan_for_runs(font_context, unscanned_ellipsis_fragments) - }); + let ellipsis_fragments = TextRunScanner::new() + .scan_for_runs(&layout_context.font_context, unscanned_ellipsis_fragments); debug_assert_eq!(ellipsis_fragments.len(), 1); ellipsis_fragment = ellipsis_fragments.fragments.into_iter().next().unwrap(); ellipsis_fragment.flags |= FragmentFlags::IS_ELLIPSIS; @@ -2346,9 +2345,10 @@ impl Fragment { return InlineMetrics::new(Au(0), Au(0), Au(0)); } // See CSS 2.1 § 10.8.1. - let font_metrics = with_thread_local_font_context(layout_context, |font_context| { - text::font_metrics_for_style(font_context, self_.style.clone_font()) - }); + let font_metrics = text::font_metrics_for_style( + &layout_context.font_context, + self_.style.clone_font(), + ); let line_height = text::line_height_from_style(&self_.style, &font_metrics); InlineMetrics::from_font_metrics(&info.run.font_metrics, line_height) } @@ -2426,10 +2426,10 @@ impl Fragment { VerticalAlign::Keyword(kw) => match kw { VerticalAlignKeyword::Baseline => {}, VerticalAlignKeyword::Middle => { - let font_metrics = - with_thread_local_font_context(layout_context, |font_context| { - text::font_metrics_for_style(font_context, self.style.clone_font()) - }); + let font_metrics = text::font_metrics_for_style( + &layout_context.font_context, + self.style.clone_font(), + ); offset += (content_inline_metrics.ascent - content_inline_metrics.space_below_baseline - font_metrics.x_height) diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs index 862e8c88191..3a50024d522 100644 --- a/components/layout/generated_content.rs +++ b/components/layout/generated_content.rs @@ -20,7 +20,7 @@ use style::servo::restyle_damage::ServoRestyleDamage; use style::values::generics::counters::ContentItem; use style::values::specified::list::{QuotePair, Quotes}; -use crate::context::{with_thread_local_font_context, LayoutContext}; +use crate::context::LayoutContext; use crate::display_list::items::OpaqueNode; use crate::flow::{Flow, FlowFlags, GetBaseFlow, ImmutableFlowUtils}; use crate::fragment::{ @@ -493,9 +493,7 @@ fn render_text( )); // FIXME(pcwalton): This should properly handle multiple marker fragments. This could happen // due to text run splitting. - let fragments = with_thread_local_font_context(layout_context, |font_context| { - TextRunScanner::new().scan_for_runs(font_context, fragments) - }); + let fragments = TextRunScanner::new().scan_for_runs(&layout_context.font_context, fragments); if fragments.is_empty() { None } else { diff --git a/components/layout/inline.rs b/components/layout/inline.rs index fa2ebb01efb..5926e4b2e03 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1239,7 +1239,7 @@ impl InlineFlow { /// `style` is the style of the block. pub fn minimum_line_metrics( &self, - font_context: &mut LayoutFontContext, + font_context: &LayoutFontContext, style: &ComputedValues, ) -> LineMetrics { InlineFlow::minimum_line_metrics_for_fragments( @@ -1255,7 +1255,7 @@ impl InlineFlow { /// `style` is the style of the block that these fragments belong to. pub fn minimum_line_metrics_for_fragments( fragments: &[Fragment], - font_context: &mut LayoutFontContext, + font_context: &LayoutFontContext, style: &ComputedValues, ) -> LineMetrics { // As a special case, if this flow contains only hypothetical fragments, then the entire diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs index 601046fc0c8..a7c44164ac1 100644 --- a/components/layout/list_item.rs +++ b/components/layout/list_item.rs @@ -14,7 +14,7 @@ use style::properties::ComputedValues; use style::servo::restyle_damage::ServoRestyleDamage; use crate::block::BlockFlow; -use crate::context::{with_thread_local_font_context, LayoutContext}; +use crate::context::LayoutContext; use crate::display_list::items::DisplayListSection; use crate::display_list::{ BorderPaintingMode, DisplayListBuildState, StackingContextCollectionState, @@ -114,13 +114,11 @@ impl ListItemFlow { fn assign_marker_block_sizes(&mut self, layout_context: &LayoutContext) { // FIXME(pcwalton): Do this during flow construction, like `InlineFlow` does? - let marker_line_metrics = with_thread_local_font_context(layout_context, |font_context| { - InlineFlow::minimum_line_metrics_for_fragments( - &self.marker_fragments, - font_context, - &self.block_flow.fragment.style, - ) - }); + let marker_line_metrics = InlineFlow::minimum_line_metrics_for_fragments( + &self.marker_fragments, + &layout_context.font_context, + &self.block_flow.fragment.style, + ); for marker in &mut self.marker_fragments { marker.assign_replaced_block_size_if_necessary(); diff --git a/components/layout/text.rs b/components/layout/text.rs index b11a25f8dee..2875ccd0d77 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -70,7 +70,7 @@ impl TextRunScanner { pub fn scan_for_runs( &mut self, - font_context: &mut LayoutFontContext, + font_context: &LayoutFontContext, mut fragments: LinkedList<Fragment>, ) -> InlineFragments { debug!( @@ -150,7 +150,7 @@ impl TextRunScanner { /// be adjusted. fn flush_clump_to_list( &mut self, - font_context: &mut LayoutFontContext, + font_context: &LayoutFontContext, out_fragments: &mut Vec<Fragment>, paragraph_bytes_processed: &mut usize, bidi_levels: Option<&[bidi::Level]>, @@ -203,10 +203,9 @@ impl TextRunScanner { .map(|l| l.into()) .unwrap_or_else(|| { let space_width = font_group - .borrow_mut() + .write() .find_by_codepoint(font_context, ' ') .and_then(|font| { - let font = font.borrow(); font.glyph_index(' ') .map(|glyph_id| font.glyph_h_advance(glyph_id)) }) @@ -248,7 +247,7 @@ impl TextRunScanner { for (byte_index, character) in text.char_indices() { if !character.is_control() { let font = font_group - .borrow_mut() + .write() .find_by_codepoint(font_context, character); let bidi_level = match bidi_levels { @@ -367,7 +366,7 @@ impl TextRunScanner { // If no font is found (including fallbacks), there's no way we can render. let font = match run_info .font - .or_else(|| font_group.borrow_mut().first(font_context)) + .or_else(|| font_group.write().first(font_context)) { Some(font) => font, None => { @@ -377,7 +376,7 @@ impl TextRunScanner { }; let (run, break_at_zero) = TextRun::new( - &mut font.borrow_mut(), + font, run_info.text, &options, run_info.bidi_level, @@ -535,14 +534,12 @@ fn bounding_box_for_run_metrics( /// Panics if no font can be found for the given font style. #[inline] pub fn font_metrics_for_style( - font_context: &mut LayoutFontContext, + font_context: &LayoutFontContext, style: crate::ServoArc<FontStyleStruct>, ) -> FontMetrics { let font_group = font_context.font_group(style); - let font = font_group.borrow_mut().first(font_context); - let font = font.as_ref().unwrap().borrow(); - - font.metrics.clone() + let font = font_group.write().first(font_context); + font.as_ref().unwrap().metrics.clone() } /// Returns the line block-size needed by the given computed style and font size. @@ -664,10 +661,8 @@ impl RunInfo { fn has_font(&self, font: &Option<FontRef>) -> bool { fn identifier_and_pt_size(font: &Option<FontRef>) -> Option<(FontIdentifier, Au)> { - font.as_ref().map(|font| { - let font = font.borrow(); - (font.identifier().clone(), font.descriptor.pt_size) - }) + font.as_ref() + .map(|font| (font.identifier().clone(), font.descriptor.pt_size)) } identifier_and_pt_size(&self.font) == identifier_and_pt_size(font) |