[tbb-commits] [tor-browser] 15/73: Bug 1782795 - Remove shared context from GlyphRasterizer. r=jnicol, a=RyanVM

gitolite role git at cupani.torproject.org
Wed Sep 21 20:17:08 UTC 2022


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 at 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) {

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the tbb-commits mailing list