From 4233e0f163ce9728dc483f7fe7ff99056aaf1794 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 11:47:02 -0800 Subject: [PATCH 1/8] gfx: Box stacking contexts to minimize `memmove` traffic. `memmove` was showing up high in the profile when concatenating and shorting display lists. This change drastically reduces the `memmove` cost in exchange for some minor additional allocation cost. --- components/gfx/display_list/mod.rs | 2 +- components/layout/block.rs | 2 +- components/layout/display_list_builder.rs | 39 +++++++++++------------ components/layout/flex.rs | 2 +- components/layout/flow.rs | 4 +-- components/layout/inline.rs | 2 +- components/layout/list_item.rs | 2 +- components/layout/multicol.rs | 4 +-- components/layout/table.rs | 2 +- components/layout/table_caption.rs | 2 +- components/layout/table_cell.rs | 2 +- components/layout/table_colgroup.rs | 2 +- components/layout/table_row.rs | 2 +- components/layout/table_rowgroup.rs | 2 +- components/layout/table_wrapper.rs | 2 +- 15 files changed, 35 insertions(+), 36 deletions(-) diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs index aa3c974a1d9..aebdc0882c0 100644 --- a/components/gfx/display_list/mod.rs +++ b/components/gfx/display_list/mod.rs @@ -508,7 +508,7 @@ pub struct StackingContext { pub layer_info: Option, /// Children of this StackingContext. - pub children: Vec, + pub children: Vec>, } impl StackingContext { diff --git a/components/layout/block.rs b/components/layout/block.rs index a538a768efd..56785dd5eda 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -2109,7 +2109,7 @@ impl Flow for BlockFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.collect_stacking_contexts_for_block(parent_id, contexts) } diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 04beaca2064..5714bdbf6ee 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -67,8 +67,7 @@ pub struct DisplayListBuildState<'a> { } impl<'a> DisplayListBuildState<'a> { - pub fn new(layout_context: &'a LayoutContext, - stacking_context_id: StackingContextId) + pub fn new(layout_context: &'a LayoutContext, stacking_context_id: StackingContextId) -> DisplayListBuildState<'a> { DisplayListBuildState { layout_context: layout_context, @@ -287,7 +286,7 @@ pub trait FragmentDisplayListBuilding { base_flow: &BaseFlow, scroll_policy: ScrollPolicy, mode: StackingContextCreationMode) - -> StackingContext; + -> Box; } fn handle_overlapping_radii(size: &Size2D, radii: &BorderRadii) -> BorderRadii { @@ -1277,7 +1276,7 @@ impl FragmentDisplayListBuilding for Fragment { base_flow: &BaseFlow, scroll_policy: ScrollPolicy, mode: StackingContextCreationMode) - -> StackingContext { + -> Box { let border_box = match mode { StackingContextCreationMode::InnerScrollWrapper => { Rect::new(Point2D::zero(), base_flow.overflow.scroll.size) @@ -1411,18 +1410,18 @@ impl FragmentDisplayListBuilding for Fragment { _ => StackingContextType::Real, }; - StackingContext::new(id, - context_type, - &border_box, - &overflow, - self.effective_z_index(), - filters, - self.style().get_effects().mix_blend_mode, - transform, - perspective, - establishes_3d_context, - scrolls_overflow_area, - layer_info) + Box::new(StackingContext::new(id, + context_type, + &border_box, + &overflow, + self.effective_z_index(), + filters, + self.style().get_effects().mix_blend_mode, + transform, + perspective, + establishes_3d_context, + scrolls_overflow_area, + layer_info)) } fn clipping_region_for_children(&self, @@ -1600,7 +1599,7 @@ impl FragmentDisplayListBuilding for Fragment { pub trait BlockFlowDisplayListBuilding { fn collect_stacking_contexts_for_block(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId; fn build_display_list_for_block(&mut self, state: &mut DisplayListBuildState, @@ -1610,7 +1609,7 @@ pub trait BlockFlowDisplayListBuilding { impl BlockFlowDisplayListBuilding for BlockFlow { fn collect_stacking_contexts_for_block(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { if !self.fragment.establishes_stacking_context() && !self.establishes_pseudo_stacking_context() { @@ -1742,7 +1741,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { pub trait InlineFlowDisplayListBuilding { fn collect_stacking_contexts_for_inline(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId; fn build_display_list_for_inline_fragment_at_index(&mut self, state: &mut DisplayListBuildState, @@ -1753,7 +1752,7 @@ pub trait InlineFlowDisplayListBuilding { impl InlineFlowDisplayListBuilding for InlineFlow { fn collect_stacking_contexts_for_inline(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.base.stacking_context_id = parent_id; diff --git a/components/layout/flex.rs b/components/layout/flex.rs index c4e08ed6ffa..5ae07604f99 100644 --- a/components/layout/flex.rs +++ b/components/layout/flex.rs @@ -430,7 +430,7 @@ impl Flow for FlexFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/flow.rs b/components/layout/flow.rs index cf8b1be451d..6dfd78ec841 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -224,7 +224,7 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static { fn collect_stacking_contexts(&mut self, _parent_id: StackingContextId, - _: &mut Vec) + _: &mut Vec>) -> StackingContextId; /// If this is a float, places it. The default implementation does nothing. @@ -1203,7 +1203,7 @@ impl BaseFlow { pub fn collect_stacking_contexts_for_children(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) { + contexts: &mut Vec>) { for kid in self.children.iter_mut() { kid.collect_stacking_contexts(parent_id, contexts); } diff --git a/components/layout/inline.rs b/components/layout/inline.rs index ed07f339720..291efea790d 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1750,7 +1750,7 @@ impl Flow for InlineFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.collect_stacking_contexts_for_inline(parent_id, contexts) } diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs index 701ac4ecdf9..337ba76120b 100644 --- a/components/layout/list_item.rs +++ b/components/layout/list_item.rs @@ -151,7 +151,7 @@ impl Flow for ListItemFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/multicol.rs b/components/layout/multicol.rs index 36815c82533..4b7ac7d8814 100644 --- a/components/layout/multicol.rs +++ b/components/layout/multicol.rs @@ -186,7 +186,7 @@ impl Flow for MulticolFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } @@ -271,7 +271,7 @@ impl Flow for MulticolColumnFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/table.rs b/components/layout/table.rs index eb0c57f615a..0c8d8865a8d 100644 --- a/components/layout/table.rs +++ b/components/layout/table.rs @@ -467,7 +467,7 @@ impl Flow for TableFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/table_caption.rs b/components/layout/table_caption.rs index 4bb783154d2..e1706fc53cd 100644 --- a/components/layout/table_caption.rs +++ b/components/layout/table_caption.rs @@ -83,7 +83,7 @@ impl Flow for TableCaptionFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/table_cell.rs b/components/layout/table_cell.rs index 0faebcb3cf2..523ea86a5f5 100644 --- a/components/layout/table_cell.rs +++ b/components/layout/table_cell.rs @@ -193,7 +193,7 @@ impl Flow for TableCellFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/table_colgroup.rs b/components/layout/table_colgroup.rs index 9f1a03e7e0b..32d2c693131 100644 --- a/components/layout/table_colgroup.rs +++ b/components/layout/table_colgroup.rs @@ -96,7 +96,7 @@ impl Flow for TableColGroupFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - _: &mut Vec) + _: &mut Vec>) -> StackingContextId { parent_id } diff --git a/components/layout/table_row.rs b/components/layout/table_row.rs index 7fc8381f76f..45e8bca12ec 100644 --- a/components/layout/table_row.rs +++ b/components/layout/table_row.rs @@ -435,7 +435,7 @@ impl Flow for TableRowFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/table_rowgroup.rs b/components/layout/table_rowgroup.rs index 6971df77787..44da4878119 100644 --- a/components/layout/table_rowgroup.rs +++ b/components/layout/table_rowgroup.rs @@ -212,7 +212,7 @@ impl Flow for TableRowGroupFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } diff --git a/components/layout/table_wrapper.rs b/components/layout/table_wrapper.rs index d3224126929..a8e16812efd 100644 --- a/components/layout/table_wrapper.rs +++ b/components/layout/table_wrapper.rs @@ -451,7 +451,7 @@ impl Flow for TableWrapperFlow { fn collect_stacking_contexts(&mut self, parent_id: StackingContextId, - contexts: &mut Vec) + contexts: &mut Vec>) -> StackingContextId { self.block_flow.collect_stacking_contexts(parent_id, contexts) } From 983576ebaaed03d5d5b52d52778fd47cff57da8a Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 11:48:49 -0800 Subject: [PATCH 2/8] gfx: Avoid copying stacking contexts around so much during stacking context creation. --- components/layout/display_list_builder.rs | 32 ++++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 5714bdbf6ee..b76bdc9ff5d 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -1643,18 +1643,24 @@ impl BlockFlowDisplayListBuilding for BlockFlow { StackingContextCreationMode::PseudoFloat }; - let mut stacking_context = - self.fragment.create_stacking_context(stacking_context_id, - &self.base, - ScrollPolicy::Scrollable, - creation_mode); - let (mut floating, mut positioned) = child_contexts.into_iter().partition(|context| { - context.context_type == StackingContextType::PseudoFloat - }); + let stacking_context_index = contexts.len(); + contexts.push(self.fragment.create_stacking_context(stacking_context_id, + &self.base, + ScrollPolicy::Scrollable, + creation_mode)); - stacking_context.children.append(&mut floating); - contexts.push(stacking_context); - contexts.append(&mut positioned); + let mut floating = vec![]; + for child_context in child_contexts.into_iter() { + if child_context.context_type == StackingContextType::PseudoFloat { + // Floating. + floating.push(child_context) + } else { + // Positioned. + contexts.push(child_context) + } + } + + contexts[stacking_context_index].children = floating; return stacking_context_id; } @@ -1670,7 +1676,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { &self.base, scroll_policy, StackingContextCreationMode::InnerScrollWrapper); - inner_stacking_context.children.append(&mut child_contexts); + inner_stacking_context.children = child_contexts; let mut outer_stacking_context = self.fragment.create_stacking_context( stacking_context_id, @@ -1685,7 +1691,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { &self.base, scroll_policy, StackingContextCreationMode::Normal); - stacking_context.children.append(&mut child_contexts); + stacking_context.children = child_contexts; stacking_context }; From 940bff1f9c5e402fe6a8049afb0cd29ff26bda48 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 11:50:21 -0800 Subject: [PATCH 3/8] gfx: Stop cloning clipping regions so much. Clipping regions can contain vectors and so can be expensive to copy. --- components/gfx/display_list/mod.rs | 9 +-- components/layout/block.rs | 13 ++-- components/layout/display_list_builder.rs | 72 +++++++++++------------ components/layout/flow.rs | 3 +- components/layout/inline.rs | 7 ++- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs index aebdc0882c0..9b9c791569f 100644 --- a/components/gfx/display_list/mod.rs +++ b/components/gfx/display_list/mod.rs @@ -761,16 +761,13 @@ impl ClippingRegion { } } - /// Returns the intersection of this clipping region and the given rectangle. + /// Mutates this clipping region to intersect with the given rectangle. /// /// TODO(pcwalton): This could more eagerly eliminate complex clipping regions, at the cost of /// complexity. #[inline] - pub fn intersect_rect(self, rect: &Rect) -> ClippingRegion { - ClippingRegion { - main: self.main.intersection(rect).unwrap_or(Rect::zero()), - complex: self.complex, - } + pub fn intersect_rect(&mut self, rect: &Rect) { + self.main = self.main.intersection(rect).unwrap_or(Rect::zero()) } /// Returns true if this clipping region might be nonempty. This can return false positives, diff --git a/components/layout/block.rs b/components/layout/block.rs index 56785dd5eda..a348c541ebf 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -1954,8 +1954,10 @@ impl Flow for BlockFlow { self.base.position.size.to_physical(self.base.writing_mode); // Compute the origin and clipping rectangle for children. + // + // `clip` is in the child coordinate system. + let mut clip; let origin_for_children; - let clip_in_child_coordinate_system; let is_stacking_context = self.fragment.establishes_stacking_context(); if is_stacking_context { // We establish a stacking context, so the position of our children is vertically @@ -1966,12 +1968,11 @@ impl Flow for BlockFlow { // FIXME(pcwalton): Is this vertical-writing-direction-safe? let margin = self.fragment.margin.to_physical(self.base.writing_mode); origin_for_children = Point2D::new(-margin.left, Au(0)); - clip_in_child_coordinate_system = - self.base.clip.translate(&-self.base.stacking_relative_position); + clip = self.base.clip.translate(&-self.base.stacking_relative_position); } else { let relative_offset = relative_offset.to_physical(self.base.writing_mode); origin_for_children = self.base.stacking_relative_position + relative_offset; - clip_in_child_coordinate_system = self.base.clip.clone(); + clip = self.base.clip.clone(); } let stacking_relative_position_of_display_port_for_children = @@ -2003,8 +2004,8 @@ impl Flow for BlockFlow { .early_absolute_position_info .relative_containing_block_mode, CoordinateSystem::Own); - let clip = self.fragment.clipping_region_for_children( - &clip_in_child_coordinate_system, + self.fragment.adjust_clipping_region_for_children( + &mut clip, &stacking_relative_border_box, self.base.flags.contains(IS_ABSOLUTELY_POSITIONED)); diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index b76bdc9ff5d..f1e1b4b806b 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -231,19 +231,17 @@ pub trait FragmentDisplayListBuilding { clip: &ClippingRegion, stacking_relative_display_port: &Rect); - /// Returns the appropriate clipping region for descendants of this fragment. - fn clipping_region_for_children(&self, - current_clip: &ClippingRegion, - stacking_relative_border_box: &Rect, - is_absolutely_positioned: bool) - -> ClippingRegion; + /// Adjusts the clipping region for descendants of this fragment as appropriate. + fn adjust_clipping_region_for_children(&self, + current_clip: &mut ClippingRegion, + stacking_relative_border_box: &Rect, + is_absolutely_positioned: bool); - /// Calculates the clipping rectangle for a fragment, taking the `clip` property into account + /// Adjusts the clipping rectangle for a fragment to take the `clip` property into account /// per CSS 2.1 § 11.1.2. - fn calculate_style_specified_clip(&self, - parent_clip: &ClippingRegion, - stacking_relative_border_box: &Rect) - -> ClippingRegion; + fn adjust_clip_for_style(&self, + parent_clip: &mut ClippingRegion, + stacking_relative_border_box: &Rect); /// Builds the display items necessary to paint the selection and/or caret for this fragment, /// if any. @@ -481,7 +479,8 @@ impl FragmentDisplayListBuilding for Fragment { // Clip. // // TODO: Check the bounds to see if a clip item is actually required. - let clip = clip.clone().intersect_rect(&bounds); + let mut clip = clip.clone(); + clip.intersect_rect(&bounds); // Background image should be positioned on the padding box basis. let border = style.logical_border_width().to_physical(style.writing_mode); @@ -580,7 +579,8 @@ impl FragmentDisplayListBuilding for Fragment { clip: &ClippingRegion, gradient: &LinearGradient, style: &ComputedValues) { - let clip = clip.clone().intersect_rect(absolute_bounds); + let mut clip = clip.clone(); + clip.intersect_rect(absolute_bounds); // This is the distance between the center and the ending point; i.e. half of the distance // between the starting point and the ending point. @@ -893,15 +893,14 @@ impl FragmentDisplayListBuilding for Fragment { }), DisplayListSection::Content); } - fn calculate_style_specified_clip(&self, - parent_clip: &ClippingRegion, - stacking_relative_border_box: &Rect) - -> ClippingRegion { + fn adjust_clip_for_style(&self, + parent_clip: &mut ClippingRegion, + stacking_relative_border_box: &Rect) { // Account for `clip` per CSS 2.1 § 11.1.2. let style_clip_rect = match (self.style().get_box().position, self.style().get_effects().clip.0) { (position::T::absolute, Some(style_clip_rect)) => style_clip_rect, - _ => return (*parent_clip).clone(), + _ => return, }; // FIXME(pcwalton, #2795): Get the real container size. @@ -910,7 +909,7 @@ impl FragmentDisplayListBuilding for Fragment { let right = style_clip_rect.right.unwrap_or(stacking_relative_border_box.size.width); let bottom = style_clip_rect.bottom.unwrap_or(stacking_relative_border_box.size.height); let clip_size = Size2D::new(right - clip_origin.x, bottom - clip_origin.y); - (*parent_clip).clone().intersect_rect(&Rect::new(clip_origin, clip_size)) + parent_clip.intersect_rect(&Rect::new(clip_origin, clip_size)) } fn build_display_items_for_selection_if_necessary(&self, @@ -995,7 +994,8 @@ impl FragmentDisplayListBuilding for Fragment { // Calculate the clip rect. If there's nothing to render at all, don't even construct // display list items. - let clip = self.calculate_style_specified_clip(clip, &stacking_relative_border_box); + let mut clip = (*clip).clone(); + self.adjust_clip_for_style(&mut clip, &stacking_relative_border_box); if !clip.might_intersect_rect(&stacking_relative_border_box) { return; } @@ -1424,19 +1424,17 @@ impl FragmentDisplayListBuilding for Fragment { layer_info)) } - fn clipping_region_for_children(&self, - current_clip: &ClippingRegion, - stacking_relative_border_box: &Rect, - is_absolutely_positioned: bool) - -> ClippingRegion { + fn adjust_clipping_region_for_children(&self, + current_clip: &mut ClippingRegion, + stacking_relative_border_box: &Rect, + is_absolutely_positioned: bool) { // Don't clip if we're text. if self.is_scanned_text_fragment() { - return (*current_clip).clone() + return } // Account for style-specified `clip`. - let mut current_clip = self.calculate_style_specified_clip(current_clip, - stacking_relative_border_box); + self.adjust_clip_for_style(current_clip, stacking_relative_border_box); // Clip according to the values of `overflow-x` and `overflow-y`. // @@ -1452,7 +1450,7 @@ impl FragmentDisplayListBuilding for Fragment { let max_x = cmp::min(bounds.max_x(), stacking_relative_border_box.max_x()); bounds.origin.x = cmp::max(bounds.origin.x, stacking_relative_border_box.origin.x); bounds.size.width = max_x - bounds.origin.x; - current_clip = current_clip.intersect_rect(&bounds) + current_clip.intersect_rect(&bounds) } _ => {} } @@ -1464,12 +1462,10 @@ impl FragmentDisplayListBuilding for Fragment { let max_y = cmp::min(bounds.max_y(), stacking_relative_border_box.max_y()); bounds.origin.y = cmp::max(bounds.origin.y, stacking_relative_border_box.origin.y); bounds.size.height = max_y - bounds.origin.y; - current_clip = current_clip.intersect_rect(&bounds) + current_clip.intersect_rect(&bounds) } _ => {} } - - current_clip } fn build_display_list_for_text_fragment(&self, @@ -1715,10 +1711,14 @@ impl BlockFlowDisplayListBuilding for BlockFlow { }; // Add the box that starts the block context. - let clip = if self.fragment.establishes_stacking_context() { - self.base.clip.translate(&-self.base.stacking_relative_position) + let translated_clip = if self.fragment.establishes_stacking_context() { + Some(self.base.clip.translate(&-self.base.stacking_relative_position)) } else { - self.base.clip.clone() + None + }; + let clip = match translated_clip { + Some(ref translated_clip) => translated_clip, + None => &self.base.clip, }; self.fragment @@ -1732,7 +1732,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { .relative_containing_block_mode, border_painting_mode, background_border_section, - &clip, + clip, &self.base.stacking_relative_position_of_display_port); // Add children. diff --git a/components/layout/flow.rs b/components/layout/flow.rs index 6dfd78ec841..a02b718d8cd 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -1184,7 +1184,8 @@ impl BaseFlow { for item in items.iter() { let base_item = item.item.base(); - let paint_bounds = base_item.clip.clone().intersect_rect(&base_item.bounds); + let mut paint_bounds = base_item.clip.clone(); + paint_bounds.intersect_rect(&base_item.bounds); if !paint_bounds.might_be_nonempty() { continue; } diff --git a/components/layout/inline.rs b/components/layout/inline.rs index 291efea790d..1212f8a3cfc 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1680,9 +1680,10 @@ impl Flow for InlineFlow { CoordinateSystem::Parent); let stacking_relative_content_box = fragment.stacking_relative_content_box(&stacking_relative_border_box); - let clip = fragment.clipping_region_for_children(&self.base.clip, - &stacking_relative_border_box, - false); + let mut clip = self.base.clip.clone(); + fragment.adjust_clipping_region_for_children(&mut clip, + &stacking_relative_border_box, + false); let is_positioned = fragment.is_positioned(); match fragment.specific { SpecificFragmentInfo::InlineBlock(ref mut info) => { From dd5c574d7fc58e1e53ca877ef257eb5990c3e0ea Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 11:51:05 -0800 Subject: [PATCH 4/8] gfx: Switch offsets to an FNV hasher. SipHash traffic was showing up high in the profile. Unfortunately, this necessitated a manual implementation of the serde traits. --- components/gfx/display_list/mod.rs | 80 ++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs index 9b9c791569f..ddd98a07d5e 100644 --- a/components/gfx/display_list/mod.rs +++ b/components/gfx/display_list/mod.rs @@ -21,15 +21,22 @@ use euclid::approxeq::ApproxEq; use euclid::num::Zero; use euclid::rect::TypedRect; use euclid::{Matrix2D, Matrix4, Point2D, Rect, SideOffsets2D, Size2D}; +use fnv::FnvHasher; use gfx_traits::{LayerId, ScrollPolicy}; use heapsize::HeapSizeOf; use msg::constellation_msg::PipelineId; use net_traits::image::base::Image; use paint_context::PaintContext; use range::Range; +use serde::de::{self, Deserialize, Deserializer, MapVisitor, Visitor}; +use serde::ser::impls::MapIteratorVisitor; +use serde::ser::{Serialize, Serializer}; use std::cmp::Ordering; use std::collections::HashMap; use std::fmt; +use std::hash::{BuildHasherDefault, Hash}; +use std::marker::PhantomData; +use std::ops::{Deref, DerefMut}; use std::sync::Arc; use style::computed_values::{border_style, cursor, filter, image_rendering, mix_blend_mode}; use style::computed_values::{pointer_events}; @@ -141,10 +148,75 @@ pub struct StackingContextOffsets { pub outlines: u32, } +/// A FNV-based hash map. This is not serializable by `serde` by default, so we provide an +/// implementation ourselves. +pub struct FnvHashMap(pub HashMap>); + +impl Deref for FnvHashMap { + type Target = HashMap>; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for FnvHashMap { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl Serialize for FnvHashMap where K: Eq + Hash + Serialize, V: Serialize { + #[inline] + fn serialize(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer { + serializer.visit_map(MapIteratorVisitor::new(self.iter(), Some(self.len()))) + } +} + +impl Deserialize for FnvHashMap where K: Eq + Hash + Deserialize, V: Deserialize { + #[inline] + fn deserialize(deserializer: &mut D) -> Result where D: Deserializer { + deserializer.visit_map(FnvHashMapVisitor::new()) + } +} +/// A visitor that produces a map. +pub struct FnvHashMapVisitor { + marker: PhantomData>, +} + +impl FnvHashMapVisitor { + /// Construct a `FnvHashMapVisitor`. + pub fn new() -> Self { + FnvHashMapVisitor { + marker: PhantomData, + } + } +} + +impl Visitor for FnvHashMapVisitor where K: Eq + Hash + Deserialize, V: Deserialize { + type Value = FnvHashMap; + + #[inline] + fn visit_unit(&mut self) -> Result, E> where E: de::Error { + Ok(FnvHashMap(HashMap::with_hasher(Default::default()))) + } + + #[inline] + fn visit_map(&mut self, mut visitor: Visitor) + -> Result, Visitor::Error> + where Visitor: MapVisitor { + let mut values = FnvHashMap(HashMap::with_hasher(Default::default())); + while let Some((key, value)) = try!(visitor.visit()) { + HashMap::insert(&mut values, key, value); + } + try!(visitor.end()); + Ok(values) + } +} + #[derive(HeapSizeOf, Deserialize, Serialize)] pub struct DisplayList { pub list: Vec, - pub offsets: HashMap, + pub offsets: FnvHashMap, pub root_stacking_context: StackingContext, } @@ -157,7 +229,7 @@ impl DisplayList { None => panic!("Tried to create empty display list."), }; - let mut offsets = HashMap::new(); + let mut offsets = FnvHashMap(HashMap::with_hasher(Default::default())); DisplayList::sort_and_count_stacking_contexts(&mut root_stacking_context, &mut offsets, 0); let mut display_list = DisplayList { @@ -201,7 +273,9 @@ impl DisplayList { fn sort_and_count_stacking_contexts( stacking_context: &mut StackingContext, - offsets: &mut HashMap, + offsets: &mut HashMap>, mut current_offset: u32) -> u32 { stacking_context.children.sort(); From 72a52e23e052ec1461ebf5d4e8e6fd7ab8019e8d Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 15:41:02 -0800 Subject: [PATCH 5/8] layout: Don't call `establishes_stacking_context()` so much. That function is expensive because it has to check a lot. --- components/layout/block.rs | 23 ++++++++++++++++++----- components/layout/display_list_builder.rs | 13 +++++++------ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/components/layout/block.rs b/components/layout/block.rs index a348c541ebf..7b04391cd14 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -1622,14 +1622,18 @@ impl BlockFlow { } } - pub fn establishes_pseudo_stacking_context(&self) -> bool { + pub fn block_stacking_context_type(&self) -> BlockStackingContextType { if self.fragment.establishes_stacking_context() { - return false; + return BlockStackingContextType::StackingContext } - self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) || - self.fragment.style.get_box().position != position::T::static_ || - self.base.flags.is_float() + if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) || + self.fragment.style.get_box().position != position::T::static_ || + self.base.flags.is_float() { + BlockStackingContextType::PseudoStackingContext + } else { + BlockStackingContextType::NonstackingContext + } } pub fn has_scrolling_overflow(&self) -> bool { @@ -3037,3 +3041,12 @@ impl ISizeAndMarginsComputer for InlineBlockReplaced { MaybeAuto::Specified(fragment.content_inline_size()) } } + +/// A stacking context, a pseudo-stacking context, or a non-stacking context. +#[derive(Copy, Clone, PartialEq)] +pub enum BlockStackingContextType { + NonstackingContext, + PseudoStackingContext, + StackingContext, +} + diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index f1e1b4b806b..500b2d9922e 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -12,7 +12,7 @@ use app_units::{Au, AU_PER_PX}; use azure::azure_hl::Color; -use block::BlockFlow; +use block::{BlockFlow, BlockStackingContextType}; use canvas_traits::{CanvasMsg, CanvasPixelData, CanvasData, FromLayoutMsg}; use context::LayoutContext; use euclid::num::Zero; @@ -1607,8 +1607,8 @@ impl BlockFlowDisplayListBuilding for BlockFlow { parent_id: StackingContextId, contexts: &mut Vec>) -> StackingContextId { - if !self.fragment.establishes_stacking_context() && - !self.establishes_pseudo_stacking_context() { + let block_stacking_context_type = self.block_stacking_context_type(); + if block_stacking_context_type == BlockStackingContextType::NonstackingContext { self.base.stacking_context_id = parent_id; self.base.collect_stacking_contexts_for_children(parent_id, contexts); return parent_id; @@ -1630,7 +1630,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { self.base.collect_stacking_contexts_for_children(inner_stacking_context_id, &mut child_contexts); - if self.establishes_pseudo_stacking_context() { + if block_stacking_context_type == BlockStackingContextType::PseudoStackingContext { let creation_mode = if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) || self.fragment.style.get_box().position != position::T::static_ { StackingContextCreationMode::PseudoPositioned @@ -1698,10 +1698,11 @@ impl BlockFlowDisplayListBuilding for BlockFlow { fn build_display_list_for_block(&mut self, state: &mut DisplayListBuildState, border_painting_mode: BorderPaintingMode) { + let establishes_stacking_context = self.fragment.establishes_stacking_context(); let background_border_section = if self.base.flags.is_float() { DisplayListSection::BackgroundAndBorders } else if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) { - if self.fragment.establishes_stacking_context() { + if establishes_stacking_context { DisplayListSection::BackgroundAndBorders } else { DisplayListSection::BlockBackgroundsAndBorders @@ -1711,7 +1712,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { }; // Add the box that starts the block context. - let translated_clip = if self.fragment.establishes_stacking_context() { + let translated_clip = if establishes_stacking_context { Some(self.base.clip.translate(&-self.base.stacking_relative_position)) } else { None From f4b95dd00b00c0b43a424712b74b28577f139d79 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 15:34:33 -0800 Subject: [PATCH 6/8] layout: Remove the `validate_display_list_geometry` debugging tool. I don't think anyone was using it, and it's incompatible with taking display lists out of flows. --- components/layout/block.rs | 4 --- components/layout/display_list_builder.rs | 4 --- components/layout/flex.rs | 5 ---- components/layout/flow.rs | 31 ----------------------- components/layout/list_item.rs | 4 --- components/util/opts.rs | 11 -------- 6 files changed, 59 deletions(-) diff --git a/components/layout/block.rs b/components/layout/block.rs index 7b04391cd14..19bb6e71ce2 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -64,7 +64,6 @@ use style::properties::ComputedValues; use style::values::computed::{LengthOrNone, LengthOrPercentageOrNone}; use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto}; use util::geometry::MAX_RECT; -use util::opts; use util::print_tree::PrintTree; /// Information specific to floated blocks. @@ -2122,9 +2121,6 @@ impl Flow for BlockFlow { fn build_display_list(&mut self, state: &mut DisplayListBuildState) { self.build_display_list_for_block(state, BorderPaintingMode::Separate); self.fragment.restyle_damage.remove(REPAINT); - if opts::get().validate_display_list_geometry { - self.base.validate_display_list_geometry(); - } } fn repair_style(&mut self, new_style: &Arc) { diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 500b2d9922e..abe06f3642e 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -1853,10 +1853,6 @@ impl InlineFlowDisplayListBuilding for InlineFlow { self.base.build_display_items_for_debugging_tint(state, self.fragments.fragments[0].node); } - - if opts::get().validate_display_list_geometry { - self.base.validate_display_list_geometry(); - } } } diff --git a/components/layout/flex.rs b/components/layout/flex.rs index 5ae07604f99..b53bb697c49 100644 --- a/components/layout/flex.rs +++ b/components/layout/flex.rs @@ -31,7 +31,6 @@ use style::logical_geometry::LogicalSize; use style::properties::ComputedValues; use style::properties::style_structs; use style::values::computed::LengthOrPercentageOrAuto; -use util::opts; // A mode describes which logical axis a flex axis is parallel with. // The logical axises are inline and block, the flex axises are main and cross. @@ -422,10 +421,6 @@ impl Flow for FlexFlow { fn build_display_list(&mut self, state: &mut DisplayListBuildState) { self.build_display_list_for_flex(state); - - if opts::get().validate_display_list_geometry { - self.block_flow.base.validate_display_list_geometry(); - } } fn collect_stacking_contexts(&mut self, diff --git a/components/layout/flow.rs b/components/layout/flow.rs index a02b718d8cd..def1f7bf8a7 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -1167,37 +1167,6 @@ impl BaseFlow { p as usize } - /// Ensures that all display list items generated by this flow are within the flow's overflow - /// rect. This should only be used for debugging. - pub fn validate_display_list_geometry(&self) { - // FIXME(pcwalton, #2795): Get the real container size. - let container_size = Size2D::zero(); - let position_with_overflow = self.position - .to_physical(self.writing_mode, container_size) - .union(&self.overflow.paint); - let bounds = Rect::new(self.stacking_relative_position, position_with_overflow.size); - - let items = match self.display_list_building_result { - Some(ref items) => items, - None => return, - }; - - for item in items.iter() { - let base_item = item.item.base(); - let mut paint_bounds = base_item.clip.clone(); - paint_bounds.intersect_rect(&base_item.bounds); - if !paint_bounds.might_be_nonempty() { - continue; - } - - if bounds.union(&paint_bounds.bounding_rect()) != bounds { - error!("DisplayList item {:?} outside of Flow overflow ({:?})", - item.item, - paint_bounds); - } - } - } - pub fn flow_id(&self) -> usize { return self as *const BaseFlow as usize; } diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs index 337ba76120b..6c8817ec1da 100644 --- a/components/layout/list_item.rs +++ b/components/layout/list_item.rs @@ -25,7 +25,6 @@ use style::computed_values::{list_style_type, position}; use style::logical_geometry::LogicalSize; use style::properties::ComputedValues; use text; -use util::opts; /// A block with the CSS `display` property equal to `list-item`. #[derive(Debug)] @@ -144,9 +143,6 @@ impl Flow for ListItemFlow { fn build_display_list(&mut self, state: &mut DisplayListBuildState) { self.build_display_list_for_list_item(state); - if opts::get().validate_display_list_geometry { - self.block_flow.base.validate_display_list_geometry(); - } } fn collect_stacking_contexts(&mut self, diff --git a/components/util/opts.rs b/components/util/opts.rs index 90135fb78ca..410b9da7c11 100644 --- a/components/util/opts.rs +++ b/components/util/opts.rs @@ -163,9 +163,6 @@ pub struct Opts { /// Emits notifications when there is a relayout. pub relayout_event: bool, - /// Whether to show an error when display list geometry escapes flow overflow regions. - pub validate_display_list_geometry: bool, - /// Whether Style Sharing Cache is used pub disable_share_style_cache: bool, @@ -254,9 +251,6 @@ pub struct DebugOptions { /// Write layout trace to an external file for debugging. pub trace_layout: bool, - /// Display an error when display list geometry escapes overflow region. - pub validate_display_list_geometry: bool, - /// Disable the style sharing cache. pub disable_share_style_cache: bool, @@ -308,7 +302,6 @@ impl DebugOptions { "show-parallel-layout" => debug_options.show_parallel_layout = true, "paint-flashing" => debug_options.paint_flashing = true, "trace-layout" => debug_options.trace_layout = true, - "validate-display-list-geometry" => debug_options.validate_display_list_geometry = true, "disable-share-style-cache" => debug_options.disable_share_style_cache = true, "convert-mouse-to-touch" => debug_options.convert_mouse_to_touch = true, "replace-surrogates" => debug_options.replace_surrogates = true, @@ -351,8 +344,6 @@ pub fn print_debug_usage(app: &str) -> ! { print_option("show-parallel-layout", "Mark which thread laid each flow out with colors."); print_option("paint-flashing", "Overlay repainted areas with a random color."); print_option("trace-layout", "Write layout trace to an external file for debugging."); - print_option("validate-display-list-geometry", - "Display an error when display list geometry escapes overflow region."); print_option("disable-share-style-cache", "Disable the style sharing cache."); print_option("parallel-display-list-building", "Build display lists in parallel."); @@ -486,7 +477,6 @@ pub fn default_opts() -> Opts { dump_display_list_optimized: false, dump_layer_tree: false, relayout_event: false, - validate_display_list_geometry: false, profile_script_events: false, profile_heartbeats: false, disable_share_style_cache: false, @@ -727,7 +717,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult { dump_display_list_optimized: debug_options.dump_display_list_optimized, dump_layer_tree: debug_options.dump_layer_tree, relayout_event: debug_options.relayout_event, - validate_display_list_geometry: debug_options.validate_display_list_geometry, disable_share_style_cache: debug_options.disable_share_style_cache, convert_mouse_to_touch: debug_options.convert_mouse_to_touch, exit_after_load: opt_match.opt_present("x"), From d3d2dd05f23918d27e554de98663ddcfd040235a Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 15:41:50 -0800 Subject: [PATCH 7/8] layout: Switch display list building from bottom-up to top-down. This eliminates a lot of allocations and O(n^2) behavior. --- components/layout/display_list_builder.rs | 33 +++-------------------- components/layout/flow.rs | 8 +----- components/layout/layout_thread.rs | 13 +++++---- components/layout/sequential.rs | 13 ++++++--- components/layout/traversal.rs | 26 ++++++++++-------- 5 files changed, 35 insertions(+), 58 deletions(-) diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index abe06f3642e..0e3b23e0912 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -18,7 +18,7 @@ use context::LayoutContext; use euclid::num::Zero; use euclid::{Matrix4, Point2D, Point3D, Rect, SideOffsets2D, Size2D}; use flex::FlexFlow; -use flow::{self, BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED}; +use flow::{BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED}; use flow_ref; use fragment::{CoordinateSystem, Fragment, HAS_LAYER, ImageFragmentInfo, ScannedTextFragmentInfo}; use fragment::{SpecificFragmentInfo}; @@ -86,21 +86,15 @@ impl<'a> DisplayListBuildState<'a> { }); } - fn append_from(&mut self, other_list: &mut Option>) { - if let Some(mut other) = other_list.take() { - self.items.append(&mut other); - } - } - fn stacking_context_id(&self) -> StackingContextId { self.stacking_context_id_stack.last().unwrap().clone() } - fn push_stacking_context_id(&mut self, stacking_context_id: StackingContextId) { + pub fn push_stacking_context_id(&mut self, stacking_context_id: StackingContextId) { self.stacking_context_id_stack.push(stacking_context_id); } - fn pop_stacking_context_id(&mut self) { + pub fn pop_stacking_context_id(&mut self) { self.stacking_context_id_stack.pop(); assert!(!self.stacking_context_id_stack.is_empty()); } @@ -1736,11 +1730,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow { clip, &self.base.stacking_relative_position_of_display_port); - // Add children. - for kid in self.base.children.iter_mut() { - state.append_from(&mut flow::mut_base(kid).display_list_building_result); - } - self.base.build_display_items_for_debugging_tint(state, self.fragment.node); } } @@ -1805,22 +1794,6 @@ impl InlineFlowDisplayListBuilding for InlineFlow { DisplayListSection::Content, &self.base.clip, &self.base.stacking_relative_position_of_display_port); - - match fragment.specific { - SpecificFragmentInfo::InlineBlock(ref mut block_flow) => { - let block_flow = flow_ref::deref_mut(&mut block_flow.flow_ref); - state.append_from(&mut flow::mut_base(block_flow).display_list_building_result) - } - SpecificFragmentInfo::InlineAbsoluteHypothetical(ref mut block_flow) => { - let block_flow = flow_ref::deref_mut(&mut block_flow.flow_ref); - state.append_from(&mut flow::mut_base(block_flow).display_list_building_result) - } - SpecificFragmentInfo::InlineAbsolute(ref mut block_flow) => { - let block_flow = flow_ref::deref_mut(&mut block_flow.flow_ref); - state.append_from(&mut flow::mut_base(block_flow).display_list_building_result) - } - _ => {} - } } fn build_display_list_for_inline(&mut self, state: &mut DisplayListBuildState) { diff --git a/components/layout/flow.rs b/components/layout/flow.rs index def1f7bf8a7..1dd4b386ad8 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -34,7 +34,7 @@ use floats::Floats; use flow_list::{FlowList, FlowListIterator, MutFlowListIterator}; use flow_ref::{self, FlowRef, WeakFlowRef}; use fragment::{Fragment, FragmentBorderBoxIterator, Overflow, SpecificFragmentInfo}; -use gfx::display_list::{ClippingRegion, DisplayListEntry, StackingContext, StackingContextId}; +use gfx::display_list::{ClippingRegion, StackingContext, StackingContextId}; use gfx_traits::{LayerId, LayerType}; use incremental::{RECONSTRUCT_FLOW, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, RestyleDamage}; use inline::InlineFlow; @@ -957,9 +957,6 @@ pub struct BaseFlow { /// per-stacking-context. pub stacking_relative_position_of_display_port: Rect, - /// The results of display list building for this flow. - pub display_list_building_result: Option>, - /// The writing mode for this flow. pub writing_mode: WritingMode, @@ -1129,7 +1126,6 @@ impl BaseFlow { block_container_writing_mode: writing_mode, block_container_explicit_block_size: None, absolute_cb: ContainingBlockLink::new(), - display_list_building_result: None, early_absolute_position_info: EarlyAbsolutePositionInfo::new(writing_mode), late_absolute_position_info: LateAbsolutePositionInfo::new(), clip: ClippingRegion::max(), @@ -1147,8 +1143,6 @@ impl BaseFlow { children: children, restyle_damage: self.restyle_damage | REPAINT | REFLOW_OUT_OF_FLOW | REFLOW, parallel: FlowParallelInfo::new(), - display_list_building_result: None, - floats: self.floats.clone(), abs_descendants: self.abs_descendants.clone(), absolute_cb: self.absolute_cb.clone(), diff --git a/components/layout/layout_thread.rs b/components/layout/layout_thread.rs index bf4a6ac9745..a361e42fb15 100644 --- a/components/layout/layout_thread.rs +++ b/components/layout/layout_thread.rs @@ -875,9 +875,10 @@ impl LayoutThread { false, None); - sequential::build_display_list_for_subtree(layout_root, - &mut root_stacking_context, - shared_layout_context); + let display_list_entries = + sequential::build_display_list_for_subtree(layout_root, + &mut root_stacking_context, + shared_layout_context); if data.goal == ReflowGoal::ForDisplay { debug!("Done building display list."); @@ -900,11 +901,9 @@ impl LayoutThread { ScrollPolicy::Scrollable, None, root_background_color)); - let display_list = DisplayList::new( - root_stacking_context, - &mut flow::mut_base(flow_ref::deref_mut(layout_root)) - .display_list_building_result); + let display_list = DisplayList::new(root_stacking_context, + &mut Some(display_list_entries)); if opts::get().dump_display_list { display_list.print(); } diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index e0207ee7ca0..a54d46848ca 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -6,13 +6,14 @@ use app_units::Au; use context::{LayoutContext, SharedLayoutContext}; +use display_list_builder::DisplayListBuildState; use euclid::point::Point2D; use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtils}; use flow_ref::{self, FlowRef}; use fragment::FragmentBorderBoxIterator; use generated_content::ResolveGeneratedContent; -use gfx::display_list::StackingContext; +use gfx::display_list::{DisplayListEntry, StackingContext}; use style::dom::TNode; use style::traversal::DomTraversalContext; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; @@ -77,13 +78,19 @@ pub fn traverse_flow_tree_preorder(root: &mut FlowRef, pub fn build_display_list_for_subtree(root: &mut FlowRef, root_stacking_context: &mut StackingContext, - shared_layout_context: &SharedLayoutContext) { + shared_layout_context: &SharedLayoutContext) + -> Vec { let flow_root = flow_ref::deref_mut(root); let layout_context = LayoutContext::new(shared_layout_context); flow_root.traverse_preorder(&ComputeAbsolutePositions { layout_context: &layout_context }); flow_root.collect_stacking_contexts(root_stacking_context.id, &mut root_stacking_context.children); - flow_root.traverse_postorder(&BuildDisplayList { layout_context: &layout_context }); + let mut build_display_list = BuildDisplayList { + state: DisplayListBuildState::new(&layout_context, + flow::base(&**root).stacking_context_id), + }; + build_display_list.traverse(&mut *flow_root); + build_display_list.state.items } pub fn iterate_through_flow_tree_fragment_border_boxes(root: &mut FlowRef, diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 696c43b210b..7e0a9e3a499 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -215,23 +215,27 @@ impl<'a> PreorderFlowTraversal for ComputeAbsolutePositions<'a> { } } -#[derive(Copy, Clone)] pub struct BuildDisplayList<'a> { - pub layout_context: &'a LayoutContext<'a>, + pub state: DisplayListBuildState<'a>, } -impl<'a> PostorderFlowTraversal for BuildDisplayList<'a> { +impl<'a> BuildDisplayList<'a> { #[inline] - fn process(&self, flow: &mut Flow) { - let mut state = DisplayListBuildState::new( - self.layout_context, flow::base(flow).stacking_context_id); - flow.build_display_list(&mut state); - flow::mut_base(flow).display_list_building_result = Some(state.items); - flow::mut_base(flow).restyle_damage.remove(REPAINT); + pub fn traverse(&mut self, flow: &mut Flow) { + if self.should_process() { + self.state.push_stacking_context_id(flow::base(flow).stacking_context_id); + flow.build_display_list(&mut self.state); + flow::mut_base(flow).restyle_damage.remove(REPAINT); + self.state.pop_stacking_context_id(); + } + + for kid in flow::child_iter(flow) { + self.traverse(kid); + } } #[inline] - fn should_process(&self, _: &mut Flow) -> bool { - self.layout_context.shared_context().goal == ReflowGoal::ForDisplay + fn should_process(&self) -> bool { + self.state.layout_context.shared_context().goal == ReflowGoal::ForDisplay } } From 117e92aefd0136e192edf132c9bd5c75bce9bd5f Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 2 Mar 2016 11:51:45 -0800 Subject: [PATCH 8/8] layout: Minor whitespace cleanup. --- components/layout/display_list_builder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 0e3b23e0912..b93c3c56c15 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -1614,8 +1614,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { self.base.stacking_context_id = stacking_context_id; let inner_stacking_context_id = if self.has_scrolling_overflow() { - StackingContextId::new_of_type(self.base.flow_id(), - self.fragment.fragment_type()) + StackingContextId::new_of_type(self.base.flow_id(), self.fragment.fragment_type()) } else { stacking_context_id };