This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch geckoview-102.3.0esr-12.0-1 in repository tor-browser.
commit d36b4c404e0e228b7d30fe629deddf757e3ebaf6 Author: Lee Salzman lsalzman@mozilla.com AuthorDate: Thu Aug 4 17:02:57 2022 +0000
Bug 1782795 - Remove shared context from GlyphRasterizer. r=jnicol, a=RyanVM
Differential Revision: https://phabricator.services.mozilla.com/D153503 --- gfx/wr/webrender/src/glyph_rasterizer/mod.rs | 74 +++++++++++++++------------ gfx/wr/webrender/src/platform/macos/font.rs | 4 -- gfx/wr/webrender/src/platform/unix/font.rs | 4 -- gfx/wr/webrender/src/platform/windows/font.rs | 4 -- 4 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/gfx/wr/webrender/src/glyph_rasterizer/mod.rs b/gfx/wr/webrender/src/glyph_rasterizer/mod.rs index a312c63679d9a..43664d611e24b 100644 --- a/gfx/wr/webrender/src/glyph_rasterizer/mod.rs +++ b/gfx/wr/webrender/src/glyph_rasterizer/mod.rs @@ -41,8 +41,10 @@ pub static GLYPH_FLASHING: AtomicBool = AtomicBool::new(false); impl FontContexts { /// Get access to the font context associated to the current thread. pub fn lock_current_context(&self) -> MutexGuard<FontContext> { - let id = self.current_worker_id(); - self.lock_context(id) + match self.current_worker_id() { + Some(id) => self.lock_context(id), + None => self.lock_any_context(), + } }
pub(in super) fn current_worker_id(&self) -> Option<usize> { @@ -1459,9 +1461,6 @@ pub struct FontContexts { // These worker are mostly accessed from their corresponding worker threads. // The goal is that there should be no noticeable contention on the mutexes. worker_contexts: Vec<Mutex<FontContext>>, - // This worker should be accessed by threads that don't belong to the thread pool - // (in theory that's only the render backend thread so no contention expected either). - shared_context: Mutex<FontContext>, // Stored here as a convenience to get the current thread index. #[allow(dead_code)] workers: Arc<ThreadPool>, @@ -1473,19 +1472,21 @@ impl FontContexts {
/// Get access to any particular font context. /// - /// The id is ```Some(i)``` where i is an index between 0 and num_worker_contexts - /// for font contexts associated to the thread pool, and None for the shared - /// global font context for use outside of the thread pool. - pub fn lock_context(&self, id: Option<usize>) -> MutexGuard<FontContext> { - match id { - Some(index) => self.worker_contexts[index].lock().unwrap(), - None => self.shared_context.lock().unwrap(), + /// The id is an index between 0 and num_worker_contexts for font contexts + /// associated to the thread pool. + pub fn lock_context(&self, id: usize) -> MutexGuard<FontContext> { + self.worker_contexts[id].lock().unwrap() + } + + // Find a context that is currently unlocked to use, otherwise defaulting + // to the first context. + pub fn lock_any_context(&self) -> MutexGuard<FontContext> { + for context in &self.worker_contexts { + if let Ok(mutex) = context.try_lock() { + return mutex; + } } - } - - /// Get access to the font context usable outside of the thread pool. - pub fn lock_shared_context(&self) -> MutexGuard<FontContext> { - self.shared_context.lock().unwrap() + self.lock_context(0) }
// number of contexts associated to workers @@ -1509,10 +1510,9 @@ impl AsyncForEach<FontContext> for Arc<FontContexts> { // Spawn a new thread on which to run the for-each off the main thread. self.workers.spawn(move || { // Lock the shared and worker contexts up front. - let mut locks = Vec::with_capacity(font_contexts.num_worker_contexts() + 1); - locks.push(font_contexts.lock_shared_context()); + let mut locks = Vec::with_capacity(font_contexts.num_worker_contexts()); for i in 0 .. font_contexts.num_worker_contexts() { - locks.push(font_contexts.lock_context(Some(i))); + locks.push(font_contexts.lock_context(i)); }
// Signal the locked condition now that all contexts are locked. @@ -1539,6 +1539,9 @@ pub struct GlyphRasterizer { workers: Arc<ThreadPool>, font_contexts: Arc<FontContexts>,
+ /// The current set of loaded fonts. + fonts: FastHashSet<FontKey>, + /// The current number of individual glyphs waiting in pending batches. pending_glyph_count: usize,
@@ -1577,22 +1580,20 @@ impl GlyphRasterizer { let num_workers = workers.current_num_threads(); let mut contexts = Vec::with_capacity(num_workers);
- let shared_context = FontContext::new()?; - for _ in 0 .. num_workers { contexts.push(Mutex::new(FontContext::new()?)); }
let font_context = FontContexts { - worker_contexts: contexts, - shared_context: Mutex::new(shared_context), - workers: Arc::clone(&workers), - locked_mutex: Mutex::new(false), - locked_cond: Condvar::new(), + worker_contexts: contexts, + workers: Arc::clone(&workers), + locked_mutex: Mutex::new(false), + locked_cond: Condvar::new(), };
Ok(GlyphRasterizer { font_contexts: Arc::new(font_context), + fonts: FastHashSet::default(), pending_glyph_jobs: 0, pending_glyph_count: 0, glyph_request_count: 0, @@ -1608,9 +1609,12 @@ impl GlyphRasterizer { }
pub fn add_font(&mut self, font_key: FontKey, template: FontTemplate) { - self.font_contexts.async_for_each(move |mut context| { - context.add_font(&font_key, &template); - }); + if self.fonts.insert(font_key.clone()) { + // Only add font to FontContexts if not previously added. + self.font_contexts.async_for_each(move |mut context| { + context.add_font(&font_key, &template); + }); + } }
pub fn delete_font(&mut self, font_key: FontKey) { @@ -1637,7 +1641,7 @@ impl GlyphRasterizer { }
pub fn has_font(&self, font_key: FontKey) -> bool { - self.font_contexts.lock_shared_context().has_font(&font_key) + self.fonts.contains(&font_key) }
pub fn get_glyph_dimensions( @@ -1652,13 +1656,13 @@ impl GlyphRasterizer { );
self.font_contexts - .lock_shared_context() + .lock_any_context() .get_glyph_dimensions(font, &glyph_key) }
pub fn get_glyph_index(&mut self, font_key: FontKey, ch: char) -> Option<u32> { self.font_contexts - .lock_shared_context() + .lock_any_context() .get_glyph_index(font_key, ch) }
@@ -1668,7 +1672,9 @@ impl GlyphRasterizer { }
profile_scope!("remove_dead_fonts"); - let fonts_to_remove = mem::replace(&mut self.fonts_to_remove, Vec::new()); + let mut fonts_to_remove = mem::replace(& mut self.fonts_to_remove, Vec::new()); + // Only remove font from FontContexts if previously added. + fonts_to_remove.retain(|font| self.fonts.remove(font)); let font_instances_to_remove = mem::replace(& mut self.font_instances_to_remove, Vec::new()); self.font_contexts.async_for_each(move |mut context| { for font_key in &fonts_to_remove { diff --git a/gfx/wr/webrender/src/platform/macos/font.rs b/gfx/wr/webrender/src/platform/macos/font.rs index 88b53b41fd717..fbf69fe438301 100644 --- a/gfx/wr/webrender/src/platform/macos/font.rs +++ b/gfx/wr/webrender/src/platform/macos/font.rs @@ -247,10 +247,6 @@ impl FontContext { }) }
- pub fn has_font(&self, font_key: &FontKey) -> bool { - self.ct_font_descs.contains_key(font_key) - } - pub fn add_raw_font(&mut self, font_key: &FontKey, bytes: Arc<Vec<u8>>, index: u32) { if self.ct_font_descs.contains_key(font_key) { return; diff --git a/gfx/wr/webrender/src/platform/unix/font.rs b/gfx/wr/webrender/src/platform/unix/font.rs index 31ecbfa27f497..0d79415d9dc05 100644 --- a/gfx/wr/webrender/src/platform/unix/font.rs +++ b/gfx/wr/webrender/src/platform/unix/font.rs @@ -343,10 +343,6 @@ impl FontContext { } }
- pub fn has_font(&self, font_key: &FontKey) -> bool { - self.faces.contains_key(font_key) - } - pub fn add_raw_font(&mut self, font_key: &FontKey, bytes: Arc<Vec<u8>>, index: u32) { if !self.faces.contains_key(font_key) { let len = bytes.len(); diff --git a/gfx/wr/webrender/src/platform/windows/font.rs b/gfx/wr/webrender/src/platform/windows/font.rs index a2fea8cbd5263..3f6bd78ab22f3 100644 --- a/gfx/wr/webrender/src/platform/windows/font.rs +++ b/gfx/wr/webrender/src/platform/windows/font.rs @@ -150,10 +150,6 @@ impl FontContext { }) }
- pub fn has_font(&self, font_key: &FontKey) -> bool { - self.fonts.contains_key(font_key) - } - fn add_font_descriptor(&mut self, font_key: &FontKey, desc: &dwrote::FontDescriptor) { let system_fc = dwrote::FontCollection::get_system(false); if let Some(font) = system_fc.get_font_from_descriptor(desc) {