From 35a869d3d6ae580fb4bd5888a49fdfc78005e59d Mon Sep 17 00:00:00 2001 From: S Pradeep Kumar Date: Tue, 28 Jan 2014 12:00:32 +0900 Subject: [PATCH] Remove unnecessary ClipDisplayItems in box_.rs For ScannedTextBox and ImageBox, the ClipDisplayItem's child_list is currently always empty. ClipDisplayItem is used to implement overflow hidden and should only be created for block containers, as per http://www.w3.org/TR/CSS2/visufx.html#propdef-overflow Take care of the case when a BlockFlow has no children display items - like an ImageBox with "display: block". --- src/components/main/layout/box_.rs | 26 +--------- src/components/main/layout/flow.rs | 77 +++++++++++++++++++----------- 2 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index 3671f77aed8..9908fa1cceb 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -967,18 +967,6 @@ impl Box { match self.specific { UnscannedTextBox(_) => fail!("Shouldn't see unscanned boxes here."), ScannedTextBox(ref text_box) => { - lists.with_mut(|lists| { - let item = ~ClipDisplayItem { - base: BaseDisplayItem { - bounds: absolute_box_bounds, - extra: ExtraDisplayListData::new(self), - }, - child_list: ~[], - need_clip: false - }; - lists.lists[index].append_item(ClipDisplayItemClass(item)); - }); - let text_color = self.style().Color.color.to_gfx_color(); // Set the various text display item flags. @@ -998,7 +986,7 @@ impl Box { text_flags.set_override_underline(flow_flags.flags.override_underline()); text_flags.set_override_overline(flow_flags.flags.override_overline()); text_flags.set_override_line_through(flow_flags.flags.override_line_through()); - + let mut bounds = absolute_box_bounds.clone(); bounds.origin.x = bounds.origin.x + self.noncontent_left() + self.noncontent_inline_left(); @@ -1104,18 +1092,6 @@ impl Box { }); }, ImageBox(ref image_box) => { - lists.with_mut(|lists| { - let item = ~ClipDisplayItem { - base: BaseDisplayItem { - bounds: absolute_box_bounds, - extra: ExtraDisplayListData::new(self), - }, - child_list: ~[], - need_clip: false - }; - lists.lists[index].append_item(ClipDisplayItemClass(item)); - }); - let mut image_ref = image_box.image.borrow_mut(); let mut bounds = absolute_box_bounds.clone(); bounds.origin.x = bounds.origin.x + self.noncontent_left() diff --git a/src/components/main/layout/flow.rs b/src/components/main/layout/flow.rs index 953a008ad0a..f3070481e04 100644 --- a/src/components/main/layout/flow.rs +++ b/src/components/main/layout/flow.rs @@ -167,6 +167,9 @@ pub trait ImmutableFlowUtils { /// Returns the number of children that this flow possesses. fn child_count(self) -> uint; + /// Return true if this flow is a Block Container. + fn is_block_container(self) -> bool; + /// Returns true if this flow is a block flow, an inline flow, or a float flow. fn starts_block_flow(self) -> bool; @@ -604,6 +607,23 @@ impl<'a> ImmutableFlowUtils for &'a Flow { base(self).children.len() } + /// Return true if this flow is a Block Container. + /// + /// Except for table boxes and replaced elements, block-level boxes (`BlockFlow`) are + /// also block container boxes. + /// Non-replaced inline blocks and non-replaced table cells are also block + /// containers. + fn is_block_container(self) -> bool { + match self.class() { + // TODO: Change this when inline-blocks are supported. + InlineFlowClass => false, + BlockFlowClass => { + // FIXME: Actually check the type of the node + self.child_count() != 0 + } + } + } + /// Returns true if this flow is a block flow, an inline-block flow, or a float flow. fn starts_block_flow(self) -> bool { match self.class() { @@ -720,37 +740,36 @@ impl<'a> MutableFlowUtils for &'a mut Flow { return true; } - let mut child_lists = DisplayListCollection::new(); - child_lists.add_list(DisplayList::new()); - let child_lists = RefCell::new(child_lists); - for kid in child_iter(self) { - kid.build_display_lists(builder, dirty, 0u, &child_lists); - } - - let mut child_lists = Some(child_lists.unwrap()); - // Find parent ClipDisplayItemClass and push all child display items - // under it - // FIXME: Once we have children for InlineFlow, this might lead to - // children display items being pushed under the ClipDisplayItemClass - // created by the last box of the InlineFlow. Fix the logic. - lists.with_mut(|lists| { - let mut child_lists = child_lists.take_unwrap(); - let result = lists.lists[index].list.mut_rev_iter().position(|item| { - match *item { - ClipDisplayItemClass(ref mut item) => { - item.child_list.push_all_move(child_lists.lists.shift().list); - true - }, - _ => false, - } - }); - - if result.is_none() { - fail!("fail to find parent item"); + if self.is_block_container() { + let mut child_lists = DisplayListCollection::new(); + child_lists.add_list(DisplayList::new()); + let child_lists = RefCell::new(child_lists); + for kid in child_iter(self) { + kid.build_display_lists(builder, dirty, 0u, &child_lists); } - lists.lists.push_all_move(child_lists.lists); - }); + let mut child_lists = Some(child_lists.unwrap()); + // Find parent ClipDisplayItemClass and push all child display items + // under it + lists.with_mut(|lists| { + let mut child_lists = child_lists.take_unwrap(); + let result = lists.lists[index].list.mut_rev_iter().position(|item| { + match *item { + ClipDisplayItemClass(ref mut item) => { + item.child_list.push_all_move(child_lists.lists.shift().list); + true + }, + _ => false, + } + }); + + if result.is_none() { + fail!("fail to find parent item"); + } + + lists.lists.push_all_move(child_lists.lists); + }); + } true }