Change line-height to be read from the font itself, rather than

a hard-coded estimate.

6 files changed, 39 insertions, 30 deletions

diff --git a/src/components/gfx/font.rs b/src/components/gfx/font.rs
index 0ee461b4418..74930da0b4a 100644
--- a/src/components/gfx/font.rs
+++ b/src/components/gfx/font.rs
@@ -77,7 +77,8 @@ pub struct FontMetrics {
     pub em_size:          Au,
     pub ascent:           Au,
     pub descent:          Au,
-    pub max_advance:      Au
+    pub max_advance:      Au,
+    pub line_gap:         Au,
 }
 
 // TODO(Issue #179): eventually this will be split into the specified
diff --git a/src/components/gfx/platform/freetype/font.rs b/src/components/gfx/platform/freetype/font.rs
index 6488ceeac65..7e58b850e2b 100644
--- a/src/components/gfx/platform/freetype/font.rs
+++ b/src/components/gfx/platform/freetype/font.rs
@@ -245,7 +245,8 @@ impl FontHandleMethods for FontHandle {
             em_size:          em_size,
             ascent:           ascent,
             descent:          -descent, // linux font's seem to use the opposite sign from mac
-            max_advance:      max_advance
+            max_advance:      max_advance,
+            line_gap:         height,
         };
 
         debug!("Font metrics (@{:f} pt): {:?}", geometry::to_pt(em_size), metrics);
diff --git a/src/components/gfx/platform/macos/font.rs b/src/components/gfx/platform/macos/font.rs
index 682c44088d2..cabadcea2b8 100644
--- a/src/components/gfx/platform/macos/font.rs
+++ b/src/components/gfx/platform/macos/font.rs
@@ -145,11 +145,13 @@ impl FontHandleMethods for FontHandle {
 
     fn get_metrics(&self) -> FontMetrics {
         let bounding_rect: CGRect = self.ctfont.bounding_box();
-        let ascent = Au::from_pt(self.ctfont.ascent() as f64);
-        let descent = Au::from_pt(self.ctfont.descent() as f64);
+        let ascent = self.ctfont.ascent() as f64;
+        let descent = self.ctfont.descent() as f64;
         let em_size = Au::from_frac_px(self.ctfont.pt_size() as f64);
+        let leading = self.ctfont.leading() as f64;
 
         let scale = px_to_pt(self.ctfont.pt_size() as f64) / (self.ctfont.ascent() as f64 + self.ctfont.descent() as f64);
+        let line_gap = (ascent + descent + leading + 0.5).floor();
 
         let metrics =  FontMetrics {
             underline_size:   Au::from_pt(self.ctfont.underline_thickness() as f64),
@@ -161,12 +163,13 @@ impl FontHandleMethods for FontHandle {
             underline_offset: Au::from_pt(self.ctfont.underline_position() as f64),
             strikeout_size:   geometry::from_pt(0.0), // FIXME(Issue #942)
             strikeout_offset: geometry::from_pt(0.0), // FIXME(Issue #942)
-            leading:          Au::from_pt(self.ctfont.leading() as f64),
+            leading:          Au::from_pt(leading),
             x_height:         Au::from_pt(self.ctfont.x_height() as f64),
             em_size:          em_size,
-            ascent:           ascent.scale_by(scale),
-            descent:          descent.scale_by(scale),
-            max_advance:      Au::from_pt(bounding_rect.size.width as f64)
+            ascent:           Au::from_pt(ascent).scale_by(scale),
+            descent:          Au::from_pt(descent).scale_by(scale),
+            max_advance:      Au::from_pt(bounding_rect.size.width as f64),
+            line_gap:         Au::from_frac_px(line_gap),
         };
 
         debug!("Font metrics (@{:f} pt): {:?}", self.ctfont.pt_size() as f64, metrics);
diff --git a/src/components/layout/fragment.rs b/src/components/layout/fragment.rs
index 66ccb6b2d86..7f61517a328 100644
--- a/src/components/layout/fragment.rs
+++ b/src/components/layout/fragment.rs
@@ -455,8 +455,10 @@ impl Fragment {
         }
     }
 
-    pub fn calculate_line_height(&self) -> Au {
-        text::line_height_from_style(self.style())
+    pub fn calculate_line_height(&self, layout_context: &LayoutContext) -> Au {
+        let font_style = text::computed_style_to_font_style(&*self.style);
+        let font_metrics = text::font_metrics_for_style(layout_context.font_context(), &font_style);
+        text::line_height_from_style(&*self.style, &font_metrics)
     }
 
     /// Returns the sum of the inline-sizes of all the borders of this fragment. This is private because
@@ -1082,7 +1084,7 @@ impl Fragment {
     }
 
     /// Returns, and computes, the block-size of this fragment.
-    pub fn content_block_size(&self) -> Au {
+    pub fn content_block_size(&self, layout_context: &LayoutContext) -> Au {
         match self.specific {
             GenericFragment | IframeFragment(_) | TableFragment | TableCellFragment | TableRowFragment |
             TableWrapperFragment => Au(0),
@@ -1091,7 +1093,7 @@ impl Fragment {
             }
             ScannedTextFragment(_) => {
                 // Compute the block-size based on the line-block-size and font size.
-                self.calculate_line_height()
+                self.calculate_line_height(layout_context)
             }
             TableColumnFragment(_) => fail!("Table column fragments do not have block_size"),
             UnscannedTextFragment(_) => fail!("Unscanned text fragments should have been scanned by now!"),
@@ -1370,7 +1372,7 @@ impl Fragment {
 
     /// Calculates block-size above baseline, depth below baseline, and ascent for this fragment when
     /// used in an inline formatting context. See CSS 2.1 § 10.8.1.
-    pub fn inline_metrics(&self) -> InlineMetrics {
+    pub fn inline_metrics(&self, layout_context: &LayoutContext) -> InlineMetrics {
         match self.specific {
             ImageFragment(ref image_fragment_info) => {
                 let computed_block_size = image_fragment_info.computed_block_size();
@@ -1382,7 +1384,7 @@ impl Fragment {
             }
             ScannedTextFragment(ref text_fragment) => {
                 // See CSS 2.1 § 10.8.1.
-                let line_height = self.calculate_line_height();
+                let line_height = self.calculate_line_height(layout_context);
                 InlineMetrics::from_font_metrics(&text_fragment.run.font_metrics, line_height)
             }
             _ => {
diff --git a/src/components/layout/inline.rs b/src/components/layout/inline.rs
index 89449ca5571..1a427f79c0b 100644
--- a/src/components/layout/inline.rs
+++ b/src/components/layout/inline.rs
@@ -296,7 +296,7 @@ impl LineBreaker {
         self.pending_line.green_zone = LogicalSize::zero(self.floats.writing_mode)
     }
 
-    pub fn scan_for_lines(&mut self, flow: &mut InlineFlow) {
+    pub fn scan_for_lines(&mut self, flow: &mut InlineFlow, layout_context: &LayoutContext) {
         self.reset_scanner();
 
         let mut old_fragments = mem::replace(&mut flow.fragments, InlineFragments::new());
@@ -322,7 +322,7 @@ impl LineBreaker {
                 };
 
                 let fragment_was_appended = match cur_fragment.white_space() {
-                    white_space::normal => self.try_append_to_line(cur_fragment, flow),
+                    white_space::normal => self.try_append_to_line(cur_fragment, flow, layout_context),
                     white_space::pre => self.try_append_to_line_by_new_line(cur_fragment),
                 };
 
@@ -360,8 +360,8 @@ impl LineBreaker {
 
     // FIXME(eatkinson): this assumes that the tallest fragment in the line determines the line block-size
     // This might not be the case with some weird text fonts.
-    fn new_block_size_for_line(&self, new_fragment: &Fragment) -> Au {
-        let fragment_block_size = new_fragment.content_block_size();
+    fn new_block_size_for_line(&self, new_fragment: &Fragment, layout_context: &LayoutContext) -> Au {
+        let fragment_block_size = new_fragment.content_block_size(layout_context);
         if fragment_block_size > self.pending_line.bounds.size.block {
             fragment_block_size
         } else {
@@ -507,7 +507,7 @@ impl LineBreaker {
 
     /// Tries to append the given fragment to the line, splitting it if necessary. Returns false only if
     /// we should break the line.
-    fn try_append_to_line(&mut self, in_fragment: Fragment, flow: &InlineFlow) -> bool {
+    fn try_append_to_line(&mut self, in_fragment: Fragment, flow: &InlineFlow, layout_context: &LayoutContext) -> bool {
         let line_is_empty = self.pending_line.range.length() == num::zero();
         if line_is_empty {
             let (line_bounds, _) = self.initial_line_placement(&in_fragment, self.cur_b, flow);
@@ -528,7 +528,7 @@ impl LineBreaker {
         // `green_zone.block-size < self.pending_line.bounds.size.block-size`, then we committed a line
         // that overlaps with floats.
 
-        let new_block_size = self.new_block_size_for_line(&in_fragment);
+        let new_block_size = self.new_block_size_for_line(&in_fragment, layout_context);
         if new_block_size > green_zone.block {
             // Uh-oh. Float collision imminent. Enter the float collision avoider
             return self.avoid_floats(in_fragment, flow, new_block_size, line_is_empty)
@@ -971,14 +971,15 @@ impl InlineFlow {
                               block_size_above_baseline: &mut Au,
                               depth_below_baseline: &mut Au,
                               largest_block_size_for_top_fragments: &mut Au,
-                              largest_block_size_for_bottom_fragments: &mut Au)
+                              largest_block_size_for_bottom_fragments: &mut Au,
+                              layout_context: &LayoutContext)
                               -> (Au, bool) {
         match fragment.vertical_align() {
             vertical_align::baseline => (-ascent, false),
             vertical_align::middle => {
                 // TODO: x-block-size value should be used from font info.
                 let xblock_size = Au(0);
-                let fragment_block_size = fragment.content_block_size();
+                let fragment_block_size = fragment.content_block_size(layout_context);
                 let offset_block_start = -(xblock_size + fragment_block_size).scale_by(0.5);
                 *block_size_above_baseline = offset_block_start.scale_by(-1.0);
                 *depth_below_baseline = fragment_block_size - *block_size_above_baseline;
@@ -1026,7 +1027,7 @@ impl InlineFlow {
             },
             vertical_align::Length(length) => (-(length + ascent), false),
             vertical_align::Percentage(p) => {
-                let line_height = fragment.calculate_line_height();
+                let line_height = fragment.calculate_line_height(layout_context);
                 let percent_offset = line_height.scale_by(p);
                 (-(percent_offset + ascent), false)
             }
@@ -1071,7 +1072,7 @@ impl InlineFlow {
                                               style: &ComputedValues) -> (Au, Au) {
         let font_style = text::computed_style_to_font_style(style);
         let font_metrics = text::font_metrics_for_style(font_context, &font_style);
-        let line_height = text::line_height_from_style(style);
+        let line_height = text::line_height_from_style(style, &font_metrics);
         let inline_metrics = InlineMetrics::from_font_metrics(&font_metrics, line_height);
         (inline_metrics.block_size_above_baseline, inline_metrics.depth_below_baseline)
     }
@@ -1144,7 +1145,7 @@ impl Flow for InlineFlow {
     }
 
     /// Calculate and set the block-size of this flow. See CSS 2.1 § 10.6.1.
-    fn assign_block_size(&mut self, _: &LayoutContext) {
+    fn assign_block_size(&mut self, ctx: &LayoutContext) {
         debug!("assign_block_size_inline: assigning block_size for flow");
 
         // Divide the fragments into lines.
@@ -1165,7 +1166,7 @@ impl Flow for InlineFlow {
 
         let scanner_floats = self.base.floats.clone();
         let mut scanner = LineBreaker::new(scanner_floats);
-        scanner.scan_for_lines(self);
+        scanner.scan_for_lines(self, ctx);
 
         // All lines use text alignment of the flow.
         let text_align = self.base.flags.text_align();
@@ -1196,7 +1197,7 @@ impl Flow for InlineFlow {
                     block_size_above_baseline: mut block_size_above_baseline,
                     depth_below_baseline: mut depth_below_baseline,
                     ascent
-                } = fragment.inline_metrics();
+                } = fragment.inline_metrics(ctx);
 
                 // To calculate text-top and text-bottom value when `vertical-align` is involved,
                 // we should find the top and bottom of the content area of the parent fragment.
@@ -1231,7 +1232,8 @@ impl Flow for InlineFlow {
                         &mut block_size_above_baseline,
                         &mut depth_below_baseline,
                         &mut largest_block_size_for_top_fragments,
-                        &mut largest_block_size_for_bottom_fragments);
+                        &mut largest_block_size_for_bottom_fragments,
+                        ctx);
 
                 // Unless the current fragment has `vertical-align` set to `top` or `bottom`,
                 // `largest_block-size_above_baseline` and `largest_depth_below_baseline` are updated.
diff --git a/src/components/layout/text.rs b/src/components/layout/text.rs
index 40ce9193af3..8aa162fd624 100644
--- a/src/components/layout/text.rs
+++ b/src/components/layout/text.rs
@@ -293,10 +293,10 @@ pub fn computed_style_to_font_style(style: &ComputedValues) -> FontStyle {
 }
 
 /// Returns the line block-size needed by the given computed style and font size.
-pub fn line_height_from_style(style: &ComputedValues) -> Au {
+pub fn line_height_from_style(style: &ComputedValues, metrics: &FontMetrics) -> Au {
     let font_size = style.get_font().font_size;
     let from_inline = match style.get_inheritedbox().line_height {
-        line_height::Normal => font_size.scale_by(1.14),
+        line_height::Normal => metrics.line_gap,
         line_height::Number(l) => font_size.scale_by(l),
         line_height::Length(l) => l
     };