From d34ebf521df9b4e8c0f9f49f4c3bac8b64a80cc7 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 4 Feb 2014 23:08:53 -0800 Subject: [PATCH] layout: Refactor flow construction to move around large structs less. 5% perf win, and a net loss in lines of code. --- src/components/main/layout/block.rs | 30 ++++++++++------------ src/components/main/layout/box_.rs | 6 ++--- src/components/main/layout/construct.rs | 33 +++++++++++-------------- src/components/main/layout/inline.rs | 17 +++---------- 4 files changed, 34 insertions(+), 52 deletions(-) diff --git a/src/components/main/layout/block.rs b/src/components/main/layout/block.rs index 40f95bf768b..cc55ed3bb2c 100644 --- a/src/components/main/layout/block.rs +++ b/src/components/main/layout/block.rs @@ -5,12 +5,14 @@ //! CSS block formatting contexts. use layout::box_::Box; +use layout::construct::FlowConstructor; use layout::context::LayoutContext; use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData}; +use layout::float_context::{FloatContext, PlacementInfo, Invalid, FloatType}; use layout::flow::{BaseFlow, BlockFlowClass, FlowClass, Flow, ImmutableFlowUtils}; use layout::flow; use layout::model::{MaybeAuto, Specified, Auto, specified_or_none, specified}; -use layout::float_context::{FloatContext, PlacementInfo, Invalid, FloatType}; +use layout::wrapper::ThreadSafeLayoutNode; use std::cell::RefCell; use geom::{Point2D, Rect, SideOffsets2D, Size2D}; @@ -66,30 +68,24 @@ pub struct BlockFlow { } impl BlockFlow { - pub fn new(base: BaseFlow) -> BlockFlow { + pub fn from_node(constructor: &mut FlowConstructor, node: ThreadSafeLayoutNode, is_fixed: bool) + -> BlockFlow { BlockFlow { - base: base, - box_: None, - is_root: false, - is_fixed: false, - float: None - } - } - - pub fn from_box(base: BaseFlow, box_: Box, is_fixed: bool) -> BlockFlow { - BlockFlow { - base: base, - box_: Some(box_), + base: BaseFlow::new(constructor.next_flow_id(), node), + box_: Some(Box::new(constructor, node)), is_root: false, is_fixed: is_fixed, float: None } } - pub fn float_from_box(base: BaseFlow, float_type: FloatType, box_: Box) -> BlockFlow { + pub fn float_from_node(constructor: &mut FlowConstructor, + node: ThreadSafeLayoutNode, + float_type: FloatType) + -> BlockFlow { BlockFlow { - base: base, - box_: Some(box_), + base: BaseFlow::new(constructor.next_flow_id(), node), + box_: Some(Box::new(constructor, node)), is_root: false, is_fixed: false, float: Some(~FloatedBlockInfo::new(float_type)) diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index 8df9a39957b..f4d694e9f99 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -14,7 +14,6 @@ use gfx::display_list::{SolidColorDisplayItem, SolidColorDisplayItemClass, TextD use gfx::display_list::{TextDisplayItemClass, TextDisplayItemFlags, ClipDisplayItem}; use gfx::display_list::{ClipDisplayItemClass, DisplayListCollection}; use gfx::font::FontStyle; - use gfx::text::text_run::TextRun; use servo_msg::constellation_msg::{FrameRectMsg, PipelineId, SubpageId}; use servo_net::image::holder::ImageHolder; @@ -33,6 +32,7 @@ use style::computed_values::{border_style, clear, font_family, line_height, posi use style::computed_values::{text_align, text_decoration, vertical_align, visibility, white_space}; use css::node_style::StyledNode; +use layout::construct::FlowConstructor; use layout::context::LayoutContext; use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData, ToGfxColor}; use layout::float_context::{ClearType, ClearLeft, ClearRight, ClearBoth}; @@ -298,7 +298,7 @@ pub struct InlineParentInfo { impl Box { /// Constructs a new `Box` instance. - pub fn new(node: ThreadSafeLayoutNode, specific: SpecificBoxInfo) -> Box { + pub fn new(constructor: &mut FlowConstructor, node: ThreadSafeLayoutNode) -> Box { Box { node: OpaqueNode::from_thread_safe_layout_node(&node), style: node.style().clone(), @@ -306,7 +306,7 @@ impl Box { border: RefCell::new(Zero::zero()), padding: RefCell::new(Zero::zero()), margin: RefCell::new(Zero::zero()), - specific: specific, + specific: constructor.build_specific_box_info_for_node(node), position_offsets: RefCell::new(Zero::zero()), inline_info: RefCell::new(None), new_line_pos: ~[], diff --git a/src/components/main/layout/construct.rs b/src/components/main/layout/construct.rs index b5f9cf21046..338c401ef27 100644 --- a/src/components/main/layout/construct.rs +++ b/src/components/main/layout/construct.rs @@ -23,10 +23,11 @@ use css::node_style::StyledNode; use layout::block::BlockFlow; use layout::box_::{Box, GenericBox, IframeBox, IframeBoxInfo, ImageBox, ImageBoxInfo}; -use layout::box_::{UnscannedTextBox, UnscannedTextBoxInfo, InlineInfo, InlineParentInfo}; +use layout::box_::{InlineInfo, InlineParentInfo, SpecificBoxInfo, UnscannedTextBox}; +use layout::box_::{UnscannedTextBoxInfo}; use layout::context::LayoutContext; use layout::float_context::FloatType; -use layout::flow::{BaseFlow, Flow, FlowLeafSet, ImmutableFlowUtils, MutableOwnedFlowUtils}; +use layout::flow::{Flow, FlowLeafSet, ImmutableFlowUtils, MutableOwnedFlowUtils}; use layout::inline::InlineFlow; use layout::text::TextRunScanner; use layout::util::{LayoutDataAccess, OpaqueNode}; @@ -227,7 +228,7 @@ impl<'fc> FlowConstructor<'fc> { } /// Returns the next flow ID and bumps the internal counter. - fn next_flow_id(&self) -> int { + pub fn next_flow_id(&self) -> int { let id = self.next_flow_id.get(); self.next_flow_id.set(id + 1); id @@ -246,9 +247,10 @@ impl<'fc> FlowConstructor<'fc> { } } - /// Builds a `Box` for the given node. - fn build_box_for_node(&mut self, node: ThreadSafeLayoutNode) -> Box { - let specific = match node.type_id() { + /// Builds specific `Box` info for the given node. + pub fn build_specific_box_info_for_node(&mut self, node: ThreadSafeLayoutNode) + -> SpecificBoxInfo { + match node.type_id() { ElementNodeTypeId(HTMLImageElementTypeId) => { match self.build_box_info_for_image(node) { None => GenericBox, @@ -258,8 +260,7 @@ impl<'fc> FlowConstructor<'fc> { ElementNodeTypeId(HTMLIframeElementTypeId) => IframeBox(IframeBoxInfo::new(&node)), TextNodeTypeId => UnscannedTextBox(UnscannedTextBoxInfo::new(&node)), _ => GenericBox, - }; - Box::new(node, specific) + } } /// Creates an inline flow from a set of inline boxes and adds it as a child of the given flow. @@ -275,8 +276,7 @@ impl<'fc> FlowConstructor<'fc> { return } - let inline_base = BaseFlow::new(self.next_flow_id(), node); - let mut inline_flow = ~InlineFlow::from_boxes(inline_base, boxes) as ~Flow; + let mut inline_flow = ~InlineFlow::from_boxes(self.next_flow_id(), node, boxes) as ~Flow; inline_flow.mark_as_leaf(self.layout_context.flow_leaf_set.get()); TextRunScanner::new().scan_for_runs(self.font_context, inline_flow); @@ -391,9 +391,7 @@ impl<'fc> FlowConstructor<'fc> { /// other `BlockFlow`s or `InlineFlow`s underneath it, depending on whether {ib} splits needed /// to happen. fn build_flow_for_block(&mut self, node: ThreadSafeLayoutNode, is_fixed: bool) -> ~Flow { - let base = BaseFlow::new(self.next_flow_id(), node); - let box_ = self.build_box_for_node(node); - let mut flow = ~BlockFlow::from_box(base, box_, is_fixed) as ~Flow; + let mut flow = ~BlockFlow::from_node(self, node, is_fixed) as ~Flow; self.build_children_of_block_flow(&mut flow, node); flow } @@ -402,10 +400,7 @@ impl<'fc> FlowConstructor<'fc> { /// a `BlockFlow` underneath it. fn build_flow_for_floated_block(&mut self, node: ThreadSafeLayoutNode, float_type: FloatType) -> ~Flow { - let base = BaseFlow::new(self.next_flow_id(), node); - let box_ = self.build_box_for_node(node); - - let mut flow = ~BlockFlow::float_from_box(base, float_type, box_) as ~Flow; + let mut flow = ~BlockFlow::float_from_node(self, node, float_type) as ~Flow; self.build_children_of_block_flow(&mut flow, node); flow } @@ -523,7 +518,7 @@ impl<'fc> FlowConstructor<'fc> { fn set_inline_info_for_inline_child(&mut self, boxes: &~[&Box], parent_node: ThreadSafeLayoutNode) { - let parent_box = self.build_box_for_node(parent_node); + let parent_box = Box::new(self, parent_node); let font_style = parent_box.font_style(); let font_group = self.font_context.get_resolved_font_for_style(&font_style); let (font_ascent,font_descent) = font_group.borrow().with_mut( |fg| { @@ -578,7 +573,7 @@ impl<'fc> FlowConstructor<'fc> { let construction_item = InlineBoxesConstructionItem(InlineBoxesConstructionResult { splits: None, boxes: ~[ - self.build_box_for_node(node) + Box::new(self, node) ], }); ConstructionItemConstructionResult(construction_item) diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 7a15cb35dc3..2135e104cc4 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -9,9 +9,9 @@ use layout::context::LayoutContext; use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData}; use layout::flow::{BaseFlow, FlowClass, Flow, InlineFlowClass}; use layout::flow; -use layout::float_context::FloatContext; +use layout::float_context::{FloatContext, FloatLeft, PlacementInfo}; use layout::util::ElementMapping; -use layout::float_context::{PlacementInfo, FloatLeft}; +use layout::wrapper::ThreadSafeLayoutNode; use extra::container::Deque; use extra::ringbuf::RingBuf; @@ -466,18 +466,9 @@ pub struct InlineFlow { } impl InlineFlow { - pub fn new(base: BaseFlow) -> InlineFlow { + pub fn from_boxes(id: int, node: ThreadSafeLayoutNode, boxes: ~[Box]) -> InlineFlow { InlineFlow { - base: base, - boxes: ~[], - lines: ~[], - elems: ElementMapping::new(), - } - } - - pub fn from_boxes(base: BaseFlow, boxes: ~[Box]) -> InlineFlow { - InlineFlow { - base: base, + base: BaseFlow::new(id, node), boxes: boxes, lines: ~[], elems: ElementMapping::new(),