[tor-commits] [tor-browser] 16/73: Bug 1783115 - Use only a single FT_Face for each font in WebRender. r=jnicol, a=RyanVM

gitolite role git at cupani.torproject.org
Wed Sep 21 20:17:09 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 f20d23c3e7723beb36939183a85c12c23536a513
Author: Lee Salzman <lsalzman at mozilla.com>
AuthorDate: Thu Aug 4 17:02:57 2022 +0000

    Bug 1783115 - Use only a single FT_Face for each font in WebRender. r=jnicol, a=RyanVM
    
    This patch uses a single FT_Library for all font threads (even across separate windows)
    within WebRender. It maintains a mapping from FontTemplate to a cached FT_Face that will
    be traded to any font thread that requests it on a first-come-first-serve basis. This
    FT_Face is guarded by a mutex to ensure that no other worker thread can use it while it
    is in-use by the first requesting thread.
    
    In general it is safe to use different FT_Faces on different threads so long as they are
    distinct. However, the snag in this is that FT_LcdFilter state is stored globally in the
    FT_Library, even while it is supplied per-FontInstance. To workaround this, we need to
    keep track of how many threads are currently rasterizing glyphs with the FT_LcdFilter
    state and wait for them to all finish before we can change the FT_LcdFilter to what
    the next group of incoming jobs expects. FontContext::begin_rasterize/end_rasterize hooks
    were added to support this. While there seems to be some support in newer FreeType
    versions for per-FT_Face LcdFilter settings, it doesn't allow disabling the LcdFilter
    or using legacy settings, so we are still suck using the global setting instead.
    
    To further reduce memory usage, this patch also no longer allocates a separate FT_Face
    for FontVariations. Instead, since the FT_Face must be locked to a single thread anyway,
    it now just uses FT_Set_Var_Design_Coordinates on the single FT_Face for the font before
    loading the batch of glyphs associated with the FontVariations. So regardless of how many
    different variations are used for a font, it will only allocate a single FT_Face.
    
    Normally, rayon's par_iter is used to scatter glyphs from a single font request onto
    multiple threads which works against the one-font-per-thread model needed here. This
    adds a FontContext::distribute_across_threads hook to control whether or not par_iter
    should be used so that we can disable that behavior for this case, where instead we just
    want to process each glyph serially within the scope of a single worker for a given
    font request.
    
    Differential Revision: https://phabricator.services.mozilla.com/D153715
---
 gfx/wr/webrender/src/glyph_rasterizer/mod.rs  |  52 ++--
 gfx/wr/webrender/src/platform/macos/font.rs   |  10 +
 gfx/wr/webrender/src/platform/unix/font.rs    | 411 ++++++++++++++------------
 gfx/wr/webrender/src/platform/windows/font.rs |  10 +
 4 files changed, 279 insertions(+), 204 deletions(-)

diff --git a/gfx/wr/webrender/src/glyph_rasterizer/mod.rs b/gfx/wr/webrender/src/glyph_rasterizer/mod.rs
index 43664d611e24b..6fc22b6619ea5 100644
--- a/gfx/wr/webrender/src/glyph_rasterizer/mod.rs
+++ b/gfx/wr/webrender/src/glyph_rasterizer/mod.rs
@@ -76,6 +76,7 @@ impl GlyphRasterizer {
         assert!(self.has_font(font.font_key));
 
         let glyph_key_cache = glyph_cache.insert_glyph_key_cache_for_font(&font);
+        let mut batch_size = 0;
 
         // select glyphs that have not been requested yet.
         for key in glyph_keys {
@@ -105,17 +106,7 @@ impl GlyphRasterizer {
             match self.pending_glyph_requests.get_mut(&font) {
                 Some(container) => {
                     container.push(*key);
-
-                    // If the batch for this font instance is big enough, kick off an async
-                    // job to start rasterizing these glyphs on other threads now.
-                    if container.len() == 8 {
-                        let glyphs = mem::replace(container, SmallVec::new());
-                        self.flush_glyph_requests(
-                            font.clone(),
-                            glyphs,
-                            true,
-                        );
-                    }
+                    batch_size = container.len();
                 }
                 None => {
                     // If no batch exists for this font instance, add the glyph to a new one.
@@ -128,6 +119,14 @@ impl GlyphRasterizer {
 
             glyph_key_cache.add_glyph(*key, GlyphCacheEntry::Pending);
         }
+
+        // If the batch for this font instance is big enough, kick off an async
+        // job to start rasterizing these glyphs on other threads now.
+        if batch_size >= 8 {
+            let container = self.pending_glyph_requests.get_mut(&font).unwrap();
+            let glyphs = mem::replace(container, SmallVec::new());
+            self.flush_glyph_requests(font, glyphs, true);
+        }
     }
 
     pub fn enable_multithreading(&mut self, enable: bool) {
@@ -150,13 +149,14 @@ impl GlyphRasterizer {
 
         let can_use_r8_format = self.can_use_r8_format;
 
+        let job_font = font.clone();
         let process_glyph = move |key: &GlyphKey| -> GlyphRasterJob {
             profile_scope!("glyph-raster");
             let mut context = font_contexts.lock_current_context();
             let mut job = GlyphRasterJob {
-                font: Arc::clone(&font),
+                font: Arc::clone(&job_font),
                 key: key.clone(),
-                result: context.rasterize_glyph(&font, key),
+                result: context.rasterize_glyph(&job_font, key),
             };
 
             if let Ok(ref mut glyph) = job.result {
@@ -185,7 +185,7 @@ impl GlyphRasterizer {
                 assert_eq!((glyph.left.fract(), glyph.top.fract()), (0.0, 0.0));
 
                 // Check if the glyph has a bitmap that needs to be downscaled.
-                glyph.downscale_bitmap_if_required(&font);
+                glyph.downscale_bitmap_if_required(&job_font);
 
                 // Convert from BGRA8 to R8 if required. In the future we can make it the
                 // backends' responsibility to output glyphs in the desired format,
@@ -209,16 +209,32 @@ impl GlyphRasterizer {
             // glyphs in the thread pool.
             profile_scope!("spawning process_glyph jobs");
             self.workers.install(|| {
-                glyphs.par_iter().for_each(|key| {
-                    let job = process_glyph(key);
-                    self.glyph_tx.send(job).unwrap();
-                });
+                FontContext::begin_rasterize(&font);
+                // If the FontContext supports distributing a font across multiple threads,
+                // then use par_iter so different glyphs of the same font are processed on
+                // multiple threads.
+                if FontContext::distribute_across_threads() {
+                    glyphs.par_iter().for_each(|key| {
+                        let job = process_glyph(key);
+                        self.glyph_tx.send(job).unwrap();
+                    });
+                } else {
+                    // For FontContexts that prefer to localize a font to a single thread,
+                    // just process all the glyphs on the same worker to avoid contention.
+                    for key in glyphs {
+                        let job = process_glyph(&key);
+                        self.glyph_tx.send(job).unwrap();
+                    }
+                }
+                FontContext::end_rasterize(&font);
             });
         } else {
+            FontContext::begin_rasterize(&font);
             for key in glyphs {
                 let job = process_glyph(&key);
                 self.glyph_tx.send(job).unwrap();
             }
+            FontContext::end_rasterize(&font);
         }
     }
 
diff --git a/gfx/wr/webrender/src/platform/macos/font.rs b/gfx/wr/webrender/src/platform/macos/font.rs
index fbf69fe438301..325cecebdf419 100644
--- a/gfx/wr/webrender/src/platform/macos/font.rs
+++ b/gfx/wr/webrender/src/platform/macos/font.rs
@@ -232,6 +232,10 @@ fn is_bitmap_font(font: &FontInstance) -> bool {
 }
 
 impl FontContext {
+    pub fn distribute_across_threads() -> bool {
+        true
+    }
+
     pub fn new() -> Result<FontContext, ResourceCacheError> {
         debug!("Test for subpixel AA support: {:?}", *FONT_SMOOTHING_MODE);
 
@@ -515,6 +519,12 @@ impl FontContext {
         }
     }
 
+    pub fn begin_rasterize(_font: &FontInstance) {
+    }
+
+    pub fn end_rasterize(_font: &FontInstance) {
+    }
+
     pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult {
         objc::rc::autoreleasepool(|| {
         let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
diff --git a/gfx/wr/webrender/src/platform/unix/font.rs b/gfx/wr/webrender/src/platform/unix/font.rs
index 0d79415d9dc05..d9faf1f9d612c 100644
--- a/gfx/wr/webrender/src/platform/unix/font.rs
+++ b/gfx/wr/webrender/src/platform/unix/font.rs
@@ -4,7 +4,7 @@
 
 use api::{ColorU, GlyphDimensions, FontKey, FontRenderMode};
 use api::{FontInstancePlatformOptions, FontLCDFilter, FontHinting};
-use api::{FontInstanceFlags, FontVariation, NativeFontHandle};
+use api::{FontInstanceFlags, FontTemplate, FontVariation, NativeFontHandle};
 use freetype::freetype::{FT_BBox, FT_Outline_Translate, FT_Pixel_Mode, FT_Render_Mode};
 use freetype::freetype::{FT_Done_Face, FT_Error, FT_Get_Char_Index, FT_Int32};
 use freetype::freetype::{FT_Done_FreeType, FT_Library_SetLcdFilter, FT_Pos};
@@ -28,9 +28,8 @@ use libc::{dlsym, RTLD_DEFAULT};
 use libc::free;
 use std::{cmp, mem, ptr, slice};
 use std::cmp::max;
-use std::collections::hash_map::Entry;
 use std::ffi::CString;
-use std::sync::Arc;
+use std::sync::{Arc, Condvar, Mutex, MutexGuard};
 
 // These constants are not present in the freetype
 // bindings due to bindgen not handling the way
@@ -150,25 +149,18 @@ pub extern "C" fn mozilla_glyphslot_embolden_less(slot: FT_GlyphSlot) {
     slot_.metrics.horiBearingY += strength;
 }
 
-enum FontFile {
-    Pathname(CString),
-    Data(Arc<Vec<u8>>),
-}
-
-struct FontFace {
-    // Raw byte data has to live until the font is deleted, according to
-    // https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#FT_New_Memory_Face
-    file: FontFile,
-    index: u32,
+struct CachedFont {
+    template: FontTemplate,
     face: FT_Face,
     mm_var: *mut FT_MM_Var,
+    variations: Vec<FontVariation>,
 }
 
-impl Drop for FontFace {
+impl Drop for CachedFont {
     fn drop(&mut self) {
         unsafe {
             if !self.mm_var.is_null() &&
-               unimplemented(FT_Done_MM_Var((*(*self.face).glyph).library, self.mm_var)) {
+                unimplemented(FT_Done_MM_Var((*(*self.face).glyph).library, self.mm_var)) {
                 free(self.mm_var as _);
             }
 
@@ -177,53 +169,121 @@ impl Drop for FontFace {
     }
 }
 
-struct VariationFace(FT_Face);
+struct FontCache {
+    lib: FT_Library,
+    // Maps a template to a cached font that may be used across all threads.
+    fonts: FastHashMap<FontTemplate, Arc<Mutex<CachedFont>>>,
+    // The current LCD filter installed in the library.
+    lcd_filter: FontLCDFilter,
+    // The number of threads currently relying on the LCD filter state.
+    lcd_filter_uses: usize,
+}
 
-impl Drop for VariationFace {
-    fn drop(&mut self) {
-        unsafe { FT_Done_Face(self.0) };
+// FreeType resources are safe to move between threads as long as they
+// are not concurrently accessed. In our case, everything is behind a
+// Mutex so it is safe to move them between threads.
+unsafe impl Send for CachedFont {}
+unsafe impl Send for FontCache {}
+
+impl FontCache {
+    fn new() -> Self {
+        let mut lib: FT_Library = ptr::null_mut();
+        let result = unsafe { FT_Init_FreeType(&mut lib) };
+        if succeeded(result) {
+            // Ensure the library uses the default LCD filter initially.
+            unsafe { FT_Library_SetLcdFilter(lib, FT_LcdFilter::FT_LCD_FILTER_DEFAULT) };
+        } else {
+            // TODO(gw): Provide detailed error values.
+            // Once this panic has been here for a while with no issues we should get rid of
+            // ResourceCacheError as this was the only place that could fail previously.
+            panic!("Failed to initialize FreeType - {}", result)
+        }
+
+        FontCache {
+            lib,
+            fonts: FastHashMap::default(),
+            lcd_filter: FontLCDFilter::Default,
+            lcd_filter_uses: 0,
+        }
+    }
+
+    fn add_font(&mut self, template: FontTemplate) -> Result<Arc<Mutex<CachedFont>>, FT_Error> {
+        if let Some(cached) = self.fonts.get(&template) {
+            return Ok(cached.clone());
+        }
+        unsafe {
+            let mut face: FT_Face = ptr::null_mut();
+            let result = match template {
+                FontTemplate::Raw(ref bytes, index) => {
+                    FT_New_Memory_Face(
+                        self.lib,
+                        bytes.as_ptr(),
+                        bytes.len() as FT_Long,
+                        index as FT_Long,
+                        &mut face,
+                    )
+                }
+                FontTemplate::Native(NativeFontHandle { ref path, index }) => {
+                    let str = path.as_os_str().to_str().unwrap();
+                    let cstr = CString::new(str).unwrap();
+                    FT_New_Face(
+                        self.lib,
+                        cstr.as_ptr(),
+                        index as FT_Long,
+                        &mut face,
+                    )
+                }
+            };
+            if !succeeded(result) || face.is_null() {
+                return Err(result);
+            }
+            let mut mm_var = ptr::null_mut();
+            if ((*face).face_flags & (FT_FACE_FLAG_MULTIPLE_MASTERS as FT_Long)) != 0 &&
+               succeeded(FT_Get_MM_Var(face, &mut mm_var)) {
+                // Calling this before FT_Set_Var_Design_Coordinates avoids a bug with font variations
+                // not initialized properly in the font face, even if we ignore the result.
+                // See bug 1647035.
+                let mut tmp = [0; 16];
+                let res = FT_Get_Var_Design_Coordinates(
+                    face,
+                    (*mm_var).num_axis.min(16),
+                    tmp.as_mut_ptr()
+                );
+                debug_assert!(succeeded(res));
+            }
+            let cached = Arc::new(Mutex::new(CachedFont {
+                template: template.clone(),
+                face,
+                mm_var,
+                variations: Vec::new(),
+            }));
+            self.fonts.insert(template, cached.clone());
+            Ok(cached)
+        }
+    }
+
+    fn delete_font(&mut self, cached: Arc<Mutex<CachedFont>>) {
+        self.fonts.remove(&cached.lock().unwrap().template);
     }
 }
 
-fn new_ft_face(font_key: &FontKey, lib: FT_Library, file: &FontFile, index: u32) -> Result<FT_Face, FT_Error> {
-    unsafe {
-        let mut face: FT_Face = ptr::null_mut();
-        let result = match file {
-            FontFile::Pathname(ref cstr) => FT_New_Face(
-                lib,
-                cstr.as_ptr(),
-                index as FT_Long,
-                &mut face,
-            ),
-            FontFile::Data(ref bytes) => FT_New_Memory_Face(
-                lib,
-                bytes.as_ptr(),
-                bytes.len() as FT_Long,
-                index as FT_Long,
-                &mut face,
-            ),
-        };
-        if succeeded(result) && !face.is_null() {
-            Ok(face)
-        } else {
-            warn!("WARN: webrender failed to load font");
-            debug!("font={:?}, result={:?}", font_key, result);
-            Err(result)
+impl Drop for FontCache {
+    fn drop(&mut self) {
+        self.fonts.clear();
+        unsafe {
+            FT_Done_FreeType(self.lib);
         }
     }
 }
 
-pub struct FontContext {
-    lib: FT_Library,
-    faces: FastHashMap<FontKey, FontFace>,
-    variations: FastHashMap<(FontKey, Vec<FontVariation>), VariationFace>,
-    lcd_extra_pixels: i64,
+lazy_static! {
+    static ref FONT_CACHE: Mutex<FontCache> = Mutex::new(FontCache::new());
+    static ref LCD_FILTER_UNUSED: Condvar = Condvar::new();
 }
 
-// FreeType resources are safe to move between threads as long as they
-// are not concurrently accessed. In our case, everything is hidden inside
-// a given FontContext so it is safe to move the latter between threads.
-unsafe impl Send for FontContext {}
+pub struct FontContext {
+    fonts: FastHashMap<FontKey, Arc<Mutex<CachedFont>>>,
+}
 
 fn get_skew_bounds(bottom: i32, top: i32, skew_factor: f32, _vertical: bool) -> (f32, f32) {
     let skew_min = (bottom as f32 + 0.5) * skew_factor;
@@ -314,129 +374,79 @@ fn flip_bitmap_y(bitmap: &mut [u8], width: usize, height: usize) {
 }
 
 impl FontContext {
-    pub fn new() -> Result<FontContext, ResourceCacheError> {
-        let mut lib: FT_Library = ptr::null_mut();
-
-        // Using an LCD filter may add one full pixel to each side if support is built in.
-        // As of FreeType 2.8.1, an LCD filter is always used regardless of settings
-        // if support for the patent-encumbered LCD filter algorithms is not built in.
-        // Thus, the only reasonable way to guess padding is to unconditonally add it if
-        // subpixel AA is used.
-        let lcd_extra_pixels = 1;
-
-        let result = unsafe {
-            FT_Init_FreeType(&mut lib)
-        };
+    pub fn distribute_across_threads() -> bool {
+        false
+    }
 
-        if succeeded(result) {
-            Ok(FontContext {
-                lib,
-                faces: FastHashMap::default(),
-                variations: FastHashMap::default(),
-                lcd_extra_pixels,
-            })
-        } else {
-            // TODO(gw): Provide detailed error values.
-            // Once this panic has been here for a while with no issues we should get rid of
-            // ResourceCacheError as this was the only place that could fail previously.
-            panic!("Failed to initialize FreeType - {}", result)
-        }
+    pub fn new() -> Result<FontContext, ResourceCacheError> {
+        Ok(FontContext {
+            fonts: FastHashMap::default(),
+        })
     }
 
     pub fn add_raw_font(&mut self, font_key: &FontKey, bytes: Arc<Vec<u8>>, index: u32) {
-        if !self.faces.contains_key(font_key) {
+        if !self.fonts.contains_key(font_key) {
             let len = bytes.len();
-            let file = FontFile::Data(bytes);
-            if let Ok(face) = new_ft_face(font_key, self.lib, &file, index) {
-                self.faces.insert(*font_key, FontFace { file, index, face, mm_var: ptr::null_mut() });
-            } else {
-                panic!("adding raw font failed: {} bytes", len);
-            }
+            match FONT_CACHE.lock().unwrap().add_font(FontTemplate::Raw(bytes, index)) {
+                Ok(font) => self.fonts.insert(*font_key, font),
+                Err(result) => panic!("adding raw font failed: {} bytes, err={:?}", len, result),
+            };
         }
     }
 
     pub fn add_native_font(&mut self, font_key: &FontKey, native_font_handle: NativeFontHandle) {
-        if !self.faces.contains_key(font_key) {
-            let str = native_font_handle.path.as_os_str().to_str().unwrap();
-            let cstr = CString::new(str).unwrap();
-            let file = FontFile::Pathname(cstr);
-            let index = native_font_handle.index;
-            match new_ft_face(font_key, self.lib, &file, index) {
-                Ok(face) => self.faces.insert(*font_key, FontFace { file, index, face, mm_var: ptr::null_mut() }),
-                Err(result) => panic!("adding native font failed: file={} err={:?}", str, result),
+        if !self.fonts.contains_key(font_key) {
+            let path = native_font_handle.path.to_string_lossy().into_owned();
+            match FONT_CACHE.lock().unwrap().add_font(FontTemplate::Native(native_font_handle)) {
+                Ok(font) => self.fonts.insert(*font_key, font),
+                Err(result) => panic!("adding native font failed: file={} err={:?}", path, result),
             };
         }
     }
 
     pub fn delete_font(&mut self, font_key: &FontKey) {
-        if self.faces.remove(font_key).is_some() {
-            self.variations.retain(|k, _| k.0 != *font_key);
+        if let Some(cached) = self.fonts.remove(font_key) {
+            // If the only references to this font are the FontCache and this FontContext,
+            // then delete the font as there are no other existing users.
+            if Arc::strong_count(&cached) <= 2 {
+                FONT_CACHE.lock().unwrap().delete_font(cached);
+            }
         }
     }
 
-    pub fn delete_font_instance(&mut self, instance: &FontInstance) {
-        // Ensure we don't keep around excessive amounts of stale variations.
-        if !instance.variations.is_empty() {
-            self.variations.remove(&(instance.font_key, instance.variations.clone()));
-        }
+    pub fn delete_font_instance(&mut self, _instance: &FontInstance) {
     }
 
-    fn get_ft_face(&mut self, font: &FontInstance) -> Option<FT_Face> {
-        if font.variations.is_empty() {
-            return Some(self.faces.get(&font.font_key)?.face);
-        }
-        match self.variations.entry((font.font_key, font.variations.clone())) {
-            Entry::Occupied(entry) => Some(entry.get().0),
-            Entry::Vacant(entry) => unsafe {
-                let normal_face = self.faces.get_mut(&font.font_key)?;
-                if ((*normal_face.face).face_flags & (FT_FACE_FLAG_MULTIPLE_MASTERS as FT_Long)) == 0 {
-                    return Some(normal_face.face);
-                }
-                // Clone a new FT face and attempt to set the variation values on it.
-                // Leave unspecified values at the defaults.
-                let var_face = new_ft_face(&font.font_key, self.lib, &normal_face.file, normal_face.index).ok()?;
-                if !normal_face.mm_var.is_null() ||
-                   succeeded(FT_Get_MM_Var(normal_face.face, &mut normal_face.mm_var)) {
-                    let mm_var = normal_face.mm_var;
-                    let num_axis = (*mm_var).num_axis;
-                    let mut coords: Vec<FT_Fixed> = Vec::with_capacity(num_axis as usize);
-
-                    // Calling this before FT_Set_Var_Design_Coordinates avoids a bug with font variations
-                    // not initialized properly in the font face, even if we ignore the result.
-                    // See bug 1647035.
-                    let mut tmp = [0; 16];
-                    let res = FT_Get_Var_Design_Coordinates(
-                        normal_face.face,
-                        num_axis.min(16),
-                        tmp.as_mut_ptr()
-                    );
-                    debug_assert!(succeeded(res));
-
-
-                    for i in 0 .. num_axis {
-                        let axis = (*mm_var).axis.offset(i as isize);
-                        let mut value = (*axis).def;
-                        for var in &font.variations {
-                            if var.tag as FT_ULong == (*axis).tag {
-                                value = (var.value * 65536.0 + 0.5) as FT_Fixed;
-                                value = cmp::min(value, (*axis).maximum);
-                                value = cmp::max(value, (*axis).minimum);
-                                break;
-                            }
+    fn load_glyph(&mut self, font: &FontInstance, glyph: &GlyphKey)
+        -> Option<(MutexGuard<CachedFont>, FT_GlyphSlot, f32)> {
+        let mut cached = self.fonts.get(&font.font_key)?.lock().ok()?;
+        let face = cached.face;
+
+        let mm_var = cached.mm_var;
+        if !mm_var.is_null() && font.variations != cached.variations {
+            cached.variations.clear();
+            cached.variations.extend_from_slice(&font.variations);
+
+            unsafe {
+                let num_axis = (*mm_var).num_axis;
+                let mut coords: Vec<FT_Fixed> = Vec::with_capacity(num_axis as usize);
+                for i in 0 .. num_axis {
+                    let axis = (*mm_var).axis.offset(i as isize);
+                    let mut value = (*axis).def;
+                    for var in &font.variations {
+                        if var.tag as FT_ULong == (*axis).tag {
+                            value = (var.value * 65536.0 + 0.5) as FT_Fixed;
+                            value = cmp::min(value, (*axis).maximum);
+                            value = cmp::max(value, (*axis).minimum);
+                            break;
                         }
-                        coords.push(value);
                     }
-                    let res = FT_Set_Var_Design_Coordinates(var_face, num_axis, coords.as_mut_ptr());
-                    debug_assert!(succeeded(res));
+                    coords.push(value);
                 }
-                entry.insert(VariationFace(var_face));
-                Some(var_face)
+                let res = FT_Set_Var_Design_Coordinates(face, num_axis, coords.as_mut_ptr());
+                debug_assert!(succeeded(res));
             }
         }
-    }
-
-    fn load_glyph(&mut self, font: &FontInstance, glyph: &GlyphKey) -> Option<(FT_GlyphSlot, f32)> {
-        let face = self.get_ft_face(font)?;
 
         let mut load_flags = FT_LOAD_DEFAULT;
         let FontInstancePlatformOptions { mut hinting, .. } = font.platform_options.unwrap_or_default();
@@ -576,9 +586,9 @@ impl FontContext {
         match format {
             FT_Glyph_Format::FT_GLYPH_FORMAT_BITMAP => {
                 let bitmap_size = unsafe { (*(*(*slot).face).size).metrics.y_ppem };
-                Some((slot, req_size as f32 / bitmap_size as f32))
+                Some((cached, slot, req_size as f32 / bitmap_size as f32))
             }
-            FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => Some((slot, 1.0)),
+            FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => Some((cached, slot, 1.0)),
             _ => {
                 error!("Unsupported format");
                 debug!("format={:?}", format);
@@ -587,10 +597,16 @@ impl FontContext {
         }
     }
 
-    fn pad_bounding_box(&self, font: &FontInstance, cbox: &mut FT_BBox) {
+    fn pad_bounding_box(font: &FontInstance, cbox: &mut FT_BBox) {
         // Apply extra pixel of padding for subpixel AA, due to the filter.
         if font.render_mode == FontRenderMode::Subpixel {
-            let padding = (self.lcd_extra_pixels * 64) as FT_Pos;
+            // Using an LCD filter may add one full pixel to each side if support is built in.
+            // As of FreeType 2.8.1, an LCD filter is always used regardless of settings
+            // if support for the patent-encumbered LCD filter algorithms is not built in.
+            // Thus, the only reasonable way to guess padding is to unconditonally add it if
+            // subpixel AA is used.
+            let lcd_extra_pixels = 1;
+            let padding = (lcd_extra_pixels * 64) as FT_Pos;
             if font.flags.contains(FontInstanceFlags::LCD_VERTICAL) {
                 cbox.yMin -= padding;
                 cbox.yMax += padding;
@@ -603,7 +619,6 @@ impl FontContext {
 
     // Get the bounding box for a glyph, accounting for sub-pixel positioning.
     fn get_bounding_box(
-        &self,
         slot: FT_GlyphSlot,
         font: &FontInstance,
         glyph: &GlyphKey,
@@ -621,7 +636,7 @@ impl FontContext {
             return cbox;
         }
 
-        self.pad_bounding_box(font, &mut cbox);
+        Self::pad_bounding_box(font, &mut cbox);
 
         // Offset the bounding box by subpixel positioning.
         // Convert to 26.6 fixed point format for FT.
@@ -645,7 +660,6 @@ impl FontContext {
     }
 
     fn get_glyph_dimensions_impl(
-        &self,
         slot: FT_GlyphSlot,
         font: &FontInstance,
         glyph: &GlyphKey,
@@ -663,7 +677,7 @@ impl FontContext {
                 ) }
             }
             FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => {
-                let cbox = self.get_bounding_box(slot, font, glyph, scale);
+                let cbox = Self::get_bounding_box(slot, font, glyph, scale);
                 (
                     (cbox.xMin >> 6) as i32,
                     (cbox.yMax >> 6) as i32,
@@ -723,7 +737,8 @@ impl FontContext {
     }
 
     pub fn get_glyph_index(&mut self, font_key: FontKey, ch: char) -> Option<u32> {
-        let face = self.faces.get(&font_key)?.face;
+        let cached = self.fonts.get(&font_key)?.lock().ok()?;
+        let face = cached.face;
         unsafe {
             let idx = FT_Get_Char_Index(face, ch as _);
             if idx != 0 {
@@ -739,8 +754,8 @@ impl FontContext {
         font: &FontInstance,
         key: &GlyphKey,
     ) -> Option<GlyphDimensions> {
-        let slot = self.load_glyph(font, key);
-        slot.and_then(|(slot, scale)| self.get_glyph_dimensions_impl(slot, &font, key, scale, true))
+        let (_cached, slot, scale) = self.load_glyph(font, key)?;
+        Self::get_glyph_dimensions_impl(slot, &font, key, scale, true)
     }
 
     fn choose_bitmap_size(&self, face: FT_Face, requested_size: f64) -> FT_Error {
@@ -776,7 +791,6 @@ impl FontContext {
     }
 
     fn rasterize_glyph_outline(
-        &mut self,
         slot: FT_GlyphSlot,
         font: &FontInstance,
         key: &GlyphKey,
@@ -795,7 +809,7 @@ impl FontContext {
             let outline = &(*slot).outline;
             let mut cbox = FT_BBox { xMin: 0, yMin: 0, xMax: 0, yMax: 0 };
             FT_Outline_Get_CBox(outline, &mut cbox);
-            self.pad_bounding_box(font, &mut cbox);
+            Self::pad_bounding_box(font, &mut cbox);
             FT_Outline_Translate(
                 outline,
                 dx - ((cbox.xMin + dx) & !63),
@@ -803,16 +817,6 @@ impl FontContext {
             );
         }
 
-        if font.render_mode == FontRenderMode::Subpixel {
-            let FontInstancePlatformOptions { lcd_filter, .. } = font.platform_options.unwrap_or_default();
-            let filter = match lcd_filter {
-                FontLCDFilter::None => FT_LcdFilter::FT_LCD_FILTER_NONE,
-                FontLCDFilter::Default => FT_LcdFilter::FT_LCD_FILTER_DEFAULT,
-                FontLCDFilter::Light => FT_LcdFilter::FT_LCD_FILTER_LIGHT,
-                FontLCDFilter::Legacy => FT_LcdFilter::FT_LCD_FILTER_LEGACY,
-            };
-            unsafe { FT_Library_SetLcdFilter(self.lib, filter) };
-        }
         let render_mode = match font.render_mode {
             FontRenderMode::Mono => FT_Render_Mode::FT_RENDER_MODE_MONO,
             FontRenderMode::Alpha => FT_Render_Mode::FT_RENDER_MODE_NORMAL,
@@ -837,13 +841,57 @@ impl FontContext {
         }
     }
 
+    pub fn begin_rasterize(font: &FontInstance) {
+        // The global LCD filter state is only used in subpixel rendering modes.
+        if font.render_mode == FontRenderMode::Subpixel {
+            let mut cache = FONT_CACHE.lock().unwrap();
+            let FontInstancePlatformOptions { lcd_filter, .. } = font.platform_options.unwrap_or_default();
+            // Check if the current LCD filter matches the requested one.
+            if cache.lcd_filter != lcd_filter {
+                // If the filter doesn't match, we have to wait for all other currently rasterizing threads
+                // that may use the LCD filter state to finish before we can override it.
+                while cache.lcd_filter_uses != 0 {
+                    cache = LCD_FILTER_UNUSED.wait(cache).unwrap();
+                }
+                // Finally set the LCD filter to the requested one now that the library is unused.
+                cache.lcd_filter = lcd_filter;
+                let filter = match lcd_filter {
+                    FontLCDFilter::None => FT_LcdFilter::FT_LCD_FILTER_NONE,
+                    FontLCDFilter::Default => FT_LcdFilter::FT_LCD_FILTER_DEFAULT,
+                    FontLCDFilter::Light => FT_LcdFilter::FT_LCD_FILTER_LIGHT,
+                    FontLCDFilter::Legacy => FT_LcdFilter::FT_LCD_FILTER_LEGACY,
+                };
+                unsafe {
+                    let result = FT_Library_SetLcdFilter(cache.lib, filter);
+                    // Setting the legacy filter may fail, so just use the default filter instead.
+                    if !succeeded(result) {
+                        FT_Library_SetLcdFilter(cache.lib, FT_LcdFilter::FT_LCD_FILTER_DEFAULT);
+                    }
+                }
+            }
+            cache.lcd_filter_uses += 1;
+        }
+    }
+
+    pub fn end_rasterize(font: &FontInstance) {
+        if font.render_mode == FontRenderMode::Subpixel {
+            let mut cache = FONT_CACHE.lock().unwrap();
+            // If this is the last use of the LCD filter, then signal that the LCD filter isn't used.
+            cache.lcd_filter_uses -= 1;
+            if cache.lcd_filter_uses == 0 {
+                LCD_FILTER_UNUSED.notify_all();
+            }
+        }
+    }
+
     pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult {
-        let (slot, scale) = self.load_glyph(font, key).ok_or(GlyphRasterError::LoadFailed)?;
+        let (_cached, slot, scale) = self.load_glyph(font, key)
+                                         .ok_or(GlyphRasterError::LoadFailed)?;
 
         // Get dimensions of the glyph, to see if we need to rasterize it.
         // Don't apply scaling to the dimensions, as the glyph cache needs to know the actual
         // footprint of the glyph.
-        let dimensions = self.get_glyph_dimensions_impl(slot, font, key, scale, false)
+        let dimensions = Self::get_glyph_dimensions_impl(slot, font, key, scale, false)
                              .ok_or(GlyphRasterError::LoadFailed)?;
         let GlyphDimensions { mut left, mut top, width, height, .. } = dimensions;
 
@@ -856,7 +904,7 @@ impl FontContext {
         match format {
             FT_Glyph_Format::FT_GLYPH_FORMAT_BITMAP => {}
             FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => {
-                if !self.rasterize_glyph_outline(slot, font, key, scale) {
+                if !Self::rasterize_glyph_outline(slot, font, key, scale) {
                     return Err(GlyphRasterError::LoadFailed);
                 }
             }
@@ -1051,12 +1099,3 @@ impl FontContext {
     }
 }
 
-impl Drop for FontContext {
-    fn drop(&mut self) {
-        self.variations.clear();
-        self.faces.clear();
-        unsafe {
-            FT_Done_FreeType(self.lib);
-        }
-    }
-}
diff --git a/gfx/wr/webrender/src/platform/windows/font.rs b/gfx/wr/webrender/src/platform/windows/font.rs
index 3f6bd78ab22f3..0fa431d7b92ca 100644
--- a/gfx/wr/webrender/src/platform/windows/font.rs
+++ b/gfx/wr/webrender/src/platform/windows/font.rs
@@ -142,6 +142,10 @@ fn is_bitmap_font(font: &FontInstance) -> bool {
 }
 
 impl FontContext {
+    pub fn distribute_across_threads() -> bool {
+        true
+    }
+
     pub fn new() -> Result<FontContext, ResourceCacheError> {
         Ok(FontContext {
             fonts: FastHashMap::default(),
@@ -547,6 +551,12 @@ impl FontContext {
         (scaled_size as f32, x_scale, y_scale, bitmaps, transform)
     }
 
+    pub fn begin_rasterize(_font: &FontInstance) {
+    }
+
+    pub fn end_rasterize(_font: &FontInstance) {
+    }
+
     pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult {
         let (size, x_scale, y_scale, bitmaps, transform) = Self::get_glyph_parameters(font, key);
         let (analysis, texture_type, bounds) = self.create_glyph_analysis(font, key, size, transform, bitmaps)

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


More information about the tor-commits mailing list