From 18fbf8cf3047806bc8bc5080fecb189d1bfddf2b Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 8 Apr 2016 17:37:22 -0700 Subject: [PATCH] gfx: Clamp the font size we supply to Core Text to 0.01pt. 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. --- components/gfx/font_context.rs | 2 +- components/gfx/platform/macos/font.rs | 2 +- .../gfx/platform/macos/font_template.rs | 41 ++++++++++++------- tests/wpt/mozilla/meta/MANIFEST.json | 24 +++++++++++ .../tests/css/inline_font_size_zero_a.html | 9 ++++ .../tests/css/inline_font_size_zero_ref.html | 9 ++++ 6 files changed, 70 insertions(+), 17 deletions(-) create mode 100644 tests/wpt/mozilla/tests/css/inline_font_size_zero_a.html create mode 100644 tests/wpt/mozilla/tests/css/inline_font_size_zero_ref.html diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs index 7cb95e6fc54..ff7715e2cb7 100644 --- a/components/gfx/font_context.rs +++ b/components/gfx/font_context.rs @@ -38,7 +38,7 @@ fn create_scaled_font(template: &Arc, pt_size: Au) -> ScaledFo #[cfg(target_os = "macos")] fn create_scaled_font(template: &Arc, 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()) } diff --git a/components/gfx/platform/macos/font.rs b/components/gfx/platform/macos/font.rs index a0667245cbe..abdb620ba36 100644 --- a/components/gfx/platform/macos/font.rs +++ b/components/gfx/platform/macos/font.rs @@ -72,7 +72,7 @@ impl FontHandleMethods for FontHandle { 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(), diff --git a/components/gfx/platform/macos/font_template.rs b/components/gfx/platform/macos/font_template.rs index 3099bbd28ed..a36b49eebe3 100644 --- a/components/gfx/platform/macos/font_template.rs +++ b/components/gfx/platform/macos/font_template.rs @@ -2,6 +2,7 @@ * 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; @@ -9,6 +10,7 @@ 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; @@ -26,7 +28,7 @@ pub struct FontTemplateData { /// `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, @@ -41,29 +43,38 @@ unsafe impl Sync for FontTemplateData {} impl FontTemplateData { pub fn new(identifier: Atom, font_data: Option>) -> 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 { - let mut ctfont = self.ctfont.lock().unwrap(); - if ctfont.is_none() { - *ctfont = match self.font_data { + 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 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 @@ -75,7 +86,7 @@ impl FontTemplateData { 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!") @@ -96,16 +107,16 @@ impl FontTemplateData { /// Returns the native font that underlies this font template, if applicable. pub fn native_font(&self) -> Option { - self.ctfont().map(|ctfont| ctfont.copy_to_CGFont()) + self.ctfont(0.0).map(|ctfont| ctfont.copy_to_CGFont()) } } #[derive(Debug)] -pub struct CachedCTFont(Mutex>); +pub struct CachedCTFont(Mutex>); impl Deref for CachedCTFont { - type Target = Mutex>; - fn deref(&self) -> &Mutex> { + type Target = Mutex>; + fn deref(&self) -> &Mutex> { &self.0 } } @@ -126,7 +137,7 @@ impl Deserialize for CachedCTFont { #[inline] fn visit_none(&mut self) -> Result where E: Error { - Ok(CachedCTFont(Mutex::new(None))) + Ok(CachedCTFont(Mutex::new(HashMap::new()))) } } diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 5b6b0a286d8..897a29b823d 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -2423,6 +2423,18 @@ "url": "/_mozilla/css/inline_element_border_a.html" } ], + "css/inline_font_size_zero_a.html": [ + { + "path": "css/inline_font_size_zero_a.html", + "references": [ + [ + "/_mozilla/css/inline_font_size_zero_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/inline_font_size_zero_a.html" + } + ], "css/inline_hypothetical_box_a.html": [ { "path": "css/inline_hypothetical_box_a.html", @@ -8923,6 +8935,18 @@ "url": "/_mozilla/css/inline_element_border_a.html" } ], + "css/inline_font_size_zero_a.html": [ + { + "path": "css/inline_font_size_zero_a.html", + "references": [ + [ + "/_mozilla/css/inline_font_size_zero_ref.html", + "==" + ] + ], + "url": "/_mozilla/css/inline_font_size_zero_a.html" + } + ], "css/inline_hypothetical_box_a.html": [ { "path": "css/inline_hypothetical_box_a.html", diff --git a/tests/wpt/mozilla/tests/css/inline_font_size_zero_a.html b/tests/wpt/mozilla/tests/css/inline_font_size_zero_a.html new file mode 100644 index 00000000000..aface0759c0 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/inline_font_size_zero_a.html @@ -0,0 +1,9 @@ + + + + +
+
+
+
+ diff --git a/tests/wpt/mozilla/tests/css/inline_font_size_zero_ref.html b/tests/wpt/mozilla/tests/css/inline_font_size_zero_ref.html new file mode 100644 index 00000000000..c370413deee --- /dev/null +++ b/tests/wpt/mozilla/tests/css/inline_font_size_zero_ref.html @@ -0,0 +1,9 @@ + + + +
+
+
+ +