From 479c6c9c424e8e36ecd8c1458b2818f14ea229df Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 9 May 2016 15:59:10 -0700 Subject: [PATCH 1/4] gfx: Perform fuzzy matching on font weights if an exact match wasn't found. Partially addresses #190. Partially addresses #9487. --- components/gfx/font_cache_thread.rs | 23 +++++-- components/gfx/font_template.rs | 101 +++++++++++++++++++--------- 2 files changed, 86 insertions(+), 38 deletions(-) diff --git a/components/gfx/font_cache_thread.rs b/components/gfx/font_cache_thread.rs index 72027689e2f..5a98e717548 100644 --- a/components/gfx/font_cache_thread.rs +++ b/components/gfx/font_cache_thread.rs @@ -18,6 +18,7 @@ use std::borrow::ToOwned; use std::collections::HashMap; use std::mem; use std::sync::{Arc, Mutex}; +use std::u32; use string_cache::Atom; use style::font_face::Source; use style::properties::longhands::font_family::computed_value::FontFamily; @@ -51,9 +52,6 @@ impl FontTemplates { // TODO(Issue #189): optimize lookup for // regular/bold/italic/bolditalic with fixed offsets and a // static decision table for fallback between these values. - - // TODO(Issue #190): if not in the fast path above, do - // expensive matching of weights, etc. for template in &mut self.templates { let maybe_template = template.data_for_descriptor(fctx, desc); if maybe_template.is_some() { @@ -61,6 +59,22 @@ impl FontTemplates { } } + // We didn't find an exact match. Do more expensive fuzzy matching. + // TODO(#190): Do a better job. + let (mut best_template_data, mut best_distance) = (None, u32::MAX); + for template in &mut self.templates { + if let Some((template_data, distance)) = + template.data_for_approximate_descriptor(fctx, desc) { + if distance < best_distance { + best_template_data = Some(template_data); + best_distance = distance + } + } + } + if best_template_data.is_some() { + return best_template_data + } + // If a request is made for a font family that exists, // pick the first valid font in the family if we failed // to find an exact match for the descriptor. @@ -81,8 +95,7 @@ impl FontTemplates { } } - let template = FontTemplate::new(identifier, - maybe_data); + let template = FontTemplate::new(identifier, maybe_data); self.templates.push(template); } } diff --git a/components/gfx/font_template.rs b/components/gfx/font_template.rs index 0b4cd6900ef..c05230a9930 100644 --- a/components/gfx/font_template.rs +++ b/components/gfx/font_template.rs @@ -7,6 +7,7 @@ use platform::font::FontHandle; use platform::font_context::FontContextHandle; use platform::font_template::FontTemplateData; use std::sync::{Arc, Weak}; +use std::u32; use string_cache::Atom; use style::computed_values::{font_stretch, font_weight}; @@ -31,13 +32,25 @@ impl FontTemplateDescriptor { italic: italic, } } + + /// Returns a score indicating how far apart visually the two font descriptors are. This is + /// used for fuzzy font selection. + /// + /// The smaller the score, the better the fonts match. 0 indicates an exact match. This must + /// be commutative (distance(A, B) == distance(B, A)). + #[inline] + fn distance_from(&self, other: &FontTemplateDescriptor) -> u32 { + if self.stretch != other.stretch || self.italic != other.italic { + // A value higher than all weights. + return 1000 + } + ((self.weight as i16) - (other.weight as i16)).abs() as u32 + } } impl PartialEq for FontTemplateDescriptor { fn eq(&self, other: &FontTemplateDescriptor) -> bool { - self.weight.is_bold() == other.weight.is_bold() && - self.stretch == other.stretch && - self.italic == other.italic + self.weight == other.weight && self.stretch == other.stretch && self.italic == other.italic } } @@ -88,52 +101,74 @@ impl FontTemplate { /// 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> { + fctx: &FontContextHandle, + requested_desc: &FontTemplateDescriptor) + -> Option> { // 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 { + Some(actual_desc) if *requested_desc == actual_desc => Some(self.data()), + Some(_) => None, + None => { + if self.instantiate(fctx).is_err() { + return None + } + + if self.descriptor + .as_ref() + .expect("Instantiation succeeded but no descriptor?") == requested_desc { Some(self.data()) } else { None } - }, - None if self.is_valid => { - let data = self.data(); - let handle: Result = - FontHandleMethods::new_from_template(fctx, data.clone(), None); - match handle { - Ok(handle) => { - let actual_desc = FontTemplateDescriptor::new(handle.boldness(), - handle.stretchiness(), - handle.is_italic()); - let desc_match = actual_desc == *requested_desc; + } + } + } - self.descriptor = Some(actual_desc); - self.is_valid = true; - if desc_match { - Some(data) - } else { - None - } - } - Err(()) => { - self.is_valid = false; - debug!("Unable to create a font from template {}", self.identifier); - None - } + /// Returns the font data along with the distance between this font's descriptor and the given + /// descriptor, if the font can be loaded. + pub fn data_for_approximate_descriptor(&mut self, + font_context: &FontContextHandle, + requested_descriptor: &FontTemplateDescriptor) + -> Option<(Arc, u32)> { + match self.descriptor { + Some(actual_descriptor) => { + Some((self.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); + Some((self.data(), distance)) + } else { + None } } - None => None, } } + fn instantiate(&mut self, font_context: &FontContextHandle) -> Result<(), ()> { + if !self.is_valid { + return Err(()) + } + + let data = self.data(); + let handle: Result = FontHandleMethods::new_from_template(font_context, + data, + None); + self.is_valid = handle.is_ok(); + let handle = try!(handle); + self.descriptor = Some(FontTemplateDescriptor::new(handle.boldness(), + handle.stretchiness(), + handle.is_italic())); + Ok(()) + } + /// Get the data for creating a font. pub fn get(&mut self) -> Option> { if self.is_valid { From 3addd775a5765c1d0c95794fca654bad56f4ca4c Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 9 May 2016 16:00:16 -0700 Subject: [PATCH 2/4] gfx: Change the mapping from Mac weights to CSS weights so that 0.0 maps to 400, not 500. CSS `normal` font-weight is specified as 400, while Mac "Regular" font weight is reported as 0.0. On the Mac, we need to center the two ranges on the same value to avoid choosing "Light" fonts where "Regular" would have been more appropriate. Closes #9487. fix for mac --- components/gfx/platform/macos/font.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/components/gfx/platform/macos/font.rs b/components/gfx/platform/macos/font.rs index abdb620ba36..c6be7c16d78 100644 --- a/components/gfx/platform/macos/font.rs +++ b/components/gfx/platform/macos/font.rs @@ -103,16 +103,20 @@ impl FontHandleMethods for FontHandle { fn boldness(&self) -> font_weight::T { let normalized = self.ctfont.all_traits().normalized_weight(); // [-1.0, 1.0] - let normalized = (normalized + 1.0) / 2.0 * 9.0; // [0.0, 9.0] - match normalized { - v if v < 1.0 => font_weight::T::Weight100, - v if v < 2.0 => font_weight::T::Weight200, - v if v < 3.0 => font_weight::T::Weight300, - v if v < 4.0 => font_weight::T::Weight400, - v if v < 5.0 => font_weight::T::Weight500, - v if v < 6.0 => font_weight::T::Weight600, - v if v < 7.0 => font_weight::T::Weight700, - v if v < 8.0 => font_weight::T::Weight800, + let normalized = if normalized <= 0.0 { + 4.0 + normalized * 3.0 // [1.0, 4.0] + } else { + 4.0 + normalized * 5.0 // [4.0, 9.0] + }; // [1.0, 9.0], centered on 4.0 + match normalized.round() as u32 { + 1 => font_weight::T::Weight100, + 2 => font_weight::T::Weight200, + 3 => font_weight::T::Weight300, + 4 => font_weight::T::Weight400, + 5 => font_weight::T::Weight500, + 6 => font_weight::T::Weight600, + 7 => font_weight::T::Weight700, + 8 => font_weight::T::Weight800, _ => font_weight::T::Weight900, } } From 376c6b34ce55aa3c55c1599aac3567702366e892 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 9 May 2016 16:03:23 -0700 Subject: [PATCH 3/4] gfx: Implement `Debug` for `FontTemplate` instances. --- components/gfx/font_template.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/gfx/font_template.rs b/components/gfx/font_template.rs index c05230a9930..1d6b271deaa 100644 --- a/components/gfx/font_template.rs +++ b/components/gfx/font_template.rs @@ -6,6 +6,7 @@ use font::FontHandleMethods; use platform::font::FontHandle; use platform::font_context::FontContextHandle; use platform::font_template::FontTemplateData; +use std::fmt::{Debug, Error, Formatter}; use std::sync::{Arc, Weak}; use std::u32; use string_cache::Atom; @@ -66,6 +67,12 @@ pub struct FontTemplate { is_valid: bool, } +impl Debug for FontTemplate { + fn fmt(&self, f: &mut Formatter) -> Result<(), Error> { + self.identifier.fmt(f) + } +} + /// Holds all of the template information for a font that /// is common, regardless of the number of instances of /// this font handle per thread. From 6d0e4de2c21fc7e0e946af3b21d6823bbd6cbd76 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 9 May 2016 21:59:48 -0700 Subject: [PATCH 4/4] tests: Update test expectations. --- .../css-text-3_dev/html/word-break-break-all-007.htm.ini | 5 ----- .../css-text-3_dev/html/word-break-normal-hi-000.htm.ini | 4 +++- 2 files changed, 3 insertions(+), 6 deletions(-) delete mode 100644 tests/wpt/metadata-css/css-text-3_dev/html/word-break-break-all-007.htm.ini diff --git a/tests/wpt/metadata-css/css-text-3_dev/html/word-break-break-all-007.htm.ini b/tests/wpt/metadata-css/css-text-3_dev/html/word-break-break-all-007.htm.ini deleted file mode 100644 index 0b32b94f7db..00000000000 --- a/tests/wpt/metadata-css/css-text-3_dev/html/word-break-break-all-007.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[word-break-break-all-007.htm] - type: reftest - expected: - if os == "linux": PASS - FAIL diff --git a/tests/wpt/metadata-css/css-text-3_dev/html/word-break-normal-hi-000.htm.ini b/tests/wpt/metadata-css/css-text-3_dev/html/word-break-normal-hi-000.htm.ini index f325fa6949a..d7bb9064224 100644 --- a/tests/wpt/metadata-css/css-text-3_dev/html/word-break-normal-hi-000.htm.ini +++ b/tests/wpt/metadata-css/css-text-3_dev/html/word-break-normal-hi-000.htm.ini @@ -1,3 +1,5 @@ [word-break-normal-hi-000.htm] type: reftest - expected: FAIL + expected: + if os == "linux": FAIL + PASS