servo: Merge #10493 - gfx: Clamp the font size we supply to Core Text to 0.01pt (from pcwalton:inline-font-size-zero); r=mbrubeck
authorPatrick Walton <pcwalton@mimiga.net>
Tue, 12 Apr 2016 04:10:07 +0500
changeset 338480 cc4bbd467e8dcf96cf48ac297e29a14fd3ea2b45
parent 338479 993f348880a66120f717a4cca374614f58a1a625
child 338481 f0a2cc1f08dec0c3f6e623894d796bb5cba9d545
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmbrubeck
servo: Merge #10493 - gfx: Clamp the font size we supply to Core Text to 0.01pt (from pcwalton:inline-font-size-zero); r=mbrubeck Core Text treats a font size of 0.0 as 12.0, which is obviously not what we want. Improves Twitter. Improves Reddit /r/rust. Closes #10492. r? @mbrubeck Source-Repo: https://github.com/servo/servo Source-Revision: 65120756c06ee2d0357cca6357e30694a0ec6559
servo/components/gfx/font_context.rs
servo/components/gfx/platform/macos/font.rs
servo/components/gfx/platform/macos/font_template.rs
--- a/servo/components/gfx/font_context.rs
+++ b/servo/components/gfx/font_context.rs
@@ -33,17 +33,17 @@ use webrender_traits;
 #[cfg(any(target_os = "linux", target_os = "android", target_os = "windows"))]
 fn create_scaled_font(template: &Arc<FontTemplateData>, pt_size: Au) -> ScaledFont {
     ScaledFont::new(BackendType::Skia, FontInfo::FontData(&template.bytes),
                     pt_size.to_f32_px())
 }
 
 #[cfg(target_os = "macos")]
 fn create_scaled_font(template: &Arc<FontTemplateData>, pt_size: Au) -> ScaledFont {
-    let cgfont = template.ctfont().as_ref().unwrap().copy_to_CGFont();
+    let cgfont = template.ctfont(pt_size.to_f64_px()).as_ref().unwrap().copy_to_CGFont();
     ScaledFont::new(BackendType::Skia, &cgfont, pt_size.to_f32_px())
 }
 
 static SMALL_CAPS_SCALE_FACTOR: f32 = 0.8;      // Matches FireFox (see gfxFont.h)
 
 #[derive(Debug)]
 struct LayoutFontCacheEntry {
     family: String,
--- a/servo/components/gfx/platform/macos/font.rs
+++ b/servo/components/gfx/platform/macos/font.rs
@@ -67,17 +67,17 @@ impl FontHandleMethods for FontHandle {
     fn new_from_template(_fctx: &FontContextHandle,
                          template: Arc<FontTemplateData>,
                          pt_size: Option<Au>)
                          -> Result<FontHandle, ()> {
         let size = match pt_size {
             Some(s) => s.to_f64_px(),
             None => 0.0
         };
-        match template.ctfont() {
+        match template.ctfont(size) {
             Some(ref ctfont) => {
                 Ok(FontHandle {
                     font_data: template.clone(),
                     ctfont: ctfont.clone_with_font_size(size),
                 })
             }
             None => {
                 Err(())
--- a/servo/components/gfx/platform/macos/font_template.rs
+++ b/servo/components/gfx/platform/macos/font_template.rs
@@ -1,19 +1,21 @@
 /* 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/. */
 
+use app_units::Au;
 use core_graphics::data_provider::CGDataProvider;
 use core_graphics::font::CGFont;
 use core_text;
 use core_text::font::CTFont;
 use serde::de::{Error, Visitor};
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::borrow::ToOwned;
+use std::collections::HashMap;
 use std::fs::File;
 use std::io::Read;
 use std::ops::Deref;
 use std::sync::Mutex;
 use string_cache::Atom;
 use url::Url;
 
 /// Platform specific font representation for mac.
@@ -21,66 +23,75 @@ use url::Url;
 /// CTFont object is cached here for use by the
 /// paint functions that create CGFont references.
 #[derive(Deserialize, Serialize, Debug)]
 pub struct FontTemplateData {
     /// The `CTFont` object, if present. This is cached here so that we don't have to keep creating
     /// `CTFont` instances over and over. It can always be recreated from the `identifier` and/or
     /// `font_data` fields.
     ///
-    /// When sending a `FontTemplateData` instance across processes, this will be set to `None` on
+    /// When sending a `FontTemplateData` instance across processes, this will be cleared out on
     /// 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 font_data: Option<Vec<u8>>
 }
 
 unsafe impl Send for FontTemplateData {}
 unsafe impl Sync for FontTemplateData {}
 
 impl FontTemplateData {
     pub fn new(identifier: Atom, font_data: Option<Vec<u8>>) -> FontTemplateData {
         FontTemplateData {
-            ctfont: CachedCTFont(Mutex::new(None)),
+            ctfont: CachedCTFont(Mutex::new(HashMap::new())),
             identifier: identifier.to_owned(),
             font_data: font_data
         }
     }
 
     /// Retrieves the Core Text font instance, instantiating it if necessary.
-    pub fn ctfont(&self) -> Option<CTFont> {
-        let mut ctfont = self.ctfont.lock().unwrap();
-        if ctfont.is_none() {
-            *ctfont = match self.font_data {
+    pub fn ctfont(&self, pt_size: f64) -> Option<CTFont> {
+        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 ctfont = match self.font_data {
                 Some(ref bytes) => {
                     let fontprov = CGDataProvider::from_buffer(bytes);
                     let cgfont_result = CGFont::from_data_provider(fontprov);
                     match cgfont_result {
-                        Ok(cgfont) => Some(core_text::font::new_from_CGFont(&cgfont, 0.0)),
+                        Ok(cgfont) => {
+                            Some(core_text::font::new_from_CGFont(&cgfont, clamped_pt_size))
+                        }
                         Err(_) => None
                     }
                 }
-                None => core_text::font::new_from_name(&*self.identifier, 0.0).ok(),
+                None => core_text::font::new_from_name(&*self.identifier, clamped_pt_size).ok(),
+            };
+            if let Some(ctfont) = ctfont {
+                ctfonts.insert(pt_size_key, ctfont);
             }
         }
-        ctfont.as_ref().map(|ctfont| (*ctfont).clone())
+        ctfonts.get(&pt_size_key).map(|ctfont| (*ctfont).clone())
     }
 
     /// 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<u8> {
         match self.bytes_if_in_memory() {
             Some(font_data) => return font_data,
             None => {}
         }
 
-        let path = Url::parse(&*self.ctfont()
+        let path = Url::parse(&*self.ctfont(0.0)
                                     .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();
@@ -91,26 +102,26 @@ impl FontTemplateData {
     /// 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<Vec<u8>> {
         self.font_data.clone()
     }
 
     /// Returns the native font that underlies this font template, if applicable.
     pub fn native_font(&self) -> Option<CGFont> {
-        self.ctfont().map(|ctfont| ctfont.copy_to_CGFont())
+        self.ctfont(0.0).map(|ctfont| ctfont.copy_to_CGFont())
     }
 }
 
 #[derive(Debug)]
-pub struct CachedCTFont(Mutex<Option<CTFont>>);
+pub struct CachedCTFont(Mutex<HashMap<Au, CTFont>>);
 
 impl Deref for CachedCTFont {
-    type Target = Mutex<Option<CTFont>>;
-    fn deref(&self) -> &Mutex<Option<CTFont>> {
+    type Target = Mutex<HashMap<Au, CTFont>>;
+    fn deref(&self) -> &Mutex<HashMap<Au, CTFont>> {
         &self.0
     }
 }
 
 impl Serialize for CachedCTFont {
     fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer {
         serializer.serialize_none()
     }
@@ -121,16 +132,16 @@ impl Deserialize for CachedCTFont {
                       where D: Deserializer {
         struct NoneOptionVisitor;
 
         impl Visitor for NoneOptionVisitor {
             type Value = CachedCTFont;
 
             #[inline]
             fn visit_none<E>(&mut self) -> Result<CachedCTFont, E> where E: Error {
-                Ok(CachedCTFont(Mutex::new(None)))
+                Ok(CachedCTFont(Mutex::new(HashMap::new())))
             }
         }
 
         deserializer.deserialize_option(NoneOptionVisitor)
     }
 }