From 0831fe35b25996851782cb55fa3cc42ba8c7f22c Mon Sep 17 00:00:00 2001 From: Jack Moffitt Date: Tue, 20 Aug 2013 17:25:58 -0600 Subject: [PATCH 1/2] Handle flow trees with block children of inlines. --- src/components/main/layout/box_builder.rs | 72 ++++++++++++++++------- src/components/main/layout/flow.rs | 9 ++- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/src/components/main/layout/box_builder.rs b/src/components/main/layout/box_builder.rs index 3e3f5a6c25b..77366fc5e01 100644 --- a/src/components/main/layout/box_builder.rs +++ b/src/components/main/layout/box_builder.rs @@ -291,34 +291,47 @@ impl LayoutTreeBuilder { /// Creates necessary box(es) and flow context(s) for the current DOM node, /// and recurses on its children. pub fn construct_recursively<'a>(&mut self, - layout_ctx: &LayoutContext, - cur_node: AbstractNode, - mut parent_generator: BoxGenerator<'a>, - mut prev_sibling_generator: Option>) - -> Option> { + layout_ctx: &LayoutContext, + cur_node: AbstractNode, + mut grandparent_generator: Option>, + mut parent_generator: BoxGenerator<'a>, + mut prev_sibling_generator: Option>) + -> Option> { debug!("Considering node: %s", cur_node.debug_str()); let box_gen_result = { + let grandparent_gen_ref = match grandparent_generator { + Some(ref mut generator) => Some(generator), + None => None, + }; let sibling_gen_ref = match prev_sibling_generator { Some(ref mut generator) => Some(generator), None => None, }; - self.box_generator_for_node(cur_node, &mut parent_generator, sibling_gen_ref) + self.box_generator_for_node(cur_node, grandparent_gen_ref, &mut parent_generator, sibling_gen_ref) }; debug!("result from generator_for_node: %?", &box_gen_result); // Skip over nodes that don't belong in the flow tree let (this_generator, next_generator) = - match box_gen_result { + match box_gen_result { NoGenerator => return prev_sibling_generator, - ParentGenerator => (parent_generator, None), + ParentGenerator => { + do parent_generator.with_clone |clone| { + (clone, None) + } + } SiblingGenerator => (prev_sibling_generator.take_unwrap(), None), NewGenerator(gen) => (gen, None), Mixed(gen, next_gen) => (gen, Some(match *next_gen { - ParentGenerator => parent_generator, + ParentGenerator => { + do parent_generator.with_clone |clone| { + clone + } + } SiblingGenerator => prev_sibling_generator.take_unwrap(), _ => fail!("Unexpect BoxGenResult") })) - }; + }; @@ -331,10 +344,17 @@ impl LayoutTreeBuilder { // recurse on child nodes. let prev_gen_cell = Cell::new(None); for child_node in cur_node.children() { - do this_generator.with_clone |clone| { - let mut prev_generator = prev_gen_cell.take(); - prev_generator = self.construct_recursively(layout_ctx, child_node, clone, prev_generator); - prev_gen_cell.put_back(prev_generator); + do parent_generator.with_clone |grandparent_clone| { + let grandparent_clone_cell = Cell::new(Some(grandparent_clone)); + do this_generator.with_clone |parent_clone| { + let mut prev_generator = prev_gen_cell.take(); + prev_generator = self.construct_recursively(layout_ctx, + child_node, + grandparent_clone_cell.take(), + parent_clone, + prev_generator); + prev_gen_cell.put_back(prev_generator); + } } } @@ -349,11 +369,12 @@ impl LayoutTreeBuilder { - pub fn box_generator_for_node<'a>(&mut self, - node: AbstractNode, - parent_generator: &mut BoxGenerator<'a>, - mut sibling_generator: Option<&mut BoxGenerator<'a>>) - -> BoxGenResult<'a> { + pub fn box_generator_for_node<'a>(&mut self, + node: AbstractNode, + grandparent_generator: Option<&mut BoxGenerator<'a>>, + parent_generator: &mut BoxGenerator<'a>, + mut sibling_generator: Option<&mut BoxGenerator<'a>>) + -> BoxGenResult<'a> { let display = if node.is_element() { match node.style().display(node.is_root()) { @@ -468,6 +489,17 @@ impl LayoutTreeBuilder { // TODO(eatkinson): blocks that are children of inlines need // to split their parent flows. + (CSSDisplayBlock, & &InlineFlow(*), _) => { + match grandparent_generator { + None => fail!("expected to have a grandparent block flow"), + Some(grandparent_gen) => { + assert!(grandparent_gen.flow.is_block_like()); + + self.create_child_generator(node, grandparent_gen, Flow_Block) + } + } + } + _ => return ParentGenerator }; @@ -590,7 +622,7 @@ impl LayoutTreeBuilder { let mut new_flow = self.make_flow(Flow_Root, root); { let new_generator = BoxGenerator::new(&mut new_flow); - self.construct_recursively(layout_ctx, root, new_generator, None); + self.construct_recursively(layout_ctx, root, None, new_generator, None); } return Ok(new_flow) } diff --git a/src/components/main/layout/flow.rs b/src/components/main/layout/flow.rs index 9228a6a9911..f5aec1d6fb6 100644 --- a/src/components/main/layout/flow.rs +++ b/src/components/main/layout/flow.rs @@ -130,7 +130,14 @@ impl FlowContext { } impl<'self> FlowContext { - pub fn leaf(&self) -> bool { + pub fn is_block_like(&self) -> bool { + match *self { + BlockFlow(*) | FloatFlow(*) => true, + _ => false, + } + } + + pub fn is_leaf(&self) -> bool { do self.with_base |base| { base.children.len() == 0 } From 7675623b348f7cdc413b71f8ef9501e5a233ab1d Mon Sep 17 00:00:00 2001 From: Jack Moffitt Date: Tue, 20 Aug 2013 20:08:22 -0600 Subject: [PATCH 2/2] Handle splitting inlines with interior blocks correctly. --- src/components/main/layout/box_builder.rs | 76 +++++++++++++++++------ 1 file changed, 56 insertions(+), 20 deletions(-) diff --git a/src/components/main/layout/box_builder.rs b/src/components/main/layout/box_builder.rs index 77366fc5e01..ce732b33be5 100644 --- a/src/components/main/layout/box_builder.rs +++ b/src/components/main/layout/box_builder.rs @@ -276,11 +276,23 @@ impl<'self> BoxGenerator<'self> { } enum BoxGenResult<'self> { - NoGenerator, - ParentGenerator, - SiblingGenerator, - NewGenerator(BoxGenerator<'self>), - Mixed(BoxGenerator<'self>, ~BoxGenResult<'self>), + NoGenerator, + ParentGenerator, + SiblingGenerator, + NewGenerator(BoxGenerator<'self>), + /// Start a new generator, but also switch the parent out for the + /// grandparent, ending the parent generator. + ReparentingGenerator(BoxGenerator<'self>), + Mixed(BoxGenerator<'self>, ~BoxGenResult<'self>), +} + +/// Determines whether the result of child box construction needs to reparent +/// or not. Reparenting is needed when a block flow is a child of an inline; +/// in that case, we need to let the level up the stack no to end the parent +/// genertor and continue with the grandparent. +enum BoxConstructResult<'self> { + Normal(Option>), + Reparent(BoxGenerator<'self>), } impl LayoutTreeBuilder { @@ -296,7 +308,7 @@ impl LayoutTreeBuilder { mut grandparent_generator: Option>, mut parent_generator: BoxGenerator<'a>, mut prev_sibling_generator: Option>) - -> Option> { + -> BoxConstructResult<'a> { debug!("Considering node: %s", cur_node.debug_str()); let box_gen_result = { let grandparent_gen_ref = match grandparent_generator { @@ -310,11 +322,13 @@ impl LayoutTreeBuilder { self.box_generator_for_node(cur_node, grandparent_gen_ref, &mut parent_generator, sibling_gen_ref) }; + let mut reparent = false; + debug!("result from generator_for_node: %?", &box_gen_result); // Skip over nodes that don't belong in the flow tree let (this_generator, next_generator) = match box_gen_result { - NoGenerator => return prev_sibling_generator, + NoGenerator => return Normal(prev_sibling_generator), ParentGenerator => { do parent_generator.with_clone |clone| { (clone, None) @@ -322,6 +336,10 @@ impl LayoutTreeBuilder { } SiblingGenerator => (prev_sibling_generator.take_unwrap(), None), NewGenerator(gen) => (gen, None), + ReparentingGenerator(gen) => { + reparent = true; + (gen, None) + } Mixed(gen, next_gen) => (gen, Some(match *next_gen { ParentGenerator => { do parent_generator.with_clone |clone| { @@ -342,18 +360,29 @@ impl LayoutTreeBuilder { debug!("point b: %s", cur_node.debug_str()); // recurse on child nodes. - let prev_gen_cell = Cell::new(None); + let prev_gen_cell = Cell::new(Normal(None)); for child_node in cur_node.children() { do parent_generator.with_clone |grandparent_clone| { let grandparent_clone_cell = Cell::new(Some(grandparent_clone)); do this_generator.with_clone |parent_clone| { - let mut prev_generator = prev_gen_cell.take(); - prev_generator = self.construct_recursively(layout_ctx, - child_node, - grandparent_clone_cell.take(), - parent_clone, - prev_generator); - prev_gen_cell.put_back(prev_generator); + match prev_gen_cell.take() { + Normal(prev_gen) => { + let prev_generator = self.construct_recursively(layout_ctx, + child_node, + grandparent_clone_cell.take(), + parent_clone, + prev_gen); + prev_gen_cell.put_back(prev_generator); + } + Reparent(prev_gen) => { + let prev_generator = self.construct_recursively(layout_ctx, + child_node, + None, + grandparent_clone_cell.take().unwrap(), + Some(prev_gen)); + prev_gen_cell.put_back(prev_generator); + } + } } } } @@ -362,8 +391,14 @@ impl LayoutTreeBuilder { self.simplify_children_of_flow(layout_ctx, this_generator.flow); match next_generator { - Some(n_gen) => Some(n_gen), - None => Some(this_generator), + Some(n_gen) => Normal(Some(n_gen)), + None => { + if reparent { + Reparent(this_generator) + } else { + Normal(Some(this_generator)) + } + } } } @@ -487,15 +522,16 @@ impl LayoutTreeBuilder { } } - // TODO(eatkinson): blocks that are children of inlines need - // to split their parent flows. + // blocks that are children of inlines need to split their parent + // flows. (CSSDisplayBlock, & &InlineFlow(*), _) => { match grandparent_generator { None => fail!("expected to have a grandparent block flow"), Some(grandparent_gen) => { assert!(grandparent_gen.flow.is_block_like()); - self.create_child_generator(node, grandparent_gen, Flow_Block) + let block_gen = self.create_child_generator(node, grandparent_gen, Flow_Block); + return ReparentingGenerator(block_gen); } } }