Auto merge of #5588 - pcwalton:line-height-float-placement-redux, r=glennw

Basically, the easiest way to describe the effect of this change is "float placement takes line height into account". It didn't before, which could lead to lines overlapping floats or floats taking up too much vertical space.

Improves Wikipedia.

r? @glennw

components/layout/floats.rs

@@ -333,7 +333,9 @@ impl Floats {
             let maybe_location = self.available_rect(float_b,
                                                      info.size.block,
                                                      info.max_inline_size);
-            debug!("place_float: Got available rect: {:?} for y-pos: {:?}", maybe_location, float_b);
+            debug!("place_float: got available rect: {:?} for block-pos: {:?}",
+                   maybe_location,
+                   float_b);
             match maybe_location {
                 // If there are no floats blocking us, return the current location
                 // TODO(eatkinson): integrate with overflow

components/layout/fragment.rs

@@ -1268,37 +1268,6 @@ impl Fragment {
         }
     }
 
-    /// Returns, and computes, the block-size of this fragment.
-    pub fn content_block_size(&self, layout_context: &LayoutContext) -> Au {
-        match self.specific {
-            SpecificFragmentInfo::Generic |
-            SpecificFragmentInfo::GeneratedContent(_) |
-            SpecificFragmentInfo::Iframe(_) |
-            SpecificFragmentInfo::Table |
-            SpecificFragmentInfo::TableCell |
-            SpecificFragmentInfo::TableRow |
-            SpecificFragmentInfo::TableWrapper |
-            SpecificFragmentInfo::InlineBlock(_) |
-            SpecificFragmentInfo::InlineAbsoluteHypothetical(_) => Au(0),
-            SpecificFragmentInfo::Image(ref image_fragment_info) => {
-                image_fragment_info.replaced_image_fragment_info.computed_block_size()
-            }
-            SpecificFragmentInfo::Canvas(ref canvas_fragment_info) => {
-                canvas_fragment_info.replaced_image_fragment_info.computed_block_size()
-            }
-            SpecificFragmentInfo::ScannedText(_) => {
-                // Compute the block-size based on the line-block-size and font size.
-                self.calculate_line_height(layout_context)
-            }
-            SpecificFragmentInfo::TableColumn(_) => {
-                panic!("Table column fragments do not have block size")
-            }
-            SpecificFragmentInfo::UnscannedText(_) => {
-                panic!("Unscanned text fragments should have been scanned by now!")
-            }
-        }
-    }
-
     /// Returns the dimensions of the content box.
     ///
     /// This is marked `#[inline]` because it is frequently called when only one or two of the

components/layout/inline.rs

@@ -148,6 +148,9 @@ pub struct Line {
     /// FFF float
     /// ~~~
     pub green_zone: LogicalSize<Au>,
+
+    /// The inline metrics for this line.
+    pub inline_metrics: InlineMetrics,
 }
 
 int_range_index! {
@@ -181,11 +184,21 @@ struct LineBreaker {
     cur_b: Au,
     /// The computed value of the indentation for the first line (`text-indent`, CSS 2.1 § 16.1).
     first_line_indentation: Au,
+    /// The minimum block-size above the baseline for each line, as specified by the line height
+    /// and font style.
+    minimum_block_size_above_baseline: Au,
+    /// The minimum depth below the baseline for each line, as specified by the line height and
+    /// font style.
+    minimum_depth_below_baseline: Au,
 }
 
 impl LineBreaker {
     /// Creates a new `LineBreaker` with a set of floats and the indentation of the first line.
-    fn new(float_context: Floats, first_line_indentation: Au) -> LineBreaker {
+    fn new(float_context: Floats,
+           first_line_indentation: Au,
+           minimum_block_size_above_baseline: Au,
+           minimum_depth_below_baseline: Au)
+           -> LineBreaker {
         LineBreaker {
             new_fragments: Vec::new(),
             work_list: VecDeque::new(),
@@ -193,11 +206,16 @@ impl LineBreaker {
                 range: Range::empty(),
                 bounds: LogicalRect::zero(float_context.writing_mode),
                 green_zone: LogicalSize::zero(float_context.writing_mode),
+                inline_metrics: InlineMetrics::new(minimum_block_size_above_baseline,
+                                                   minimum_depth_below_baseline,
+                                                   minimum_block_size_above_baseline),
             },
             floats: float_context,
             lines: Vec::new(),
             cur_b: Au(0),
             first_line_indentation: first_line_indentation,
+            minimum_block_size_above_baseline: minimum_block_size_above_baseline,
+            minimum_depth_below_baseline: minimum_depth_below_baseline,
         }
     }
 
@@ -218,6 +236,10 @@ impl LineBreaker {
                                                     Au(0),
                                                     Au(0));
         self.pending_line.green_zone = LogicalSize::zero(self.floats.writing_mode);
+        self.pending_line.inline_metrics =
+            InlineMetrics::new(self.minimum_block_size_above_baseline,
+                               self.minimum_depth_below_baseline,
+                               self.minimum_block_size_above_baseline)
     }
 
     /// Reflows fragments for the given inline flow.
@@ -370,14 +392,15 @@ 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_inline_metrics_for_line(&self, new_fragment: &Fragment, layout_context: &LayoutContext)
+                                   -> InlineMetrics {
+        self.pending_line.inline_metrics.max(&new_fragment.inline_metrics(layout_context))
+    }
+
     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 {
-            self.pending_line.bounds.size.block
-        }
+        Au::max(self.pending_line.bounds.size.block,
+                self.new_inline_metrics_for_line(new_fragment, layout_context).block_size())
     }
 
     /// Computes the position of a line that has only the provided fragment. Returns the bounding
@@ -616,7 +639,7 @@ impl LineBreaker {
         }
 
         if !need_ellipsis {
-            self.push_fragment_to_line_ignoring_text_overflow(fragment);
+            self.push_fragment_to_line_ignoring_text_overflow(fragment, layout_context);
         } else {
             let ellipsis = fragment.transform_into_ellipsis(layout_context);
             if let Some(truncation_info) =
@@ -624,9 +647,9 @@ impl LineBreaker {
                                                      ellipsis.border_box.size.inline) {
                 let fragment = fragment.transform_with_split_info(&truncation_info.split,
                                                                   truncation_info.text_run);
-                self.push_fragment_to_line_ignoring_text_overflow(fragment);
+                self.push_fragment_to_line_ignoring_text_overflow(fragment, layout_context);
             }
-            self.push_fragment_to_line_ignoring_text_overflow(ellipsis);
+            self.push_fragment_to_line_ignoring_text_overflow(ellipsis, layout_context);
         }
 
         if line_flush_mode == LineFlushMode::Flush {
@@ -636,14 +659,18 @@ impl LineBreaker {
 
     /// Pushes a fragment to the current line unconditionally, without placing an ellipsis in the
     /// case of `text-overflow: ellipsis`.
-    fn push_fragment_to_line_ignoring_text_overflow(&mut self, fragment: Fragment) {
+    fn push_fragment_to_line_ignoring_text_overflow(&mut self,
+                                                    fragment: Fragment,
+                                                    layout_context: &LayoutContext) {
         let indentation = self.indentation_for_pending_fragment();
         self.pending_line.range.extend_by(FragmentIndex(1));
         self.pending_line.bounds.size.inline = self.pending_line.bounds.size.inline +
             fragment.border_box.size.inline +
             indentation;
-        self.pending_line.bounds.size.block = max(self.pending_line.bounds.size.block,
-                                                  fragment.border_box.size.block);
+        self.pending_line.inline_metrics =
+            self.new_inline_metrics_for_line(&fragment, layout_context);
+        self.pending_line.bounds.size.block =
+            self.new_block_size_for_line(&fragment, layout_context);
         self.new_fragments.push(fragment);
     }
 
@@ -730,11 +757,11 @@ pub struct InlineFlow {
     /// lines.
     pub lines: Vec<Line>,
 
-    /// The minimum block-size above the baseline for each line, as specified by the line block-
-    /// size and font style.
+    /// The minimum block-size above the baseline for each line, as specified by the line height
+    /// and font style.
     pub minimum_block_size_above_baseline: Au,
 
-    /// The minimum depth below the baseline for each line, as specified by the line block-size and
+    /// The minimum depth below the baseline for each line, as specified by the line height and
     /// font style.
     pub minimum_depth_below_baseline: Au,
 
@@ -1148,7 +1175,10 @@ impl Flow for InlineFlow {
         };
 
         // Perform line breaking.
-        let mut scanner = LineBreaker::new(self.base.floats.clone(), indentation);
+        let mut scanner = LineBreaker::new(self.base.floats.clone(),
+                                           indentation,
+                                           self.minimum_block_size_above_baseline,
+                                           self.minimum_depth_below_baseline);
         scanner.scan_for_lines(self, layout_context);
 
         // Now, go through each line and lay out the fragments inside.
@@ -1417,6 +1447,7 @@ fn inline_contexts_are_equal(inline_context_a: &Option<InlineFragmentContext>,
 
 /// Block-size above the baseline, depth below the baseline, and ascent for a fragment. See CSS 2.1
 /// § 10.8.1.
+#[derive(Clone, Copy, Debug, RustcEncodable)]
 pub struct InlineMetrics {
     pub block_size_above_baseline: Au,
     pub depth_below_baseline: Au,
@@ -1424,6 +1455,16 @@ pub struct InlineMetrics {
 }
 
 impl InlineMetrics {
+    /// Creates a new set of inline metrics.
+    pub fn new(block_size_above_baseline: Au, depth_below_baseline: Au, ascent: Au)
+               -> InlineMetrics {
+        InlineMetrics {
+            block_size_above_baseline: block_size_above_baseline,
+            depth_below_baseline: depth_below_baseline,
+            ascent: ascent,
+        }
+    }
+
     /// Calculates inline metrics from font metrics and line block-size per CSS 2.1 § 10.8.1.
     #[inline]
     pub fn from_font_metrics(font_metrics: &FontMetrics, line_height: Au) -> InlineMetrics {
@@ -1445,6 +1486,19 @@ impl InlineMetrics {
             ascent: font_metrics.ascent + leading.scale_by(0.5),
         }
     }
+
+    pub fn block_size(&self) -> Au {
+        self.block_size_above_baseline + self.depth_below_baseline
+    }
+
+    pub fn max(&self, other: &InlineMetrics) -> InlineMetrics {
+        InlineMetrics {
+            block_size_above_baseline: Au::max(self.block_size_above_baseline,
+                                               other.block_size_above_baseline),
+            depth_below_baseline: Au::max(self.depth_below_baseline, other.depth_below_baseline),
+            ascent: Au::max(self.ascent, other.ascent),
+        }
+    }
 }
 
 #[derive(Copy, Clone, PartialEq)]

tests/ref/basic.list

@@ -188,6 +188,7 @@ flaky_cpu == append_style_a.html append_style_b.html
 == letter_spacing_a.html letter_spacing_ref.html
 == line_breaking_whitespace_collapse_a.html line_breaking_whitespace_collapse_ref.html
 == line_height_a.html line_height_ref.html
+== line_height_float_placement_a.html line_height_float_placement_ref.html
 != linear_gradients_corners_a.html linear_gradients_corners_ref.html
 == linear_gradients_lengths_a.html linear_gradients_lengths_ref.html
 == linear_gradients_parsing_a.html linear_gradients_parsing_ref.html

tests/ref/line_height_float_placement_a.html

@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" type="text/css" href="css/ahem.css">
+<style>
+* {
+    margin: 0;
+    padding: 0;
+}
+body {
+    font-size: 16px;
+    font-family: Ahem, monospace;
+    padding-top: 5px;
+}
+#floaty {
+    float: left;
+    height: 20px;
+    width: 100px;
+}
+#pre {
+    line-height: 32px;
+    white-space: pre;
+}
+</style>
+</head>
+<body>
+<div id=floaty></div>
+<div id=pre>x
+x</div>
+</body>
+</html>
+

tests/ref/line_height_float_placement_ref.html

@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" type="text/css" href="css/ahem.css">
+<style>
+* {
+    margin: 0;
+    padding: 0;
+}
+body {
+    font-size: 16px;
+    font-family: Ahem, monospace;
+}
+#floaty {
+    float: left;
+    height: 20px;
+    width: 100px;
+}
+#pre {
+    white-space: pre;
+}
+.spacer {
+    color: transparent;
+    font-size: 32px;
+}
+</style>
+</head>
+<body>
+<div id=floaty></div>
+<div id=pre>x<span class=spacer>x</span>
+x<span class=spacer>x</span></div>
+</body>
+</html>
+