From e5fbb3d48781765745f5fcc007e469f048e9ebd7 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Thu, 14 Mar 2024 12:31:00 +0100 Subject: [PATCH] fonts: Add `FontIdentifier` and `LocalFontIdentifier` (#31658) Instead of using a simple `Atom` to identify a local font, use a data structure. This allows us to carry more information necessary to identify a local font (such as a path on MacOS). We need this for the new version of WebRender, as fonts on MacOS now require a path. This has a lot of benefits: 1. We can avoid loading fonts without paths on MacOS, which should avoid a lot of problems with flakiness and ensure we always load the same font for a given identifier. 2. This clarifies the difference between web fonts and local fonts, though there is more work to do here. 3. This avoid a *lot* of font shenanigans, such as trying to work backwards from the name of the font to the path of the font we actually matched. In general, we can remove a lot of code trying to accomplish these shenanigans. 4. Getting the font bytes always returns an `Arc` now avoiding an extra full font copy in the case of Canvas. --- components/canvas/canvas_data.rs | 3 +- components/gfx/font.rs | 5 +- components/gfx/font_cache_thread.rs | 29 ++-- components/gfx/font_template.rs | 11 +- .../platform/freetype/android/font_list.rs | 27 ++- components/gfx/platform/freetype/font.rs | 43 ++--- components/gfx/platform/freetype/font_list.rs | 40 +++-- .../gfx/platform/freetype/font_template.rs | 96 +++++++---- components/gfx/platform/macos/font.rs | 6 +- components/gfx/platform/macos/font_list.rs | 23 ++- .../gfx/platform/macos/font_template.rs | 159 ++++++------------ components/gfx/platform/windows/font.rs | 42 ++--- components/gfx/platform/windows/font_list.rs | 55 +++--- .../gfx/platform/windows/font_template.rs | 83 +++++---- components/gfx/tests/font_context.rs | 49 ++++-- components/gfx/tests/font_template.rs | 8 +- components/layout/text.rs | 6 +- 17 files changed, 365 insertions(+), 320 deletions(-) diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index 7525574a927..b125eb4e65c 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -501,8 +501,7 @@ impl<'a> CanvasData<'a> { .first(font_context) .expect("couldn't find font"); let font = font.borrow_mut(); - let template = font.handle.template(); - Font::from_bytes(Arc::new(template.bytes()), 0) + Font::from_bytes(font.handle.template().bytes(), 0) .ok() .or_else(|| load_system_font_from_style(Some(style))) }) diff --git a/components/gfx/font.rs b/components/gfx/font.rs index 885a89db25a..fe5de070421 100644 --- a/components/gfx/font.rs +++ b/components/gfx/font.rs @@ -24,6 +24,7 @@ use style::values::computed::font::{GenericFontFamily, SingleFontFamily}; use unicode_script::Script; use webrender_api::FontInstanceKey; +use crate::font_cache_thread::FontIdentifier; use crate::font_context::{FontContext, FontSource}; use crate::font_template::FontTemplateDescriptor; use crate::platform::font::{FontHandle, FontTable}; @@ -78,7 +79,7 @@ pub trait FontHandleMethods: Sized { fn table_for_tag(&self, _: FontTableTag) -> Option; /// A unique identifier for the font, allowing comparison. - fn identifier(&self) -> Atom; + fn identifier(&self) -> &FontIdentifier; } // Used to abstract over the shaper's choice of fixed int representation. @@ -202,7 +203,7 @@ impl Font { } /// A unique identifier for the font, allowing comparison. - pub fn identifier(&self) -> Atom { + pub fn identifier(&self) -> &FontIdentifier { self.handle.identifier() } } diff --git a/components/gfx/font_cache_thread.rs b/components/gfx/font_cache_thread.rs index 773fe64fe46..aaa5f629d13 100644 --- a/components/gfx/font_cache_thread.rs +++ b/components/gfx/font_cache_thread.rs @@ -28,7 +28,8 @@ use crate::font_context::FontSource; use crate::font_template::{FontTemplate, FontTemplateDescriptor}; use crate::platform::font_context::FontContextHandle; use crate::platform::font_list::{ - for_each_available_family, for_each_variation, system_default_family, SANS_SERIF_FONT_FAMILY, + for_each_available_family, for_each_variation, system_default_family, LocalFontIdentifier, + SANS_SERIF_FONT_FAMILY, }; use crate::platform::font_template::FontTemplateData; @@ -50,9 +51,15 @@ pub struct SerializedFontTemplateInfo { pub font_key: FontKey, } +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub enum FontIdentifier { + Local(LocalFontIdentifier), + Web(ServoUrl), +} + #[derive(Debug, Deserialize, Serialize)] pub struct SerializedFontTemplate { - identifier: Atom, + identifier: FontIdentifier, bytes_receiver: ipc_channel::ipc::IpcBytesReceiver, } @@ -110,7 +117,7 @@ impl FontTemplates { None } - pub fn add_template(&mut self, identifier: Atom, maybe_data: Option>) { + pub fn add_template(&mut self, identifier: FontIdentifier, maybe_data: Option>) { for template in &self.templates { if *template.identifier() == identifier { return; @@ -155,7 +162,7 @@ struct FontCache { font_context: FontContextHandle, core_resource_thread: CoreResourceThread, webrender_api: Box, - webrender_fonts: HashMap, + webrender_fonts: HashMap, font_instances: HashMap<(FontKey, Au), FontInstanceKey>, } @@ -235,7 +242,7 @@ impl FontCache { }, Command::AddDownloadedWebFont(family_name, url, bytes, result) => { let templates = &mut self.web_families.get_mut(&family_name).unwrap(); - templates.add_template(Atom::from(url.to_string()), Some(bytes)); + templates.add_template(FontIdentifier::Web(url), Some(bytes)); drop(result.send(())); }, Command::Ping => (), @@ -346,9 +353,9 @@ impl FontCache { let font_face_name = LowercaseString::new(&font.name); let templates = &mut self.web_families.get_mut(&family_name).unwrap(); let mut found = false; - for_each_variation(&font_face_name, |path| { + for_each_variation(&font_face_name, |local_font_identifier| { found = true; - templates.add_template(Atom::from(&*path), None); + templates.add_template(FontIdentifier::Local(local_font_identifier), None); }); if found { sender.send(()).unwrap(); @@ -389,8 +396,8 @@ impl FontCache { let s = self.local_families.get_mut(&family_name).unwrap(); if s.templates.is_empty() { - for_each_variation(&family_name, |path| { - s.add_template(Atom::from(&*path), None); + for_each_variation(&family_name, |local_font_identifier| { + s.add_template(FontIdentifier::Local(local_font_identifier), None); }); } @@ -430,9 +437,9 @@ impl FontCache { .entry(template.identifier.clone()) .or_insert_with(|| { let font = match (template.bytes_if_in_memory(), template.native_font()) { - (Some(bytes), _) => FontData::Raw(bytes), + (Some(bytes), _) => FontData::Raw((*bytes).clone()), (None, Some(native_font)) => FontData::Native(native_font), - (None, None) => FontData::Raw(template.bytes()), + (None, None) => FontData::Raw((*template.bytes()).clone()), }; webrender_api.add_font(font) }); diff --git a/components/gfx/font_template.rs b/components/gfx/font_template.rs index ff4e1d49824..0cb7ca80828 100644 --- a/components/gfx/font_template.rs +++ b/components/gfx/font_template.rs @@ -7,13 +7,13 @@ use std::io::Error as IoError; use std::sync::{Arc, Weak}; use serde::{Deserialize, Serialize}; -use servo_atoms::Atom; use style::computed_values::font_stretch::T as FontStretch; use style::computed_values::font_style::T as FontStyle; use style::properties::style_structs::Font as FontStyleStruct; use style::values::computed::font::FontWeight; use crate::font::FontHandleMethods; +use crate::font_cache_thread::FontIdentifier; use crate::platform::font::FontHandle; use crate::platform::font_context::FontContextHandle; use crate::platform::font_template::FontTemplateData; @@ -85,7 +85,7 @@ impl<'a> From<&'a FontStyleStruct> for FontTemplateDescriptor { /// font instance handles. It contains a unique /// FontTemplateData structure that is platform specific. pub struct FontTemplate { - identifier: Atom, + identifier: FontIdentifier, descriptor: Option, weak_ref: Option>, // GWTODO: Add code path to unset the strong_ref for web fonts! @@ -103,7 +103,10 @@ impl Debug for FontTemplate { /// is common, regardless of the number of instances of /// this font handle per thread. impl FontTemplate { - pub fn new(identifier: Atom, maybe_bytes: Option>) -> Result { + pub fn new( + identifier: FontIdentifier, + maybe_bytes: Option>, + ) -> Result { let maybe_data = match maybe_bytes { Some(_) => Some(FontTemplateData::new(identifier.clone(), maybe_bytes)?), None => None, @@ -122,7 +125,7 @@ impl FontTemplate { }) } - pub fn identifier(&self) -> &Atom { + pub fn identifier(&self) -> &FontIdentifier { &self.identifier } diff --git a/components/gfx/platform/freetype/android/font_list.rs b/components/gfx/platform/freetype/android/font_list.rs index 44c6640468c..968cb460589 100644 --- a/components/gfx/platform/freetype/android/font_list.rs +++ b/components/gfx/platform/freetype/android/font_list.rs @@ -5,6 +5,8 @@ use std::path::Path; use log::warn; +use serde::{Deserialize, Serialize}; +use style::Atom; use ucd::{Codepoint, UnicodeBlock}; use super::xml::{Attribute, Node}; @@ -14,6 +16,13 @@ lazy_static::lazy_static! { static ref FONT_LIST: FontList = FontList::new(); } +/// An identifier for a local font on Android systems. +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub struct LocalFontIdentifier { + /// The path to the font. + pub path: Atom, +} + // Android doesn't provide an API to query system fonts until Android O: // https://developer.android.com/reference/android/text/FontConfig.html // System font configuration files must be parsed until Android O version is set as the minimum target. @@ -426,11 +435,17 @@ where pub fn for_each_variation(family_name: &str, mut callback: F) where - F: FnMut(String), + F: FnMut(LocalFontIdentifier), { + let mut produce_font = |font: &Font| { + callback(LocalFontIdentifier { + path: Atom::from(FontList::font_absolute_path(&font.filename)), + }) + }; + if let Some(family) = FONT_LIST.find_family(family_name) { for font in &family.fonts { - callback(FontList::font_absolute_path(&font.filename)); + produce_font(font); } return; } @@ -439,12 +454,8 @@ where if let Some(family) = FONT_LIST.find_family(&alias.to) { for font in &family.fonts { match (alias.weight, font.weight) { - (None, _) => callback(FontList::font_absolute_path(&font.filename)), - (Some(w1), Some(w2)) => { - if w1 == w2 { - callback(FontList::font_absolute_path(&font.filename)) - } - }, + (None, _) => produce_font(font), + (Some(w1), Some(w2)) if w1 == w2 => produce_font(font), _ => {}, } } diff --git a/components/gfx/platform/freetype/font.rs b/components/gfx/platform/freetype/font.rs index 39cd4958d5f..1fd429aaefa 100644 --- a/components/gfx/platform/freetype/font.rs +++ b/components/gfx/platform/freetype/font.rs @@ -17,7 +17,6 @@ use freetype::freetype::{ use freetype::succeeded; use freetype::tt_os2::TT_OS2; use log::debug; -use servo_atoms::Atom; use style::computed_values::font_stretch::T as FontStretch; use style::computed_values::font_weight::T as FontWeight; use style::values::computed::font::FontStyle; @@ -27,6 +26,7 @@ use crate::font::{ FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, GPOS, GSUB, KERN, }; +use crate::font_cache_thread::FontIdentifier; use crate::platform::font_context::FontContextHandle; use crate::platform::font_template::FontTemplateData; use crate::text::glyph::GlyphId; @@ -102,23 +102,26 @@ fn create_face( let mut face: FT_Face = ptr::null_mut(); let face_index = 0 as FT_Long; - let result = if let Some(ref bytes) = template.bytes { - FT_New_Memory_Face( - lib, - bytes.as_ptr(), - bytes.len() as FT_Long, - face_index, - &mut face, - ) - } else { - // This will trigger a synchronous file read during layout, which we may want to - // revisit at some point. See discussion here: - // - // https://github.com/servo/servo/pull/20506#issuecomment-378838800 - - let filename = - CString::new(&*template.identifier).expect("filename contains NUL byte!"); - FT_New_Face(lib, filename.as_ptr(), face_index, &mut face) + let result = match template.identifier { + FontIdentifier::Web(_) => { + let bytes = template.bytes(); + FT_New_Memory_Face( + lib, + bytes.as_ptr(), + bytes.len() as FT_Long, + face_index, + &mut face, + ) + }, + FontIdentifier::Local(ref local_identifier) => { + // This will trigger a synchronous file read during layout, which we may want to + // revisit at some point. See discussion here: + // + // https://github.com/servo/servo/pull/20506#issuecomment-378838800 + let filename = + CString::new(&*local_identifier.path).expect("filename contains NUL byte!"); + FT_New_Face(lib, filename.as_ptr(), face_index, &mut face) + }, }; if !succeeded(result) || face.is_null() { @@ -361,8 +364,8 @@ impl FontHandleMethods for FontHandle { } } - fn identifier(&self) -> Atom { - self.font_data.identifier.clone() + fn identifier(&self) -> &FontIdentifier { + &self.font_data.identifier } } diff --git a/components/gfx/platform/freetype/font_list.rs b/components/gfx/platform/freetype/font_list.rs index e9e9ad7a531..bd1df3661e9 100644 --- a/components/gfx/platform/freetype/font_list.rs +++ b/components/gfx/platform/freetype/font_list.rs @@ -13,6 +13,8 @@ use fontconfig_sys::{ }; use libc::{c_char, c_int}; use log::debug; +use serde::{Deserialize, Serialize}; +use style::Atom; use super::c_str_to_string; use crate::text::util::is_cjk; @@ -22,6 +24,15 @@ static FC_FILE: &[u8] = b"file\0"; static FC_INDEX: &[u8] = b"index\0"; static FC_FONTFORMAT: &[u8] = b"fontformat\0"; +/// An identifier for a local font on systems using Freetype. +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub struct LocalFontIdentifier { + /// The path to the font. + pub path: Atom, + /// The variation index within the font. + pub variation_index: i32, +} + pub fn for_each_available_family(mut callback: F) where F: FnMut(String), @@ -59,7 +70,7 @@ where pub fn for_each_variation(family_name: &str, mut callback: F) where - F: FnMut(String), + F: FnMut(LocalFontIdentifier), { debug!("getting variations for {}", family_name); unsafe { @@ -84,31 +95,24 @@ where FcObjectSetAdd(object_set, FC_INDEX.as_ptr() as *mut c_char); let matches = FcFontSetList(config, font_set_array_ptr, 1, pattern, object_set); - debug!("found {} variations", (*matches).nfont); for i in 0..((*matches).nfont as isize) { let font = (*matches).fonts.offset(i); - let mut file: *mut FcChar8 = ptr::null_mut(); - let result = FcPatternGetString(*font, FC_FILE.as_ptr() as *mut c_char, 0, &mut file); - let file = if result == FcResultMatch { - c_str_to_string(file as *const c_char) - } else { - panic!(); - }; + + let mut path: *mut FcChar8 = ptr::null_mut(); + let result = FcPatternGetString(*font, FC_FILE.as_ptr() as *mut c_char, 0, &mut path); + assert_eq!(result, FcResultMatch); + let mut index: libc::c_int = 0; let result = FcPatternGetInteger(*font, FC_INDEX.as_ptr() as *mut c_char, 0, &mut index); - let index = if result == FcResultMatch { - index - } else { - panic!(); - }; + assert_eq!(result, FcResultMatch); - debug!("variation file: {}", file); - debug!("variation index: {}", index); - - callback(file); + callback(LocalFontIdentifier { + path: Atom::from(c_str_to_string(path as *const c_char)), + variation_index: index as i32, + }); } FcFontSetDestroy(matches); diff --git a/components/gfx/platform/freetype/font_template.rs b/components/gfx/platform/freetype/font_template.rs index b65bb8f4b04..b9896e15ea8 100644 --- a/components/gfx/platform/freetype/font_template.rs +++ b/components/gfx/platform/freetype/font_template.rs @@ -5,67 +5,89 @@ use std::fmt; use std::fs::File; use std::io::{Error, Read}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, RwLock}; use serde::{Deserialize, Serialize}; -use servo_atoms::Atom; use webrender_api::NativeFontHandle; +use crate::font_cache_thread::FontIdentifier; + /// Platform specific font representation for Linux. -/// The identifier is an absolute path, and the bytes -/// field is the loaded data that can be passed to -/// freetype and Raqote directly. #[derive(Deserialize, Serialize)] pub struct FontTemplateData { - // If you add members here, review the Debug impl below - pub bytes: Option>, - pub identifier: Atom, + /// Lazily-loaded (for local fonts) byte data that can be passed + /// to Freetype or Raqote directly. + pub font_data: RwLock>>>, + pub identifier: FontIdentifier, } impl fmt::Debug for FontTemplateData { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("FontTemplateData") - .field( - "bytes", - &self.bytes.as_ref().map(|b| format!("[{} bytes]", b.len())), - ) .field("identifier", &self.identifier) + .field( + "font_data", + &self + .font_data + .read() + .unwrap() + .as_ref() + .map(|bytes| format!("[{} bytes]", bytes.len())), + ) .finish() } } impl FontTemplateData { - pub fn new(identifier: Atom, bytes: Option>) -> Result { - Ok(FontTemplateData { bytes, identifier }) - } - - /// Returns a clone of the data in this font. This may be a hugely expensive - /// operation (depending on the platform) which performs synchronous disk I/O - /// and should never be done lightly. - pub fn bytes(&self) -> Vec { - self.bytes_if_in_memory().unwrap_or_else(|| { - let mut file = File::open(&*self.identifier).expect("Couldn't open font file!"); - let mut buffer = vec![]; - file.read_to_end(&mut buffer).unwrap(); - buffer + pub fn new( + identifier: FontIdentifier, + font_data: Option>, + ) -> Result { + Ok(FontTemplateData { + identifier, + font_data: RwLock::new(font_data.map(Arc::new)), }) } - /// Returns a clone of the bytes in this font if they are in memory. This function never - /// performs disk I/O. - pub fn bytes_if_in_memory(&self) -> Option> { - self.bytes.clone() + /// Returns a reference to the data in this font. This may be a hugely expensive + /// operation (depending on the platform) which performs synchronous disk I/O + /// and should never be done lightly. + pub fn bytes(&self) -> Arc> { + self.font_data + .write() + .unwrap() + .get_or_insert_with(|| { + let path = match &self.identifier { + FontIdentifier::Local(local) => local.path.clone(), + FontIdentifier::Web(_) => unreachable!("Web fonts should always have data."), + }; + let mut bytes = Vec::new(); + File::open(Path::new(&*path)) + .expect("Couldn't open font file!") + .read_to_end(&mut bytes) + .unwrap(); + Arc::new(bytes) + }) + .clone() + } + + /// Returns a reference to the bytes in this font if they are in memory. This function + /// never performs disk I/O. + pub fn bytes_if_in_memory(&self) -> Option>> { + self.font_data.read().unwrap().as_ref().cloned() } /// Returns the native font that underlies this font template, if applicable. pub fn native_font(&self) -> Option { - if self.bytes.is_none() { - Some(NativeFontHandle { - path: PathBuf::from(&*self.identifier), - index: 0, - }) - } else { - None - } + let local_identifier = match &self.identifier { + FontIdentifier::Local(local_identifier) => local_identifier, + FontIdentifier::Web(_) => return None, + }; + + Some(NativeFontHandle { + path: PathBuf::from(&*local_identifier.path), + index: 0, + }) } } diff --git a/components/gfx/platform/macos/font.rs b/components/gfx/platform/macos/font.rs index 552004d98ed..b9681e246fd 100644 --- a/components/gfx/platform/macos/font.rs +++ b/components/gfx/platform/macos/font.rs @@ -19,13 +19,13 @@ use core_text::font_descriptor::{ kCTFontDefaultOrientation, SymbolicTraitAccessors, TraitAccessors, }; use log::debug; -use servo_atoms::Atom; use style::values::computed::font::{FontStretch, FontStyle, FontWeight}; use crate::font::{ FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, GPOS, GSUB, KERN, }; +use crate::font_cache_thread::FontIdentifier; use crate::platform::font_template::FontTemplateData; use crate::platform::macos::font_context::FontContextHandle; use crate::text::glyph::GlyphId; @@ -325,7 +325,7 @@ impl FontHandleMethods for FontHandle { result.and_then(|data| Some(FontTable::wrap(data))) } - fn identifier(&self) -> Atom { - self.font_data.identifier.clone() + fn identifier(&self) -> &FontIdentifier { + &self.font_data.identifier } } diff --git a/components/gfx/platform/macos/font_list.rs b/components/gfx/platform/macos/font_list.rs index 32b8b93b21f..490f5e44028 100644 --- a/components/gfx/platform/macos/font_list.rs +++ b/components/gfx/platform/macos/font_list.rs @@ -3,10 +3,21 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use log::debug; +use serde::{Deserialize, Serialize}; +use style::Atom; use ucd::{Codepoint, UnicodeBlock}; use crate::text::util::unicode_plane; +/// An identifier for a local font on a MacOS system. These values comes from the CoreText +/// CTFontCollection. Note that `path` here is required. We do not load fonts that do not +/// have paths. +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub struct LocalFontIdentifier { + pub postscript_name: Atom, + pub path: Atom, +} + pub fn for_each_available_family(mut callback: F) where F: FnMut(String), @@ -19,7 +30,7 @@ where pub fn for_each_variation(family_name: &str, mut callback: F) where - F: FnMut(String), + F: FnMut(LocalFontIdentifier), { debug!("Looking for faces of family: {}", family_name); @@ -27,7 +38,15 @@ where if let Some(family_collection) = family_collection { if let Some(family_descriptors) = family_collection.get_descriptors() { for family_descriptor in family_descriptors.iter() { - callback(family_descriptor.font_name()); + let path = family_descriptor.font_path(); + let path = match path.as_ref().and_then(|path| path.to_str()) { + Some(path) => path, + None => continue, + }; + callback(LocalFontIdentifier { + postscript_name: Atom::from(family_descriptor.font_name()), + path: Atom::from(path), + }) } } } diff --git a/components/gfx/platform/macos/font_template.rs b/components/gfx/platform/macos/font_template.rs index 9e3c4a72f1b..a770996a81a 100644 --- a/components/gfx/platform/macos/font_template.rs +++ b/components/gfx/platform/macos/font_template.rs @@ -2,34 +2,27 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use std::borrow::ToOwned; +use std::collections::hash_map::Entry; use std::collections::HashMap; use std::fmt; -use std::fs::{self, File}; +use std::fs::File; use std::io::{Error as IoError, Read}; use std::ops::Deref; use std::path::Path; use std::sync::{Arc, Mutex, RwLock}; use app_units::Au; -use core_foundation::array::CFArray; -use core_foundation::base::{CFType, TCFType}; -use core_foundation::dictionary::CFDictionary; -use core_foundation::string::CFString; use core_graphics::data_provider::CGDataProvider; use core_graphics::font::CGFont; use core_text::font::CTFont; -use core_text::{font_collection, font_descriptor}; use serde::de::{Error, Visitor}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use servo_atoms::Atom; -use servo_url::ServoUrl; use webrender_api::NativeFontHandle; -/// Platform specific font representation for mac. -/// The identifier is a PostScript font name. The -/// CTFont object is cached here for use by the -/// paint functions that create CGFont references. +use crate::font_cache_thread::FontIdentifier; + +/// Platform specific font representation for MacOS. CTFont object is cached here for use +/// by the paint functions that create CGFont references. #[derive(Deserialize, Serialize)] pub struct FontTemplateData { // If you add members here, review the Debug impl below @@ -41,8 +34,7 @@ pub struct FontTemplateData { /// the other side, because `CTFont` instances cannot be sent across processes. This is /// harmless, however, because it can always be recreated. ctfont: CachedCTFont, - - pub identifier: Atom, + pub identifier: FontIdentifier, pub font_data: RwLock>>>, } @@ -68,10 +60,13 @@ unsafe impl Send for FontTemplateData {} unsafe impl Sync for FontTemplateData {} impl FontTemplateData { - pub fn new(identifier: Atom, font_data: Option>) -> Result { + pub fn new( + identifier: FontIdentifier, + font_data: Option>, + ) -> Result { Ok(FontTemplateData { ctfont: CachedCTFont(Mutex::new(HashMap::new())), - identifier: identifier.to_owned(), + identifier, font_data: RwLock::new(font_data.map(Arc::new)), }) } @@ -79,105 +74,53 @@ impl FontTemplateData { /// Retrieves the Core Text font instance, instantiating it if necessary. pub fn ctfont(&self, pt_size: f64) -> Option { let mut ctfonts = self.ctfont.lock().unwrap(); - let pt_size_key = Au::from_f64_px(pt_size); - if !ctfonts.contains_key(&pt_size_key) { - // If you pass a zero font size to one of the Core Text APIs, it'll replace it with - // 12.0. We don't want that! (Issue #10492.) - let clamped_pt_size = pt_size.max(0.01); - let mut font_data = self.font_data.write().unwrap(); - let ctfont = match *font_data { - Some(ref bytes) => { - let fontprov = CGDataProvider::from_buffer(bytes.clone()); - let cgfont_result = CGFont::from_data_provider(fontprov); - match cgfont_result { - Ok(cgfont) => { - Some(core_text::font::new_from_CGFont(&cgfont, clamped_pt_size)) - }, - Err(_) => None, - } - }, - None => { - // We can't rely on Core Text to load a font for us by postscript - // name here, due to https://github.com/servo/servo/issues/23290. - // The APIs will randomly load the wrong font, forcing us to use - // the roundabout route of creating a Core Graphics font from a - // a set of descriptors and then creating a Core Text font from - // that one. - let attributes: CFDictionary = - CFDictionary::from_CFType_pairs(&[( - CFString::new("NSFontNameAttribute"), - CFString::new(&*self.identifier).as_CFType(), - )]); - - let descriptor = font_descriptor::new_from_attributes(&attributes); - let descriptors = CFArray::from_CFTypes(&[descriptor]); - let collection = font_collection::new_from_descriptors(&descriptors); - collection.get_descriptors().and_then(|descriptors| { - let descriptor = descriptors.get(0).unwrap(); - let font_path = Path::new(&descriptor.font_path().unwrap()).to_owned(); - fs::read(&font_path).ok().and_then(|bytes| { - let font_bytes = Arc::new(bytes); - let fontprov = CGDataProvider::from_buffer(font_bytes.clone()); - CGFont::from_data_provider(fontprov).ok().map(|cgfont| { - *font_data = Some(font_bytes); - core_text::font::new_from_CGFont(&cgfont, clamped_pt_size) - }) - }) - }) - }, - }; - if let Some(ctfont) = ctfont { - ctfonts.insert(pt_size_key, ctfont); - } + let entry = ctfonts.entry(Au::from_f64_px(pt_size)); + match entry { + Entry::Occupied(entry) => return Some(entry.get().clone()), + Entry::Vacant(_) => {}, } - ctfonts.get(&pt_size_key).map(|ctfont| (*ctfont).clone()) + + // If you pass a zero font size to one of the Core Text APIs, it'll replace it with + // 12.0. We don't want that! (Issue #10492.) + let clamped_pt_size = pt_size.max(0.01); + + let provider = CGDataProvider::from_buffer(self.bytes()); + let cgfont = CGFont::from_data_provider(provider).ok()?; + let ctfont = core_text::font::new_from_CGFont(&cgfont, clamped_pt_size); + + // Cache the newly created CTFont font. + entry.or_insert(ctfont.clone()); + + Some(ctfont) } - /// Returns a clone of the data in this font. This may be a hugely expensive + /// Returns a reference to the data in this font. This may be a hugely expensive /// operation (depending on the platform) which performs synchronous disk I/O /// and should never be done lightly. - pub fn bytes(&self) -> Vec { - if let Some(font_data) = self.bytes_if_in_memory() { - return font_data; - } - - // This is spooky action at a distance, but getting a CTFont from this template - // will (in the common case) bring the bytes into memory if they were not there - // already. This also helps work around intermittent panics like - // https://github.com/servo/servo/issues/24622 that occur for unclear reasons. - let ctfont = self.ctfont(0.0); - if let Some(font_data) = self.bytes_if_in_memory() { - return font_data; - } - - let path = ServoUrl::parse( - &*ctfont - .expect("No Core Text font available!") - .url() - .expect("No URL for Core Text font!") - .get_string() - .to_string(), - ) - .expect("Couldn't parse Core Text font URL!") - .to_file_path() - .expect("Core Text font didn't name a path!"); - let mut bytes = Vec::new(); - File::open(path) - .expect("Couldn't open font file!") - .read_to_end(&mut bytes) - .unwrap(); - bytes + pub fn bytes(&self) -> Arc> { + self.font_data + .write() + .unwrap() + .get_or_insert_with(|| { + let path = match &self.identifier { + FontIdentifier::Local(local) => local.path.clone(), + FontIdentifier::Web(_) => unreachable!("Web fonts should always have data."), + }; + let mut bytes = Vec::new(); + File::open(Path::new(&*path)) + .expect("Couldn't open font file!") + .read_to_end(&mut bytes) + .unwrap(); + Arc::new(bytes) + }) + .clone() } - /// Returns a clone of the bytes in this font if they are in memory. This function never - /// performs disk I/O. - pub fn bytes_if_in_memory(&self) -> Option> { - self.font_data - .read() - .unwrap() - .as_ref() - .map(|bytes| (**bytes).clone()) + /// Returns a reference to the bytes in this font if they are in memory. + /// This function never performs disk I/O. + pub fn bytes_if_in_memory(&self) -> Option>> { + self.font_data.read().unwrap().as_ref().cloned() } /// Returns the native font that underlies this font template, if applicable. diff --git a/components/gfx/platform/windows/font.rs b/components/gfx/platform/windows/font.rs index 4e57d00c187..8b05eb3cf07 100644 --- a/components/gfx/platform/windows/font.rs +++ b/components/gfx/platform/windows/font.rs @@ -13,7 +13,6 @@ use std::sync::Arc; use app_units::Au; use dwrote::{Font, FontFace, FontFile, FontStretch, FontStyle}; use log::debug; -use servo_atoms::Atom; use style::computed_values::font_stretch::T as StyleFontStretch; use style::computed_values::font_weight::T as StyleFontWeight; use style::values::computed::font::FontStyle as StyleFontStyle; @@ -22,9 +21,9 @@ use style::values::specified::font::FontStretchKeyword; use crate::font::{ FontHandleMethods, FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, }; +use crate::font_cache_thread::FontIdentifier; use crate::platform::font_template::FontTemplateData; use crate::platform::windows::font_context::FontContextHandle; -use crate::platform::windows::font_list::font_from_atom; use crate::text::glyph::GlyphId; // 1em = 12pt = 16px, assuming 72 points per inch and 96 px per inch @@ -208,7 +207,6 @@ pub struct FontHandle { face: Nondebug, info: FontInfo, em_size: f32, - du_per_em: f32, du_to_px: f32, scaled_du_to_px: f32, } @@ -236,24 +234,17 @@ impl FontHandleMethods for FontHandle { template: Arc, pt_size: Option, ) -> Result { - let (info, face) = if let Some(ref raw_font) = template.bytes { - let font_file = FontFile::new_from_data(Arc::new(raw_font.clone())); - if font_file.is_none() { - // failed to load raw font - return Err(()); - } - - let face = font_file - .unwrap() - .create_face(0, dwrote::DWRITE_FONT_SIMULATIONS_NONE) - .map_err(|_| ())?; - let info = FontInfo::new_from_face(&face)?; - (info, face) - } else { - let font = font_from_atom(&template.identifier); - let face = font.create_font_face(); - let info = FontInfo::new_from_font(&font)?; - (info, face) + let (face, info) = match template.get_font() { + Some(font) => (font.create_font_face(), FontInfo::new_from_font(&font)?), + None => { + let bytes = template.bytes(); + let font_file = FontFile::new_from_data(bytes).ok_or(())?; + let face = font_file + .create_face(0, dwrote::DWRITE_FONT_SIMULATIONS_NONE) + .map_err(|_| ())?; + let info = FontInfo::new_from_face(&face)?; + (face, info) + }, }; let pt_size = pt_size.unwrap_or(au_from_pt(12.)); @@ -268,9 +259,8 @@ impl FontHandleMethods for FontHandle { Ok(FontHandle { font_data: template.clone(), face: Nondebug(face), - info: info, - em_size: em_size, - du_per_em: du_per_em, + info, + em_size, du_to_px: design_units_to_pixels, scaled_du_to_px: scaled_design_units_to_pixels, }) @@ -366,7 +356,7 @@ impl FontHandleMethods for FontHandle { .map(|bytes| FontTable { data: bytes }) } - fn identifier(&self) -> Atom { - self.font_data.identifier.clone() + fn identifier(&self) -> &FontIdentifier { + &self.font_data.identifier } } diff --git a/components/gfx/platform/windows/font_list.rs b/components/gfx/platform/windows/font_list.rs index bf9c019cf97..6da723cdc38 100644 --- a/components/gfx/platform/windows/font_list.rs +++ b/components/gfx/platform/windows/font_list.rs @@ -3,22 +3,18 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::collections::HashMap; -use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::Mutex; +use std::hash::Hash; +use std::sync::{Arc, Mutex}; -use dwrote::{Font, FontCollection, FontDescriptor}; +use dwrote::{FontCollection, FontDescriptor}; use lazy_static::lazy_static; +use serde::{Deserialize, Serialize}; use servo_atoms::Atom; use ucd::{Codepoint, UnicodeBlock}; use crate::text::util::unicode_plane; -lazy_static! { - static ref FONT_ATOM_COUNTER: AtomicUsize = AtomicUsize::new(1); - static ref FONT_ATOM_MAP: Mutex> = Mutex::new(HashMap::new()); -} - -pub static SANS_SERIF_FONT_FAMILY: &'static str = "Arial"; +pub static SANS_SERIF_FONT_FAMILY: &str = "Arial"; pub fn system_default_family(_: &str) -> Option { Some("Verdana".to_owned()) @@ -34,6 +30,24 @@ where } } +/// An identifier for a local font on a Windows system. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub struct LocalFontIdentifier { + /// The FontDescriptor of this font. + pub font_descriptor: Arc, +} + +impl Eq for LocalFontIdentifier {} + +impl Hash for LocalFontIdentifier { + fn hash(&self, state: &mut H) { + self.font_descriptor.family_name.hash(state); + self.font_descriptor.weight.to_u32().hash(state); + self.font_descriptor.stretch.to_u32().hash(state); + self.font_descriptor.style.to_u32().hash(state); + } +} + // for_each_variation is supposed to return a string that can be // atomized and then uniquely used to return back to this font. // Some platforms use the full postscript name (MacOS X), or @@ -45,35 +59,20 @@ where pub fn for_each_variation(family_name: &str, mut callback: F) where - F: FnMut(String), + F: FnMut(LocalFontIdentifier), { let system_fc = FontCollection::system(); if let Some(family) = system_fc.get_font_family_by_name(family_name) { let count = family.get_font_count(); for i in 0..count { let font = family.get_font(i); - let index = FONT_ATOM_COUNTER.fetch_add(1, Ordering::Relaxed); - let index_str = format!("{}", index); - let atom = Atom::from(index_str.clone()); - - { - let descriptor = font.to_descriptor(); - let mut fonts = FONT_ATOM_MAP.lock().unwrap(); - fonts.insert(atom, descriptor); - } - - callback(index_str); + callback(LocalFontIdentifier { + font_descriptor: Arc::new(font.to_descriptor()), + }); } } } -pub fn font_from_atom(ident: &Atom) -> Font { - let fonts = FONT_ATOM_MAP.lock().unwrap(); - FontCollection::system() - .get_font_from_descriptor(fonts.get(ident).unwrap()) - .unwrap() -} - // Based on gfxWindowsPlatform::GetCommonFallbackFonts() in Gecko pub fn fallback_font_families(codepoint: Option) -> Vec<&'static str> { let mut families = vec!["Arial"]; diff --git a/components/gfx/platform/windows/font_template.rs b/components/gfx/platform/windows/font_template.rs index 85477d36b36..5e2469550dc 100644 --- a/components/gfx/platform/windows/font_template.rs +++ b/components/gfx/platform/windows/font_template.rs @@ -2,28 +2,32 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use std::sync::{Arc, RwLock}; use std::{fmt, io}; +use dwrote::{Font, FontCollection}; use serde::{Deserialize, Serialize}; -use servo_atoms::Atom; use webrender_api::NativeFontHandle; -use crate::platform::windows::font_list::font_from_atom; +use crate::font_cache_thread::FontIdentifier; #[derive(Deserialize, Serialize)] pub struct FontTemplateData { - // If you add members here, review the Debug impl below - pub bytes: Option>, - pub identifier: Atom, + /// The identifier for this font. + pub identifier: FontIdentifier, + /// The bytes of this font, lazily loaded if this is a local font. + pub font_data: RwLock>>>, } impl fmt::Debug for FontTemplateData { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("FontTemplateData") .field( - "bytes", + "font_data", &self - .bytes + .font_data + .read() + .unwrap() .as_ref() .map(|bytes| format!("[{} bytes]", bytes.len())), ) @@ -34,40 +38,61 @@ impl fmt::Debug for FontTemplateData { impl FontTemplateData { pub fn new( - identifier: Atom, + identifier: FontIdentifier, font_data: Option>, ) -> Result { Ok(FontTemplateData { - bytes: font_data, - identifier: identifier, + identifier, + font_data: RwLock::new(font_data.map(Arc::new)), }) } - pub fn bytes(&self) -> Vec { - if self.bytes.is_some() { - self.bytes.as_ref().unwrap().clone() - } else { - let font = font_from_atom(&self.identifier); - let face = font.create_font_face(); - let files = face.get_files(); - assert!(files.len() > 0); + /// Returns a reference to the data in this font. This may be a hugely expensive + /// operation (depending on the platform) which performs synchronous disk I/O + /// and should never be done lightly. + pub fn bytes(&self) -> Arc> { + self.font_data + .write() + .unwrap() + .get_or_insert_with(|| { + let font_descriptor = match &self.identifier { + FontIdentifier::Local(local_identifier) => { + local_identifier.font_descriptor.clone() + }, + FontIdentifier::Web(_) => unreachable!("Created a web font without data."), + }; - files[0].get_font_file_bytes() - } + let font = FontCollection::system() + .get_font_from_descriptor(&font_descriptor) + .unwrap(); + let face = font.create_font_face(); + let files = face.get_files(); + assert!(!files.is_empty()); + Arc::new(files[0].get_font_file_bytes()) + }) + .clone() } - pub fn bytes_if_in_memory(&self) -> Option> { - self.bytes.clone() + /// Returns a reference to the bytes in this font if they are in memory. + /// This function never performs disk I/O. + pub fn bytes_if_in_memory(&self) -> Option>> { + self.font_data.read().unwrap().as_ref().cloned() + } + + /// Get a [`FontFace`] for this font if it is a local font or return `None` if it's a + /// web font. + pub fn get_font(&self) -> Option { + let font_descriptor = match &self.identifier { + FontIdentifier::Local(local_identifier) => local_identifier.font_descriptor.clone(), + FontIdentifier::Web(_) => return None, + }; + + FontCollection::system().get_font_from_descriptor(&font_descriptor) } pub fn native_font(&self) -> Option { - if self.bytes.is_some() { - return None; - } - let font = font_from_atom(&self.identifier); - let face = font.create_font_face(); - let files = face.get_files(); - let path = files.iter().next()?.get_font_file_path()?; + let face = self.get_font()?.create_font_face(); + let path = face.get_files().first()?.get_font_file_path()?; Some(NativeFontHandle { path, index: face.get_index(), diff --git a/components/gfx/tests/font_context.rs b/components/gfx/tests/font_context.rs index 972fbca2afe..088d61b4d3f 100644 --- a/components/gfx/tests/font_context.rs +++ b/components/gfx/tests/font_context.rs @@ -13,11 +13,12 @@ use app_units::Au; use gfx::font::{ fallback_font_families, FontDescriptor, FontFamilyDescriptor, FontFamilyName, FontSearchScope, }; -use gfx::font_cache_thread::{FontTemplateInfo, FontTemplates}; +use gfx::font_cache_thread::{FontIdentifier, FontTemplateInfo, FontTemplates}; use gfx::font_context::{FontContext, FontContextHandle, FontSource}; use gfx::font_template::FontTemplateDescriptor; use servo_arc::Arc; use servo_atoms::Atom; +use servo_url::ServoUrl; use style::properties::longhands::font_variant_caps::computed_value::T as FontVariantCaps; use style::properties::style_structs::Font as FontStyleStruct; use style::values::computed::font::{ @@ -36,13 +37,13 @@ struct TestFontSource { impl TestFontSource { fn new() -> TestFontSource { let mut csstest_ascii = FontTemplates::default(); - Self::add_face(&mut csstest_ascii, "csstest-ascii", None); + Self::add_face(&mut csstest_ascii, "csstest-ascii"); let mut csstest_basic = FontTemplates::default(); - Self::add_face(&mut csstest_basic, "csstest-basic-regular", None); + Self::add_face(&mut csstest_basic, "csstest-basic-regular"); let mut fallback = FontTemplates::default(); - Self::add_face(&mut fallback, "csstest-basic-regular", Some("fallback")); + Self::add_face(&mut fallback, "csstest-basic-regular"); let mut families = HashMap::new(); families.insert("CSSTest ASCII".to_owned(), csstest_ascii); @@ -56,16 +57,25 @@ impl TestFontSource { } } - fn add_face(family: &mut FontTemplates, name: &str, identifier: Option<&str>) { + fn identifier_for_font_name(name: &str) -> FontIdentifier { + let mut path: PathBuf = [env!("CARGO_MANIFEST_DIR"), "tests", "support", "CSSTest"] + .iter() + .collect(); + path.push(format!("{}.ttf", name)); + FontIdentifier::Web(ServoUrl::from_file_path(path).unwrap()) + } + + fn add_face(family: &mut FontTemplates, name: &str) { let mut path: PathBuf = [env!("CARGO_MANIFEST_DIR"), "tests", "support", "CSSTest"] .iter() .collect(); path.push(format!("{}.ttf", name)); let file = File::open(path).unwrap(); - let identifier = Atom::from(identifier.unwrap_or(name)); - - family.add_template(identifier, Some(file.bytes().map(|b| b.unwrap()).collect())) + family.add_template( + Self::identifier_for_font_name(name), + Some(file.bytes().map(|b| b.unwrap()).collect()), + ) } } @@ -165,7 +175,10 @@ fn test_font_group_find_by_codepoint() { .borrow_mut() .find_by_codepoint(&mut context, 'a') .unwrap(); - assert_eq!(&*font.borrow().identifier(), "csstest-ascii"); + assert_eq!( + *font.borrow().identifier(), + TestFontSource::identifier_for_font_name("csstest-ascii") + ); assert_eq!( count.get(), 1, @@ -176,7 +189,10 @@ fn test_font_group_find_by_codepoint() { .borrow_mut() .find_by_codepoint(&mut context, 'a') .unwrap(); - assert_eq!(&*font.borrow().identifier(), "csstest-ascii"); + assert_eq!( + *font.borrow().identifier(), + TestFontSource::identifier_for_font_name("csstest-ascii") + ); assert_eq!( count.get(), 1, @@ -187,7 +203,10 @@ fn test_font_group_find_by_codepoint() { .borrow_mut() .find_by_codepoint(&mut context, 'á') .unwrap(); - assert_eq!(&*font.borrow().identifier(), "csstest-basic-regular"); + assert_eq!( + *font.borrow().identifier(), + TestFontSource::identifier_for_font_name("csstest-basic-regular") + ); assert_eq!(count.get(), 2, "both fonts should now have been loaded"); } @@ -206,8 +225,8 @@ fn test_font_fallback() { .find_by_codepoint(&mut context, 'a') .unwrap(); assert_eq!( - &*font.borrow().identifier(), - "csstest-ascii", + *font.borrow().identifier(), + TestFontSource::identifier_for_font_name("csstest-ascii"), "a family in the group should be used if there is a matching glyph" ); @@ -216,8 +235,8 @@ fn test_font_fallback() { .find_by_codepoint(&mut context, 'á') .unwrap(); assert_eq!( - &*font.borrow().identifier(), - "fallback", + *font.borrow().identifier(), + TestFontSource::identifier_for_font_name("csstest-basic-regular"), "a fallback font should be used if there is no matching glyph in the group" ); } diff --git a/components/gfx/tests/font_template.rs b/components/gfx/tests/font_template.rs index d3958ce4a2e..1c8420bc45c 100644 --- a/components/gfx/tests/font_template.rs +++ b/components/gfx/tests/font_template.rs @@ -10,9 +10,10 @@ fn test_font_template_descriptor() { use std::io::prelude::*; use std::path::PathBuf; + use gfx::font_cache_thread::FontIdentifier; use gfx::font_context::FontContextHandle; use gfx::font_template::{FontTemplate, FontTemplateDescriptor}; - use servo_atoms::Atom; + use servo_url::ServoUrl; use style::values::computed::font::{FontStretch, FontStyle, FontWeight}; fn descriptor(filename: &str) -> FontTemplateDescriptor { @@ -27,10 +28,9 @@ fn test_font_template_descriptor() { .collect(); path.push(format!("{}.ttf", filename)); - let file = File::open(path).unwrap(); - + let file = File::open(path.clone()).unwrap(); let mut template = FontTemplate::new( - Atom::from(filename), + FontIdentifier::Web(ServoUrl::from_file_path(path).unwrap()), Some(file.bytes().map(|b| b.unwrap()).collect()), ) .unwrap(); diff --git a/components/layout/text.rs b/components/layout/text.rs index 128bdfbe141..4b62deeb7a3 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -11,12 +11,12 @@ use std::sync::Arc; use app_units::Au; use gfx::font::{self, FontMetrics, FontRef, RunMetrics, ShapingFlags, ShapingOptions}; +use gfx::font_cache_thread::FontIdentifier; use gfx::text::glyph::ByteIndex; use gfx::text::text_run::TextRun; use gfx::text::util::{self, CompressionMode}; use log::{debug, warn}; use range::Range; -use servo_atoms::Atom; use style::computed_values::text_rendering::T as TextRendering; use style::computed_values::white_space::T as WhiteSpace; use style::computed_values::word_break::T as WordBreak; @@ -665,10 +665,10 @@ impl RunInfo { } fn has_font(&self, font: &Option) -> bool { - fn identifier_and_pt_size(font: &Option) -> Option<(Atom, Au)> { + fn identifier_and_pt_size(font: &Option) -> Option<(FontIdentifier, Au)> { font.as_ref().map(|font| { let font = font.borrow(); - (font.identifier(), font.descriptor.pt_size) + (font.identifier().clone(), font.descriptor.pt_size) }) }