Line breaking around floats

src/components/main/layout/float_context.rs

@@ -8,6 +8,7 @@ use geom::rect::Rect;
 use gfx::geometry::{Au, max, min};
 use std::util::replace;
 use std::vec;
+use std::i32::max_value;
 
 pub enum FloatType{
     FloatLeft,
@@ -146,10 +147,10 @@ impl FloatContextBase{
             (max(top_1, top_2), min(bottom_1, bottom_2))
         }
 
-        debug!("available_rect: trying to find space at %?", top);
-
         let top = top - self.offset.y;
 
+        debug!("available_rect: trying to find space at %?", top);
+
         // Relevant dimensions for the right-most left float
         let mut max_left = Au(0) - self.offset.x;
         let mut l_top = None;
@@ -167,6 +168,7 @@ impl FloatContextBase{
                 Some(data) => {
                     let float_pos = data.bounds.origin;
                     let float_size = data.bounds.size;
+                    debug!("float_pos: %?, float_size: %?", float_pos, float_size);
                     match data.f_type {
                         FloatLeft => {
                             if(float_pos.x + float_size.width > max_left && 
@@ -187,8 +189,8 @@ impl FloatContextBase{
 
                                 r_top = Some(float_pos.y);
                                 r_bottom = Some(float_pos.y + float_size.height);
-                                debug!("available_rect: collision with right float: new max_left is %?",
+                                debug!("available_rect: collision with right float: new min_right is %?",
-                                        max_left);
+                                        min_right);
                             }
                         }
                     }
@@ -198,13 +200,14 @@ impl FloatContextBase{
 
         // Extend the vertical range of the rectangle to the closest floats.
         // If there are floats on both sides, take the intersection of the
-        // two areas.
+        // two areas. Also make sure we never return a top smaller than the
+        // given upper bound.
         let (top, bottom) = match (r_top, r_bottom, l_top, l_bottom) {
             (Some(r_top), Some(r_bottom), Some(l_top), Some(l_bottom)) => 
-                range_intersect(r_top, r_bottom, l_top, l_bottom),
+                range_intersect(max(top, r_top), r_bottom, max(top, l_top), l_bottom),
 
-            (None, None, Some(l_top), Some(l_bottom)) => (l_top, l_bottom),
+            (None, None, Some(l_top), Some(l_bottom)) => (max(top, l_top), l_bottom),
-            (Some(r_top), Some(r_bottom), None, None) => (r_top, r_bottom),
+            (Some(r_top), Some(r_bottom), None, None) => (max(top, r_top), r_bottom),
             (None, None, None, None) => return None,
             _ => fail!("Reached unreachable state when computing float area")
         };
@@ -215,7 +218,7 @@ impl FloatContextBase{
         // assertion here.
         //assert!(max_left < min_right); 
         
-        assert!(top < bottom, "Float position error");
+        assert!(top <= bottom, "Float position error");
 
         Some(Rect{
             origin: Point2D(max_left, top) + self.offset,
@@ -236,6 +239,8 @@ impl FloatContextBase{
             f_type: info.f_type
         };
 
+        debug!("add_float: added float with info %?", new_info);
+
         let new_float = FloatData {    
             bounds: Rect {
                 origin: self.place_between_floats(&new_info).origin - self.offset,
@@ -264,6 +269,34 @@ impl FloatContextBase{
         return false;
     }
 
+    /// Given the top 3 sides of the rectange, finds the largest height that
+    /// will result in the rectange not colliding with any floats. Returns
+    /// None if that height is infinite.
+    fn max_height_for_bounds(&self, left: Au, top: Au, width: Au) -> Option<Au> {
+        let top = top - self.offset.y;
+        let left = left - self.offset.x;
+        let mut max_height = None;
+
+        for self.float_data.iter().advance |float| {
+            match *float {
+                None => (),
+                Some(f_data) => {
+                    if f_data.bounds.origin.y + f_data.bounds.size.height > top &&
+                       f_data.bounds.origin.x + f_data.bounds.size.width > left &&
+                       f_data.bounds.origin.x < left + width {
+                           let new_y = f_data.bounds.origin.y;
+                           max_height = Some(min(max_height.get_or_default(new_y), new_y));
+                       }
+                }
+            }
+        }
+
+        match max_height {
+            None => None,
+            Some(h) => Some(h + self.offset.y)
+        }
+    }
+
     /// Given necessary info, finds the closest place a box can be positioned
     /// without colliding with any floats.
     fn place_between_floats(&self, info: &PlacementInfo) -> Rect<Au>{
@@ -280,10 +313,10 @@ impl FloatContextBase{
                 // TODO(eatknson): integrate with overflow
                 None => return match info.f_type { 
                     FloatLeft => Rect(Point2D(Au(0), float_y), 
-                                      Size2D(info.max_width, info.height)),
+                                      Size2D(info.max_width, Au(max_value))),
 
                     FloatRight => Rect(Point2D(info.max_width - info.width, float_y), 
-                                       Size2D(info.max_width, info.height))
+                                       Size2D(info.max_width, Au(max_value)))
                 },
 
                 Some(rect) => {
@@ -292,12 +325,16 @@ impl FloatContextBase{
                     
                     // Place here if there is enough room
                     if (rect.size.width >= info.width) {
+                        let height = self.max_height_for_bounds(rect.origin.x, 
+                                                                rect.origin.y, 
+                                                                rect.size.width);
+                        let height = height.get_or_default(Au(max_value));
                         return match info.f_type {
                             FloatLeft => Rect(Point2D(rect.origin.x, float_y),
-                                              Size2D(rect.size.width, info.height)),
+                                              Size2D(rect.size.width, height)),
                             FloatRight => {
                                 Rect(Point2D(rect.origin.x + rect.size.width - info.width, float_y),
-                                     Size2D(rect.size.width, info.height))
+                                     Size2D(rect.size.width, height))
                             }
                         };
                     }

src/components/main/layout/inline.rs

@@ -44,12 +44,18 @@ things like "start outer box, text, start inner box, text, end inner
 box, text, end outer box, text". This seems a little complicated to
 serve as the starting point, but the current design doesn't make it
 hard to try out that alternative.
+
+Line boxes also contain some metadata used during line breaking. The
+green zone is the area that the line can expand to before it collides
+with a float or a horizontal wall of the containing block. The top
+left corner of the green zone is the same as that of the line, but
+the green zone can be taller and wider than the line itself.
 */
 
 struct LineBox {
     range: Range,
     bounds: Rect<Au>,
-    available_width: Au
+    green_zone: Size2D<Au>
 }
 
 struct LineboxScanner {
@@ -74,7 +80,7 @@ impl LineboxScanner {
             pending_line: LineBox {
                 range: Range::empty(), 
                 bounds: Rect(Point2D(Au(0), Au(0)), Size2D(Au(0), Au(0))), 
-                available_width: inline.position().size.width
+                green_zone: Size2D(Au(0), Au(0))
             },
             lines: ~[],
             cur_y: Au(0)
@@ -96,7 +102,7 @@ impl LineboxScanner {
     fn reset_linebox(&mut self) {
         self.pending_line.range.reset(0,0);
         self.pending_line.bounds = Rect(Point2D(Au(0), self.cur_y), Size2D(Au(0), Au(0)));
-        self.pending_line.available_width = self.flow.position().size.width;
+        self.pending_line.green_zone = Size2D(Au(0), Au(0))     
     }
 
     pub fn scan_for_lines(&mut self, ctx: &LayoutContext) {
@@ -171,6 +177,8 @@ impl LineboxScanner {
             ImageRenderBoxClass(image_box) => {
                 let size = image_box.image.get_size();
                 let height = Au::from_px(size.get_or_default(Size2D(0, 0)).height);
+                image_box.base.position.size.height = height;
+                debug!("box_height: found image height: %?", height);
                 height
             }
             TextRenderBoxClass(text_box) => {
@@ -191,7 +199,7 @@ impl LineboxScanner {
 
                 line_height
             }
-            GenericRenderBoxClass(generic_box) => {
+            GenericRenderBoxClass(_) => {
                 Au(0)
             }
             _ => {
@@ -211,130 +219,228 @@ impl LineboxScanner {
         }
     }
 
-    fn try_append_to_line(&mut self, ctx: &LayoutContext, in_box: RenderBox) -> bool {
+    /// Computes the position of a line that has only the provided RenderBox.
+    /// Returns: the bounding rect of the line's green zone (whose origin coincides
+    /// with the line's origin) and the actual width of the first box after splitting.
+    fn initial_line_placement (&self, ctx: &LayoutContext, first_box: RenderBox, ceiling: Au) -> (Rect<Au>, Au) {
+        debug!("LineboxScanner: Trying to place first box of line %?", self.lines.len());
+        debug!("LineboxScanner: box size: %?", first_box.position().size);
+        let splitable = first_box.can_split();
         let line_is_empty: bool = self.pending_line.range.length() == 0;
-        let new_height = self.new_height_for_line(in_box);
 
-        if line_is_empty {
+        // Initally, pretend a splitable box has 0 width.
-            let info = PlacementInfo {
+        // We will move it later if it has nonzero width
-                width: Au(0),
+        // and that causes problems.
-                height: in_box.position().size.height,
+        let placement_width = if splitable {
-                ceiling: self.cur_y,
+            Au(0)
-                max_width: self.flow.position().size.width,
+        } else {
-                f_type: FloatLeft
+            first_box.position().size.width
-            };
+        };
 
-            let line_bounds = self.floats.place_between_floats(&info);
+        let mut info = PlacementInfo {
+            width: placement_width,
+            height: first_box.position().size.height,
+            ceiling: ceiling,
+            max_width: self.flow.position().size.width,
+            f_type: FloatLeft
+        };
 
-            self.pending_line.bounds.origin = line_bounds.origin;
+        let line_bounds = self.floats.place_between_floats(&info);
-            self.pending_line.available_width = line_bounds.size.width;
 
-        } else if new_height > self.pending_line.bounds.size.height {
+        debug!("LineboxScanner: found position for line: %? using placement_info: %?", line_bounds, info);
-            // Uh-oh, adding this box increases the height of the line, so we may collide
+        
-            // with some floats.
+        // Simple case: if the box fits, then we can stop here
-
+        if line_bounds.size.width > first_box.position().size.width {
-            let info = PlacementInfo {
+            debug!("LineboxScanner: case=box fits");
-                width: in_box.position().size.width,
+            return (line_bounds, first_box.position().size.width);
-                height: new_height,
-                ceiling: self.pending_line.bounds.origin.y,
-                max_width: self.flow.position().size.width,
-                f_type: FloatLeft
-            };
-
-            let new_bounds = self.floats.place_between_floats(&info);
-
-            let bounds_width = new_bounds.size.width;
-            let line_width = self.pending_line.bounds.size.width;
-            let box_width = in_box.position().size.width;
-
-            // if the line and the new box can fit inside the bounds, 
-            // move the line.
-            if bounds_width >= box_width + line_width {
-                debug!("LineboxScanner: new box fits, so moving line");
-                self.pending_line.bounds.origin = new_bounds.origin;
-                self.pending_line.available_width = 
-                    new_bounds.size.width - self.pending_line.bounds.size.width;
-            }
-
-            // otherwise, we'll eventually try to split the box.
-            // if this is not possible, we'll make a new line.
         }
 
-        let in_box_width = in_box.position().size.width;
+        // If not, but we can't split the box, then we'll place
+        // the line here and it will overflow.
+        if !splitable {
+            debug!("LineboxScanner: case=line doesn't fit, but is unsplittable");
+            return (line_bounds, first_box.position().size.width);
+        }
 
-        debug!("LineboxScanner: Trying to append box to line %u (box width: %?, remaining width: \
+        // Otherwise, try and split the box
+        // FIXME(eatkinson): calling split_to_width here seems excessive and expensive.
+        // We should find a better abstraction or merge it with the call in
+        // try_append_to_line.
+        match first_box.split_to_width(ctx, line_bounds.size.width, line_is_empty) {
+            CannotSplit(_) => {
+                error!("LineboxScanner: Tried to split unsplittable render box! %s",
+                        first_box.debug_str());
+                return (line_bounds, first_box.position().size.width);
+            }
+            SplitDidFit(left, right) => {
+
+                debug!("LineboxScanner: case=box split and fit");
+                let actual_box_width = match (left, right) {
+                    (Some(l_box), Some(_))  => l_box.position().size.width,
+                    (Some(l_box), None)     => l_box.position().size.width,
+                    (None, Some(r_box))     => r_box.position().size.width,
+                    (None, None)            => fail!("This cas makes no sense.")
+                };
+                return (line_bounds, actual_box_width);
+            }
+            SplitDidNotFit(left, right) => {
+                // The split didn't fit, but we might be able to
+                // push it down past floats.
+
+
+                debug!("LineboxScanner: case=box split and fit didn't fit; trying to push it down");
+                let actual_box_width = match (left, right) {
+                    (Some(l_box), Some(_))  => l_box.position().size.width,
+                    (Some(l_box), None)     => l_box.position().size.width,
+                    (None, Some(r_box))     => r_box.position().size.width,
+                    (None, None)            => fail!("This cas makes no sense.")
+                };
+
+                info.width = actual_box_width;
+                let new_bounds = self.floats.place_between_floats(&info);
+
+                debug!("LineboxScanner: case=new line position: %?", new_bounds);
+                return (new_bounds, actual_box_width);
+            }
+        }
+        
+    }
+
+    /// Returns false only if we should break the line.
+    fn try_append_to_line(&mut self, ctx: &LayoutContext, in_box: RenderBox) -> bool {
+        let line_is_empty: bool = self.pending_line.range.length() == 0;
+
+        if line_is_empty {
+            let (line_bounds, _) = self.initial_line_placement(ctx, in_box, self.cur_y);
+            self.pending_line.bounds.origin = line_bounds.origin;
+            self.pending_line.green_zone = line_bounds.size;
+        }
+
+        debug!("LineboxScanner: Trying to append box to line %u (box size: %?, green zone: \
                 %?): %s",
                self.lines.len(),
-               in_box_width,
+               in_box.position().size,
-               self.pending_line.available_width,
+               self.pending_line.green_zone,
                in_box.debug_str());
 
-        if in_box_width <= self.pending_line.available_width {
+
+        let green_zone = self.pending_line.green_zone;
+
+        //assert!(green_zone.width >= self.pending_line.bounds.size.width &&
+        //        green_zone.height >= self.pending_line.bounds.size.height,
+        //        "Committed a line that overlaps with floats");
+
+        let new_height = self.new_height_for_line(in_box);
+        if new_height > green_zone.height {
+            // Uh-oh. Adding this box is going to increase the height,
+            // and because of that we will collide with some floats.
+
+            // We have two options here:
+            // 1) Move the entire line so that it doesn't collide any more.
+            // 2) Break the line and put the new box on the next line.
+
+            // The problem with option 1 is that we might move the line
+            // and then wind up breaking anyway, which violates the standard.
+            // But option 2 is going to look weird sometimes.
+
+            // So we'll try to move the line whenever we can, but break
+            // if we have to.
+
+            // First predict where the next line is going to be
+            let this_line_y = self.pending_line.bounds.origin.y;
+            let (next_line, first_box_width) = self.initial_line_placement(ctx, in_box, this_line_y);
+            let next_green_zone = next_line.size;
+
+            let new_width = self.pending_line.bounds.size.width + first_box_width;
+            // Now, see if everything can fit at the new location.
+            if next_green_zone.width >= new_width && next_green_zone.height >= new_height{
+                debug!("LineboxScanner: case=adding box collides vertically with floats: moving line");
+
+                self.pending_line.bounds.origin = next_line.origin;
+                self.pending_line.green_zone = next_green_zone;
+
+                assert!(!line_is_empty, "Non-terminating line breaking");
+                self.work_list.add_front(in_box);
+                return true;
+            } else {
+                debug!("LineboxScanner: case=adding box collides vertically with floats: breaking line");
+                self.work_list.add_front(in_box);
+                return false;
+            }
+        }
+
+        // If we're not going to overflow the green zone vertically, we might still do so
+        // horizontally. We'll try to place the whole box on this line and break somewhere
+        // if it doesn't fit.
+
+        let new_width = self.pending_line.bounds.size.width + in_box.position().size.width;
+
+        if(new_width <= green_zone.width){
             debug!("LineboxScanner: case=box fits without splitting");
             self.push_box_to_line(in_box);
             return true;
         }
 
         if !in_box.can_split() {
-            // force it onto the line anyway, if its otherwise empty
             // TODO(Issue #224): signal that horizontal overflow happened?
             if line_is_empty {
                 debug!("LineboxScanner: case=box can't split and line %u is empty, so \
                         overflowing.",
-                       self.lines.len());
+                        self.lines.len());
                 self.push_box_to_line(in_box);
                 return true;
             } else {
                 debug!("LineboxScanner: Case=box can't split, not appending.");
                 return false;
             }
-        }
+        } else {
+            let available_width = green_zone.width - self.pending_line.bounds.size.width;
 
-        // not enough width; try splitting?
+            match in_box.split_to_width(ctx, available_width, line_is_empty) {
-        match in_box.split_to_width(ctx, self.pending_line.available_width, line_is_empty) {
+                CannotSplit(_) => {
-            CannotSplit(_) => {
+                    error!("LineboxScanner: Tried to split unsplittable render box! %s",
-                error!("LineboxScanner: Tried to split unsplittable render box! %s",
+                            in_box.debug_str());
-                       in_box.debug_str());
+                    return false;
-                return false;
-            },
-            SplitDidFit(left, right) => {
-                debug!("LineboxScanner: case=split box did fit; deferring remainder box.");
-                match (left, right) {
-                    (Some(left_box), Some(right_box)) => {
-                        self.push_box_to_line(left_box);
-                        self.work_list.add_front(right_box);
-                    },
-                    (Some(left_box), None) => self.push_box_to_line(left_box),
-                    (None, Some(right_box)) => self.push_box_to_line(right_box),
-                    (None, None) => error!("LineboxScanner: This split case makes no sense!"),
                 }
-                return true;
+                SplitDidFit(left, right) => {
-            },
+                    debug!("LineboxScanner: case=split box did fit; deferring remainder box.");
-            SplitDidNotFit(left, right) => {
-                if line_is_empty {
-                    debug!("LineboxScanner: case=split box didn't fit and line %u is empty, so overflowing and deferring remainder box.",
-                          self.lines.len());
-                    // TODO(Issue #224): signal that horizontal overflow happened?
                     match (left, right) {
                         (Some(left_box), Some(right_box)) => {
                             self.push_box_to_line(left_box);
                             self.work_list.add_front(right_box);
-                        },
-                        (Some(left_box), None) => {
-                            self.push_box_to_line(left_box);
-                        }
-                        (None, Some(right_box)) => {
-                            self.push_box_to_line(right_box);
-                        },
-                        (None, None) => {
-                            error!("LineboxScanner: This split case makes no sense!");
                         }
+                        (Some(left_box), None) => self.push_box_to_line(left_box),
+                        (None, Some(right_box)) => self.push_box_to_line(right_box),
+                        (None, None) => error!("LineboxScanner: This split case makes no sense!"),
                     }
                     return true;
-                } else {
+                }
-                    debug!("LineboxScanner: case=split box didn't fit, not appending and deferring original box.");
+                SplitDidNotFit(left, right) => {
-                    self.work_list.add_front(in_box);
+                    if line_is_empty {
-                    return false;
+                        debug!("LineboxScanner: case=split box didn't fit and line %u is empty, so overflowing and deferring remainder box.",
+                                self.lines.len());
+                        // TODO(Issue #224): signal that horizontal overflow happened?
+                        match (left, right) {
+                            (Some(left_box), Some(right_box)) => {
+                                self.push_box_to_line(left_box);
+                                self.work_list.add_front(right_box);
+                            }
+                            (Some(left_box), None) => {
+                                self.push_box_to_line(left_box);
+                            }
+                            (None, Some(right_box)) => {
+                                self.push_box_to_line(right_box);
+                            }
+                            (None, None) => {
+                                error!("LineboxScanner: This split case makes no sense!");
+                            }
+                        }
+                        return true;
+                    } else {
+                        debug!("LineboxScanner: case=split box didn't fit, not appending and deferring original box.");
+                        self.work_list.add_front(in_box);
+                        return false;
+                    }
                 }
             }
         }
@@ -350,7 +456,6 @@ impl LineboxScanner {
         }
         self.pending_line.range.extend_by(1);
         self.pending_line.bounds.size.width = self.pending_line.bounds.size.width + box.position().size.width;
-        self.pending_line.available_width = self.pending_line.available_width - box.position().size.width;
         self.pending_line.bounds.size.height = Au::max(self.pending_line.bounds.size.height, 
                                                              box.position().size.height);
         self.new_boxes.push(box);
@@ -494,12 +599,11 @@ impl InlineFlowData {
         // determine its height for computing linebox height.
         let mut scanner = LineboxScanner::new(InlineFlow(self), self.common.floats_in.clone());
         scanner.scan_for_lines(ctx);
-        self.common.floats_out = scanner.floats_out();
 
         // Now, go through each line and lay out the boxes inside
         for self.lines.iter().advance |line| {
             // We need to distribute extra width based on text-align.
-            let mut slack_width = line.available_width;
+            let mut slack_width = line.green_zone.width - line.bounds.size.width;
             if slack_width < Au(0) {
                 slack_width = Au(0);
             }
@@ -632,7 +736,10 @@ impl InlineFlowData {
                 self.lines.last().bounds.origin.y + self.lines.last().bounds.size.height
             } else {
                 Au(0)
-            }
+            };
+
+        self.common.floats_out = scanner.floats_out().translate(Point2D(Au(0), 
+                                                                -self.common.position.size.height));
     }
 
     pub fn build_display_list_inline<E:ExtraDisplayListData>(&self,