From 650bd6546019c92de55d49d1f7c17a8d2e960bec Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Tue, 25 Jun 2013 16:19:08 -0700 Subject: [PATCH 1/7] Add right floats and fix bug where max_y was not being set --- src/components/main/layout/box_builder.rs | 28 +++++++++++---------- src/components/main/layout/float.rs | 12 ++++++--- src/components/main/layout/float_context.rs | 15 +++++++++-- src/components/main/layout/flow.rs | 4 +-- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/components/main/layout/box_builder.rs b/src/components/main/layout/box_builder.rs index 07e9e6e3b3b..9311627d8dd 100644 --- a/src/components/main/layout/box_builder.rs +++ b/src/components/main/layout/box_builder.rs @@ -24,7 +24,8 @@ use newcss::values::{CSSDisplayTableRowGroup, CSSDisplayTableHeaderGroup, CSSDis use newcss::values::{CSSDisplayTableRow, CSSDisplayTableColumnGroup, CSSDisplayTableColumn}; use newcss::values::{CSSDisplayTableCell, CSSDisplayTableCaption}; use newcss::values::{CSSDisplayNone}; -use newcss::values::{CSSFloatNone}; +use newcss::values::{CSSFloatNone, CSSFloatLeft, CSSFloatRight}; +use layout::float_context::{FloatLeft, FloatRight}; use script::dom::element::*; use script::dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId}; use script::dom::node::{ElementNodeTypeId, LayoutView, TextNodeTypeId}; @@ -370,17 +371,18 @@ impl LayoutTreeBuilder { // determine whether to float left or right. let is_float = if (node.is_element()) { match node.style().float() { - CSSFloatNone => false, - _ => true + CSSFloatNone => None, + CSSFloatLeft => Some(FloatLeft), + CSSFloatRight => Some(FloatRight) } } else { - false + None }; let new_generator = match (display, parent_generator.flow, sibling_flow) { - (CSSDisplayBlock, BlockFlow(_), _) if is_float => { - self.create_child_generator(node, parent_generator, Flow_Float) + (CSSDisplayBlock, BlockFlow(_), _) if !is_float.is_none() => { + self.create_child_generator(node, parent_generator, Flow_Float(is_float.get())) } (CSSDisplayBlock, BlockFlow(info), _) => match (info.is_root, node.parent_node()) { @@ -574,13 +576,13 @@ impl LayoutTreeBuilder { pub fn make_flow(&mut self, ty: FlowContextType, node: AbstractNode) -> FlowContext { let info = FlowData::new(self.next_flow_id(), node); let result = match ty { - Flow_Absolute => AbsoluteFlow(@mut info), - Flow_Block => BlockFlow(@mut BlockFlowData::new(info)), - Flow_Float => FloatFlow(@mut FloatFlowData::new(info)), - Flow_InlineBlock => InlineBlockFlow(@mut info), - Flow_Inline => InlineFlow(@mut InlineFlowData::new(info)), - Flow_Root => BlockFlow(@mut BlockFlowData::new_root(info)), - Flow_Table => TableFlow(@mut info), + Flow_Absolute => AbsoluteFlow(@mut info), + Flow_Block => BlockFlow(@mut BlockFlowData::new(info)), + Flow_Float(f_type) => FloatFlow(@mut FloatFlowData::new(info, f_type)), + Flow_InlineBlock => InlineBlockFlow(@mut info), + Flow_Inline => InlineFlow(@mut InlineFlowData::new(info)), + Flow_Root => BlockFlow(@mut BlockFlowData::new_root(info)), + Flow_Table => TableFlow(@mut info), }; debug!("LayoutTreeBuilder: created flow: %s", result.debug_str()); result diff --git a/src/components/main/layout/float.rs b/src/components/main/layout/float.rs index 426ae8d6281..3d67a742fa0 100644 --- a/src/components/main/layout/float.rs +++ b/src/components/main/layout/float.rs @@ -8,7 +8,7 @@ use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData}; use layout::display_list_builder::{FlowDisplayListBuilderMethods}; use layout::flow::{FloatFlow, FlowData}; use layout::model::{MaybeAuto}; -use layout::float_context::{FloatContext, PlacementInfo, FloatLeft}; +use layout::float_context::{FloatContext, PlacementInfo, FloatLeft, FloatType}; use std::cell::Cell; use geom::point::Point2D; @@ -27,21 +27,25 @@ pub struct FloatFlowData { containing_width: Au, - /// Parent clobbers our position, so store it separately + /// Offset relative to where the parent tried to position this flow rel_pos: Point2D, + /// Left or right? + float_type: FloatType, + /// Index into the box list for inline floats index: Option, } impl FloatFlowData { - pub fn new(common: FlowData) -> FloatFlowData { + pub fn new(common: FlowData, float_type: FloatType) -> FloatFlowData { FloatFlowData { common: common, containing_width: Au(0), box: None, index: None, + float_type: float_type, rel_pos: Point2D(Au(0), Au(0)), } } @@ -210,7 +214,7 @@ impl FloatFlowData { height: height, ceiling: Au(0), max_width: self.containing_width, - f_type: FloatLeft, + f_type: self.float_type, }; // Place the float and return the FloatContext back to the parent flow. diff --git a/src/components/main/layout/float_context.rs b/src/components/main/layout/float_context.rs index f384c718d60..5b364705587 100644 --- a/src/components/main/layout/float_context.rs +++ b/src/components/main/layout/float_context.rs @@ -225,6 +225,7 @@ impl FloatContextBase{ f_type: info.f_type }; self.float_data[self.floats_used] = Some(new_float); + self.max_y = max(self.max_y, new_float.bounds.origin.y); self.floats_used += 1; } @@ -240,16 +241,26 @@ impl FloatContextBase{ let maybe_location = self.available_rect(float_y, info.height, info.max_width); debug!("place_float: Got available rect: %? for y-pos: %?", maybe_location, float_y); match maybe_location { + // If there are no floats blocking us, return the current location // TODO(eatknson): integrate with overflow - None => return Point2D(Au(0), float_y), + None => return match info.f_type { + FloatLeft => Point2D(Au(0), float_y), + FloatRight => Point2D(info.max_width - info.width, float_y) + }, + Some(rect) => { assert!(rect.origin.y + rect.size.height != float_y, "Non-terminating float placement"); // Place here if there is enough room if (rect.size.width >= info.width) { - return Point2D(rect.origin.x, float_y); + return match info.f_type { + FloatLeft => Point2D(rect.origin.x, float_y), + FloatRight => { + Point2D(rect.origin.x + rect.size.width - info.width, float_y) + } + }; } // Try to place at the next-lowest location. diff --git a/src/components/main/layout/flow.rs b/src/components/main/layout/flow.rs index ccfe21b9250..d6bc65d73c0 100644 --- a/src/components/main/layout/flow.rs +++ b/src/components/main/layout/flow.rs @@ -31,7 +31,7 @@ use layout::box::RenderBox; use layout::context::LayoutContext; use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData}; use layout::inline::{InlineFlowData}; -use layout::float_context::{FloatContext, Invalid}; +use layout::float_context::{FloatContext, Invalid, FloatType}; use std::cell::Cell; use std::uint; @@ -56,7 +56,7 @@ pub enum FlowContext { pub enum FlowContextType { Flow_Absolute, Flow_Block, - Flow_Float, + Flow_Float(FloatType), Flow_InlineBlock, Flow_Inline, Flow_Root, From 602334a3162cfda0a93e1ab23919fa40c203bfee Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Tue, 25 Jun 2013 18:21:14 -0700 Subject: [PATCH 2/7] Fix float placement bugs & add hacky non-percen heights to blocks. --- src/components/main/layout/block.rs | 21 +++++++++++++++------ src/components/main/layout/float.rs | 2 +- src/components/main/layout/float_context.rs | 14 ++++++-------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/components/main/layout/block.rs b/src/components/main/layout/block.rs index d1827561f5c..afabf4336c4 100644 --- a/src/components/main/layout/block.rs +++ b/src/components/main/layout/block.rs @@ -249,12 +249,14 @@ impl BlockFlowData { pub fn assign_height_block(@mut self, ctx: &mut LayoutContext) { let mut cur_y = Au(0); let mut top_offset = Au(0); + let mut bottom_offset = Au(0); let mut left_offset = Au(0); for self.box.iter().advance |&box| { do box.with_model |model| { top_offset = model.margin.top + model.border.top + model.padding.top; cur_y = cur_y + top_offset; + bottom_offset = model.margin.bottom + model.border.bottom + model.padding.bottom; left_offset = model.offset(); }; } @@ -277,7 +279,7 @@ impl BlockFlowData { } kid.assign_height(ctx); do kid.with_mut_base |child_node| { - float_ctx = child_node.floats_out.translate(Point2D(Au(0), -child_node.position.size.height)); + float_ctx = child_node.floats_out.clone(); } } @@ -288,12 +290,19 @@ impl BlockFlowData { }; } - let height = if self.is_root { + let mut height = if self.is_root { Au::max(ctx.screen_size.size.height, cur_y) } else { - cur_y - top_offset + cur_y - top_offset }; - + + for self.box.each |&box| { + let style = box.style(); + let maybe_height = MaybeAuto::from_height(style.height(), Au(0)); + let maybe_height = maybe_height.spec_or_default(Au(0)); + height = geometry::max(height, maybe_height); + } + let mut noncontent_height = Au(0); self.box.map(|&box| { do box.with_mut_base |base| { @@ -311,8 +320,8 @@ impl BlockFlowData { //TODO(eatkinson): compute heights using the 'height' property. self.common.position.size.height = height + noncontent_height; - - self.common.floats_out = float_ctx.translate(Point2D(left_offset, self.common.position.size.height)); + let extra_height = height - (cur_y - top_offset) + bottom_offset; + self.common.floats_out = float_ctx.translate(Point2D(left_offset, -extra_height)); } pub fn build_display_list_block(@mut self, diff --git a/src/components/main/layout/float.rs b/src/components/main/layout/float.rs index 3d67a742fa0..2c23b90138d 100644 --- a/src/components/main/layout/float.rs +++ b/src/components/main/layout/float.rs @@ -8,7 +8,7 @@ use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData}; use layout::display_list_builder::{FlowDisplayListBuilderMethods}; use layout::flow::{FloatFlow, FlowData}; use layout::model::{MaybeAuto}; -use layout::float_context::{FloatContext, PlacementInfo, FloatLeft, FloatType}; +use layout::float_context::{FloatContext, PlacementInfo, FloatType}; use std::cell::Cell; use geom::point::Point2D; diff --git a/src/components/main/layout/float_context.rs b/src/components/main/layout/float_context.rs index 5b364705587..cdddf7090aa 100644 --- a/src/components/main/layout/float_context.rs +++ b/src/components/main/layout/float_context.rs @@ -196,14 +196,12 @@ impl FloatContextBase{ _ => fail!("Reached unreachable state when computing float area") }; - // When the window is smaller than the float, we will return a rect - // with negative width. - assert!(max_left < min_right - || max_left > max_x - self.offset.x - || min_right < Au(0) - self.offset.x - ,"Float position error"); - - //TODO(eatkinson): do we need to do something similar for heights? + // This assertion is too strong and fails in some cases. It is OK to + // return negative widths since we check against that right away, but + // we should still undersrtand why they occur and add a stronger + // assertion here. + //assert!(max_left < min_right); + assert!(top < bottom, "Float position error"); Some(Rect{ From 3cbe9156d7b9034adf6a25bb491142cc4617233b Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Wed, 26 Jun 2013 10:12:15 -0700 Subject: [PATCH 3/7] Add test and address comments --- src/test/html/test_float_placement.html | 62 +++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 src/test/html/test_float_placement.html diff --git a/src/test/html/test_float_placement.html b/src/test/html/test_float_placement.html new file mode 100644 index 00000000000..f983c6260e8 --- /dev/null +++ b/src/test/html/test_float_placement.html @@ -0,0 +1,62 @@ + + + + + +
+
+
+
+
+
+
+
+
+
+ + From e09cd30dc87711718c5b3bb85137b8b4351ec441 Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Fri, 28 Jun 2013 09:36:44 -0700 Subject: [PATCH 4/7] Stop crashing --- src/components/main/layout/block.rs | 2 +- src/components/main/layout/box_builder.rs | 86 +++++++++++++++++------ 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/src/components/main/layout/block.rs b/src/components/main/layout/block.rs index afabf4336c4..ff06f05dd62 100644 --- a/src/components/main/layout/block.rs +++ b/src/components/main/layout/block.rs @@ -296,7 +296,7 @@ impl BlockFlowData { cur_y - top_offset }; - for self.box.each |&box| { + for self.box.iter().advance |&box| { let style = box.style(); let maybe_height = MaybeAuto::from_height(style.height(), Au(0)); let maybe_height = maybe_height.spec_or_default(Au(0)); diff --git a/src/components/main/layout/box_builder.rs b/src/components/main/layout/box_builder.rs index 9311627d8dd..717bfd61a49 100644 --- a/src/components/main/layout/box_builder.rs +++ b/src/components/main/layout/box_builder.rs @@ -30,7 +30,7 @@ use script::dom::element::*; use script::dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId}; use script::dom::node::{ElementNodeTypeId, LayoutView, TextNodeTypeId}; use servo_util::range::Range; -use servo_util::tree::{TreeNodeRef, TreeUtils}; +use servo_util::tree::{TreeNodeRef, TreeNode, TreeUtils}; pub struct LayoutTreeBuilder { root_flow: Option, @@ -166,15 +166,42 @@ impl BoxGenerator { }, FloatFlow(float) => { debug!("BoxGenerator[f%d]: point b", float.common.id); - let new_box = self.make_box(ctx, box_type, node, self.flow, builder); - debug!("BoxGenerator[f%d]: attaching box[b%d] to float flow (node: %s)", - float.common.id, - new_box.id(), - node.debug_str()); + let mut parent_flow = None; - assert!(float.box.is_none()); - float.box = Some(new_box); + do self.flow.with_base |base| { + parent_flow = base.parent_node(); + } + + match parent_flow { + None => fail!("Float flow as root node"), + Some(BlockFlow(*)) | + Some(FloatFlow(*)) => { + let new_box = self.make_box(ctx, box_type, node, self.flow, builder); + + debug!("BoxGenerator[f%d]: attaching box[b%d] to float flow (node: %s)", + float.common.id, + new_box.id(), + node.debug_str()); + + assert!(float.box.is_none() && float.index.is_none()); + float.box = Some(new_box); + } + Some(InlineFlow(inline)) => { + let new_box = self.make_box(ctx, box_type, node, self.flow, builder); + + debug!("BoxGenerator[f%d]: attaching box[b%d] to float flow (node: %s)", + float.common.id, + new_box.id(), + node.debug_str()); + + + assert!(float.box.is_none() && float.index.is_none()); + inline.boxes.push(new_box); + float.index = Some(inline.boxes.len() - 1); + } + _ => warn!("push_node() not implemented for flow f%d", self.flow.id()) + } }, _ => warn!("push_node() not implemented for flow f%d", self.flow.id()), } @@ -215,7 +242,7 @@ impl BoxGenerator { /// Disambiguate between different methods here instead of inlining, since each case has very /// different complexity. - fn make_box(&mut self, + fn make_box(&self, layout_ctx: &LayoutContext, ty: RenderBoxType, node: AbstractNode, @@ -232,7 +259,7 @@ impl BoxGenerator { result } - fn make_image_box(&mut self, + fn make_image_box(&self, layout_ctx: &LayoutContext, node: AbstractNode, base: RenderBoxBase) @@ -287,10 +314,11 @@ impl LayoutTreeBuilder { -> Option<@mut BoxGenerator> { debug!("Considering node: %s", cur_node.debug_str()); - let this_generator = match self.box_generator_for_node(cur_node, - parent_generator, - prev_sibling_generator) { - Some(gen) => gen, + // Skip over nodes that don't belong in the flow tree + let (this_generator, next_generator) = + match self.box_generator_for_node(cur_node, parent_generator, prev_sibling_generator) { + + Some((gen, n_gen)) => (gen, n_gen), None => { return prev_sibling_generator; } }; @@ -319,14 +347,14 @@ impl LayoutTreeBuilder { dom_node.layout_data().flow = Some(child_flow); } } - Some(this_generator) + Some(next_generator) } pub fn box_generator_for_node(&mut self, node: AbstractNode, parent_generator: @mut BoxGenerator, sibling_generator: Option<@mut BoxGenerator>) - -> Option<@mut BoxGenerator> { + -> Option<(@mut BoxGenerator, @mut BoxGenerator)> { fn is_root(node: AbstractNode) -> bool { match node.parent_node() { @@ -334,7 +362,7 @@ impl LayoutTreeBuilder { Some(_) => false } } - let display = if (node.is_element()) { + let display = if node.is_element() { match node.style().display(is_root(node)) { CSSDisplayNone => return None, // tree ends here if 'display: none' // TODO(eatkinson) these are hacks so that the code doesn't crash @@ -381,9 +409,25 @@ impl LayoutTreeBuilder { let new_generator = match (display, parent_generator.flow, sibling_flow) { + // Floats (CSSDisplayBlock, BlockFlow(_), _) if !is_float.is_none() => { self.create_child_generator(node, parent_generator, Flow_Float(is_float.get())) } + // If we're placing a float after an inline, append the float to the inline flow, + // then continue building from the inline flow in case there are more inlines + // afterward. + (CSSDisplayBlock, _, Some(InlineFlow(_))) if !is_float.is_none() => { + let float_generator = self.create_child_generator(node, + sibling_generator.get(), + Flow_Float(is_float.get())); + return Some((float_generator, sibling_generator.get())); + } + // This is a catch-all case for when: + // a) sibling_flow is None + // b) sibling_flow is a BlockFlow + (CSSDisplayBlock, InlineFlow(_), _) if !is_float.is_none() => { + self.create_child_generator(node, parent_generator, Flow_Float(is_float.get())) + } (CSSDisplayBlock, BlockFlow(info), _) => match (info.is_root, node.parent_node()) { // If this is the root node, then use the root flow's @@ -410,8 +454,8 @@ impl LayoutTreeBuilder { self.create_child_generator(node, parent_generator, Flow_Inline) } - // FIXME(eatkinson): this is bogus. Floats should not be able to split - // inlines. They should be appended as children of the inline flow. + // The first two cases should only be hit when a FloatFlow + // is the first child of a BlockFlow. Other times, we will (CSSDisplayInline, _, Some(FloatFlow(*))) | (CSSDisplayInlineBlock, _, Some(FloatFlow(*))) | (CSSDisplayInline, FloatFlow(*), _) | @@ -434,7 +478,9 @@ impl LayoutTreeBuilder { _ => parent_generator }; - Some(new_generator) + // Usually, the node we add boxes to will be prev_sibling on the + // next call to this function. + Some((new_generator, new_generator)) } pub fn create_child_generator(&mut self, From a6f6ce926c11d0630cb694eaa6fac4297b820bcc Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Wed, 10 Jul 2013 12:46:24 -0700 Subject: [PATCH 5/7] Semi-correct text wrapping --- src/components/main/layout/float_context.rs | 61 +++- src/components/main/layout/inline.rs | 332 +++++++++++++------- 2 files changed, 263 insertions(+), 130 deletions(-) diff --git a/src/components/main/layout/float_context.rs b/src/components/main/layout/float_context.rs index cdddf7090aa..5de03e89e01 100644 --- a/src/components/main/layout/float_context.rs +++ b/src/components/main/layout/float_context.rs @@ -94,6 +94,13 @@ impl FloatContext { replace(self, Invalid) } + #[inline(always)] + pub fn place_between_floats(&self, info: &PlacementInfo) -> Rect { + do self.with_base |base| { + base.place_between_floats(info) + } + } + #[inline(always)] pub fn last_float_pos(&mut self) -> Point2D { do self.with_base |base| { @@ -154,6 +161,7 @@ impl FloatContextBase{ // Find the float collisions for the given vertical range. for self.float_data.iter().advance |float| { + debug!("available_rect: Checking for collision against float"); match *float{ None => (), Some(data) => { @@ -167,6 +175,9 @@ impl FloatContextBase{ l_top = Some(float_pos.y); l_bottom = Some(float_pos.y + float_size.height); + + debug!("available_rect: collision with left float: new max_left is %?", + max_left); } } FloatRight => { @@ -176,6 +187,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 %?", + max_left); } } } @@ -215,9 +228,17 @@ impl FloatContextBase{ assert!(self.floats_used < self.float_data.len() && self.float_data[self.floats_used].is_none()); + let new_info = PlacementInfo { + width: info.width, + height: info.height, + ceiling: max(info.ceiling, self.max_y + self.offset.y), + max_width: info.max_width, + f_type: info.f_type + }; + let new_float = FloatData { bounds: Rect { - origin: self.place_float(info) - self.offset, + origin: self.place_between_floats(&new_info).origin - self.offset, size: Size2D(info.width, info.height) }, f_type: info.f_type @@ -227,14 +248,29 @@ impl FloatContextBase{ self.floats_used += 1; } - /// Given necessary info, finds the position of the float in - /// LOCAL COORDINATES. i.e. must be translated before placed - /// in the float list - fn place_float(&self, info: &PlacementInfo) -> Point2D{ + /// Returns true if the given rect overlaps with any floats. + fn collides_with_float(&self, bounds: &Rect) -> bool { + for self.float_data.each |float| { + match *float{ + None => (), + Some(data) => { + if data.bounds.translate(&self.offset).intersects(bounds) { + return true; + } + } + }; + } + + return false; + } + + /// 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{ debug!("place_float: Placing float with width %? and height %?", info.width, info.height); // Can't go any higher than previous floats or // previous elements in the document. - let mut float_y = max(info.ceiling, self.max_y + self.offset.y); + let mut float_y = info.ceiling; loop { let maybe_location = self.available_rect(float_y, info.height, info.max_width); debug!("place_float: Got available rect: %? for y-pos: %?", maybe_location, float_y); @@ -243,8 +279,11 @@ impl FloatContextBase{ // If there are no floats blocking us, return the current location // TODO(eatknson): integrate with overflow None => return match info.f_type { - FloatLeft => Point2D(Au(0), float_y), - FloatRight => Point2D(info.max_width - info.width, float_y) + FloatLeft => Rect(Point2D(Au(0), float_y), + Size2D(info.max_width, info.height)), + + FloatRight => Rect(Point2D(info.max_width - info.width, float_y), + Size2D(info.max_width, info.height)) }, Some(rect) => { @@ -254,9 +293,11 @@ impl FloatContextBase{ // Place here if there is enough room if (rect.size.width >= info.width) { return match info.f_type { - FloatLeft => Point2D(rect.origin.x, float_y), + FloatLeft => Rect(Point2D(rect.origin.x, float_y), + Size2D(rect.size.width, info.height)), FloatRight => { - Point2D(rect.origin.x + rect.size.width - info.width, float_y) + Rect(Point2D(rect.origin.x + rect.size.width - info.width, float_y), + Size2D(rect.size.width, info.height)) } }; } diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 616f142c76f..60552c480c8 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -10,6 +10,7 @@ use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData}; use layout::flow::{FlowContext, FlowData, InlineFlow}; use layout::float_context::FloatContext; use layout::util::{ElementMapping}; +use layout::float_context::{PlacementInfo, FloatLeft}; use std::u16; use std::util; @@ -45,42 +46,57 @@ serve as the starting point, but the current design doesn't make it hard to try out that alternative. */ -struct PendingLine { +struct LineBox { range: Range, - bounds: Rect + bounds: Rect, + available_width: Au } struct LineboxScanner { flow: FlowContext, + floats: FloatContext, new_boxes: ~[RenderBox], work_list: @mut Deque, - pending_line: PendingLine, - line_spans: ~[Range], + pending_line: LineBox, + lines: ~[LineBox], + cur_y: Au, } impl LineboxScanner { - pub fn new(inline: FlowContext) -> LineboxScanner { + pub fn new(inline: FlowContext, float_ctx: FloatContext) -> LineboxScanner { assert!(inline.starts_inline_flow()); LineboxScanner { flow: inline, + floats: float_ctx, new_boxes: ~[], work_list: @mut Deque::new(), - pending_line: PendingLine {range: Range::empty(), bounds: Rect(Point2D(Au(0), Au(0)), Size2D(Au(0), Au(0)))}, - line_spans: ~[], + pending_line: LineBox { + range: Range::empty(), + bounds: Rect(Point2D(Au(0), Au(0)), Size2D(Au(0), Au(0))), + available_width: inline.position().size.width + }, + lines: ~[], + cur_y: Au(0) } } + + pub fn floats_out(&mut self) -> FloatContext { + self.floats.clone() + } fn reset_scanner(&mut self) { debug!("Resetting line box scanner's state for flow f%d.", self.flow.id()); - self.line_spans = ~[]; + self.lines = ~[]; self.new_boxes = ~[]; + self.cur_y = Au(0); self.reset_linebox(); } fn reset_linebox(&mut self) { self.pending_line.range.reset(0,0); - self.pending_line.bounds = Rect(Point2D(Au(0), Au(0)), Size2D(Au(0), Au(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; } pub fn scan_for_lines(&mut self, ctx: &LayoutContext) { @@ -108,16 +124,16 @@ impl LineboxScanner { let box_was_appended = self.try_append_to_line(ctx, cur_box); if !box_was_appended { debug!("LineboxScanner: Box wasn't appended, because line %u was full.", - self.line_spans.len()); + self.lines.len()); self.flush_current_line(); } else { - debug!("LineboxScanner: appended a box to line %u", self.line_spans.len()); + debug!("LineboxScanner: appended a box to line %u", self.lines.len()); } } if self.pending_line.range.length() > 0 { debug!("LineboxScanner: Partially full linebox %u left at end of scanning.", - self.line_spans.len()); + self.lines.len()); self.flush_current_line(); } } @@ -131,88 +147,129 @@ impl LineboxScanner { fn swap_out_results(&mut self) { debug!("LineboxScanner: Propagating scanned lines[n=%u] to inline flow f%d", - self.line_spans.len(), + self.lines.len(), self.flow.id()); let inline: &mut InlineFlowData = self.flow.inline(); util::swap(&mut inline.boxes, &mut self.new_boxes); - util::swap(&mut inline.lines, &mut self.line_spans); + util::swap(&mut inline.lines, &mut self.lines); } fn flush_current_line(&mut self) { debug!("LineboxScanner: Flushing line %u: %?", - self.line_spans.len(), self.pending_line); - // set box horizontal offsets - let line_range = self.pending_line.range; - let mut offset_x = Au(0); - // TODO(Issue #199): interpretation of CSS 'direction' will change how boxes are positioned. - debug!("LineboxScanner: Setting horizontal offsets for boxes in line %u range: %?", - self.line_spans.len(), line_range); - - // Get the text alignment. - // TODO(Issue #222): use 'text-align' property from InlineFlow's - // block container, not from the style of the first box child. - let linebox_align; - if self.pending_line.range.begin() < self.new_boxes.len() { - let first_box = self.new_boxes[self.pending_line.range.begin()]; - linebox_align = first_box.text_align(); - } else { - // Nothing to lay out, so assume left alignment. - linebox_align = CSSTextAlignLeft; - } - - let slack_width = self.flow.position().size.width - self.pending_line.bounds.size.width; - match linebox_align { - // So sorry, but justified text is more complicated than shuffling linebox coordinates. - // TODO(Issue #213): implement `text-align: justify` - CSSTextAlignLeft | CSSTextAlignJustify => { - for line_range.eachi |i| { - do self.new_boxes[i].with_mut_base |base| { - base.position.origin.x = offset_x; - offset_x = offset_x + base.position.size.width; - }; - } - }, - CSSTextAlignCenter => { - offset_x = slack_width.scale_by(0.5f); - for line_range.eachi |i| { - do self.new_boxes[i].with_mut_base |base| { - base.position.origin.x = offset_x; - offset_x = offset_x + base.position.size.width; - }; - } - }, - CSSTextAlignRight => { - offset_x = slack_width; - for line_range.eachi |i| { - do self.new_boxes[i].with_mut_base |base| { - base.position.origin.x = offset_x; - offset_x = offset_x + base.position.size.width; - }; - } - }, - } + self.lines.len(), self.pending_line); // clear line and add line mapping - debug!("LineboxScanner: Saving information for flushed line %u.", self.line_spans.len()); - self.line_spans.push(line_range); + debug!("LineboxScanner: Saving information for flushed line %u.", self.lines.len()); + self.lines.push(self.pending_line); + self.cur_y = self.pending_line.bounds.origin.y + self.pending_line.bounds.size.height; self.reset_linebox(); } - // return value: whether any box was appended. + fn box_height(&self, box: RenderBox) -> Au { + match box { + ImageRenderBoxClass(image_box) => { + let size = image_box.image.get_size(); + let height = Au::from_px(size.get_or_default(Size2D(0, 0)).height); + height + } + TextRenderBoxClass(text_box) => { + let range = &text_box.range; + let run = &text_box.run; + + // Compute the height based on the line-height and font size + let text_bounds = run.metrics_for_range(range).bounding_box; + let em_size = text_bounds.size.height; + let line_height = match box.line_height() { + CSSLineHeightNormal => em_size.scale_by(1.14f), + CSSLineHeightNumber(l) => em_size.scale_by(l), + CSSLineHeightLength(Em(l)) => em_size.scale_by(l), + CSSLineHeightLength(Px(l)) => Au::from_frac_px(l), + CSSLineHeightLength(Pt(l)) => Au::from_pt(l), + CSSLineHeightPercentage(p) => em_size.scale_by(p / 100.0f) + }; + + line_height + } + GenericRenderBoxClass(generic_box) => { + Au(0) + } + _ => { + fail!(fmt!("Tried to get height of unknown Box variant: %s", box.debug_str())) + } + } + } + + // FIXME(eatkinson): this assumes that the tallest box in the line determines the line height + // This might not be the case with some weird text fonts. + fn new_height_for_line(&self, new_box: RenderBox) -> Au { + let box_height = self.box_height(new_box); + if box_height > self.pending_line.bounds.size.height { + box_height + } else { + self.pending_line.bounds.size.height + } + } + fn try_append_to_line(&mut self, ctx: &LayoutContext, in_box: RenderBox) -> bool { - let remaining_width = self.flow.position().size.width - self.pending_line.bounds.size.width; - let in_box_width = in_box.position().size.width; 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 { + let info = PlacementInfo { + width: Au(0), + height: in_box.position().size.height, + ceiling: self.cur_y, + max_width: self.flow.position().size.width, + f_type: FloatLeft + }; + + let line_bounds = self.floats.place_between_floats(&info); + + self.pending_line.bounds.origin = line_bounds.origin; + self.pending_line.available_width = line_bounds.size.width; + + } else if new_height > self.pending_line.bounds.size.height { + // Uh-oh, adding this box increases the height of the line, so we may collide + // with some floats. + + let info = PlacementInfo { + width: in_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; debug!("LineboxScanner: Trying to append box to line %u (box width: %?, remaining width: \ %?): %s", - self.line_spans.len(), + self.lines.len(), in_box_width, - remaining_width, + self.pending_line.available_width, in_box.debug_str()); - if in_box_width <= remaining_width { + if in_box_width <= self.pending_line.available_width { debug!("LineboxScanner: case=box fits without splitting"); self.push_box_to_line(in_box); return true; @@ -224,7 +281,7 @@ impl LineboxScanner { if line_is_empty { debug!("LineboxScanner: case=box can't split and line %u is empty, so \ overflowing.", - self.line_spans.len()); + self.lines.len()); self.push_box_to_line(in_box); return true; } else { @@ -234,7 +291,7 @@ impl LineboxScanner { } // not enough width; try splitting? - match in_box.split_to_width(ctx, remaining_width, line_is_empty) { + match in_box.split_to_width(ctx, self.pending_line.available_width, line_is_empty) { CannotSplit(_) => { error!("LineboxScanner: Tried to split unsplittable render box! %s", in_box.debug_str()); @@ -256,7 +313,7 @@ impl LineboxScanner { 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.line_spans.len()); + self.lines.len()); // TODO(Issue #224): signal that horizontal overflow happened? match (left, right) { (Some(left_box), Some(right_box)) => { @@ -285,7 +342,7 @@ impl LineboxScanner { // unconditional push fn push_box_to_line(&mut self, box: RenderBox) { - debug!("LineboxScanner: Pushing box b%d to line %u", box.id(), self.line_spans.len()); + debug!("LineboxScanner: Pushing box b%d to line %u", box.id(), self.lines.len()); if self.pending_line.range.length() == 0 { assert!(self.new_boxes.len() <= (u16::max_value as uint)); @@ -293,6 +350,9 @@ 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 -= 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); } } @@ -306,7 +366,8 @@ pub struct InlineFlowData { boxes: ~[RenderBox], // vec of ranges into boxes that represents line positions. // these ranges are disjoint, and are the result of inline layout. - lines: ~[Range], + // also some metadata used for positioning lines + lines: ~[LineBox], // vec of ranges into boxes that represent elements. These ranges // must be well-nested, and are only related to the content of // boxes (not lines). Ranges are only kept for non-leaf elements. @@ -425,42 +486,84 @@ impl InlineFlowData { kid.assign_height(ctx); } - - // TODO(eatkinson): line boxes need to shrink if there are floats - let mut scanner = LineboxScanner::new(InlineFlow(self)); - scanner.scan_for_lines(ctx); - self.common.floats_out = self.common.floats_in.clone(); - + // Divide the boxes into lines // TODO(#226): Get the CSS `line-height` property from the containing block's style to // determine minimum linebox height. // // TODO(#226): Get the CSS `line-height` property from each non-replaced inline element to // 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(); - let mut cur_y = Au(0); + // Now, go through each line and lay out the boxes inside + for self.lines.eachi |i, line| { + // We need to distribute extra width based on text-align. + let mut slack_width = line.available_width; + if slack_width < Au(0) { + slack_width = Au(0); + } + //assert!(slack_width >= Au(0), "Too many boxes on line"); - for self.lines.iter().enumerate().advance |(i, line_span)| { - debug!("assign_height_inline: processing line %u with box span: %?", i, line_span); + // Get the text alignment. + // TODO(Issue #222): use 'text-align' property from InlineFlow's + // block container, not from the style of the first box child. + let linebox_align; + if line.range.begin() < self.boxes.len() { + let first_box = self.boxes[line.range.begin()]; + linebox_align = first_box.text_align(); + } else { + // Nothing to lay out, so assume left alignment. + linebox_align = CSSTextAlignLeft; + } - // These coordinates are relative to the left baseline. - let mut linebox_bounding_box = Au::zero_rect(); - let mut linebox_height = Au(0); + // Set the box x positions + let mut offset_x = line.bounds.origin.x; + match linebox_align { + // So sorry, but justified text is more complicated than shuffling linebox coordinates. + // TODO(Issue #213): implement `text-align: justify` + CSSTextAlignLeft | CSSTextAlignJustify => { + for line.range.eachi |i| { + do self.boxes[i].with_mut_base |base| { + base.position.origin.x = offset_x; + offset_x += base.position.size.width; + } + } + } + CSSTextAlignCenter => { + offset_x += slack_width.scale_by(0.5f); + for line.range.eachi |i| { + do self.boxes[i].with_mut_base |base| { + base.position.origin.x = offset_x; + offset_x += base.position.size.width; + } + } + } + CSSTextAlignRight => { + offset_x += slack_width; + for line.range.eachi |i| { + do self.boxes[i].with_mut_base |base| { + base.position.origin.x = offset_x; + offset_x += base.position.size.width; + } + } + } + }; + + + // Get the baseline offset, assuming that the tallest text box will determine + // the baseline. let mut baseline_offset = Au(0); - - for line_span.eachi |box_i| { + let mut max_height = Au(0); + for line.range.eachi |box_i| { let cur_box = self.boxes[box_i]; - // Compute the height and bounding box of each box. - let bounding_box = match cur_box { + match cur_box { 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; - if height > linebox_height { - linebox_height = height; - } - image_box.base.position.translate(&Point2D(Au(0), -height)) } TextRenderBoxClass(text_box) => { @@ -484,8 +587,9 @@ impl InlineFlowData { // calculations. // TODO: this will need to take into account type of line-height // and the vertical-align value. - if line_height > linebox_height { - linebox_height = line_height; + if line_height > max_height { + max_height = line_height; + let linebox_height = line.bounds.size.height; // Offset from the top of the linebox is 1/2 of the leading + ascent baseline_offset = text_box.run.font.metrics.ascent + (linebox_height - em_size).scale_by(0.5f); @@ -493,13 +597,6 @@ impl InlineFlowData { text_bounds.translate(&Point2D(text_box.base.position.origin.x, Au(0))) } GenericRenderBoxClass(generic_box) => { - // TODO(Issue #225): There will be different cases here for `inline-block` - // and other replaced content. - // FIXME(pcwalton): This seems clownshoes; can we remove? - generic_box.position.size.height = Au::from_px(30); - if generic_box.position.size.height > linebox_height { - linebox_height = generic_box.position.size.height; - } generic_box.position } // FIXME(pcwalton): This isn't very type safe! @@ -508,18 +605,10 @@ impl InlineFlowData { cur_box.debug_str())) } }; - - debug!("assign_height_inline: bounding box for box b%d = %?", - cur_box.id(), - bounding_box); - - linebox_bounding_box = linebox_bounding_box.union(&bounding_box); - - debug!("assign_height_inline: linebox bounding box = %?", linebox_bounding_box); } // Now go back and adjust the Y coordinates to match the baseline we determined. - for line_span.eachi |box_i| { + for line.range.eachi |box_i| { let cur_box = self.boxes[box_i]; // TODO(#226): This is completely wrong. We need to use the element's `line-height` @@ -533,14 +622,17 @@ impl InlineFlowData { }; do cur_box.with_mut_base |base| { - base.position.origin.y = offset + cur_y; + base.position.origin.y = offset + line.bounds.origin.y; } } - - cur_y = cur_y + linebox_height; } // End of `lines.each` loop. - self.common.position.size.height = cur_y; + self.common.position.size.height = + if self.lines.len() > 0 { + self.lines.last().bounds.origin.y + self.lines.last().bounds.size.height + } else { + Au(0) + } } pub fn build_display_list_inline(&self, From 7b933ac952d4f2a492e519c1e4c0bea0058f1948 Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Wed, 10 Jul 2013 14:37:09 -0700 Subject: [PATCH 6/7] Fix rebase fallout --- src/components/main/layout/float_context.rs | 2 +- src/components/main/layout/inline.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/main/layout/float_context.rs b/src/components/main/layout/float_context.rs index 5de03e89e01..a9ddded71b1 100644 --- a/src/components/main/layout/float_context.rs +++ b/src/components/main/layout/float_context.rs @@ -250,7 +250,7 @@ impl FloatContextBase{ /// Returns true if the given rect overlaps with any floats. fn collides_with_float(&self, bounds: &Rect) -> bool { - for self.float_data.each |float| { + for self.float_data.iter().advance |float| { match *float{ None => (), Some(data) => { diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 60552c480c8..286b1a8f21d 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -350,7 +350,7 @@ 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 -= 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); @@ -497,7 +497,7 @@ impl InlineFlowData { self.common.floats_out = scanner.floats_out(); // Now, go through each line and lay out the boxes inside - for self.lines.eachi |i, line| { + for self.lines.iter().advance |line| { // We need to distribute extra width based on text-align. let mut slack_width = line.available_width; if slack_width < Au(0) { @@ -526,25 +526,25 @@ impl InlineFlowData { for line.range.eachi |i| { do self.boxes[i].with_mut_base |base| { base.position.origin.x = offset_x; - offset_x += base.position.size.width; + offset_x = offset_x + base.position.size.width; } } } CSSTextAlignCenter => { - offset_x += slack_width.scale_by(0.5f); + offset_x = offset_x + slack_width.scale_by(0.5f); for line.range.eachi |i| { do self.boxes[i].with_mut_base |base| { base.position.origin.x = offset_x; - offset_x += base.position.size.width; + offset_x = offset_x + base.position.size.width; } } } CSSTextAlignRight => { - offset_x += slack_width; + offset_x = offset_x + slack_width; for line.range.eachi |i| { do self.boxes[i].with_mut_base |base| { base.position.origin.x = offset_x; - offset_x += base.position.size.width; + offset_x = offset_x + base.position.size.width; } } } From f68a548916363db5987e686de6bde5a5f2538209 Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Thu, 11 Jul 2013 20:19:13 -0700 Subject: [PATCH 7/7] Line breaking around floats --- src/components/main/layout/float_context.rs | 63 ++++- src/components/main/layout/inline.rs | 297 +++++++++++++------- 2 files changed, 252 insertions(+), 108 deletions(-) diff --git a/src/components/main/layout/float_context.rs b/src/components/main/layout/float_context.rs index a9ddded71b1..6ccef10939e 100644 --- a/src/components/main/layout/float_context.rs +++ b/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 %?", - max_left); + debug!("available_rect: collision with right float: new min_right is %?", + 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), - (Some(r_top), Some(r_bottom), None, None) => (r_top, r_bottom), + (None, None, Some(l_top), Some(l_bottom)) => (max(top, l_top), l_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 { + 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{ @@ -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)) } }; } diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 286b1a8f21d..16ad686e14e 100644 --- a/src/components/main/layout/inline.rs +++ b/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, - available_width: Au + green_zone: Size2D } 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) { + 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 { - let info = PlacementInfo { - width: Au(0), - height: in_box.position().size.height, - ceiling: self.cur_y, - max_width: self.flow.position().size.width, - f_type: FloatLeft - }; + // Initally, pretend a splitable box has 0 width. + // We will move it later if it has nonzero width + // and that causes problems. + let placement_width = if splitable { + Au(0) + } else { + 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; - self.pending_line.available_width = line_bounds.size.width; + let line_bounds = self.floats.place_between_floats(&info); - } else if new_height > self.pending_line.bounds.size.height { - // Uh-oh, adding this box increases the height of the line, so we may collide - // with some floats. - - let info = PlacementInfo { - width: in_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. + debug!("LineboxScanner: found position for line: %? using placement_info: %?", line_bounds, info); + + // Simple case: if the box fits, then we can stop here + if line_bounds.size.width > first_box.position().size.width { + debug!("LineboxScanner: case=box fits"); + return (line_bounds, first_box.position().size.width); } - 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, - self.pending_line.available_width, + in_box.position().size, + 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, self.pending_line.available_width, line_is_empty) { - CannotSplit(_) => { - error!("LineboxScanner: Tried to split unsplittable render box! %s", - in_box.debug_str()); - 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!"), + match in_box.split_to_width(ctx, available_width, line_is_empty) { + CannotSplit(_) => { + error!("LineboxScanner: Tried to split unsplittable render box! %s", + in_box.debug_str()); + return false; } - return true; - }, - 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? + 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!"); } + (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; + } + 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!"); + } + } + 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(&self,