diff --git a/components/fonts/font.rs b/components/fonts/font.rs index 67236debef1..14c55b928de 100644 --- a/components/fonts/font.rs +++ b/components/fonts/font.rs @@ -12,6 +12,7 @@ use std::{iter, str}; use app_units::Au; use bitflags::bitflags; use euclid::default::{Point2D, Rect, Size2D}; +use euclid::num::Zero; use log::debug; use malloc_size_of_derive::MallocSizeOf; use parking_lot::RwLock; @@ -137,6 +138,9 @@ pub struct FontMetrics { pub line_gap: Au, pub zero_horizontal_advance: Option, pub ic_horizontal_advance: Option, + /// The advance of the space character (' ') in this font or if there is no space, + /// the average char advance. + pub space_advance: Au, } impl FontMetrics { @@ -144,20 +148,21 @@ impl FontMetrics { /// no font can be found. pub fn empty() -> Self { Self { - underline_size: Au(0), - underline_offset: Au(0), - strikeout_size: Au(0), - strikeout_offset: Au(0), - leading: Au(0), - x_height: Au(0), - em_size: Au(0), - ascent: Au(0), - descent: Au(0), - max_advance: Au(0), - average_advance: Au(0), - line_gap: Au(0), + underline_size: Au::zero(), + underline_offset: Au::zero(), + strikeout_size: Au::zero(), + strikeout_offset: Au::zero(), + leading: Au::zero(), + x_height: Au::zero(), + em_size: Au::zero(), + ascent: Au::zero(), + descent: Au::zero(), + max_advance: Au::zero(), + average_advance: Au::zero(), + line_gap: Au::zero(), zero_horizontal_advance: None, ic_horizontal_advance: None, + space_advance: Au::zero(), } } } @@ -517,6 +522,14 @@ impl FontGroup { codepoint: char, next_codepoint: Option, ) -> Option { + // Tab characters are converted into spaces when rendering. + // TODO: We should not render a tab character. Instead they should be converted into tab stops + // based upon the width of a space character in inline formatting contexts. + let codepoint = match codepoint { + '\t' => ' ', + _ => codepoint, + }; + let options = FallbackFontSelectionOptions::new(codepoint, next_codepoint); let should_look_for_small_caps = self.descriptor.variant == font_variant_caps::T::SmallCaps && diff --git a/components/fonts/platform/freetype/font.rs b/components/fonts/platform/freetype/font.rs index 2f0fe23eb80..3a77ab2d8d9 100644 --- a/components/fonts/platform/freetype/font.rs +++ b/components/fonts/platform/freetype/font.rs @@ -351,6 +351,10 @@ impl PlatformFontMethods for PlatformFont { .glyph_index('\u{6C34}') .and_then(|idx| self.glyph_h_advance(idx)) .map(Au::from_f64_px); + let space_advance = self + .glyph_index(' ') + .and_then(|idx| self.glyph_h_advance(idx)) + .unwrap_or(average_advance); FontMetrics { underline_size: Au::from_f64_px(underline_size), @@ -367,6 +371,7 @@ impl PlatformFontMethods for PlatformFont { line_gap: Au::from_f64_px(line_height), zero_horizontal_advance, ic_horizontal_advance, + space_advance: Au::from_f64_px(space_advance), } } diff --git a/components/fonts/platform/macos/font.rs b/components/fonts/platform/macos/font.rs index 14e3382d5a2..13af8ae034e 100644 --- a/components/fonts/platform/macos/font.rs +++ b/components/fonts/platform/macos/font.rs @@ -272,11 +272,17 @@ impl PlatformFontMethods for PlatformFont { .glyph_index('0') .and_then(|idx| self.glyph_h_advance(idx)) .map(Au::from_f64_px); + let average_advance = zero_horizontal_advance.unwrap_or(max_advance); + let ic_horizontal_advance = self .glyph_index('\u{6C34}') .and_then(|idx| self.glyph_h_advance(idx)) .map(Au::from_f64_px); - let average_advance = zero_horizontal_advance.unwrap_or(max_advance); + let space_advance = self + .glyph_index(' ') + .and_then(|index| self.glyph_h_advance(index)) + .map(Au::from_f64_px) + .unwrap_or(average_advance); let metrics = FontMetrics { underline_size: Au::from_f64_au(underline_thickness), @@ -301,6 +307,7 @@ impl PlatformFontMethods for PlatformFont { line_gap: Au::from_f64_px(line_gap), zero_horizontal_advance, ic_horizontal_advance, + space_advance, }; debug!( "Font metrics (@{} pt): {:?}", diff --git a/components/fonts/platform/windows/font.rs b/components/fonts/platform/windows/font.rs index 529ac67c910..e49fa22bbff 100644 --- a/components/fonts/platform/windows/font.rs +++ b/components/fonts/platform/windows/font.rs @@ -19,6 +19,7 @@ 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::Zero; use truetype::tables::WindowsMetrics; use truetype::value::Read; use webrender_api::FontInstanceFlags; @@ -242,6 +243,17 @@ impl PlatformFontMethods for PlatformFont { .and_then(|idx| self.glyph_h_advance(idx)) .map(Au::from_f64_px); + // TODO: These should be retrieved from the OS/2 table if possible and then + // fall back to measuring 'x' if that is not available. + let average_advance = Au::zero(); + let max_advance = Au::zero(); + + let space_advance = self + .glyph_index(' ') + .and_then(|index| self.glyph_h_advance(index)) + .map(Au::from_f64_px) + .unwrap_or(average_advance); + let metrics = FontMetrics { underline_size: au_from_du(dm.underlineThickness as i32), underline_offset: au_from_du_s(dm.underlinePosition as i32), @@ -252,11 +264,12 @@ impl PlatformFontMethods for PlatformFont { em_size: au_from_em(self.em_size as f64), ascent: au_from_du_s(dm.ascent as i32), descent: au_from_du_s(dm.descent as i32), - max_advance: au_from_pt(0.0), // FIXME - average_advance: au_from_pt(0.0), // FIXME + max_advance, + average_advance, line_gap: au_from_du_s((dm.ascent + dm.descent + dm.lineGap as u16) as i32), zero_horizontal_advance, ic_horizontal_advance, + space_advance, }; debug!("Font metrics (@{} pt): {:?}", self.em_size * 12., metrics); metrics diff --git a/components/fonts/shaper.rs b/components/fonts/shaper.rs index eb8f92b789d..6b61a16183a 100644 --- a/components/fonts/shaper.rs +++ b/components/fonts/shaper.rs @@ -545,20 +545,27 @@ impl Shaper { let character = text[byte_range.clone()].chars().next().unwrap(); if is_bidi_control(character) { // Don't add any glyphs for bidi control chars - } else if character == '\t' { - // Treat tabs in pre-formatted text as a fixed number of spaces. - // - // TODO: Proper tab stops. - const TAB_COLS: i32 = 8; - let (space_glyph_id, space_advance) = glyph_space_advance(self.font); - let advance = Au::from_f64_px(space_advance) * TAB_COLS; - let data = - GlyphData::new(space_glyph_id, advance, Default::default(), true, true); - glyphs.add_glyph_for_byte_index(byte_idx, character, &data); } else { - let shape = glyph_data.entry_for_glyph(glyph_span.start, &mut y_pos); - let advance = self.advance_for_shaped_glyph(shape.advance, character, options); - let data = GlyphData::new(shape.codepoint, advance, shape.offset, true, true); + let (glyph_id, advance, offset) = if character == '\t' { + // Treat tabs in pre-formatted text as a fixed number of spaces. The glyph id does + // not matter here as Servo doesn't render any glyphs for whitespace. + // + // TODO: Proper tab stops. This should happen in layout and be based on the + // size of the space character of the inline formatting context. + let font = unsafe { &(*self.font) }; + ( + font.glyph_index(' ').unwrap_or(0) as hb_codepoint_t, + font.metrics.space_advance * 8, + Default::default(), + ) + } else { + let shape = glyph_data.entry_for_glyph(glyph_span.start, &mut y_pos); + let advance = + self.advance_for_shaped_glyph(shape.advance, character, options); + (shape.codepoint, advance, shape.offset) + }; + + let data = GlyphData::new(glyph_id, advance, offset, true, true); glyphs.add_glyph_for_byte_index(byte_idx, character, &data); } } else { @@ -708,16 +715,6 @@ extern "C" fn glyph_h_advance_func( } } -fn glyph_space_advance(font: *const Font) -> (hb_codepoint_t, f64) { - let space_unicode = ' '; - let space_glyph: hb_codepoint_t = match unsafe { (*font).glyph_index(space_unicode) } { - Some(g) => g as hb_codepoint_t, - None => panic!("No space info"), - }; - let space_advance = unsafe { (*font).glyph_h_advance(space_glyph as GlyphId) }; - (space_glyph, space_advance) -} - // Callback to get a font table out of a font. extern "C" fn font_table_func( _: *mut hb_face_t, diff --git a/tests/wpt/meta-legacy-layout/css/css-text/tab-size/tab-size-integer-005.html.ini b/tests/wpt/meta-legacy-layout/css/css-text/tab-size/tab-size-integer-005.html.ini index 50ef08178ba..ce1e35d2ee0 100644 --- a/tests/wpt/meta-legacy-layout/css/css-text/tab-size/tab-size-integer-005.html.ini +++ b/tests/wpt/meta-legacy-layout/css/css-text/tab-size/tab-size-integer-005.html.ini @@ -1,2 +1,2 @@ [tab-size-integer-005.html] - expected: CRASH + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-005.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-005.html.ini index 50ef08178ba..ce1e35d2ee0 100644 --- a/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-005.html.ini +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-005.html.ini @@ -1,2 +1,2 @@ [tab-size-integer-005.html] - expected: CRASH + expected: FAIL