From e2cd3f51e63875ad036e3b3eb2993a9905f25193 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 17 Dec 2015 09:18:03 +0501 Subject: [PATCH] servo: Merge #8990 - Optimize `TextRun::advance_for_range` (from pcwalton:advance-for-range-optzns); r=mbrubeck The combined effects of these optimizations move `advance_for_range` from #1 in the layout profile on all sites I tested to #2, #3, or #4, depending on the site. r? @mbrubeck Source-Repo: https://github.com/servo/servo Source-Revision: 67c3cb37073068fe8be7b35b26469e2e329ce385 --- servo/components/gfx/text/glyph.rs | 22 ++++++++++++++- servo/components/gfx/text/text_run.rs | 39 ++++++++++++++++++++++++++- servo/components/util/range.rs | 12 +++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/servo/components/gfx/text/glyph.rs b/servo/components/gfx/text/glyph.rs index fa250bd1ba6d..c17517429377 100644 --- a/servo/components/gfx/text/glyph.rs +++ b/servo/components/gfx/text/glyph.rs @@ -403,6 +403,9 @@ pub struct GlyphStore { /// `entry_buffer` point to locations in this data structure. detail_store: DetailedGlyphStore, + /// A cache of the advance of the entire glyph store. + total_advance: Au, + /// Used to check if fast path should be used in glyph iteration. has_detailed_glyphs: bool, is_whitespace: bool, @@ -427,6 +430,7 @@ impl<'a> GlyphStore { GlyphStore { entry_buffer: vec![GlyphEntry::initial(); length], detail_store: DetailedGlyphStore::new(), + total_advance: Au(0), has_detailed_glyphs: false, is_whitespace: is_whitespace, is_rtl: is_rtl, @@ -443,6 +447,20 @@ impl<'a> GlyphStore { pub fn finalize_changes(&mut self) { self.detail_store.ensure_sorted(); + self.cache_total_advance() + } + + #[inline(never)] + fn cache_total_advance(&mut self) { + let mut total_advance = Au(0); + for glyph in self.iter_glyphs_for_char_range(&Range::new(CharIndex(0), self.char_len())) { + total_advance = total_advance + glyph.advance() + } + self.total_advance = total_advance + } + + pub fn total_advance(&self) -> Au { + self.total_advance } /// Adds a single glyph. If `character` is present, this represents a single character; @@ -532,7 +550,9 @@ impl<'a> GlyphStore { #[inline] pub fn advance_for_char_range(&self, rang: &Range) -> Au { - if !self.has_detailed_glyphs { + if rang.begin() == CharIndex(0) && rang.end() == self.char_len() { + self.total_advance + } else if !self.has_detailed_glyphs { self.advance_for_char_range_simple_glyphs(rang) } else { self.advance_for_char_range_slow_path(rang) diff --git a/servo/components/gfx/text/text_run.rs b/servo/components/gfx/text/text_run.rs index 7d4023a58e07..5b8acb5e1c59 100644 --- a/servo/components/gfx/text/text_run.rs +++ b/servo/components/gfx/text/text_run.rs @@ -6,6 +6,7 @@ use app_units::Au; use font::{Font, FontHandleMethods, FontMetrics, IS_WHITESPACE_SHAPING_FLAG, RunMetrics}; use font::{ShapingOptions}; use platform::font_template::FontTemplateData; +use std::cell::Cell; use std::cmp::{Ordering, max}; use std::slice::Iter; use std::sync::Arc; @@ -13,6 +14,11 @@ use text::glyph::{CharIndex, GlyphStore}; use util::range::Range; use util::vec::{Comparator, FullBinarySearchMethods}; +thread_local! { + static INDEX_OF_FIRST_GLYPH_RUN_CACHE: Cell> = + Cell::new(None) +} + /// A single "paragraph" of text in one font size and style. #[derive(Clone, Deserialize, Serialize)] pub struct TextRun { @@ -26,6 +32,19 @@ pub struct TextRun { pub bidi_level: u8, } +impl Drop for TextRun { + fn drop(&mut self) { + // Invalidate the glyph run cache if it was our text run that got freed. + INDEX_OF_FIRST_GLYPH_RUN_CACHE.with(|index_of_first_glyph_run_cache| { + if let Some((text_run_ptr, _, _)) = index_of_first_glyph_run_cache.get() { + if text_run_ptr == (self as *const TextRun) { + index_of_first_glyph_run_cache.set(None); + } + } + }) + } +} + /// A single series of glyphs within a text run. #[derive(Clone, Deserialize, Serialize)] pub struct GlyphRun { @@ -248,6 +267,10 @@ impl<'a> TextRun { } pub fn advance_for_range(&self, range: &Range) -> Au { + if range.is_empty() { + return Au(0) + } + // TODO(Issue #199): alter advance direction for RTL // TODO(Issue #98): using inter-char and inter-word spacing settings when measuring text self.natural_word_slices_in_range(range) @@ -279,7 +302,21 @@ impl<'a> TextRun { /// Returns the index of the first glyph run containing the given character index. fn index_of_first_glyph_run_containing(&self, index: CharIndex) -> Option { - (&**self.glyphs).binary_search_index_by(&index, CharIndexComparator) + let self_ptr = self as *const TextRun; + INDEX_OF_FIRST_GLYPH_RUN_CACHE.with(|index_of_first_glyph_run_cache| { + if let Some((last_text_run, last_index, last_result)) = + index_of_first_glyph_run_cache.get() { + if last_text_run == self_ptr && last_index == index { + return Some(last_result) + } + } + + let result = (&**self.glyphs).binary_search_index_by(&index, CharIndexComparator); + if let Some(result) = result { + index_of_first_glyph_run_cache.set(Some((self_ptr, index, result))); + } + result + }) } /// Returns an iterator that will iterate over all slices of glyphs that represent natural diff --git a/servo/components/util/range.rs b/servo/components/util/range.rs index 4549a915635f..07e66b28a6f6 100644 --- a/servo/components/util/range.rs +++ b/servo/components/util/range.rs @@ -22,15 +22,23 @@ pub trait Int: fn from_usize(n: usize) -> Option; } impl Int for isize { + #[inline] fn zero() -> isize { 0 } + #[inline] fn one() -> isize { 1 } + #[inline] fn max_value() -> isize { ::std::isize::MAX } + #[inline] fn from_usize(n: usize) -> Option { num_lib::NumCast::from(n) } } impl Int for usize { + #[inline] fn zero() -> usize { 0 } + #[inline] fn one() -> usize { 1 } + #[inline] fn max_value() -> usize { ::std::usize::MAX } + #[inline] fn from_usize(n: usize) -> Option { Some(n) } } @@ -88,9 +96,13 @@ macro_rules! int_range_index { } impl $crate::range::Int for $Self_ { + #[inline] fn zero() -> $Self_ { $Self_($crate::range::Int::zero()) } + #[inline] fn one() -> $Self_ { $Self_($crate::range::Int::one()) } + #[inline] fn max_value() -> $Self_ { $Self_($crate::range::Int::max_value()) } + #[inline] fn from_usize(n: usize) -> Option<$Self_> { $crate::range::Int::from_usize(n).map($Self_) } }