Fix FontTemplateDescriptor under FreeType

Issue #17321. Under Linux, using "font-family: sans-serif" previously
caused Servo to select the "UltraLight" face (of DejaVu Sans). There
were two reasons for this:

1. Font weight was only retrieved from the OS/2 table for bold faces.
   This neglected to retrieve the weight information for "lighter than
   normal" weight faces. This meant that the UltraLight face appeared as
   normal weight, and was selected.

2. Retrieval of font stretch information from the OS/2 table was not
   implemented at all.

components/gfx/font_context.rs

@@ -9,7 +9,7 @@ use font_cache_thread::FontCacheThread;
 use font_template::FontTemplateDescriptor;
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use platform::font::FontHandle;
-use platform::font_context::FontContextHandle;
+pub use platform::font_context::FontContextHandle;
 use platform::font_template::FontTemplateData;
 use servo_arc::Arc as ServoArc;
 use smallvec::SmallVec;

components/gfx/font_template.rs

@@ -107,33 +107,35 @@ impl FontTemplate {
         &self.identifier
     }
 
-    /// Get the data for creating a font if it matches a given descriptor.
-    pub fn data_for_descriptor(&mut self,
-                               fctx: &FontContextHandle,
-                               requested_desc: &FontTemplateDescriptor)
-                               -> Option<Arc<FontTemplateData>> {
+    /// Get the descriptor. Returns `None` when instantiating the data fails.
+    pub fn descriptor(&mut self, font_context: &FontContextHandle) -> Option<FontTemplateDescriptor> {
         // The font template data can be unloaded when nothing is referencing
         // it (via the Weak reference to the Arc above). However, if we have
         // already loaded a font, store the style information about it separately,
         // so that we can do font matching against it again in the future
         // without having to reload the font (unless it is an actual match).
-        match self.descriptor {
-            Some(actual_desc) if *requested_desc == actual_desc => self.data().ok(),
-            Some(_) => None,
-            None => {
-                if self.instantiate(fctx).is_err() {
-                    return None
-                }
-
-                if self.descriptor
-                       .as_ref()
-                       .expect("Instantiation succeeded but no descriptor?") == requested_desc {
-                    self.data().ok()
-                } else {
-                    None
-                }
+
+        self.descriptor.or_else(|| {
+            if self.instantiate(font_context).is_err() {
+                return None
+            };
+
+            Some(self.descriptor.expect("Instantiation succeeded but no descriptor?"))
+        })
+    }
+
+    /// Get the data for creating a font if it matches a given descriptor.
+    pub fn data_for_descriptor(&mut self,
+                               fctx: &FontContextHandle,
+                               requested_desc: &FontTemplateDescriptor)
+                               -> Option<Arc<FontTemplateData>> {
+        self.descriptor(&fctx).and_then(|descriptor| {
+            if *requested_desc == descriptor {
+                self.data().ok()
+            } else {
+                None
             }
-        }
+        })
     }
 
     /// Returns the font data along with the distance between this font's descriptor and the given
@@ -142,24 +144,11 @@ impl FontTemplate {
                                            font_context: &FontContextHandle,
                                            requested_descriptor: &FontTemplateDescriptor)
                                            -> Option<(Arc<FontTemplateData>, u32)> {
-        match self.descriptor {
-            Some(actual_descriptor) => {
-                self.data().ok().map(|data| {
-                    (data, actual_descriptor.distance_from(requested_descriptor))
-                })
-            }
-            None => {
-                if self.instantiate(font_context).is_ok() {
-                    let distance = self.descriptor
-                                       .as_ref()
-                                       .expect("Instantiation successful but no descriptor?")
-                                       .distance_from(requested_descriptor);
-                    self.data().ok().map(|data| (data, distance))
-                } else {
-                    None
-                }
-            }
-        }
+        self.descriptor(&font_context).and_then(|descriptor| {
+            self.data().ok().map(|data| {
+                (data, descriptor.distance_from(requested_descriptor))
+            })
+        })
     }
 
     fn instantiate(&mut self, font_context: &FontContextHandle) -> Result<(), ()> {

components/gfx/platform/freetype/font.rs

@@ -10,7 +10,7 @@ use freetype::freetype::{FT_F26Dot6, FT_Face, FT_FaceRec};
 use freetype::freetype::{FT_Get_Char_Index, FT_Get_Postscript_Name};
 use freetype::freetype::{FT_Get_Kerning, FT_Get_Sfnt_Table, FT_Load_Sfnt_Table};
 use freetype::freetype::{FT_GlyphSlot, FT_Library, FT_Long, FT_ULong};
-use freetype::freetype::{FT_Int32, FT_Kerning_Mode, FT_STYLE_FLAG_BOLD, FT_STYLE_FLAG_ITALIC};
+use freetype::freetype::{FT_Int32, FT_Kerning_Mode, FT_STYLE_FLAG_ITALIC};
 use freetype::freetype::{FT_Load_Glyph, FT_Set_Char_Size};
 use freetype::freetype::{FT_SizeRec, FT_Size_Metrics, FT_UInt, FT_Vector};
 use freetype::freetype::FT_Sfnt_Tag;
@@ -52,6 +52,17 @@ impl FontTableMethods for FontTable {
     }
 }
 
+/// Data from the OS/2 table of an OpenType font.
+/// See https://www.microsoft.com/typography/otspec/os2.htm
+#[derive(Debug)]
+struct OS2Table {
+    us_weight_class: u16,
+    us_width_class: u16,
+    y_strikeout_size: i16,
+    y_strikeout_position: i16,
+    sx_height: i16,
+}
+
 #[derive(Debug)]
 pub struct FontHandle {
     // The font binary. This must stay valid for the lifetime of the font,
@@ -141,32 +152,38 @@ impl FontHandleMethods for FontHandle {
     }
 
     fn boldness(&self) -> FontWeight {
-        let default_weight = FontWeight::normal();
-        if unsafe { (*self.face).style_flags & FT_STYLE_FLAG_BOLD as c_long == 0 } {
-            default_weight
-        } else {
-            unsafe {
-                let os2 = FT_Get_Sfnt_Table(self.face, FT_Sfnt_Tag::FT_SFNT_OS2) as *mut TT_OS2;
-                let valid = !os2.is_null() && (*os2).version != 0xffff;
-                if valid {
-                    let weight =(*os2).usWeightClass as i32;
-                    if weight < 10 {
-                        FontWeight::from_int(weight * 100).unwrap()
-                    } else if weight >= 100 && weight < 1000 {
-                        FontWeight::from_int(weight / 100 * 100).unwrap()
-                    } else {
-                        default_weight
-                    }
-                } else {
-                    default_weight
-                }
+        if let Some(os2) = self.os2_table() {
+            let weight = os2.us_weight_class as i32;
+
+            if weight < 10 {
+                FontWeight::from_int(weight * 100).unwrap()
+            } else if weight >= 100 && weight < 1000 {
+                FontWeight::from_int(weight / 100 * 100).unwrap()
+            } else {
+                FontWeight::normal()
             }
+        } else {
+            FontWeight::normal()
         }
     }
 
     fn stretchiness(&self) -> FontStretch {
-        // TODO(pcwalton): Implement this.
-        FontStretch::Normal
+        if let Some(os2) = self.os2_table() {
+            match os2.us_width_class {
+                1 => FontStretch::UltraCondensed,
+                2 => FontStretch::ExtraCondensed,
+                3 => FontStretch::Condensed,
+                4 => FontStretch::SemiCondensed,
+                5 => FontStretch::Normal,
+                6 => FontStretch::SemiExpanded,
+                7 => FontStretch::Expanded,
+                8 => FontStretch::ExtraExpanded,
+                9 => FontStretch::UltraExpanded,
+                _ => FontStretch::Normal
+            }
+        } else {
+            FontStretch::Normal
+        }
     }
 
     fn glyph_index(&self, codepoint: char) -> Option<GlyphId> {
@@ -242,14 +259,11 @@ impl FontHandleMethods for FontHandle {
         let mut strikeout_size = Au(0);
         let mut strikeout_offset = Au(0);
         let mut x_height = Au(0);
-        unsafe {
-            let os2 = FT_Get_Sfnt_Table(face, FT_Sfnt_Tag::FT_SFNT_OS2) as *mut TT_OS2;
-            let valid = !os2.is_null() && (*os2).version != 0xffff;
-            if valid {
-               strikeout_size = self.font_units_to_au((*os2).yStrikeoutSize as f64);
-               strikeout_offset = self.font_units_to_au((*os2).yStrikeoutPosition as f64);
-               x_height = self.font_units_to_au((*os2).sxHeight as f64);
-            }
+
+        if let Some(os2) = self.os2_table() {
+            strikeout_size = self.font_units_to_au(os2.y_strikeout_size as f64);
+            strikeout_offset = self.font_units_to_au(os2.y_strikeout_position as f64);
+            x_height = self.font_units_to_au(os2.sx_height as f64);
         }
 
         let average_advance = self.glyph_index('0')
@@ -332,4 +346,23 @@ impl<'a> FontHandle {
 
         Au::from_f64_px(value * x_scale)
     }
+
+    fn os2_table(&self) -> Option<OS2Table> {
+        unsafe {
+            let os2 = FT_Get_Sfnt_Table(self.face_rec_mut(), FT_Sfnt_Tag::FT_SFNT_OS2) as *mut TT_OS2;
+            let valid = !os2.is_null() && (*os2).version != 0xffff;
+
+            if !valid {
+                return None
+            }
+
+            Some(OS2Table {
+                us_weight_class: (*os2).usWeightClass,
+                us_width_class: (*os2).usWidthClass,
+                y_strikeout_size: (*os2).yStrikeoutSize,
+                y_strikeout_position: (*os2).yStrikeoutPosition,
+                sx_height: (*os2).sxHeight,
+            })
+        }
+    }
 }

components/gfx/tests/font_template.rs

@@ -0,0 +1,67 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#[cfg(not(target_os = "macos"))] extern crate gfx;
+#[cfg(not(target_os = "macos"))] extern crate servo_atoms;
+#[cfg(not(target_os = "macos"))] extern crate style;
+
+// Test doesn't yet run on Mac, see https://github.com/servo/servo/pull/19928 for explanation.
+#[cfg(not(target_os = "macos"))]
+#[test]
+fn test_font_template_descriptor() {
+    use gfx::font_context::FontContextHandle;
+    use gfx::font_template::{FontTemplate, FontTemplateDescriptor};
+    use servo_atoms::Atom;
+    use std::fs::File;
+    use std::io::prelude::*;
+    use std::path::PathBuf;
+    use style::computed_values::font_stretch::T as FontStretch;
+    use style::values::computed::font::FontWeight;
+
+    fn descriptor(filename: &str) -> FontTemplateDescriptor {
+        let mut path: PathBuf = [
+            env!("CARGO_MANIFEST_DIR"),
+            "tests",
+            "support",
+            "dejavu-fonts-ttf-2.37",
+            "ttf",
+        ].iter().collect();
+        path.push(format!("{}.ttf", filename));
+
+        let file = File::open(path).unwrap();
+
+        let mut template = FontTemplate::new(
+            Atom::from(filename),
+            Some(file.bytes().map(|b| b.unwrap()).collect())
+        ).unwrap();
+
+        let context = FontContextHandle::new();
+
+        template.descriptor(&context).unwrap()
+    }
+
+    assert_eq!(descriptor("DejaVuSans"), FontTemplateDescriptor {
+        weight: FontWeight::normal(),
+        stretch: FontStretch::Normal,
+        italic: false,
+    });
+
+    assert_eq!(descriptor("DejaVuSans-Bold"), FontTemplateDescriptor {
+        weight: FontWeight::bold(),
+        stretch: FontStretch::Normal,
+        italic: false,
+    });
+
+    assert_eq!(descriptor("DejaVuSans-Oblique"), FontTemplateDescriptor {
+        weight: FontWeight::normal(),
+        stretch: FontStretch::Normal,
+        italic: true,
+    });
+
+    assert_eq!(descriptor("DejaVuSansCondensed-BoldOblique"), FontTemplateDescriptor {
+        weight: FontWeight::bold(),
+        stretch: FontStretch::SemiCondensed,
+        italic: true,
+    });
+}

components/gfx/tests/support/dejavu-fonts-ttf-2.37/AUTHORS

@@ -0,0 +1,57 @@
+abysta at yandex.ru
+Adrian Schroeter
+Aleksey Chalabyan
+Andrey Valentinovich Panov
+Ben Laenen
+Besarion Gugushvili
+Bhikkhu Pesala
+Clayborne Arevalo
+Dafydd Harries
+Danilo Segan
+Davide Viti
+David Jez
+David Lawrence Ramsey
+Denis Jacquerye
+Dwayne Bailey
+Eugeniy Meshcheryakov
+Frédéric Wang
+Gee Fung Sit
+Heikki Lindroos
+James Cloos
+James Crippen
+John Karp
+Keenan Pepper
+Lars Næsbye Christensen
+Lior Halphon
+MaEr
+Mashrab Kuvatov
+Max Berger
+Mederic Boquien
+Michael Everson
+MihailJP
+Misu Moldovan
+Nguyen Thai Ngoc Duy
+Nicolas Mailhot
+Norayr Chilingarian
+Olleg Samoylov
+Ognyan Kulev
+Ondrej Koala Vacha
+Peter Cernak
+Remy Oudompheng
+Roozbeh Pournader
+Rouben Hakobian
+Sahak Petrosyan
+Sami Tarazi
+Sander Vesik
+Stepan Roh
+Stephen Hartke
+Steve Tinney
+Tavmjong Bah
+Thomas Henlich
+Tim May
+Valentin Stoykov
+Vasek Stodulka
+Wesley Transue
+Yoshiki Ohshima
+
+$Id$

components/gfx/tests/support/dejavu-fonts-ttf-2.37/BUGS

@@ -0,0 +1,3 @@
+See http://dejavu.sourceforge.net/wiki/index.php/Bugs
+
+$Id$