fonts: Merge multiple methods into PlatformFont::descriptor() (#32115)

This combines `style()`, `boldness()`, `stretchiness()` into a
`descriptor()` method which is used when creating `FontTemplate`s for
web fonts. Eventually this method will simply read font tables using
skrifa. This is the first step.

In addition, `family_name()` and `face_name()` are removed. They were
only used for debugging and the `FontIdentifier` serves for that. On
Windows, this was adding another way in which font loading could fail,
without buying us very much. The path or URL to the font is more
important when debugging than the names in the font tables.

Closes #15103.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by
`[X]` when the step is complete, and replace `___` with appropriate
data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because they should not change
observable behavior.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox
is checked, so that we can help you if you get stuck somewhere along the
way.-->

<!-- Pull requests that do not address these steps are welcome, but they
will require additional verification as part of the review process. -->

components/gfx/font.rs

@@ -18,7 +18,7 @@ use log::debug;
 use serde::{Deserialize, Serialize};
 use servo_atoms::{atom, Atom};
 use smallvec::SmallVec;
-use style::computed_values::{font_stretch, font_style, font_variant_caps, font_weight};
+use style::computed_values::font_variant_caps;
 use style::properties::style_structs::Font as FontStyleStruct;
 use style::values::computed::font::{GenericFontFamily, SingleFontFamily};
 use unicode_script::Script;
@@ -70,12 +70,9 @@ pub trait PlatformFontMethods: Sized {
         pt_size: Option<Au>,
     ) -> Result<PlatformFont, &'static str>;
 
-    fn family_name(&self) -> Option<String>;
+    /// Get a [`FontTemplateDescriptor`] from a [`PlatformFont`]. This is used to get
-    fn face_name(&self) -> Option<String>;
+    /// descriptors for web fonts.
-
+    fn descriptor(&self) -> FontTemplateDescriptor;
-    fn style(&self) -> font_style::T;
-    fn boldness(&self) -> font_weight::T;
-    fn stretchiness(&self) -> font_stretch::T;
 
     fn glyph_index(&self, codepoint: char) -> Option<GlyphId>;
     fn glyph_h_advance(&self, _: GlyphId) -> Option<FractionalPixel>;
@@ -349,15 +346,11 @@ impl Font {
         };
 
         debug!(
-            "{} font table[{}] with family={}, face={}",
+            "{} font table[{}] in {:?},",
             status,
             tag.tag_to_str(),
-            self.handle
+            self.identifier()
-                .family_name()
-                .unwrap_or("unavailable".to_owned()),
-            self.handle.face_name().unwrap_or("unavailable".to_owned())
         );
-
         result
     }
 

components/gfx/font_cache_thread.rs

@@ -260,12 +260,7 @@ impl FontCache {
                         return;
                     };
 
-                    let descriptor = FontTemplateDescriptor::new(
+                    let descriptor = handle.descriptor();
-                        handle.boldness(),
-                        handle.stretchiness(),
-                        handle.style(),
-                    );
-
                     templates.add_template(FontTemplate::new_web_font(url, descriptor, data));
                     drop(result.send(()));
                 },

components/gfx/font_template.rs

@@ -31,6 +31,16 @@ pub struct FontTemplateDescriptor {
     pub style: FontStyle,
 }
 
+impl Default for FontTemplateDescriptor {
+    fn default() -> Self {
+        FontTemplateDescriptor {
+            weight: FontWeight::normal(),
+            stretch: FontStretch::NORMAL,
+            style: FontStyle::NORMAL,
+        }
+    }
+}
+
 /// FontTemplateDescriptor contains floats, which are not Eq because of NaN. However,
 /// we know they will never be NaN, so we can manually implement Eq.
 impl Eq for FontTemplateDescriptor {}

components/gfx/platform/freetype/font.rs

@@ -3,16 +3,16 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::convert::TryInto;
-use std::os::raw::{c_char, c_long};
+use std::os::raw::c_long;
 use std::sync::Arc;
 use std::{mem, ptr};
 
 use app_units::Au;
 use freetype::freetype::{
     FT_Done_Face, FT_F26Dot6, FT_Face, FT_FaceRec, FT_Get_Char_Index, FT_Get_Kerning,
-    FT_Get_Postscript_Name, FT_Get_Sfnt_Table, FT_GlyphSlot, FT_Int32, FT_Kerning_Mode,
+    FT_Get_Sfnt_Table, FT_GlyphSlot, FT_Int32, FT_Kerning_Mode, FT_Load_Glyph, FT_Load_Sfnt_Table,
-    FT_Load_Glyph, FT_Load_Sfnt_Table, FT_Long, FT_New_Memory_Face, FT_Set_Char_Size, FT_Sfnt_Tag,
+    FT_Long, FT_New_Memory_Face, FT_Set_Char_Size, FT_Sfnt_Tag, FT_SizeRec, FT_Size_Metrics,
-    FT_SizeRec, FT_Size_Metrics, FT_UInt, FT_ULong, FT_Vector, FT_STYLE_FLAG_ITALIC,
+    FT_UInt, FT_ULong, FT_Vector, FT_STYLE_FLAG_ITALIC,
 };
 use freetype::succeeded;
 use freetype::tt_os2::TT_OS2;
@@ -21,13 +21,13 @@ use style::computed_values::font_stretch::T as FontStretch;
 use style::computed_values::font_weight::T as FontWeight;
 use style::values::computed::font::FontStyle;
 
-use super::c_str_to_string;
 use super::library_handle::FreeTypeLibraryHandle;
 use crate::font::{
     FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, PlatformFontMethods, GPOS, GSUB,
     KERN,
 };
 use crate::font_cache_thread::FontIdentifier;
+use crate::font_template::FontTemplateDescriptor;
 use crate::text::glyph::GlyphId;
 use crate::text::util::fixed_to_float;
 
@@ -145,65 +145,39 @@ impl PlatformFontMethods for PlatformFont {
         Ok(handle)
     }
 
-    fn family_name(&self) -> Option<String> {
+    fn descriptor(&self) -> FontTemplateDescriptor {
-        unsafe {
+        let style = if unsafe { (*self.face).style_flags & FT_STYLE_FLAG_ITALIC as c_long != 0 } {
-            let family_name = (*self.face).family_name;
-            if !family_name.is_null() {
-                Some(c_str_to_string(family_name as *const c_char))
-            } else {
-                None
-            }
-        }
-    }
-
-    fn face_name(&self) -> Option<String> {
-        unsafe {
-            let name = FT_Get_Postscript_Name(self.face) as *const c_char;
-
-            if !name.is_null() {
-                Some(c_str_to_string(name))
-            } else {
-                None
-            }
-        }
-    }
-
-    fn style(&self) -> FontStyle {
-        if unsafe { (*self.face).style_flags & FT_STYLE_FLAG_ITALIC as c_long != 0 } {
             FontStyle::ITALIC
         } else {
             FontStyle::NORMAL
-        }
-    }
-
-    fn boldness(&self) -> FontWeight {
-        let os2 = match self.os2_table() {
-            None => return FontWeight::normal(),
-            Some(os2) => os2,
         };
-        let weight = os2.us_weight_class as f32;
-        FontWeight::from_float(weight)
-    }
 
-    fn stretchiness(&self) -> FontStretch {
+        let os2_table = self.os2_table();
-        use style::values::specified::font::FontStretchKeyword;
+        let weight = os2_table
-        if let Some(os2) = self.os2_table() {
+            .as_ref()
-            match os2.us_width_class {
+            .map(|os2| FontWeight::from_float(os2.us_weight_class as f32))
-                1 => FontStretchKeyword::UltraCondensed,
+            .unwrap_or_else(FontWeight::normal);
-                2 => FontStretchKeyword::ExtraCondensed,
+        let stretch = os2_table
-                3 => FontStretchKeyword::Condensed,
+            .as_ref()
-                4 => FontStretchKeyword::SemiCondensed,
+            .map(|os2| match os2.us_width_class {
-                5 => FontStretchKeyword::Normal,
+                1 => FontStretch::ULTRA_CONDENSED,
-                6 => FontStretchKeyword::SemiExpanded,
+                2 => FontStretch::EXTRA_CONDENSED,
-                7 => FontStretchKeyword::Expanded,
+                3 => FontStretch::CONDENSED,
-                8 => FontStretchKeyword::ExtraExpanded,
+                4 => FontStretch::SEMI_CONDENSED,
-                9 => FontStretchKeyword::UltraExpanded,
+                5 => FontStretch::NORMAL,
-                _ => FontStretchKeyword::Normal,
+                6 => FontStretch::SEMI_EXPANDED,
+                7 => FontStretch::EXPANDED,
+                8 => FontStretch::EXTRA_EXPANDED,
+                9 => FontStretch::ULTRA_EXPANDED,
+                _ => FontStretch::NORMAL,
+            })
+            .unwrap_or(FontStretch::NORMAL);
+
+        FontTemplateDescriptor {
+            weight,
+            stretch,
+            style,
         }
-        } else {
-            FontStretchKeyword::Normal
-        }
-        .compute()
     }
 
     fn glyph_index(&self, codepoint: char) -> Option<GlyphId> {

components/gfx/platform/macos/font.rs

@@ -26,6 +26,7 @@ use crate::font::{
     FractionalPixel, PlatformFontMethods, GPOS, GSUB, KERN,
 };
 use crate::font_cache_thread::FontIdentifier;
+use crate::font_template::FontTemplateDescriptor;
 use crate::text::glyph::GlyphId;
 
 const KERN_PAIR_LEN: usize = 6;
@@ -187,24 +188,13 @@ impl PlatformFontMethods for PlatformFont {
         Ok(handle)
     }
 
-    fn family_name(&self) -> Option<String> {
+    fn descriptor(&self) -> FontTemplateDescriptor {
-        Some(self.ctfont.family_name())
+        let traits = self.ctfont.all_traits();
+        FontTemplateDescriptor {
+            weight: traits.weight(),
+            stretch: traits.stretch(),
+            style: traits.style(),
         }
-
-    fn face_name(&self) -> Option<String> {
-        Some(self.ctfont.face_name())
-    }
-
-    fn style(&self) -> FontStyle {
-        self.ctfont.all_traits().style()
-    }
-
-    fn boldness(&self) -> FontWeight {
-        self.ctfont.all_traits().weight()
-    }
-
-    fn stretchiness(&self) -> FontStretch {
-        self.ctfont.all_traits().stretch()
     }
 
     fn glyph_index(&self, codepoint: char) -> Option<GlyphId> {

components/gfx/platform/windows/font.rs

@@ -6,22 +6,26 @@
 // information for an approach that we'll likely need to take when the
 // renderer moves to a sandboxed process.
 
+use std::cmp::{max, min};
 use std::fmt;
+use std::io::Cursor;
 use std::ops::Deref;
 use std::sync::Arc;
 
 use app_units::Au;
 use dwrote::{FontFace, FontFile};
-use log::debug;
+use log::{debug, warn};
 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;
-use style::values::specified::font::FontStretchKeyword;
+use truetype::tables::WindowsMetrics;
+use truetype::value::Read;
 
 use crate::font::{
     FontMetrics, FontTableMethods, FontTableTag, FractionalPixel, PlatformFontMethods,
 };
 use crate::font_cache_thread::FontIdentifier;
+use crate::font_template::FontTemplateDescriptor;
 use crate::text::glyph::GlyphId;
 
 // 1em = 12pt = 16px, assuming 72 points per inch and 96 px per inch
@@ -56,23 +60,6 @@ impl FontTableMethods for FontTable {
     }
 }
 
-// We need the font (DWriteFont) in order to be able to query things like
-// the family name, face name, weight, etc.  On Windows 10, the
-// DWriteFontFace3 interface provides this on the FontFace, but that's only
-// available on Win10+.
-//
-// Instead, we do the parsing work using the truetype crate for raw fonts.
-// We're just extracting basic info, so this is sufficient for now.
-
-#[derive(Debug)]
-struct FontInfo {
-    family_name: String,
-    face_name: String,
-    weight: StyleFontWeight,
-    stretch: StyleFontStretch,
-    style: StyleFontStyle,
-}
-
 #[macro_export]
 /// Packs the components of a font tag name into 32 bytes, while respecting the
 /// necessary Rust 4-byte alignment for pointers. This is similar to
@@ -83,104 +70,12 @@ macro_rules! font_tag {
     };
 }
 
-impl FontInfo {
-    fn new_from_face(face: &FontFace) -> Result<FontInfo, &'static str> {
-        use std::cmp::{max, min};
-        use std::collections::HashMap;
-        use std::io::Cursor;
-
-        use truetype::tables::names::{NameID, Names};
-        use truetype::tables::WindowsMetrics;
-        use truetype::value::Read;
-
-        let name_tag = font_tag!('n', 'a', 'm', 'e');
-        let os2_tag = font_tag!('O', 'S', '/', '2');
-        let names_bytes = face.get_font_table(name_tag);
-        let windows_metrics_bytes = face.get_font_table(os2_tag);
-        if names_bytes.is_none() || windows_metrics_bytes.is_none() {
-            return Err("No 'name' or 'OS/2' tables");
-        }
-
-        let mut cursor = Cursor::new(names_bytes.as_ref().unwrap());
-        let table = Names::read(&mut cursor).map_err(|_| "Could not read 'name' table")?;
-        let language_tags = table.language_tags().collect::<Vec<_>>();
-        let mut names = table
-            .iter()
-            .filter(|((_, _, language_id, _), value)| {
-                value.is_some() &&
-                    language_id
-                        .tag(&language_tags)
-                        .map_or(false, |tag| tag.starts_with("en"))
-            })
-            .map(|((_, _, _, name_id), value)| (name_id, value.unwrap()))
-            .collect::<HashMap<_, _>>();
-        let family = match names.remove(&NameID::FontFamilyName) {
-            Some(family) => family,
-            _ => return Err("Could not find family"),
-        };
-        let face = match names.remove(&NameID::FontSubfamilyName) {
-            Some(face) => face,
-            _ => return Err("Could not find subfamily"),
-        };
-
-        let mut cursor = Cursor::new(windows_metrics_bytes.as_ref().unwrap());
-        let table = WindowsMetrics::read(&mut cursor).map_err(|_| "Could not read OS/2 table")?;
-        let (weight_val, width_val, italic_bool) = match table {
-            WindowsMetrics::Version0(ref m) => {
-                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
-            },
-            WindowsMetrics::Version1(ref m) => {
-                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
-            },
-            WindowsMetrics::Version2(ref m) |
-            WindowsMetrics::Version3(ref m) |
-            WindowsMetrics::Version4(ref m) => {
-                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
-            },
-            WindowsMetrics::Version5(ref m) => {
-                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
-            },
-        };
-
-        let weight = StyleFontWeight::from_float(weight_val as f32);
-
-        let stretch = match min(9, max(1, width_val)) {
-            1 => FontStretchKeyword::UltraCondensed,
-            2 => FontStretchKeyword::ExtraCondensed,
-            3 => FontStretchKeyword::Condensed,
-            4 => FontStretchKeyword::SemiCondensed,
-            5 => FontStretchKeyword::Normal,
-            6 => FontStretchKeyword::SemiExpanded,
-            7 => FontStretchKeyword::Expanded,
-            8 => FontStretchKeyword::ExtraExpanded,
-            9 => FontStretchKeyword::UltraExpanded,
-            _ => return Err("Unknown stretch size"),
-        }
-        .compute();
-
-        let style = if italic_bool {
-            StyleFontStyle::ITALIC
-        } else {
-            StyleFontStyle::NORMAL
-        };
-
-        Ok(FontInfo {
-            family_name: family,
-            face_name: face,
-            weight,
-            stretch,
-            style,
-        })
-    }
-}
-
 #[derive(Debug)]
 pub struct PlatformFont {
     face: Nondebug<FontFace>,
     /// A reference to this data used to create this [`PlatformFont`], ensuring the
     /// data stays alive of the lifetime of this struct.
     _data: Arc<Vec<u8>>,
-    info: FontInfo,
     em_size: f32,
     du_to_px: f32,
     scaled_du_to_px: f32,
@@ -212,7 +107,6 @@ impl PlatformFontMethods for PlatformFont {
         let face = font_file
             .create_face(face_index, dwrote::DWRITE_FONT_SIMULATIONS_NONE)
             .map_err(|_| "Could not create FontFace")?;
-        let info = FontInfo::new_from_face(&face)?;
 
         let pt_size = pt_size.unwrap_or(au_from_pt(12.));
         let du_per_em = face.metrics().metrics0().designUnitsPerEm as f32;
@@ -226,31 +120,77 @@ impl PlatformFontMethods for PlatformFont {
         Ok(PlatformFont {
             face: Nondebug(face),
             _data: data,
-            info,
             em_size,
             du_to_px: design_units_to_pixels,
             scaled_du_to_px: scaled_design_units_to_pixels,
         })
     }
 
-    fn family_name(&self) -> Option<String> {
+    fn descriptor(&self) -> FontTemplateDescriptor {
-        Some(self.info.family_name.clone())
+        // We need the font (DWriteFont) in order to be able to query things like
+        // the family name, face name, weight, etc.  On Windows 10, the
+        // DWriteFontFace3 interface provides this on the FontFace, but that's only
+        // available on Win10+.
+        //
+        // Instead, we do the parsing work using the truetype crate for raw fonts.
+        // We're just extracting basic info, so this is sufficient for now.
+        let windows_metrics_bytes = self.face.get_font_table(font_tag!('O', 'S', '/', '2'));
+        if windows_metrics_bytes.is_none() {
+            warn!("Could not find OS/2 table in font.");
+            return FontTemplateDescriptor::default();
         }
 
-    fn face_name(&self) -> Option<String> {
+        let mut cursor = Cursor::new(windows_metrics_bytes.as_ref().unwrap());
-        Some(self.info.face_name.clone())
+        let Ok(table) = WindowsMetrics::read(&mut cursor) else {
-    }
+            warn!("Could not read OS/2 table in font.");
+            return FontTemplateDescriptor::default();
+        };
 
-    fn style(&self) -> StyleFontStyle {
+        let (weight_val, width_val, italic_bool) = match table {
-        self.info.style
+            WindowsMetrics::Version0(ref m) => {
-    }
+                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
+            },
+            WindowsMetrics::Version1(ref m) => {
+                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
+            },
+            WindowsMetrics::Version2(ref m) |
+            WindowsMetrics::Version3(ref m) |
+            WindowsMetrics::Version4(ref m) => {
+                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
+            },
+            WindowsMetrics::Version5(ref m) => {
+                (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1)
+            },
+        };
 
-    fn boldness(&self) -> StyleFontWeight {
+        let weight = StyleFontWeight::from_float(weight_val as f32);
-        self.info.weight
+        let stretch = match min(9, max(1, width_val)) {
-    }
+            1 => StyleFontStretch::ULTRA_CONDENSED,
+            2 => StyleFontStretch::EXTRA_CONDENSED,
+            3 => StyleFontStretch::CONDENSED,
+            4 => StyleFontStretch::SEMI_CONDENSED,
+            5 => StyleFontStretch::NORMAL,
+            6 => StyleFontStretch::SEMI_EXPANDED,
+            7 => StyleFontStretch::EXPANDED,
+            8 => StyleFontStretch::EXTRA_EXPANDED,
+            9 => StyleFontStretch::ULTRA_CONDENSED,
+            _ => {
+                warn!("Unknown stretch size.");
+                StyleFontStretch::NORMAL
+            },
+        };
 
-    fn stretchiness(&self) -> StyleFontStretch {
+        let style = if italic_bool {
-        self.info.stretch
+            StyleFontStyle::ITALIC
+        } else {
+            StyleFontStyle::NORMAL
+        };
+
+        FontTemplateDescriptor {
+            weight,
+            stretch,
+            style,
+        }
     }
 
     fn glyph_index(&self, codepoint: char) -> Option<GlyphId> {

components/gfx/tests/font_context.rs

@@ -86,11 +86,9 @@ impl TestFontSource {
             None,
         )
         .unwrap();
-        let descriptor =
-            FontTemplateDescriptor::new(handle.boldness(), handle.stretchiness(), handle.style());
         family.add_template(FontTemplate::new_web_font(
             Self::url_for_font_name(name),
-            descriptor,
+            handle.descriptor(),
             data,
         ));
     }

components/gfx/tests/font_template.rs

@@ -34,7 +34,7 @@ fn test_font_template_descriptor() {
         let file = File::open(path.clone()).unwrap();
         let data = file.bytes().map(|b| b.unwrap()).collect();
         let handle = PlatformFont::new_from_data(identifier, Arc::new(data), 0, None).unwrap();
-        FontTemplateDescriptor::new(handle.boldness(), handle.stretchiness(), handle.style())
+        handle.descriptor()
     }
 
     assert_eq!(