From a178a7a5d182a0aba04d27315646f514a9a619f2 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 3 May 2013 15:36:58 -0700 Subject: [PATCH 01/12] Fix style in `CharacterData`. --- src/servo/dom/characterdata.rs | 41 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/servo/dom/characterdata.rs b/src/servo/dom/characterdata.rs index b078d6dd03c..a8d81158d9c 100644 --- a/src/servo/dom/characterdata.rs +++ b/src/servo/dom/characterdata.rs @@ -1,3 +1,9 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! DOM bindings for `CharacterData`. + use dom::bindings::utils::{DOMString, null_string, str}; use dom::node::{Node, NodeTypeId}; @@ -8,50 +14,51 @@ pub struct CharacterData { data: DOMString } -pub impl CharacterData { - fn new(id: NodeTypeId, data: ~str) -> CharacterData { +impl CharacterData { + pub fn new(id: NodeTypeId, data: ~str) -> CharacterData { CharacterData { parent: Node::new(id), data: str(data) } } - fn GetData(&self) -> DOMString { + pub fn GetData(&self) -> DOMString { copy self.data } - fn SetData(&mut self, arg: DOMString) { + pub fn SetData(&mut self, arg: DOMString) { self.data = arg; } - fn Length(&self) -> u32 { + pub fn Length(&self) -> u32 { match self.data { - str(ref s) => s.len() as u32, - null_string => 0 + str(ref s) => s.len() as u32, + null_string => 0 } } - fn SubstringData(&self, offset: u32, count: u32) -> DOMString { + pub fn SubstringData(&self, offset: u32, count: u32) -> DOMString { match self.data { - str(ref s) => str(s.slice(offset as uint, count as uint).to_str()), - null_string => null_string + str(ref s) => str(s.slice(offset as uint, count as uint).to_str()), + null_string => null_string } } - fn AppendData(&mut self, arg: DOMString) { + pub fn AppendData(&mut self, arg: DOMString) { let s = self.data.to_str(); self.data = str(str::append(s, arg.to_str())); } - fn InsertData(&mut self, _offset: u32, _arg: DOMString) { - fail!(~"nyi") + pub fn InsertData(&mut self, _offset: u32, _arg: DOMString) { + fail!("CharacterData::InsertData() is unimplemented") } - fn DeleteData(&mut self, _offset: u32, _count: u32) { - fail!(~"nyi") + pub fn DeleteData(&mut self, _offset: u32, _count: u32) { + fail!("CharacterData::DeleteData() is unimplemented") } - fn ReplaceData(&mut self, _offset: u32, _count: u32, _arg: DOMString) { - fail!(~"nyi") + pub fn ReplaceData(&mut self, _offset: u32, _count: u32, _arg: DOMString) { + fail!("CharacterData::ReplaceData() is unimplemented") } } + From b17fffa9d926f4a2a805445c68f120c7912f2c57 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 3 May 2013 15:41:23 -0700 Subject: [PATCH 02/12] Fix code style in DOMParser --- src/servo/dom/domparser.rs | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/servo/dom/domparser.rs b/src/servo/dom/domparser.rs index 2b08540e40f..6ab6655398f 100644 --- a/src/servo/dom/domparser.rs +++ b/src/servo/dom/domparser.rs @@ -1,6 +1,10 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + use content::content_task::global_content; -use dom::bindings::utils::{DOMString, ErrorResult, WrapperCache, CacheableWrapper}; use dom::bindings::codegen::DOMParserBinding; +use dom::bindings::utils::{DOMString, ErrorResult, WrapperCache, CacheableWrapper}; use dom::document::Document; use dom::element::{Element, HTMLHtmlElement, HTMLHtmlElementTypeId}; use dom::node::Node; @@ -11,12 +15,13 @@ pub struct DOMParser { wrapper: WrapperCache } -pub impl DOMParser { - fn new(owner: @mut Window) -> @mut DOMParser { +impl DOMParser { + pub fn new(owner: @mut Window) -> @mut DOMParser { let parser = @mut DOMParser { owner: owner, wrapper: WrapperCache::new() }; + let cx = global_content().compartment.get().cx.ptr; let cache = owner.get_wrappercache(); let scope = cache.get_wrapper(); @@ -24,13 +29,23 @@ pub impl DOMParser { parser } - fn Constructor(owner: @mut Window, _rv: &mut ErrorResult) -> @mut DOMParser { + pub fn Constructor(owner: @mut Window, _rv: &mut ErrorResult) -> @mut DOMParser { DOMParser::new(owner) } - fn ParseFromString(&self, _s: DOMString, _type_: DOMParserBinding::SupportedType, _rv: &mut ErrorResult) -> @mut Document { - let root = ~HTMLHtmlElement { parent: Element::new(HTMLHtmlElementTypeId, ~"html") }; - let root = unsafe { Node::as_abstract_node(root) }; - Document(root, None) + pub fn ParseFromString(&self, + _s: DOMString, + _type: DOMParserBinding::SupportedType, + _rv: &mut ErrorResult) + -> @mut Document { + unsafe { + let root = ~HTMLHtmlElement { + parent: Element::new(HTMLHtmlElementTypeId, ~"html") + }; + + let root = Node::as_abstract_node(root); + Document(root, None) + } } -} \ No newline at end of file +} + From 5749ffcf7afeefcbeaf38f1a29dabde7c7da9626 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 3 May 2013 15:42:42 -0700 Subject: [PATCH 03/12] Remove ancient `net.rs` --- src/servo/net.rs | 33 --------------------------------- 1 file changed, 33 deletions(-) delete mode 100644 src/servo/net.rs diff --git a/src/servo/net.rs b/src/servo/net.rs deleted file mode 100644 index cd1c5df0f06..00000000000 --- a/src/servo/net.rs +++ /dev/null @@ -1,33 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -export uri, input_stream, channel, io_service, file_channel; - -import uri::uri; - -iface input_stream { - fn close(); - fn read() -> ~[u8]; -} - -iface channel { - fn uri() -> uri; - fn open() -> input_stream; -} - -iface io_service { - fn new_uri(spec: ~str) -> uri; - fn new_channel(uri: uri) -> channel; -} - -class file_channel: channel { - let bogus : int; - - new() { - self.bogus = 0; - } - - fn uri() -> uri { fail } - fn open() -> input_stream { fail } -} From 1d7a3f916d56220206d4f1b086a334917c009eb8 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 15:41:47 -0700 Subject: [PATCH 04/12] servo: Refactor the `Flow` type to save memory and allow upcasting and downcasting more naturally --- src/servo/dom/node.rs | 2 +- src/servo/layout/block.rs | 88 ++++--- src/servo/layout/box.rs | 10 +- src/servo/layout/box_builder.rs | 282 ++++++++++---------- src/servo/layout/display_list_builder.rs | 41 +-- src/servo/layout/flow.rs | 315 +++++++++++++---------- src/servo/layout/inline.rs | 284 ++++++++++---------- src/servo/layout/layout_task.rs | 13 +- src/servo/layout/root.rs | 67 ++--- src/servo/layout/traverse.rs | 12 +- 10 files changed, 603 insertions(+), 511 deletions(-) diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs index 446dc036d62..d8745ae00fb 100644 --- a/src/servo/dom/node.rs +++ b/src/servo/dom/node.rs @@ -71,7 +71,7 @@ pub enum NodeTypeId { pub struct LayoutData { style: Option, - flow: Option<@mut FlowContext>, + flow: Option, } impl LayoutData { diff --git a/src/servo/layout/block.rs b/src/servo/layout/block.rs index 7b1fb67faa2..6d4cda5b31d 100644 --- a/src/servo/layout/block.rs +++ b/src/servo/layout/block.rs @@ -2,12 +2,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -// Block layout. +//! CSS block layout. use layout::box::{RenderBox}; use layout::context::LayoutContext; use layout::display_list_builder::{DisplayListBuilder, FlowDisplayListBuilderMethods}; -use layout::flow::{BlockFlow, FlowContext, InlineBlockFlow, RootFlow}; +use layout::flow::{BlockFlow, FlowContext, FlowData, InlineBlockFlow, RootFlow}; use layout::inline::InlineLayout; use au = gfx::geometry; @@ -18,23 +18,33 @@ use gfx::display_list::DisplayList; use gfx::geometry::Au; pub struct BlockFlowData { + /// Data common to all flows. + common: FlowData, + + /// The associated render box. box: Option<@mut RenderBox> } -pub fn BlockFlowData() -> BlockFlowData { - BlockFlowData { - box: None +impl BlockFlowData { + pub fn new(common: FlowData) -> BlockFlowData { + BlockFlowData { + common: common, + box: None, + } } } +/// NB: These are part of FlowContext, not part of BlockFlowData, because the root flow calls these +/// as well. It is not clear to me whether this needs to be the case, or whether `RootFlow` can be +/// merged into this. pub trait BlockLayout { fn starts_block_flow(&self) -> bool; - fn with_block_box(@mut self, &fn(box: &@mut RenderBox) -> ()) -> (); + fn with_block_box(&self, &fn(box: &@mut RenderBox) -> ()) -> (); - fn bubble_widths_block(@mut self, ctx: &LayoutContext); - fn assign_widths_block(@mut self, ctx: &LayoutContext); - fn assign_height_block(@mut self, ctx: &LayoutContext); - fn build_display_list_block(@mut self, + fn bubble_widths_block(&self, ctx: &LayoutContext); + fn assign_widths_block(&self, ctx: &LayoutContext); + fn assign_height_block(&self, ctx: &LayoutContext); + fn build_display_list_block(&self, a: &DisplayListBuilder, b: &Rect, c: &Point2D, @@ -49,17 +59,21 @@ impl BlockLayout for FlowContext { } } - /* Get the current flow's corresponding block box, if it exists, and do something with it. - This works on both BlockFlow and RootFlow, since they are mostly the same. */ - fn with_block_box(@mut self, cb: &fn(box: &@mut RenderBox) -> ()) -> () { + /// Get the current flow's corresponding block box, if it exists, and do something with it. + /// This works on both BlockFlow and RootFlow, since they are mostly the same. + fn with_block_box(&self, callback: &fn(box: &@mut RenderBox) -> ()) -> () { match *self { BlockFlow(*) => { let box = self.block().box; - for box.each |b| { cb(b); } + for box.each |b| { + callback(b); + } }, RootFlow(*) => { let mut box = self.root().box; - for box.each |b| { cb(b); } + for box.each |b| { + callback(b); + } }, _ => fail!(fmt!("Tried to do something with_block_box(), but this is a %?", self)) } @@ -74,7 +88,7 @@ impl BlockLayout for FlowContext { /* TODO: floats */ /* TODO: absolute contexts */ /* TODO: inline-blocks */ - fn bubble_widths_block(@mut self, ctx: &LayoutContext) { + fn bubble_widths_block(&self, ctx: &LayoutContext) { assert!(self.starts_block_flow()); let mut min_width = Au(0); @@ -84,8 +98,10 @@ impl BlockLayout for FlowContext { for self.each_child |child_ctx| { assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow()); - min_width = au::max(min_width, child_ctx.d().min_width); - pref_width = au::max(pref_width, child_ctx.d().pref_width); + do child_ctx.with_common_info |child_info| { + min_width = au::max(min_width, child_info.min_width); + pref_width = au::max(pref_width, child_info.pref_width); + } } /* if not an anonymous block context, add in block box's widths. @@ -95,8 +111,10 @@ impl BlockLayout for FlowContext { pref_width = pref_width.add(&box.get_pref_width(ctx)); } - self.d().min_width = min_width; - self.d().pref_width = pref_width; + do self.with_common_info |info| { + info.min_width = min_width; + info.pref_width = pref_width; + } } /* Recursively (top-down) determines the actual width of child @@ -106,10 +124,10 @@ impl BlockLayout for FlowContext { Dual boxes consume some width first, and the remainder is assigned to all child (block) contexts. */ - fn assign_widths_block(@mut self, _ctx: &LayoutContext) { + fn assign_widths_block(&self, _ctx: &LayoutContext) { assert!(self.starts_block_flow()); - let mut remaining_width = self.d().position.size.width; + let mut remaining_width = self.with_common_info(|info| info.position.size.width); let mut _right_used = Au(0); let mut left_used = Au(0); @@ -123,22 +141,28 @@ impl BlockLayout for FlowContext { for self.each_child |child_ctx| { assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow()); - child_ctx.d().position.origin.x = left_used; - child_ctx.d().position.size.width = remaining_width; + do child_ctx.with_common_info |child_info| { + child_info.position.origin.x = left_used; + child_info.position.size.width = remaining_width; + } } } - fn assign_height_block(@mut self, _ctx: &LayoutContext) { + fn assign_height_block(&self, _ctx: &LayoutContext) { assert!(self.starts_block_flow()); let mut cur_y = Au(0); for self.each_child |child_ctx| { - child_ctx.d().position.origin.y = cur_y; - cur_y += child_ctx.d().position.size.height; + do child_ctx.with_common_info |child_info| { + child_info.position.origin.y = cur_y; + cur_y += child_info.position.size.height; + } } - self.d().position.size.height = cur_y; + do self.with_common_info |info| { + info.position.size.height = cur_y; + } let _used_top = Au(0); let _used_bot = Au(0); @@ -150,9 +174,11 @@ impl BlockLayout for FlowContext { } } - fn build_display_list_block(@mut self, builder: &DisplayListBuilder, dirty: &Rect, - offset: &Point2D, list: &Cell) { - + fn build_display_list_block(&self, + builder: &DisplayListBuilder, + dirty: &Rect, + offset: &Point2D, + list: &Cell) { assert!(self.starts_block_flow()); // add box that starts block context diff --git a/src/servo/layout/box.rs b/src/servo/layout/box.rs index a7a56109525..b6b46f7f5d3 100644 --- a/src/servo/layout/box.rs +++ b/src/servo/layout/box.rs @@ -71,13 +71,13 @@ padding, backgrounds. It is analogous to a CSS nonreplaced content box. */ pub struct RenderBoxData { /* originating DOM node */ - node : AbstractNode, + node: AbstractNode, /* reference to containing flow context, which this box participates in */ - ctx : @mut FlowContext, + ctx: FlowContext, /* position of this box relative to owning flow */ - position : Rect, - font_size : Length, + position: Rect, + font_size: Length, /* TODO (Issue #87): debug only */ id: int } @@ -103,7 +103,7 @@ pub enum SplitBoxResult { SplitDidNotFit(Option<@mut RenderBox>, Option<@mut RenderBox>) } -pub fn RenderBoxData(node: AbstractNode, ctx: @mut FlowContext, id: int) -> RenderBoxData { +pub fn RenderBoxData(node: AbstractNode, ctx: FlowContext, id: int) -> RenderBoxData { RenderBoxData { node : node, ctx : ctx, diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index f127efa03e7..d0fb0deb55d 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/** Creates CSS boxes from a DOM. */ +//! Creates CSS boxes from a DOM tree. use dom::element::*; use dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId}; @@ -16,12 +16,12 @@ use layout::inline::{InlineFlowData, InlineLayout}; use layout::root::RootFlowData; use gfx::image::holder::ImageHolder; -use servo_util::range::Range; use newcss::values::{CSSDisplay, CSSDisplayBlock, CSSDisplayInline, CSSDisplayInlineBlock}; use newcss::values::{CSSDisplayNone}; +use servo_util::range::Range; pub struct LayoutTreeBuilder { - root_flow: Option<@mut FlowContext>, + root_flow: Option, next_bid: int, next_cid: int } @@ -39,7 +39,7 @@ pub impl LayoutTreeBuilder { // helper object for building the initial box list and making the // mapping between DOM nodes and boxes. struct BoxGenerator { - flow: @mut FlowContext, + flow: FlowContext, range_stack: ~[uint], } @@ -58,7 +58,9 @@ priv fn simulate_UA_display_rules(node: AbstractNode) -> CSSDisplay { };*/ let resolved = CSSDisplayInline; - if (resolved == CSSDisplayNone) { return resolved; } + if resolved == CSSDisplayNone { + return resolved; + } match node.type_id() { DoctypeNodeTypeId | CommentNodeTypeId => CSSDisplayNone, @@ -81,7 +83,7 @@ priv fn simulate_UA_display_rules(node: AbstractNode) -> CSSDisplay { } impl BoxGenerator { - fn new(flow: @mut FlowContext) -> BoxGenerator { + fn new(flow: FlowContext) -> BoxGenerator { debug!("Creating box generator for flow: %s", flow.debug_str()); BoxGenerator { flow: flow, @@ -99,103 +101,110 @@ impl BoxGenerator { _: &LayoutContext, _: AbstractNode, _: InlineSpacerSide) - -> Option<@mut RenderBox> { + -> Option<@mut RenderBox> { None } - pub fn push_node(@mut self, ctx: &LayoutContext, builder: &mut LayoutTreeBuilder, node: AbstractNode) { - debug!("BoxGenerator[f%d]: pushing node: %s", self.flow.d().id, node.debug_str()); + pub fn push_node(&mut self, + ctx: &LayoutContext, + builder: &mut LayoutTreeBuilder, + node: AbstractNode) { + debug!("BoxGenerator[f%d]: pushing node: %s", self.flow.id(), node.debug_str()); // first, determine the box type, based on node characteristics let simulated_display = simulate_UA_display_rules(node); // TODO: remove this once UA styles work let box_type = builder.decide_box_type(node, simulated_display); - debug!("BoxGenerator[f%d]: point a", self.flow.d().id); + debug!("BoxGenerator[f%d]: point a", self.flow.id()); // depending on flow, make a box for this node. match self.flow { - @InlineFlow(*) => { - let node_range_start = match self.flow { - @InlineFlow(*) => { - let inline_flow = self.flow.inline(); - inline_flow.boxes.len() - } - _ => 0 - }; + InlineFlow(inline) => { + let mut inline = &mut *inline; + let node_range_start = inline.boxes.len(); self.range_stack.push(node_range_start); // if a leaf, make a box. if node.is_leaf() { let new_box = builder.make_box(ctx, box_type, node, self.flow); - let boxes = &mut self.flow.inline().boxes; - boxes.push(new_box); + inline.boxes.push(new_box); } else if self.inline_spacers_needed_for_node(node) { // else, maybe make a spacer for "left" margin, border, padding for self.make_inline_spacer_for_node_side(ctx, node, LogicalBefore).each |spacer: &@mut RenderBox| { - let boxes = &mut self.flow.inline().boxes; - boxes.push(*spacer); + inline.boxes.push(*spacer); } } // TODO: cases for inline-block, etc. }, - @BlockFlow(*) => { - debug!("BoxGenerator[f%d]: point b", self.flow.d().id); - let new_box = builder.make_box(ctx, box_type, node, self.flow); - debug!("BoxGenerator[f%d]: attaching box[b%d] to block flow (node: %s)", - self.flow.d().id, new_box.d().id, node.debug_str()); + BlockFlow(*) => { + do self.flow.with_common_info |flow_info| { + debug!("BoxGenerator[f%d]: point b", flow_info.id); + let new_box = builder.make_box(ctx, box_type, node, self.flow); + debug!("BoxGenerator[f%d]: attaching box[b%d] to block flow (node: %s)", + flow_info.id, + new_box.d().id, + node.debug_str()); - assert!(self.flow.block().box.is_none()); - //XXXjdm We segfault when returning without this temporary. - let block = self.flow.block(); - block.box = Some(new_box); + assert!(self.flow.block().box.is_none()); + //XXXjdm We segfault when returning without this temporary. + let block = self.flow.block(); + block.box = Some(new_box); + } }, - @RootFlow(*) => { - debug!("BoxGenerator[f%d]: point c", self.flow.d().id); - let new_box = builder.make_box(ctx, box_type, node, self.flow); - debug!("BoxGenerator[f%d]: (node is: %s)", self.flow.d().id, node.debug_str()); - debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)", - self.flow.d().id, new_box.d().id, node.debug_str()); + RootFlow(*) => { + do self.flow.with_common_info |info| { + debug!("BoxGenerator[f%d]: point c", info.id); + let new_box = builder.make_box(ctx, box_type, node, self.flow); + debug!("BoxGenerator[f%d]: (node is: %s)", info.id, node.debug_str()); + debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)", + info.id, + new_box.d().id, + node.debug_str()); - assert!(self.flow.root().box.is_none()); - //XXXjdm We segfault when returning without this temporary. - let root = self.flow.root(); - root.box = Some(new_box); + assert!(self.flow.root().box.is_none()); + //XXXjdm We segfault when returning without this temporary. + let root = self.flow.root(); + root.box = Some(new_box); + } }, - _ => { warn!("push_node() not implemented for flow f%d", self.flow.d().id) } + _ => { + do self.flow.with_common_info |flow_info| { + warn!("push_node() not implemented for flow f%d", flow_info.id) + } + } } } - pub fn pop_node(&mut self, ctx: &LayoutContext, _builder: &LayoutTreeBuilder, node: AbstractNode) { - debug!("BoxGenerator[f%d]: popping node: %s", self.flow.d().id, node.debug_str()); + pub fn pop_node(&mut self, + ctx: &LayoutContext, + _builder: &LayoutTreeBuilder, + node: AbstractNode) { + debug!("BoxGenerator[f%d]: popping node: %s", self.flow.id(), node.debug_str()); match self.flow { - @InlineFlow(*) => { + InlineFlow(inline) => { + let inline = &mut *inline; + if self.inline_spacers_needed_for_node(node) { - // if this non-leaf box generates extra horizontal - // spacing, add a SpacerBox for it. - for self.make_inline_spacer_for_node_side(ctx, node, LogicalAfter).each |spacer: &@mut RenderBox| { + // If this non-leaf box generates extra horizontal spacing, add a SpacerBox for + // it. + let result = self.make_inline_spacer_for_node_side(ctx, node, LogicalAfter); + for result.each |spacer| { let boxes = &mut self.flow.inline().boxes; boxes.push(*spacer); } } let mut node_range: Range = Range::new(self.range_stack.pop(), 0); - let inline_flow = self.flow.inline(); // FIXME: borrow checker workaround - node_range.extend_to(inline_flow.boxes.len()); + node_range.extend_to(inline.boxes.len()); assert!(node_range.length() > 0); debug!("BoxGenerator: adding element range=%?", node_range); - let elems = &mut inline_flow.elems; - elems.add_mapping(node, &node_range); + inline.elems.add_mapping(node, &node_range); }, - @BlockFlow(*) | @RootFlow(*) => { - assert!(self.range_stack.len() == 0); - }, - _ => { - let d = self.flow.d(); // FIXME: borrow checker workaround - warn!("pop_node() not implemented for flow %?", d.id) - } + BlockFlow(*) | RootFlow(*) => assert!(self.range_stack.len() == 0), + _ => warn!("pop_node() not implemented for flow %?", self.flow.id()), } } } @@ -207,7 +216,11 @@ struct BuilderContext { impl BuilderContext { fn new(collector: @mut BoxGenerator) -> BuilderContext { - debug!("Creating new BuilderContext for flow: %s", collector.flow.debug_str()); + { + let collector = &mut *collector; + debug!("Creating new BuilderContext for flow: %s", collector.flow.debug_str()); + } + BuilderContext { default_collector: collector, inline_collector: None, @@ -219,11 +232,16 @@ impl BuilderContext { copy self } - priv fn attach_child_flow(&self, child: @mut FlowContext) { - let d = self.default_collector.flow.d(); // FIXME: borrow checker workaround - let cd = child.d(); // FIXME: borrow checker workaround - debug!("BuilderContext: Adding child flow f%? of f%?", d.id, cd.id); - self.default_collector.flow.add_child(child); + priv fn attach_child_flow(&self, child: FlowContext) { + let default_collector = &mut *self.default_collector; + do default_collector.flow.with_common_info |flow_info| { + do child.with_common_info |child_flow_info| { + debug!("BuilderContext: Adding child flow f%? of f%?", + flow_info.id, + child_flow_info.id); + default_collector.flow.add_child(child); + } + } } priv fn create_child_flow_of_type(&self, @@ -236,7 +254,8 @@ impl BuilderContext { BuilderContext::new(@mut BoxGenerator::new(new_flow)) } - priv fn make_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode) -> BuilderContext { + priv fn make_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode) + -> BuilderContext { debug!("BuilderContext: making new inline collector flow"); let new_flow = builder.make_flow(Flow_Inline, node); let new_generator = @mut BoxGenerator::new(new_flow); @@ -247,7 +266,8 @@ impl BuilderContext { BuilderContext::new(new_generator) } - priv fn get_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode) -> BuilderContext { + priv fn get_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode) + -> BuilderContext { match copy self.inline_collector { Some(collector) => BuilderContext::new(collector), None => self.make_inline_collector(builder, node) @@ -261,10 +281,8 @@ impl BuilderContext { // returns a context for the current node, or None if the document subtree rooted // by the node should not generate a layout tree. For example, nodes with style 'display:none' // should just not generate any flows or boxes. - fn containing_context_for_node(&mut self, - node: AbstractNode, - builder: &mut LayoutTreeBuilder) - -> Option { + fn containing_context_for_node(&mut self, node: AbstractNode, builder: &mut LayoutTreeBuilder) + -> Option { // TODO: remove this once UA styles work // TODO: handle interactions with 'float', 'position' (CSS 2.1, Section 9.7) let simulated_display = match simulate_UA_display_rules(node) { @@ -273,7 +291,7 @@ impl BuilderContext { }; let containing_context = match (simulated_display, self.default_collector.flow) { - (CSSDisplayBlock, @RootFlow(*)) => { + (CSSDisplayBlock, RootFlow(*)) => { // If this is the root node, then use the root flow's // context. Otherwise, make a child block context. match node.parent_node() { @@ -281,14 +299,14 @@ impl BuilderContext { None => { self.clone() }, } }, - (CSSDisplayBlock, @BlockFlow(*)) => { + (CSSDisplayBlock, BlockFlow(*)) => { self.clear_inline_collector(); self.create_child_flow_of_type(Flow_Block, builder, node) }, - (CSSDisplayInline, @InlineFlow(*)) => self.clone(), - (CSSDisplayInlineBlock, @InlineFlow(*)) => self.clone(), - (CSSDisplayInline, @BlockFlow(*)) => self.get_inline_collector(builder, node), - (CSSDisplayInlineBlock, @BlockFlow(*)) => self.get_inline_collector(builder, node), + (CSSDisplayInline, InlineFlow(*)) => self.clone(), + (CSSDisplayInlineBlock, InlineFlow(*)) => self.clone(), + (CSSDisplayInline, BlockFlow(*)) => self.get_inline_collector(builder, node), + (CSSDisplayInlineBlock, BlockFlow(*)) => self.get_inline_collector(builder, node), _ => self.clone() }; @@ -330,10 +348,12 @@ pub impl LayoutTreeBuilder { // eventually be elided or split, but the mapping between // nodes and FlowContexts should not change during layout. let flow = &mut this_ctx.default_collector.flow; - for flow.each_child |child_flow: @mut FlowContext| { - let node = child_flow.d().node; - assert!(node.has_layout_data()); - node.layout_data().flow = Some(child_flow); + for flow.each_child |child_flow| { + do child_flow.with_common_info |child_flow_info| { + let node = child_flow_info.node; + assert!(node.has_layout_data()); + node.layout_data().flow = Some(child_flow); + } } } @@ -347,14 +367,14 @@ pub impl LayoutTreeBuilder { // beginning or end of a block flow. Otherwise, the whitespace // might affect whitespace collapsing with adjacent text. fn simplify_children_of_flow(&self, _: &LayoutContext, parent_ctx: &BuilderContext) { - match *parent_ctx.default_collector.flow { + match parent_ctx.default_collector.flow { InlineFlow(*) => { let mut found_child_inline = false; let mut found_child_block = false; let flow = &mut parent_ctx.default_collector.flow; - for flow.each_child |child_ctx: @mut FlowContext| { - match *child_ctx { + for flow.each_child |child_ctx| { + match child_ctx { InlineFlow(*) | InlineBlockFlow(*) => found_child_inline = true, BlockFlow(*) => found_child_block = true, _ => {} @@ -370,34 +390,32 @@ pub impl LayoutTreeBuilder { // of its RenderBox or FlowContext children, and possibly keep alive other junk let parent_flow = parent_ctx.default_collector.flow; - // FIXME: Workaround for the borrow check. - let (first_child, last_child) = { - let parent_flow: &mut FlowContext = parent_flow; - let parent_flow_data = parent_flow.d(); - (parent_flow_data.first_child, parent_flow_data.last_child) - }; + let (first_child, last_child) = + do parent_flow.with_common_info |parent_flow_info| { + (parent_flow_info.first_child, parent_flow_info.last_child) + }; // check first/last child for whitespace-ness - for first_child.each |first_flow: &@mut FlowContext| { + for first_child.each |first_flow| { if first_flow.starts_inline_flow() { let boxes = &mut first_flow.inline().boxes; if boxes.len() == 1 && boxes[0].is_whitespace_only() { debug!("LayoutTreeBuilder: pruning whitespace-only first child flow \ f%d from parent f%d", - first_flow.d().id, - parent_flow.d().id); + first_flow.id(), + parent_flow.id()); parent_flow.remove_child(*first_flow); } } } - for last_child.each |last_flow: &@mut FlowContext| { + for last_child.each |last_flow| { if last_flow.starts_inline_flow() { let boxes = &mut last_flow.inline().boxes; if boxes.len() == 1 && boxes.last().is_whitespace_only() { debug!("LayoutTreeBuilder: pruning whitespace-only last child flow \ f%d from parent f%d", - last_flow.d().id, - parent_flow.d().id); + last_flow.id(), + parent_flow.id()); parent_flow.remove_child(*last_flow); } } @@ -407,15 +425,14 @@ pub impl LayoutTreeBuilder { } } - fn fixup_split_inline(&self, _: @mut FlowContext) { + fn fixup_split_inline(&self, _: FlowContext) { // TODO: finish me. fail!(~"TODO: handle case where an inline is split by a block") } - /** entry point for box creation. Should only be - called on root DOM element. */ + /// Entry point for box creation. Should only be called on the root DOM element. fn construct_trees(&mut self, layout_ctx: &LayoutContext, root: AbstractNode) - -> Result<@mut FlowContext, ()> { + -> Result { let new_flow = self.make_flow(Flow_Root, root); let new_generator = @mut BoxGenerator::new(new_flow); let mut root_ctx = BuilderContext::new(new_generator); @@ -425,53 +442,46 @@ pub impl LayoutTreeBuilder { return Ok(new_flow) } - fn make_flow(&mut self, ty: FlowContextType, node: AbstractNode) -> @mut FlowContext { - let data = FlowData::new(self.next_flow_id(), node); - let ret = match ty { - Flow_Absolute => @mut AbsoluteFlow(data), - Flow_Block => @mut BlockFlow(data, BlockFlowData()), - Flow_Float => @mut FloatFlow(data), - Flow_InlineBlock => @mut InlineBlockFlow(data), - Flow_Inline => @mut InlineFlow(data, InlineFlowData()), - Flow_Root => @mut RootFlow(data, RootFlowData()), - Flow_Table => @mut TableFlow(data) + /// Creates a flow of the given type for the supplied node. + 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 info), + Flow_InlineBlock => InlineBlockFlow(@mut info), + Flow_Inline => InlineFlow(@mut InlineFlowData::new(info)), + Flow_Root => RootFlow(@mut RootFlowData::new(info)), + Flow_Table => TableFlow(@mut info), }; - debug!("LayoutTreeBuilder: created flow: %s", ret.debug_str()); - ret + debug!("LayoutTreeBuilder: created flow: %s", result.debug_str()); + result } - /** - disambiguate between different methods here instead of inlining, since each - case has very different complexity - */ + /// Disambiguate between different methods here instead of inlining, since each case has very + /// different complexity. fn make_box(&mut self, layout_ctx: &LayoutContext, ty: RenderBoxType, node: AbstractNode, - ctx: @mut FlowContext) - -> @mut RenderBox { - let ret = match ty { + ctx: FlowContext) + -> @mut RenderBox { + let result = match ty { RenderBox_Generic => self.make_generic_box(layout_ctx, node, ctx), RenderBox_Text => self.make_text_box(layout_ctx, node, ctx), RenderBox_Image => self.make_image_box(layout_ctx, node, ctx), }; - debug!("LayoutTreeBuilder: created box: %s", ret.debug_str()); - ret + debug!("LayoutTreeBuilder: created box: %s", result.debug_str()); + result } - fn make_generic_box(&mut self, - _: &LayoutContext, - node: AbstractNode, - ctx: @mut FlowContext) - -> @mut RenderBox { + fn make_generic_box(&mut self, _: &LayoutContext, node: AbstractNode, ctx: FlowContext) + -> @mut RenderBox { @mut GenericBox(RenderBoxData(copy node, ctx, self.next_box_id())) } - fn make_image_box(&mut self, - layout_ctx: &LayoutContext, - node: AbstractNode, - ctx: @mut FlowContext) - -> @mut RenderBox { + fn make_image_box(&mut self, layout_ctx: &LayoutContext, node: AbstractNode, ctx: FlowContext) + -> @mut RenderBox { if !node.is_image_element() { fail!(~"WAT error: why couldn't we make an image box?"); } @@ -482,17 +492,15 @@ pub impl LayoutTreeBuilder { layout_ctx.image_cache); @mut ImageBox(RenderBoxData(node, ctx, self.next_box_id()), holder) } else { - info!("Tried to make image box, but couldn't find image. Made generic box instead."); + info!("Tried to make image box, but couldn't find image. Made generic box \ + instead."); self.make_generic_box(layout_ctx, node, ctx) } } } - fn make_text_box(&mut self, - _: &LayoutContext, - node: AbstractNode, - ctx: @mut FlowContext) - -> @mut RenderBox { + fn make_text_box(&mut self, _: &LayoutContext, node: AbstractNode, ctx: FlowContext) + -> @mut RenderBox { if !node.is_text() { fail!(~"WAT error: why couldn't we make a text box?"); } diff --git a/src/servo/layout/display_list_builder.rs b/src/servo/layout/display_list_builder.rs index e106347e3f7..85eed2db274 100644 --- a/src/servo/layout/display_list_builder.rs +++ b/src/servo/layout/display_list_builder.rs @@ -29,17 +29,17 @@ pub struct DisplayListBuilder<'self> { } pub trait FlowDisplayListBuilderMethods { - fn build_display_list(@mut self, a: &DisplayListBuilder, b: &Rect, c: &Cell); - fn build_display_list_for_child(@mut self, + fn build_display_list(&self, a: &DisplayListBuilder, b: &Rect, c: &Cell); + fn build_display_list_for_child(&self, a: &DisplayListBuilder, - b: @mut FlowContext, + b: FlowContext, c: &Rect, d: &Point2D, e: &Cell); } impl FlowDisplayListBuilderMethods for FlowContext { - fn build_display_list(@mut self, + fn build_display_list(&self, builder: &DisplayListBuilder, dirty: &Rect, list: &Cell) { @@ -47,27 +47,28 @@ impl FlowDisplayListBuilderMethods for FlowContext { self.build_display_list_recurse(builder, dirty, &zero, list); } - fn build_display_list_for_child(@mut self, + fn build_display_list_for_child(&self, builder: &DisplayListBuilder, - child_flow: @mut FlowContext, - dirty: &Rect, offset: &Point2D, + child_flow: FlowContext, + dirty: &Rect, + offset: &Point2D, list: &Cell) { - // adjust the dirty rect to child flow context coordinates - let d = child_flow.d(); // FIXME: borrow checker workaround - let abs_flow_bounds = d.position.translate(offset); - let adj_offset = offset.add(&d.position.origin); + do child_flow.with_common_info |child_flow_info| { + let abs_flow_bounds = child_flow_info.position.translate(offset); + let adj_offset = offset.add(&child_flow_info.position.origin); - debug!("build_display_list_for_child: rel=%?, abs=%?", - d.position, abs_flow_bounds); - debug!("build_display_list_for_child: dirty=%?, offset=%?", - dirty, offset); + debug!("build_display_list_for_child: rel=%?, abs=%?", + child_flow_info.position, + abs_flow_bounds); + debug!("build_display_list_for_child: dirty=%?, offset=%?", dirty, offset); - if dirty.intersects(&abs_flow_bounds) { - debug!("build_display_list_for_child: intersected. recursing into child flow..."); - child_flow.build_display_list_recurse(builder, dirty, &adj_offset, list); - } else { - debug!("build_display_list_for_child: Did not intersect..."); + if dirty.intersects(&abs_flow_bounds) { + debug!("build_display_list_for_child: intersected. recursing into child flow..."); + child_flow.build_display_list_recurse(builder, dirty, &adj_offset, list); + } else { + debug!("build_display_list_for_child: Did not intersect..."); + } } } } diff --git a/src/servo/layout/flow.rs b/src/servo/layout/flow.rs index 701136c2db2..a6ef3c36d5b 100644 --- a/src/servo/layout/flow.rs +++ b/src/servo/layout/flow.rs @@ -28,7 +28,7 @@ use dom::node::AbstractNode; use layout::block::{BlockFlowData, BlockLayout}; use layout::box::RenderBox; use layout::context::LayoutContext; -use layout::debug::BoxedMutDebugMethods; +use layout::debug::DebugMethods; use layout::display_list_builder::DisplayListBuilder; use layout::inline::{InlineFlowData, InlineLayout}; use layout::root::{RootFlowData, RootLayout}; @@ -43,13 +43,13 @@ use gfx::geometry::Au; /// The type of the formatting context and data specific to each context, such as line box /// structures or float lists. pub enum FlowContext { - AbsoluteFlow(FlowData), - BlockFlow(FlowData, BlockFlowData), - FloatFlow(FlowData), - InlineBlockFlow(FlowData), - InlineFlow(FlowData, InlineFlowData), - RootFlow(FlowData, RootFlowData), - TableFlow(FlowData) + AbsoluteFlow(@mut FlowData), + BlockFlow(@mut BlockFlowData), + FloatFlow(@mut FlowData), + InlineBlockFlow(@mut FlowData), + InlineFlow(@mut InlineFlowData), + RootFlow(@mut RootFlowData), + TableFlow(@mut FlowData), } pub enum FlowContextType { @@ -62,16 +62,18 @@ pub enum FlowContextType { Flow_Table } -/* A particular kind of layout context. It manages the positioning of - render boxes within the context. */ +/// Data common to all flows. +/// +/// FIXME: We need a naming convention for pseudo-inheritance like this. How about +/// `CommonFlowInfo`? pub struct FlowData { node: AbstractNode, - parent: Option<@mut FlowContext>, - first_child: Option<@mut FlowContext>, - last_child: Option<@mut FlowContext>, - prev_sibling: Option<@mut FlowContext>, - next_sibling: Option<@mut FlowContext>, + parent: Option, + first_child: Option, + last_child: Option, + prev_sibling: Option, + next_sibling: Option, /* TODO (Issue #87): debug only */ id: int, @@ -105,31 +107,53 @@ impl FlowData { } impl<'self> FlowContext { - pub fn d(&'self mut self) -> &'self mut FlowData { - unsafe { - match *self { - AbsoluteFlow(ref d) => cast::transmute(d), - BlockFlow(ref d, _) => cast::transmute(d), - FloatFlow(ref d) => cast::transmute(d), - InlineBlockFlow(ref d) => cast::transmute(d), - InlineFlow(ref d, _) => cast::transmute(d), - RootFlow(ref d, _) => cast::transmute(d), - TableFlow(ref d) => cast::transmute(d) + #[inline(always)] + pub fn with_common_info(&self, block: &fn(&mut FlowData) -> R) -> R { + match *self { + AbsoluteFlow(info) => block(info), + BlockFlow(info) => { + let info = &mut *info; // FIXME: Borrow check workaround. + block(&mut info.common) } + FloatFlow(info) => block(info), + InlineBlockFlow(info) => block(info), + InlineFlow(info) => { + let info = &mut *info; // FIXME: Borrow check workaround. + block(&mut info.common) + } + RootFlow(info) => { + let info = &mut *info; // FIXME: Borrow check workaround. + block(&mut info.common) + } + TableFlow(info) => block(info), + } + } + + pub fn position(&self) -> Rect { + do self.with_common_info |common_info| { + common_info.position + } + } + + /// Returns the ID of this flow. + #[inline(always)] + pub fn id(&self) -> int { + do self.with_common_info |info| { + info.id } } /// Iterates over the immediate children of this flow. /// /// TODO: Fold me into `util::tree`. - pub fn each_child(@mut self, f: &fn(@mut FlowContext) -> bool) { - let mut current_opt = self.d().first_child; + pub fn each_child(&self, f: &fn(FlowContext) -> bool) { + let mut current_opt = self.with_common_info(|info| info.first_child); while !current_opt.is_none() { let current = current_opt.get(); if !f(current) { break; } - current_opt = current.d().next_sibling; + current_opt = current.with_common_info(|info| info.next_sibling); } } @@ -137,173 +161,187 @@ impl<'self> FlowContext { /// detached from the tree before calling this method. /// /// TODO: Fold me into `util::tree`. - pub fn add_child(@mut self, child: @mut FlowContext) { - let self_data = self.d(), child_data = child.d(); + pub fn add_child(&self, child: FlowContext) { + do self.with_common_info |self_info| { + do child.with_common_info |child_info| { + assert!(child_info.parent.is_none()); + assert!(child_info.prev_sibling.is_none()); + assert!(child_info.next_sibling.is_none()); - assert!(child_data.parent.is_none()); - assert!(child_data.prev_sibling.is_none()); - assert!(child_data.next_sibling.is_none()); + match self_info.last_child { + None => { + self_info.first_child = Some(child); + } + Some(last_child) => { + do last_child.with_common_info |last_child_info| { + assert!(last_child_info.next_sibling.is_none()); + last_child_info.next_sibling = Some(child); + child_info.prev_sibling = Some(last_child); + } + } + } - match self_data.last_child { - None => { - self_data.first_child = Some(child); - } - Some(last_child) => { - assert!(last_child.d().next_sibling.is_none()); - last_child.d().next_sibling = Some(child); - child_data.prev_sibling = Some(last_child); + self_info.last_child = Some(child); + child_info.parent = Some(*self); } } - - self_data.last_child = Some(child); - child_data.parent = Some(self); } /// Removes the given flow from the tree. /// /// TODO: Fold me into `util::tree`. - pub fn remove_child(@mut self, child: @mut FlowContext) { - let self_data = self.d(), child_data = child.d(); + pub fn remove_child(&self, child: FlowContext) { + do self.with_common_info |self_info| { + do child.with_common_info |child_info| { + assert!(child_info.parent.is_some()); - assert!(child_data.parent.is_some()); - assert!(ptr::ref_eq(&*child_data.parent.get(), self)); + match child_info.prev_sibling { + None => self_info.first_child = child_info.next_sibling, + Some(prev_sibling) => { + do prev_sibling.with_common_info |prev_sibling_info| { + prev_sibling_info.next_sibling = child_info.next_sibling; + child_info.prev_sibling = None; + } + } + } - match child_data.prev_sibling { - None => self_data.first_child = child_data.next_sibling, - Some(prev_sibling) => { - prev_sibling.d().next_sibling = child_data.next_sibling; - child_data.prev_sibling = None; + match child_info.next_sibling { + None => { + do child.with_common_info |child_info| { + self_info.last_child = child_info.prev_sibling; + } + } + Some(next_sibling) => { + do next_sibling.with_common_info |next_sibling_info| { + next_sibling_info.prev_sibling = Some(next_sibling); + child_info.next_sibling = None; + } + } + } + + child_info.parent = None; } } - - match child_data.next_sibling { - None => self_data.last_child = child.d().prev_sibling, - Some(next_sibling) => { - next_sibling.d().prev_sibling = Some(next_sibling); - child_data.next_sibling = None; - } - } - - child_data.parent = None; } - pub fn inline(&'self mut self) -> &'self mut InlineFlowData { - match self { - &InlineFlow(_, ref i) => unsafe { cast::transmute(i) }, - _ => fail!(fmt!("Tried to access inline data of non-inline: f%d", self.d().id)) + pub fn inline(&self) -> @mut InlineFlowData { + match *self { + InlineFlow(info) => info, + _ => fail!(fmt!("Tried to access inline data of non-inline: f%d", self.id())) } } - pub fn block(&'self mut self) -> &'self mut BlockFlowData { - match self { - &BlockFlow(_, ref mut b) => unsafe { cast::transmute(b) }, - _ => fail!(fmt!("Tried to access block data of non-block: f%d", self.d().id)) + pub fn block(&self) -> @mut BlockFlowData { + match *self { + BlockFlow(info) => info, + _ => fail!(fmt!("Tried to access block data of non-block: f%d", self.id())) } } - pub fn root(&'self mut self) -> &'self mut RootFlowData { - match self { - &RootFlow(_, ref r) => unsafe { cast::transmute(r) }, - _ => fail!(fmt!("Tried to access root data of non-root: f%d", self.d().id)) + pub fn root(&self) -> @mut RootFlowData { + match *self { + RootFlow(info) => info, + _ => fail!(fmt!("Tried to access root data of non-root: f%d", self.id())) } } - pub fn bubble_widths(@mut self, ctx: &mut LayoutContext) { - match self { - @BlockFlow(*) => self.bubble_widths_block(ctx), - @InlineFlow(*) => self.bubble_widths_inline(ctx), - @RootFlow(*) => self.bubble_widths_root(ctx), - _ => fail!(fmt!("Tried to bubble_widths of flow: f%d", self.d().id)) + pub fn bubble_widths(&self, ctx: &mut LayoutContext) { + match *self { + BlockFlow(*) => self.bubble_widths_block(ctx), + InlineFlow(info) => info.bubble_widths_inline(ctx), + RootFlow(info) => info.bubble_widths_root(ctx), + _ => fail!(fmt!("Tried to bubble_widths of flow: f%d", self.id())) } } - pub fn assign_widths(@mut self, ctx: &mut LayoutContext) { - match self { - @BlockFlow(*) => self.assign_widths_block(ctx), - @InlineFlow(*) => self.assign_widths_inline(ctx), - @RootFlow(*) => self.assign_widths_root(ctx), - _ => fail!(fmt!("Tried to assign_widths of flow: f%d", self.d().id)) + pub fn assign_widths(&self, ctx: &mut LayoutContext) { + match *self { + BlockFlow(*) => self.assign_widths_block(ctx), + InlineFlow(info) => info.assign_widths_inline(ctx), + RootFlow(info) => info.assign_widths_root(ctx), + _ => fail!(fmt!("Tried to assign_widths of flow: f%d", self.id())) } } - pub fn assign_height(@mut self, ctx: &mut LayoutContext) { - match self { - @BlockFlow(*) => self.assign_height_block(ctx), - @InlineFlow(*) => self.assign_height_inline(ctx), - @RootFlow(*) => self.assign_height_root(ctx), - _ => fail!(fmt!("Tried to assign_height of flow: f%d", self.d().id)) + pub fn assign_height(&self, ctx: &mut LayoutContext) { + match *self { + BlockFlow(*) => self.assign_height_block(ctx), + InlineFlow(info) => info.assign_height_inline(ctx), + RootFlow(info) => info.assign_height_root(ctx), + _ => fail!(fmt!("Tried to assign_height of flow: f%d", self.id())) } } - pub fn build_display_list_recurse(@mut self, + pub fn build_display_list_recurse(&self, builder: &DisplayListBuilder, dirty: &Rect, offset: &Point2D, list: &Cell) { - let d = self.d(); // FIXME: borrow checker workaround - debug!("FlowContext::build_display_list at %?: %s", d.position, self.debug_str()); + do self.with_common_info |info| { + debug!("FlowContext::build_display_list at %?: %s", info.position, self.debug_str()); + } - match self { - @RootFlow(*) => self.build_display_list_root(builder, dirty, offset, list), - @BlockFlow(*) => self.build_display_list_block(builder, dirty, offset, list), - @InlineFlow(*) => self.build_display_list_inline(builder, dirty, offset, list), + match *self { + RootFlow(info) => info.build_display_list_root(builder, dirty, offset, list), + BlockFlow(*) => self.build_display_list_block(builder, dirty, offset, list), + InlineFlow(info) => info.build_display_list_inline(builder, dirty, offset, list), _ => fail!(fmt!("Tried to build_display_list_recurse of flow: %?", self)) } } // Actual methods that do not require much flow-specific logic - pub fn foldl_all_boxes(&mut self, - seed: B, - cb: &fn(a: B, b: @mut RenderBox) -> B) - -> B { - match self { - &RootFlow(*) => { - let root = self.root(); // FIXME: borrow checker workaround + pub fn foldl_all_boxes(&self, seed: B, cb: &fn(a: B, b: @mut RenderBox) -> B) -> B { + match *self { + RootFlow(root) => { + let root = &mut *root; root.box.map_default(seed, |box| { cb(seed, *box) }) } - &BlockFlow(*) => { - let block = self.block(); // FIXME: borrow checker workaround + BlockFlow(block) => { + let block = &mut *block; block.box.map_default(seed, |box| { cb(seed, *box) }) } - &InlineFlow(*) => { - let inline = self.inline(); // FIXME: borrow checker workaround + InlineFlow(inline) => { + let inline = &mut *inline; inline.boxes.foldl(seed, |acc, box| { cb(*acc, *box) }) } _ => fail!(fmt!("Don't know how to iterate node's RenderBoxes for %?", self)) } } - pub fn foldl_boxes_for_node(&mut self, + pub fn foldl_boxes_for_node(&self, node: AbstractNode, seed: B, - cb: &fn(a: B, @mut RenderBox) -> B) + callback: &fn(a: B, @mut RenderBox) -> B) -> B { do self.foldl_all_boxes(seed) |acc, box| { - if box.d().node == node { cb(acc, box) } - else { acc } + if box.d().node == node { + callback(acc, box) + } else { + acc + } } } - pub fn iter_all_boxes(&mut self, cb: &fn(@mut RenderBox) -> bool) { - match self { - &RootFlow(*) => { - let root = self.root(); // FIXME: borrow checker workaround + pub fn iter_all_boxes(&self, cb: &fn(@mut RenderBox) -> bool) { + match *self { + RootFlow(root) => { + let root = &mut *root; for root.box.each |box| { if !cb(*box) { break; } } } - &BlockFlow(*) => { - let block = self.block(); // FIXME: borrow checker workaround + BlockFlow(block) => { + let block = &mut *block; for block.box.each |box| { if !cb(*box) { break; } } } - &InlineFlow(*) => { - let inline = self.inline(); // FIXME: borrow checker workaround + InlineFlow(inline) => { + let inline = &mut *inline; for inline.boxes.each |box| { if !cb(*box) { break; @@ -314,10 +352,10 @@ impl<'self> FlowContext { } } - pub fn iter_boxes_for_node(&mut self, node: AbstractNode, cb: &fn(@mut RenderBox) -> bool) { + pub fn iter_boxes_for_node(&self, node: AbstractNode, callback: &fn(@mut RenderBox) -> bool) { for self.iter_all_boxes |box| { if box.d().node == node { - if !cb(box) { + if !callback(box) { break; } } @@ -325,13 +363,13 @@ impl<'self> FlowContext { } } -impl BoxedMutDebugMethods for FlowContext { - fn dump(@mut self) { +impl DebugMethods for FlowContext { + fn dump(&self) { self.dump_indent(0); } /// Dumps the flow tree, for debugging, with indentation. - fn dump_indent(@mut self, indent: uint) { + fn dump_indent(&self, indent: uint) { let mut s = ~"|"; for uint::range(0, indent) |_i| { s += ~"---- "; @@ -346,24 +384,26 @@ impl BoxedMutDebugMethods for FlowContext { } } - fn debug_str(@mut self) -> ~str { + fn debug_str(&self) -> ~str { let repr = match *self { - InlineFlow(*) => { - let inline = self.inline(); // FIXME: borrow checker workaround + InlineFlow(inline) => { + let inline = &mut *inline; let mut s = inline.boxes.foldl(~"InlineFlow(children=", |s, box| { fmt!("%s b%d", *s, box.d().id) }); s += ~")"; s }, - BlockFlow(*) => { - match self.block().box { + BlockFlow(block) => { + let block = &mut *block; + match block.box { Some(box) => fmt!("BlockFlow(box=b%d)", box.d().id), None => ~"BlockFlow", } }, - RootFlow(*) => { - match self.root().box { + RootFlow(root) => { + let root = &mut *root; + match root.box { Some(box) => fmt!("RootFlo(box=b%d)", box.d().id), None => ~"RootFlow", } @@ -371,8 +411,9 @@ impl BoxedMutDebugMethods for FlowContext { _ => ~"(Unknown flow)" }; - let d = self.d(); // FIXME: borrow checker workaround - fmt!("f%? %?", d.id, repr) + do self.with_common_info |info| { + fmt!("f%? %?", info.id, repr) + } } } diff --git a/src/servo/layout/inline.rs b/src/servo/layout/inline.rs index a5a6f5d355b..57cbe17de56 100644 --- a/src/servo/layout/inline.rs +++ b/src/servo/layout/inline.rs @@ -2,26 +2,27 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use core; use core::cell::Cell; +use core; use dom::node::AbstractNode; use layout::box::*; use layout::context::LayoutContext; use layout::debug::{BoxedDebugMethods, BoxedMutDebugMethods, DebugMethods}; use layout::display_list_builder::DisplayListBuilder; -use layout::flow::{FlowContext, InlineFlow}; +use layout::flow::{FlowContext, FlowData, InlineFlow}; use layout::text::{UnscannedMethods, adapt_textbox_with_range}; +use core::util; use geom::{Point2D, Rect, Size2D}; use gfx::display_list::DisplayList; use gfx::geometry::Au; use gfx::image::holder; use gfx::text::text_run::TextRun; use gfx::text::util::*; +use newcss::values::{CSSTextAlignCenter, CSSTextAlignJustify, CSSTextAlignLeft}; +use newcss::values::{CSSTextAlignRight}; use servo_util::range::Range; -use newcss::values::{CSSTextAlignCenter, CSSTextAlignJustify, CSSTextAlignLeft, CSSTextAlignRight}; use std::deque::Deque; -use core::util; /* Lineboxes are represented as offsets into the child list, rather than @@ -81,7 +82,9 @@ pub impl ElementMapping { do self.entries.eachi |i, nr| { cb(i, nr) } } - fn repair_for_box_changes(&mut self, old_boxes: &[@mut RenderBox], new_boxes: &[@mut RenderBox]) { + fn repair_for_box_changes(&mut self, + old_boxes: &[@mut RenderBox], + new_boxes: &[@mut RenderBox]) { let entries = &mut self.entries; debug!("--- Old boxes: ---"); @@ -166,34 +169,30 @@ priv impl TextRunScanner { } priv impl TextRunScanner { - fn scan_for_runs(&mut self, ctx: &mut LayoutContext, flow: @mut FlowContext) { - let inline = flow.inline(); + fn scan_for_runs(&mut self, ctx: &mut LayoutContext, flow: FlowContext) { + let inline = &mut *flow.inline(); assert!(inline.boxes.len() > 0); + debug!("TextRunScanner: scanning %u boxes for text runs...", inline.boxes.len()); - let in_boxes = &mut flow.inline().boxes; - //do boxes.swap |in_boxes| { - debug!("TextRunScanner: scanning %u boxes for text runs...", in_boxes.len()); - let mut out_boxes = ~[]; - - for uint::range(0, in_boxes.len()) |box_i| { - debug!("TextRunScanner: considering box: %?", in_boxes[box_i].debug_str()); - if box_i > 0 && !can_coalesce_text_nodes(*in_boxes, box_i-1, box_i) { - self.flush_clump_to_list(ctx, flow, *in_boxes, &mut out_boxes); - } - self.clump.extend_by(1); - } - // handle remaining clumps - if self.clump.length() > 0 { - self.flush_clump_to_list(ctx, flow, *in_boxes, &mut out_boxes); + let mut out_boxes = ~[]; + for uint::range(0, inline.boxes.len()) |box_i| { + debug!("TextRunScanner: considering box: %?", inline.boxes[box_i].debug_str()); + if box_i > 0 && !can_coalesce_text_nodes(inline.boxes, box_i-1, box_i) { + self.flush_clump_to_list(ctx, flow, inline.boxes, &mut out_boxes); } + self.clump.extend_by(1); + } + // handle remaining clumps + if self.clump.length() > 0 { + self.flush_clump_to_list(ctx, flow, inline.boxes, &mut out_boxes); + } - debug!("TextRunScanner: swapping out boxes."); - // swap out old and new box list of flow, by supplying - // temp boxes as return value to boxes.swap |...| - util::swap(in_boxes, &mut out_boxes); - //} + debug!("TextRunScanner: swapping out boxes."); - // helper functions + // Swap out the old and new box list of the flow. + inline.boxes = out_boxes; + + // A helper function. fn can_coalesce_text_nodes(boxes: &[@mut RenderBox], left_i: uint, right_i: uint) -> bool { assert!(left_i < boxes.len()); assert!(right_i > 0 && right_i < boxes.len()); @@ -221,7 +220,7 @@ priv impl TextRunScanner { // boxes are appended, the caller swaps the flow's box list. fn flush_clump_to_list(&mut self, ctx: &mut LayoutContext, - flow: @mut FlowContext, + flow: FlowContext, in_boxes: &[@mut RenderBox], out_boxes: &mut ~[@mut RenderBox]) { assert!(self.clump.length() > 0); @@ -337,14 +336,14 @@ struct PendingLine { } struct LineboxScanner { - flow: @mut FlowContext, + flow: FlowContext, new_boxes: ~[@mut RenderBox], work_list: @mut Deque<@mut RenderBox>, pending_line: PendingLine, line_spans: ~[Range], } -fn LineboxScanner(inline: @mut FlowContext) -> LineboxScanner { +fn LineboxScanner(inline: FlowContext) -> LineboxScanner { assert!(inline.starts_inline_flow()); LineboxScanner { @@ -358,7 +357,7 @@ fn LineboxScanner(inline: @mut FlowContext) -> LineboxScanner { impl LineboxScanner { priv fn reset_scanner(&mut self) { - debug!("Resetting line box scanner's state for flow f%d.", self.flow.d().id); + debug!("Resetting line box scanner's state for flow f%d.", self.flow.id()); self.line_spans = ~[]; self.new_boxes = ~[]; self.reset_linebox(); @@ -412,20 +411,13 @@ impl LineboxScanner { priv fn swap_out_results(&mut self) { debug!("LineboxScanner: Propagating scanned lines[n=%u] to inline flow f%d", - self.line_spans.len(), self.flow.d().id); + self.line_spans.len(), + self.flow.id()); - //do self.new_boxes.swap |boxes| { - let inline_boxes = &mut self.flow.inline().boxes; + let inline_boxes = &mut self.flow.inline().boxes; util::swap(inline_boxes, &mut self.new_boxes); - //inline_boxes = boxes; - // ~[] - //}; - //do self.line_spans.swap |boxes| { - let lines = &mut self.flow.inline().lines; + let lines = &mut self.flow.inline().lines; util::swap(lines, &mut self.line_spans); - // lines = boxes; - // ~[] - //}; } priv fn flush_current_line(&mut self) { @@ -450,7 +442,7 @@ impl LineboxScanner { linebox_align = CSSTextAlignLeft; } - let slack_width = self.flow.d().position.size.width - self.pending_line.width; + let slack_width = self.flow.position().size.width - self.pending_line.width; match linebox_align { // So sorry, but justified text is more complicated than shuffling linebox coordinates. // TODO(Issue #213): implement `text-align: justify` @@ -478,6 +470,7 @@ impl LineboxScanner { } }, } + // clear line and add line mapping debug!("LineboxScanner: Saving information for flushed line %u.", self.line_spans.len()); self.line_spans.push(line_range); @@ -486,7 +479,7 @@ impl LineboxScanner { // return value: whether any box was appended. priv fn try_append_to_line(&mut self, ctx: &LayoutContext, in_box: @mut RenderBox) -> bool { - let remaining_width = self.flow.d().position.size.width - self.pending_line.width; + let remaining_width = self.flow.position().size.width - self.pending_line.width; let in_box_width = in_box.d().position.size.width; let line_is_empty: bool = self.pending_line.range.length() == 0; @@ -579,6 +572,9 @@ impl LineboxScanner { } pub struct InlineFlowData { + /// Data common to all flows. + common: FlowData, + // A vec of all inline render boxes. Several boxes may // correspond to one Node/Element. boxes: ~[@mut RenderBox], @@ -591,74 +587,79 @@ pub struct InlineFlowData { elems: ElementMapping } -pub fn InlineFlowData() -> InlineFlowData { - InlineFlowData { - boxes: ~[], - lines: ~[], - elems: ElementMapping::new(), +impl InlineFlowData { + pub fn new(common: FlowData) -> InlineFlowData { + InlineFlowData { + common: common, + boxes: ~[], + lines: ~[], + elems: ElementMapping::new(), + } } } pub trait InlineLayout { fn starts_inline_flow(&self) -> bool; - - fn bubble_widths_inline(@mut self, ctx: &mut LayoutContext); - fn assign_widths_inline(@mut self, ctx: &mut LayoutContext); - fn assign_height_inline(@mut self, ctx: &mut LayoutContext); - fn build_display_list_inline(@mut self, a: &DisplayListBuilder, b: &Rect, c: &Point2D, - d: &Cell); } impl InlineLayout for FlowContext { - fn starts_inline_flow(&self) -> bool { match *self { InlineFlow(*) => true, _ => false } } - - fn bubble_widths_inline(@mut self, ctx: &mut LayoutContext) { - assert!(self.starts_inline_flow()); - - let mut scanner = TextRunScanner::new(); - scanner.scan_for_runs(ctx, self); - - let mut min_width = Au(0); - let mut pref_width = Au(0); - - let boxes = &mut self.inline().boxes; - for boxes.each |box| { - debug!("FlowContext[%d]: measuring %s", self.d().id, box.debug_str()); - min_width = Au::max(min_width, box.get_min_width(ctx)); - pref_width = Au::max(pref_width, box.get_pref_width(ctx)); + fn starts_inline_flow(&self) -> bool { + match *self { + InlineFlow(*) => true, + _ => false } + } +} - self.d().min_width = min_width; - self.d().pref_width = pref_width; +impl InlineFlowData { + pub fn bubble_widths_inline(@mut self, ctx: &mut LayoutContext) { + let mut scanner = TextRunScanner::new(); + scanner.scan_for_runs(ctx, InlineFlow(self)); + + { + let this = &mut *self; + + let mut min_width = Au(0); + let mut pref_width = Au(0); + + for this.boxes.each |box| { + debug!("FlowContext[%d]: measuring %s", self.common.id, box.debug_str()); + min_width = Au::max(min_width, box.get_min_width(ctx)); + pref_width = Au::max(pref_width, box.get_pref_width(ctx)); + } + + this.common.min_width = min_width; + this.common.pref_width = pref_width; + } } - /* Recursively (top-down) determines the actual width of child - contexts and boxes. When called on this context, the context has - had its width set by the parent context. */ - fn assign_widths_inline(@mut self, ctx: &mut LayoutContext) { - assert!(self.starts_inline_flow()); + /// Recursively (top-down) determines the actual width of child contexts and boxes. When called + /// on this context, the context has had its width set by the parent context. + pub fn assign_widths_inline(@mut self, ctx: &mut LayoutContext) { + { + // initialize (content) box widths, if they haven't been + // already. This could be combined with LineboxScanner's walk + // over the box list, and/or put into RenderBox. + let this = &mut *self; + for this.boxes.each |&box| { + let box2 = &mut *box; + box.d().position.size.width = match *box2 { + ImageBox(_, ref img) => { + let img2: &mut holder::ImageHolder = unsafe { cast::transmute(img) }; + Au::from_px(img2.get_size().get_or_default(Size2D(0,0)).width) + } + TextBox(*) => { + // Text boxes are initialized with dimensions. + box.d().position.size.width + }, + // TODO(Issue #225): different cases for 'inline-block', other replaced content + GenericBox(*) => Au::from_px(45), + _ => fail!(fmt!("Tried to assign width to unknown Box variant: %?", box)) + }; + } // for boxes.each |box| + } - // initialize (content) box widths, if they haven't been - // already. This could be combined with LineboxScanner's walk - // over the box list, and/or put into RenderBox. - let boxes = &mut self.inline().boxes; - for boxes.each |&box| { - let box2 = &mut *box; - box.d().position.size.width = match *box2 { - ImageBox(_, ref img) => { - let img2: &mut holder::ImageHolder = unsafe { cast::transmute(img) }; - Au::from_px(img2.get_size().get_or_default(Size2D(0,0)).width) - } - TextBox(*) => { /* text boxes are initialized with dimensions */ - box.d().position.size.width - }, - // TODO(Issue #225): different cases for 'inline-block', other replaced content - GenericBox(*) => Au::from_px(45), - _ => fail!(fmt!("Tried to assign width to unknown Box variant: %?", box)) - }; - } // for boxes.each |box| - - let mut scanner = LineboxScanner(self); + let mut scanner = LineboxScanner(InlineFlow(self)); scanner.scan_for_lines(ctx); /* There are no child contexts, so stop here. */ @@ -670,31 +671,34 @@ impl InlineLayout for FlowContext { // 'inline-block' box that created this flow before recursing. } - fn assign_height_inline(@mut self, _ctx: &mut LayoutContext) { - // TODO(Issue #226): get CSS 'line-height' property from - // containing block's style to determine minimum linebox height. - // TODO(Issue #226): get CSS 'line-height' property from each non-replaced - // inline element to determine its height for computing linebox height. + pub fn assign_height_inline(&mut self, _ctx: &mut LayoutContext) { + // 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 line_height = Au::from_px(20); let mut cur_y = Au(0); - let lines = &mut self.inline().lines; - for lines.eachi |i, line_span| { + for self.lines.eachi |i, line_span| { debug!("assign_height_inline: processing line %u with box span: %?", i, line_span); // coords relative to left baseline let mut linebox_bounding_box = Au::zero_rect(); - let boxes = &mut self.inline().boxes; + let boxes = &mut self.boxes; for line_span.eachi |box_i| { - let cur_box : &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround + let cur_box: &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround - // compute box height. + // Compute the height of each box. let d = cur_box.d(); // FIXME: borrow checker workaround - let cur_box : &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround + let cur_box: &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround + d.position.size.height = match *cur_box { ImageBox(_, ref img) => { Au::from_px(img.size().height) } - TextBox(*) => { /* text boxes are initialized with dimensions */ + TextBox(*) => { + // Text boxes are initialized with dimensions. d.position.size.height }, // TODO(Issue #225): different cases for 'inline-block', other replaced content @@ -706,31 +710,38 @@ impl InlineLayout for FlowContext { } }; - // compute bounding rect, with left baseline as origin. - // so, linebox height is a matter of lining up ideal baselines, - // and then using the union of all these rects. + // Compute the bounding rect with the left baseline as origin. Linebox height is a + // matter of lining up ideal baselines and then using the union of all these rects. let bounding_box = match *cur_box { - // adjust to baseline coords - // TODO(Issue #227): use left/right margins, border, padding for nonreplaced content, - // and also use top/bottom margins, border, padding for replaced or inline-block content. - // TODO(Issue #225): use height, width for 'inline-block', other replaced content + // Adjust to baseline coordinates. + // + // TODO(#227): Use left/right margins, border, padding for nonreplaced content, + // and also use top/bottom margins, border, padding for replaced or + // inline-block content. + // + // TODO(#225): Use height, width for 'inline-block' and other replaced content. ImageBox(*) | GenericBox(*) => { let box_bounds = d.position; box_bounds.translate(&Point2D(Au(0), -d.position.size.height)) }, - // adjust bounding box metric to box's horizontal offset - // TODO: we can use font metrics directly instead of re-measuring for the bounding box. + + // Adjust the bounding box metric to the box's horizontal offset. + // TODO: We can use font metrics directly instead of re-measuring for the + // bounding box. TextBox(_, data) => { let text_bounds = data.run.metrics_for_range(&data.range).bounding_box; text_bounds.translate(&Point2D(d.position.origin.x, Au(0))) }, + _ => { let cur_box = boxes[box_i]; // FIXME: borrow checker workaround fail!(fmt!("Tried to compute bounding box of unknown Box variant: %s", cur_box.debug_str())) } }; - debug!("assign_height_inline: bounding box for box b%d = %?", cur_box.d().id, bounding_box); + debug!("assign_height_inline: bounding box for box b%d = %?", + cur_box.d().id, + bounding_box); linebox_bounding_box = linebox_bounding_box.union(&bounding_box); debug!("assign_height_inline: linebox bounding box = %?", linebox_bounding_box); } @@ -743,30 +754,33 @@ impl InlineLayout for FlowContext { // 'line-height' when calculating linebox height. Then, go back over // and set y offsets according to 'vertical-align' property of containing block. let halfleading = match cur_box { - @TextBox(_, data) => { (data.run.font.metrics.em_size - line_height).scale_by(0.5f) }, - _ => { Au(0) } + @TextBox(_, data) => { + (data.run.font.metrics.em_size - line_height).scale_by(0.5f) + }, + _ => Au(0), }; - cur_box.d().position.origin.y = cur_y + halfleading + (baseline_offset - cur_box.d().position.size.height); + cur_box.d().position.origin.y = + cur_y + halfleading + (baseline_offset - cur_box.d().position.size.height); } cur_y += Au::max(line_height, linebox_height); } // /lines.each |line_span| - self.d().position.size.height = cur_y; + self.common.position.size.height = cur_y; } - fn build_display_list_inline(@mut self, builder: &DisplayListBuilder, dirty: &Rect, - offset: &Point2D, list: &Cell) { - - assert!(self.starts_inline_flow()); - + pub fn build_display_list_inline(&mut self, + builder: &DisplayListBuilder, + dirty: &Rect, + offset: &Point2D, + list: &Cell) { // TODO(Issue #228): once we form line boxes and have their cached bounds, we can be // smarter and not recurse on a line if nothing in it can intersect dirty - let inline = self.inline(); // FIXME: borrow checker workaround debug!("FlowContext[%d]: building display list for %u inline boxes", - self.d().id, inline.boxes.len()); - let boxes = &mut self.inline().boxes; - for boxes.each |box| { + self.common.id, + self.boxes.len()); + + for self.boxes.each |box| { box.build_display_list(builder, dirty, offset, list) } diff --git a/src/servo/layout/layout_task.rs b/src/servo/layout/layout_task.rs index 3a3b106324d..c8f7bf23708 100644 --- a/src/servo/layout/layout_task.rs +++ b/src/servo/layout/layout_task.rs @@ -205,10 +205,9 @@ impl Layout { } } - let layout_root: @mut FlowContext = do time("layout: tree construction") { + let layout_root: FlowContext = do time("layout: tree construction") { let mut builder = LayoutTreeBuilder::new(); - let layout_root: @mut FlowContext = match builder.construct_trees(&layout_ctx, - *node) { + let layout_root: FlowContext = match builder.construct_trees(&layout_ctx, *node) { Ok(root) => root, Err(*) => fail!(~"Root flow should always exist") }; @@ -235,9 +234,11 @@ impl Layout { // TODO: set options on the builder before building // TODO: be smarter about what needs painting - layout_root.build_display_list(&builder, - © layout_root.d().position, - display_list); + do layout_root.with_common_info |layout_root_info| { + layout_root.build_display_list(&builder, + © layout_root_info.position, + display_list); + } let render_layer = RenderLayer { display_list: display_list.take(), diff --git a/src/servo/layout/root.rs b/src/servo/layout/root.rs index af92b037d78..d74d8bb345b 100644 --- a/src/servo/layout/root.rs +++ b/src/servo/layout/root.rs @@ -10,27 +10,28 @@ use gfx::geometry::Au; use layout::block::BlockLayout; use layout::box::RenderBox; use layout::context::LayoutContext; -use layout::flow::{FlowContext, RootFlow}; +use layout::flow::{FlowContext, FlowData, RootFlow}; use layout::display_list_builder::DisplayListBuilder; pub struct RootFlowData { + /// Data common to all flows. + common: FlowData, + + /// The render box at the root of the tree. box: Option<@mut RenderBox> } -pub fn RootFlowData() -> RootFlowData { - RootFlowData { - box: None +impl RootFlowData { + pub fn new(common: FlowData) -> RootFlowData { + RootFlowData { + common: common, + box: None, + } } } pub trait RootLayout { fn starts_root_flow(&self) -> bool; - - fn bubble_widths_root(@mut self, ctx: &LayoutContext); - fn assign_widths_root(@mut self, ctx: &LayoutContext); - fn assign_height_root(@mut self, ctx: &LayoutContext); - fn build_display_list_root(@mut self, a: &DisplayListBuilder, b: &Rect, - c: &Point2D, d: &Cell); } impl RootLayout for FlowContext { @@ -40,47 +41,47 @@ impl RootLayout for FlowContext { _ => false } } +} - /* defer to the block algorithm */ - fn bubble_widths_root(@mut self, ctx: &LayoutContext) { - assert!(self.starts_root_flow()); - self.bubble_widths_block(ctx) +impl RootFlowData { + /// Defer to the block algorithm. + pub fn bubble_widths_root(@mut self, ctx: &LayoutContext) { + RootFlow(self).bubble_widths_block(ctx) } - fn assign_widths_root(@mut self, ctx: &LayoutContext) { - assert!(self.starts_root_flow()); + pub fn assign_widths_root(@mut self, ctx: &LayoutContext) { + self.common.position.origin = Au::zero_point(); + self.common.position.size.width = ctx.screen_size.size.width; - self.d().position.origin = Au::zero_point(); - self.d().position.size.width = ctx.screen_size.size.width; - - self.assign_widths_block(ctx) + RootFlow(self).assign_widths_block(ctx) } - fn assign_height_root(@mut self, ctx: &LayoutContext) { - assert!(self.starts_root_flow()); - + pub fn assign_height_root(@mut self, ctx: &LayoutContext) { // this is essentially the same as assign_height_block(), except // the root adjusts self height to at least cover the viewport. let mut cur_y = Au(0); - for self.each_child |child_ctx| { - child_ctx.d().position.origin.y = cur_y; - cur_y += child_ctx.d().position.size.height; + for RootFlow(self).each_child |child_flow| { + do child_flow.with_common_info |child_common_info| { + child_common_info.position.origin.y = cur_y; + cur_y += child_common_info.position.size.height; + } } - self.d().position.size.height = Au::max(ctx.screen_size.size.height, cur_y); + self.common.position.size.height = Au::max(ctx.screen_size.size.height, cur_y); - do self.with_block_box |box| { + do RootFlow(self).with_block_box |box| { box.d().position.origin.y = Au(0); box.d().position.size.height = Au::max(ctx.screen_size.size.height, cur_y); let (_used_top, _used_bot) = box.get_used_height(); } } - fn build_display_list_root(@mut self, builder: &DisplayListBuilder, dirty: &Rect, - offset: &Point2D, list: &Cell) { - assert!(self.starts_root_flow()); - - self.build_display_list_block(builder, dirty, offset, list); + pub fn build_display_list_root(@mut self, + builder: &DisplayListBuilder, + dirty: &Rect, + offset: &Point2D, + list: &Cell) { + RootFlow(self).build_display_list_block(builder, dirty, offset, list); } } diff --git a/src/servo/layout/traverse.rs b/src/servo/layout/traverse.rs index b4710b974bc..d242bcae4f5 100644 --- a/src/servo/layout/traverse.rs +++ b/src/servo/layout/traverse.rs @@ -6,22 +6,22 @@ use layout::flow::FlowContext; /// A trait for running tree-based traversals over flows contexts. pub trait FlowContextTraversals { - fn traverse_preorder(@mut self, preorder_cb: &fn(@mut FlowContext)); - fn traverse_postorder(@mut self, postorder_cb: &fn(@mut FlowContext)); + fn traverse_preorder(&self, preorder_cb: &fn(FlowContext)); + fn traverse_postorder(&self, postorder_cb: &fn(FlowContext)); } impl FlowContextTraversals for FlowContext { - fn traverse_preorder(@mut self, preorder_cb: &fn(@mut FlowContext)) { - preorder_cb(self); + fn traverse_preorder(&self, preorder_cb: &fn(FlowContext)) { + preorder_cb(*self); for self.each_child |child| { child.traverse_preorder(preorder_cb); } } - fn traverse_postorder(@mut self, postorder_cb: &fn(@mut FlowContext)) { + fn traverse_postorder(&self, postorder_cb: &fn(FlowContext)) { for self.each_child |child| { child.traverse_postorder(postorder_cb); } - postorder_cb(self); + postorder_cb(*self); } } From d374d6561de07ac8488dc7f85cccca33423314c0 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 15:46:12 -0700 Subject: [PATCH 05/12] layout: Document the root flow. --- src/servo/layout/flow.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/servo/layout/flow.rs b/src/servo/layout/flow.rs index a6ef3c36d5b..67e9dc31964 100644 --- a/src/servo/layout/flow.rs +++ b/src/servo/layout/flow.rs @@ -14,15 +14,18 @@ /// /// Currently, the important types of flows are: /// -/// * `BlockFlow`: a flow that establishes a block context. It has several child flows, each of +/// * `BlockFlow`: A flow that establishes a block context. It has several child flows, each of /// which are positioned according to block formatting context rules (CSS block boxes). Block /// flows also contain a single `GenericBox` to represent their rendered borders, padding, etc. /// (In the future, this render box may be folded into `BlockFlow` to save space.) /// -/// * `InlineFlow`: a flow that establishes an inline context. It has a flat list of child +/// * `InlineFlow`: A flow that establishes an inline context. It has a flat list of child /// boxes/flows that are subject to inline layout and line breaking and structs to represent /// line breaks and mapping to CSS boxes, for the purpose of handling `getClientRects()` and /// similar methods. +/// +/// * `RootFlow`: The flow at the root of the tree. This flow behaves like a `BlockFlow`, except +/// that stretches to the boundaries of the viewport. use dom::node::AbstractNode; use layout::block::{BlockFlowData, BlockLayout}; @@ -30,11 +33,10 @@ use layout::box::RenderBox; use layout::context::LayoutContext; use layout::debug::DebugMethods; use layout::display_list_builder::DisplayListBuilder; -use layout::inline::{InlineFlowData, InlineLayout}; -use layout::root::{RootFlowData, RootLayout}; +use layout::inline::{InlineFlowData}; +use layout::root::{RootFlowData}; use core::cell::Cell; -use core::ptr; use geom::point::Point2D; use geom::rect::Rect; use gfx::display_list::DisplayList; From 58679216b3a4849050b27cf1569434a99310fd51 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 18:54:49 -0700 Subject: [PATCH 06/12] util: Add a new `tree` module to avoid duplicating pointer stitching. Add some basic support to AbstractNode for it. --- src/servo-util/servo_util.rc | 1 + src/servo-util/tree.rs | 171 +++++++++++++++++++++++++++++++++++ src/servo/css/matching.rs | 2 + src/servo/dom/document.rs | 3 +- src/servo/dom/node.rs | 74 +++++++++++++-- 5 files changed, 243 insertions(+), 8 deletions(-) create mode 100644 src/servo-util/tree.rs diff --git a/src/servo-util/servo_util.rc b/src/servo-util/servo_util.rc index 0f67d299cb3..5779542af3a 100644 --- a/src/servo-util/servo_util.rc +++ b/src/servo-util/servo_util.rc @@ -13,6 +13,7 @@ extern mod std; pub mod cache; pub mod range; pub mod time; +pub mod tree; pub mod url; pub mod vec; diff --git a/src/servo-util/tree.rs b/src/servo-util/tree.rs new file mode 100644 index 00000000000..2e46ee541fa --- /dev/null +++ b/src/servo-util/tree.rs @@ -0,0 +1,171 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! Helper functions for garbage collected doubly-linked trees. + +/// The basic trait. This function is meant to encapsulate a clonable reference to a tree node. +pub trait TreeNodeRef : Clone { + /// Borrows this node as immutable. + fn with_immutable_node(&self, callback: &fn(&N) -> R) -> R; + + /// Borrows this node as mutable. + fn with_mutable_node(&self, callback: &fn(&mut N) -> R) -> R; +} + +/// The contents of a tree node. +pub trait TreeNode { + /// Returns the parent of this node. + fn parent_node(&self) -> Option; + + /// Returns the first child of this node. + fn first_child(&self) -> Option; + + /// Returns the last child of this node. + fn last_child(&self) -> Option; + + /// Returns the previous sibling of this node. + fn prev_sibling(&self) -> Option; + + /// Returns the next sibling of this node. + fn next_sibling(&self) -> Option; + + /// Sets the parent of this node. + fn set_parent_node(&mut self, new_parent: Option); + + /// Sets the first child of this node. + fn set_first_child(&mut self, new_first_child: Option); + + /// Sets the last child of this node. + fn set_last_child(&mut self, new_last_child: Option); + + /// Sets the previous sibling of this node. + fn set_prev_sibling(&mut self, new_prev_sibling: Option); + + /// Sets the next sibling of this node. + fn set_next_sibling(&mut self, new_next_sibling: Option); +} + +/// A set of helper functions useful for operating on trees. +pub trait TreeUtils { + /// Returns true if this node is disconnected from the tree or has no children. + fn is_leaf(&self) -> bool; + + /// Adds a new child to the end of this node's list of children. + /// + /// Fails unless `new_child` is disconnected from the tree. + fn add_child(&self, new_child: Self); + + /// Removes the given child from this node's list of children. + /// + /// Fails unless `child` is a child of this node. (FIXME: This is not yet checked.) + fn remove_child(&self, child: Self); + + /// Iterates over all children of this node. + fn each_child(&self, callback: &fn(Self) -> bool); + + /// Iterates over this node and all its descendants, in preorder. + fn traverse_preorder(&self, callback: &fn(Self) -> bool) -> bool; + + /// Iterates over this node and all its descendants, in postorder. + fn traverse_postorder(&self, callback: &fn(Self) -> bool) -> bool; +} + +impl,N:TreeNode> TreeUtils for NR { + fn is_leaf(&self) -> bool { + do self.with_immutable_node |this_node| { + this_node.first_child().is_none() + } + } + + fn add_child(&self, new_child: NR) { + do self.with_mutable_node |this_node| { + do new_child.with_mutable_node |new_child_node| { + assert!(new_child_node.parent_node().is_none()); + assert!(new_child_node.prev_sibling().is_none()); + assert!(new_child_node.next_sibling().is_none()); + + match this_node.last_child() { + None => this_node.set_first_child(Some(new_child.clone())), + Some(last_child) => { + do last_child.with_mutable_node |last_child_node| { + assert!(last_child_node.next_sibling().is_none()); + last_child_node.set_next_sibling(Some(new_child.clone())); + new_child_node.set_prev_sibling(Some(last_child.clone())); + } + } + } + + this_node.set_last_child(Some(new_child.clone())); + new_child_node.set_parent_node(Some((*self).clone())); + } + } + } + + fn remove_child(&self, child: NR) { + do self.with_mutable_node |this_node| { + do child.with_mutable_node |child_node| { + assert!(child_node.parent_node().is_some()); + + match child_node.prev_sibling() { + None => this_node.set_first_child(child_node.next_sibling()), + Some(prev_sibling) => { + do prev_sibling.with_mutable_node |prev_sibling_node| { + prev_sibling_node.set_next_sibling(child_node.next_sibling()); + } + } + } + + match child_node.next_sibling() { + None => this_node.set_last_child(child_node.prev_sibling()), + Some(next_sibling) => { + do next_sibling.with_mutable_node |next_sibling_node| { + next_sibling_node.set_prev_sibling(child_node.prev_sibling()); + } + } + } + + child_node.set_prev_sibling(None); + child_node.set_next_sibling(None); + child_node.set_parent_node(None); + } + } + } + + fn each_child(&self, callback: &fn(NR) -> bool) { + let mut maybe_current = self.with_immutable_node(|n| n.first_child()); + while !maybe_current.is_none() { + let current = maybe_current.get_ref().clone(); + if !callback(current.clone()) { + break; + } + + maybe_current = current.with_immutable_node(|n| n.next_sibling()); + } + } + + fn traverse_preorder(&self, callback: &fn(NR) -> bool) -> bool { + if !callback((*self).clone()) { + return false; + } + + for self.each_child |kid| { + if !kid.traverse_preorder(callback) { + return false; + } + } + + true + } + + fn traverse_postorder(&self, callback: &fn(NR) -> bool) -> bool { + for self.each_child |kid| { + if !kid.traverse_postorder(callback) { + return false; + } + } + + callback((*self).clone()) + } +} + diff --git a/src/servo/css/matching.rs b/src/servo/css/matching.rs index 4739c4928a1..3a95aceac07 100644 --- a/src/servo/css/matching.rs +++ b/src/servo/css/matching.rs @@ -10,6 +10,8 @@ use dom::node::AbstractNode; use newcss::complete::CompleteSelectResults; use newcss::select::{SelectCtx, SelectResults}; +use servo_util::tree::TreeUtils; + pub trait MatchMethods { fn restyle_subtree(&self, select_ctx: &SelectCtx); } diff --git a/src/servo/dom/document.rs b/src/servo/dom/document.rs index 11fc5de0457..fc841534718 100644 --- a/src/servo/dom/document.rs +++ b/src/servo/dom/document.rs @@ -11,6 +11,7 @@ use dom::node::AbstractNode; use dom::window::Window; use js::jsapi::bindgen::{JS_AddObjectRoot, JS_RemoveObjectRoot}; +use servo_util::tree::TreeUtils; pub struct Document { root: AbstractNode, @@ -69,4 +70,4 @@ pub impl Document { chan.send(ReflowEvent) }; } -} \ No newline at end of file +} diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs index d8745ae00fb..8f8ffe45ed2 100644 --- a/src/servo/dom/node.rs +++ b/src/servo/dom/node.rs @@ -7,20 +7,21 @@ // use content::content_task::global_content; -use dom::bindings; use dom::bindings::codegen; use dom::bindings::node; use dom::bindings::utils::WrapperCache; +use dom::bindings; use dom::characterdata::CharacterData; use dom::document::Document; use dom::element::{Element, ElementTypeId, HTMLImageElement, HTMLImageElementTypeId}; use dom::element::{HTMLStyleElementTypeId}; +use js::rust::Compartment; use layout::debug::DebugMethods; use layout::flow::FlowContext; use newcss::complete::CompleteSelectResults; -use js::rust::Compartment; use core::cast::transmute; +use servo_util::tree::{TreeNode, TreeNodeRef, TreeUtils}; // // The basic Node structure @@ -135,17 +136,76 @@ impl Text { } } -pub impl AbstractNode { - // - // Convenience accessors - // - // FIXME: Fold these into util::tree. +impl Clone for AbstractNode { + fn clone(&self) -> AbstractNode { + *self + } +} +impl TreeNode for Node { + fn parent_node(&self) -> Option { + self.parent_node + } + fn first_child(&self) -> Option { + self.first_child + } + fn last_child(&self) -> Option { + self.last_child + } + fn prev_sibling(&self) -> Option { + self.prev_sibling + } + fn next_sibling(&self) -> Option { + self.next_sibling + } + + fn set_parent_node(&mut self, new_parent_node: Option) { + self.parent_node = new_parent_node + } + fn set_first_child(&mut self, new_first_child: Option) { + self.first_child = new_first_child + } + fn set_last_child(&mut self, new_last_child: Option) { + self.last_child = new_last_child + } + fn set_prev_sibling(&mut self, new_prev_sibling: Option) { + self.prev_sibling = new_prev_sibling + } + fn set_next_sibling(&mut self, new_next_sibling: Option) { + self.next_sibling = new_next_sibling + } +} + +impl TreeNodeRef for AbstractNode { + // FIXME: The duplication between `with_imm_node` and `with_immutable_node` is ugly. + fn with_immutable_node(&self, callback: &fn(&Node) -> R) -> R { + self.with_imm_node(callback) + } + + fn with_mutable_node(&self, callback: &fn(&mut Node) -> R) -> R { + self.with_mut_node(callback) + } +} + +pub impl AbstractNode { + // Convenience accessors + + /// Returns the type ID of this node. fn type_id(self) -> NodeTypeId { self.with_imm_node(|n| n.type_id) } + + /// Returns the parent node of this node. fn parent_node(self) -> Option { self.with_imm_node(|n| n.parent_node) } + + /// Returns the first child of this node. fn first_child(self) -> Option { self.with_imm_node(|n| n.first_child) } + + /// Returns the last child of this node. fn last_child(self) -> Option { self.with_imm_node(|n| n.last_child) } + + /// Returns the previous sibling of this node. fn prev_sibling(self) -> Option { self.with_imm_node(|n| n.prev_sibling) } + + /// Returns the next sibling of this node. fn next_sibling(self) -> Option { self.with_imm_node(|n| n.next_sibling) } // NB: You must not call these if you are not layout. We should do something with scoping to From 6a6cad1e39e541f372f6aeb4cf04c18025395b6c Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 19:26:52 -0700 Subject: [PATCH 07/12] dom: Document the node structure better and remove the node pointer stitching routines --- src/servo/dom/node.rs | 190 +++++++++++---------------- src/servo/html/hubbub_html_parser.rs | 5 +- src/servo/layout/aux.rs | 8 +- src/servo/layout/box_builder.rs | 10 +- 4 files changed, 94 insertions(+), 119 deletions(-) diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs index 8f8ffe45ed2..5b97149aa1d 100644 --- a/src/servo/dom/node.rs +++ b/src/servo/dom/node.rs @@ -2,9 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -// -// The core DOM types. Defines the basic DOM hierarchy as well as all the HTML elements. -// +//! The core DOM types. Defines the basic DOM hierarchy as well as all the HTML elements. use content::content_task::global_content; use dom::bindings::codegen; @@ -36,28 +34,47 @@ pub struct AbstractNode { } impl Eq for AbstractNode { - fn eq(&self, other: &AbstractNode) -> bool { self.obj == other.obj } - fn ne(&self, other: &AbstractNode) -> bool { self.obj != other.obj } + fn eq(&self, other: &AbstractNode) -> bool { + self.obj == other.obj + } + fn ne(&self, other: &AbstractNode) -> bool { + self.obj != other.obj + } } +/// An HTML node. pub struct Node { + /// The JavaScript wrapper for this node. wrapper: WrapperCache, + + /// The type of node that this is. type_id: NodeTypeId, abstract: Option, + /// The parent of this node. parent_node: Option, + + /// The first child of this node. first_child: Option, + + /// The last child of this node. last_child: Option, + + /// The next sibling of this node. next_sibling: Option, + + /// The previous sibling of this node. prev_sibling: Option, + /// The document that this node belongs to. owner_doc: Option<@mut Document>, - // You must not touch this if you are not layout. + /// Layout information. You must not touch this if you are not layout. priv layout_data: Option<@mut LayoutData> } +/// The different types of nodes. #[deriving(Eq)] pub enum NodeTypeId { DoctypeNodeTypeId, @@ -70,12 +87,17 @@ pub enum NodeTypeId { // Auxiliary layout data // +/// Data that layout associates with a node. pub struct LayoutData { + /// The results of CSS styling for this node. style: Option, + + /// The CSS flow that this node is associated with. flow: Option, } impl LayoutData { + /// Creates new layout data. pub fn new() -> LayoutData { LayoutData { style: None, @@ -88,6 +110,7 @@ impl LayoutData { // Basic node types // +/// The `DOCTYPE` tag. pub struct Doctype { parent: Node, name: ~str, @@ -97,6 +120,7 @@ pub struct Doctype { } impl Doctype { + /// Creates a new `DOCTYPE` tag. pub fn new(name: ~str, public_id: Option<~str>, system_id: Option<~str>, @@ -112,11 +136,13 @@ impl Doctype { } } +/// An HTML comment. pub struct Comment { parent: CharacterData, } impl Comment { + /// Creates a new HTML comment. pub fn new(text: ~str) -> Comment { Comment { parent: CharacterData::new(CommentNodeTypeId, text) @@ -124,11 +150,13 @@ impl Comment { } } +/// An HTML text node. pub struct Text { parent: CharacterData, } impl Text { + /// Creates a new HTML text node. pub fn new(text: ~str) -> Text { Text { parent: CharacterData::new(TextNodeTypeId, text) @@ -187,118 +215,56 @@ impl TreeNodeRef for AbstractNode { } } -pub impl AbstractNode { +impl AbstractNode { // Convenience accessors - /// Returns the type ID of this node. - fn type_id(self) -> NodeTypeId { self.with_imm_node(|n| n.type_id) } + /// Returns the type ID of this node. Fails if this node is borrowed mutably. + pub fn type_id(self) -> NodeTypeId { + self.with_imm_node(|n| n.type_id) + } - /// Returns the parent node of this node. - fn parent_node(self) -> Option { self.with_imm_node(|n| n.parent_node) } + /// Returns the parent node of this node. Fails if this node is borrowed mutably. + pub fn parent_node(self) -> Option { + self.with_imm_node(|n| n.parent_node) + } - /// Returns the first child of this node. - fn first_child(self) -> Option { self.with_imm_node(|n| n.first_child) } + /// Returns the first child of this node. Fails if this node is borrowed mutably. + pub fn first_child(self) -> Option { + self.with_imm_node(|n| n.first_child) + } - /// Returns the last child of this node. - fn last_child(self) -> Option { self.with_imm_node(|n| n.last_child) } + /// Returns the last child of this node. Fails if this node is borrowed mutably. + pub fn last_child(self) -> Option { + self.with_imm_node(|n| n.last_child) + } - /// Returns the previous sibling of this node. - fn prev_sibling(self) -> Option { self.with_imm_node(|n| n.prev_sibling) } + /// Returns the previous sibling of this node. Fails if this node is borrowed mutably. + pub fn prev_sibling(self) -> Option { + self.with_imm_node(|n| n.prev_sibling) + } - /// Returns the next sibling of this node. - fn next_sibling(self) -> Option { self.with_imm_node(|n| n.next_sibling) } + /// Returns the next sibling of this node. Fails if this node is borrowed mutably. + pub fn next_sibling(self) -> Option { + self.with_imm_node(|n| n.next_sibling) + } // NB: You must not call these if you are not layout. We should do something with scoping to // ensure this. - fn layout_data(self) -> @mut LayoutData { + pub fn layout_data(self) -> @mut LayoutData { self.with_imm_node(|n| n.layout_data.get()) } - fn has_layout_data(self) -> bool { + pub fn has_layout_data(self) -> bool { self.with_imm_node(|n| n.layout_data.is_some()) } - fn set_layout_data(self, data: @mut LayoutData) { + pub fn set_layout_data(self, data: @mut LayoutData) { self.with_mut_node(|n| n.layout_data = Some(data)) } - // - // Tree operations - // - // FIXME: Fold this into util::tree. - // - - fn is_leaf(self) -> bool { self.first_child().is_none() } - - // Invariant: `child` is disconnected from the document. - fn append_child(self, child: AbstractNode) { - assert!(self != child); - - do self.with_mut_node |parent_n| { - do child.with_mut_node |child_n| { - assert!(child_n.parent_node.is_none()); - assert!(child_n.prev_sibling.is_none()); - assert!(child_n.next_sibling.is_none()); - - child_n.parent_node = Some(self); - - match parent_n.last_child { - None => parent_n.first_child = Some(child), - Some(last_child) => { - do last_child.with_mut_node |last_child_n| { - assert!(last_child_n.next_sibling.is_none()); - last_child_n.next_sibling = Some(child); - } - } - } - - child_n.prev_sibling = parent_n.last_child; - parent_n.last_child = Some(child); - } - } - } - - // - // Tree traversal - // - // FIXME: Fold this into util::tree. - // - - fn each_child(self, f: &fn(AbstractNode) -> bool) { - let mut current_opt = self.first_child(); - while !current_opt.is_none() { - let current = current_opt.get(); - if !f(current) { - break; - } - current_opt = current.next_sibling(); - } - } - - fn traverse_preorder(self, f: &fn(AbstractNode) -> bool) -> bool { - if !f(self) { - return false; - } - for self.each_child |kid| { - if !kid.traverse_preorder(f) { - return false; - } - } - true - } - - fn traverse_postorder(self, f: &fn(AbstractNode) -> bool) -> bool { - for self.each_child |kid| { - if !kid.traverse_postorder(f) { - return false; - } - } - f(self) - } - // // Downcasting borrows // - fn transmute(self, f: &fn(&T) -> R) -> R { + pub fn transmute(self, f: &fn(&T) -> R) -> R { unsafe { let node_box: *mut bindings::utils::rust_box = transmute(self.obj); let node = &mut (*node_box).payload; @@ -311,7 +277,7 @@ pub impl AbstractNode { } } - fn transmute_mut(self, f: &fn(&mut T) -> R) -> R { + pub fn transmute_mut(self, f: &fn(&mut T) -> R) -> R { unsafe { let node_box: *mut bindings::utils::rust_box = transmute(self.obj); let node = &mut (*node_box).payload; @@ -324,25 +290,27 @@ pub impl AbstractNode { } } - fn with_imm_node(self, f: &fn(&Node) -> R) -> R { + pub fn with_imm_node(self, f: &fn(&Node) -> R) -> R { self.transmute(f) } - fn with_mut_node(self, f: &fn(&mut Node) -> R) -> R { + pub fn with_mut_node(self, f: &fn(&mut Node) -> R) -> R { self.transmute_mut(f) } - fn is_text(self) -> bool { self.type_id() == TextNodeTypeId } + pub fn is_text(self) -> bool { + self.type_id() == TextNodeTypeId + } // FIXME: This should be doing dynamic borrow checking for safety. - fn with_imm_text(self, f: &fn(&Text) -> R) -> R { + pub fn with_imm_text(self, f: &fn(&Text) -> R) -> R { if !self.is_text() { fail!(~"node is not text"); } self.transmute(f) } - fn is_element(self) -> bool { + pub fn is_element(self) -> bool { match self.type_id() { ElementNodeTypeId(*) => true, _ => false @@ -350,7 +318,7 @@ pub impl AbstractNode { } // FIXME: This should be doing dynamic borrow checking for safety. - fn with_imm_element(self, f: &fn(&Element) -> R) -> R { + pub fn with_imm_element(self, f: &fn(&Element) -> R) -> R { if !self.is_element() { fail!(~"node is not an element"); } @@ -358,40 +326,40 @@ pub impl AbstractNode { } // FIXME: This should be doing dynamic borrow checking for safety. - fn as_mut_element(self, f: &fn(&mut Element) -> R) -> R { + pub fn as_mut_element(self, f: &fn(&mut Element) -> R) -> R { if !self.is_element() { fail!(~"node is not an element"); } self.transmute_mut(f) } - fn is_image_element(self) -> bool { + pub fn is_image_element(self) -> bool { self.type_id() == ElementNodeTypeId(HTMLImageElementTypeId) } - fn with_imm_image_element(self, f: &fn(&HTMLImageElement) -> R) -> R { + pub fn with_imm_image_element(self, f: &fn(&HTMLImageElement) -> R) -> R { if !self.is_image_element() { fail!(~"node is not an image element"); } self.transmute(f) } - fn with_mut_image_element(self, f: &fn(&mut HTMLImageElement) -> R) -> R { + pub fn with_mut_image_element(self, f: &fn(&mut HTMLImageElement) -> R) -> R { if !self.is_image_element() { fail!(~"node is not an image element"); } self.transmute_mut(f) } - fn is_style_element(self) -> bool { + pub fn is_style_element(self) -> bool { self.type_id() == ElementNodeTypeId(HTMLStyleElementTypeId) } - unsafe fn raw_object(self) -> *mut Node { + pub unsafe fn raw_object(self) -> *mut Node { self.obj } - fn from_raw(raw: *mut Node) -> AbstractNode { + pub fn from_raw(raw: *mut Node) -> AbstractNode { AbstractNode { obj: raw } diff --git a/src/servo/html/hubbub_html_parser.rs b/src/servo/html/hubbub_html_parser.rs index 2a7f37a2113..fef7c6584a6 100644 --- a/src/servo/html/hubbub_html_parser.rs +++ b/src/servo/html/hubbub_html_parser.rs @@ -14,8 +14,9 @@ use util::task::spawn_conversation; use core::cell::Cell; use core::comm::{Chan, Port, SharedChan}; use core::str::eq_slice; -use servo_util::url::make_url; use hubbub::hubbub; +use servo_util::tree::TreeUtils; +use servo_util::url::make_url; use std::net::url::Url; use std::net::url; @@ -336,7 +337,7 @@ pub fn parse_html(url: Url, debug!("append child %x %x", cast::transmute(parent), cast::transmute(child)); let parent: AbstractNode = NodeWrapping::from_hubbub_node(parent); let child: AbstractNode = NodeWrapping::from_hubbub_node(child); - parent.append_child(child); + parent.add_child(child); append_hook(parent, child); } child diff --git a/src/servo/layout/aux.rs b/src/servo/layout/aux.rs index b803a036bac..f56f204e0d1 100644 --- a/src/servo/layout/aux.rs +++ b/src/servo/layout/aux.rs @@ -2,12 +2,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/** -Code for managing the DOM aux pointer -*/ +//! Code for managing the layout data in the DOM. use dom::node::{AbstractNode, LayoutData}; +use servo_util::tree::TreeUtils; + pub trait LayoutAuxMethods { fn initialize_layout_data(self) -> Option<@mut LayoutData>; fn initialize_style_for_subtree(self, refs: &mut ~[@mut LayoutData]); @@ -36,5 +36,5 @@ impl LayoutAuxMethods for AbstractNode { } }; } - } + diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index d0fb0deb55d..8bc838b18d1 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -121,6 +121,8 @@ impl BoxGenerator { // depending on flow, make a box for this node. match self.flow { InlineFlow(inline) => { + use servo_util::tree::TreeUtils; // For `is_leaf()`. + let mut inline = &mut *inline; let node_range_start = inline.boxes.len(); self.range_stack.push(node_range_start); @@ -336,8 +338,12 @@ pub impl LayoutTreeBuilder { debug!("point b: %s", cur_node.debug_str()); // recurse on child nodes. - for cur_node.each_child |child_node| { - self.construct_recursively(layout_ctx, child_node, &mut this_ctx); + { + use servo_util::tree::TreeUtils; // For `each_child()`. + + for cur_node.each_child |child_node| { + self.construct_recursively(layout_ctx, child_node, &mut this_ctx); + } } this_ctx.default_collector.pop_node(layout_ctx, self, cur_node); From fe36f1dccbbf1e5978057fbf10671c3a529457d4 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 20:05:17 -0700 Subject: [PATCH 08/12] layout: Port the flow tree over to the new generic tree functions --- src/servo/layout/block.rs | 1 + src/servo/layout/box_builder.rs | 12 +-- src/servo/layout/flow.rs | 163 ++++++++++++++++---------------- src/servo/layout/layout_task.rs | 39 +++++--- src/servo/layout/root.rs | 2 + src/servo/layout/traverse.rs | 27 ------ src/servo/servo.rc | 1 - 7 files changed, 116 insertions(+), 129 deletions(-) delete mode 100644 src/servo/layout/traverse.rs diff --git a/src/servo/layout/block.rs b/src/servo/layout/block.rs index 6d4cda5b31d..43864617714 100644 --- a/src/servo/layout/block.rs +++ b/src/servo/layout/block.rs @@ -16,6 +16,7 @@ use geom::point::Point2D; use geom::rect::Rect; use gfx::display_list::DisplayList; use gfx::geometry::Au; +use servo_util::tree::TreeUtils; pub struct BlockFlowData { /// Data common to all flows. diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index 8bc838b18d1..f7638e07c5d 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -19,6 +19,7 @@ use gfx::image::holder::ImageHolder; use newcss::values::{CSSDisplay, CSSDisplayBlock, CSSDisplayInline, CSSDisplayInlineBlock}; use newcss::values::{CSSDisplayNone}; use servo_util::range::Range; +use servo_util::tree::TreeUtils; pub struct LayoutTreeBuilder { root_flow: Option, @@ -121,7 +122,6 @@ impl BoxGenerator { // depending on flow, make a box for this node. match self.flow { InlineFlow(inline) => { - use servo_util::tree::TreeUtils; // For `is_leaf()`. let mut inline = &mut *inline; let node_range_start = inline.boxes.len(); @@ -338,12 +338,8 @@ pub impl LayoutTreeBuilder { debug!("point b: %s", cur_node.debug_str()); // recurse on child nodes. - { - use servo_util::tree::TreeUtils; // For `each_child()`. - - for cur_node.each_child |child_node| { - self.construct_recursively(layout_ctx, child_node, &mut this_ctx); - } + for cur_node.each_child |child_node| { + self.construct_recursively(layout_ctx, child_node, &mut this_ctx); } this_ctx.default_collector.pop_node(layout_ctx, self, cur_node); @@ -354,6 +350,7 @@ pub impl LayoutTreeBuilder { // eventually be elided or split, but the mapping between // nodes and FlowContexts should not change during layout. let flow = &mut this_ctx.default_collector.flow; + let flow: &FlowContext = flow; for flow.each_child |child_flow| { do child_flow.with_common_info |child_flow_info| { let node = child_flow_info.node; @@ -379,6 +376,7 @@ pub impl LayoutTreeBuilder { let mut found_child_block = false; let flow = &mut parent_ctx.default_collector.flow; + let flow: &FlowContext = flow; for flow.each_child |child_ctx| { match child_ctx { InlineFlow(*) | InlineBlockFlow(*) => found_child_inline = true, diff --git a/src/servo/layout/flow.rs b/src/servo/layout/flow.rs index 67e9dc31964..6086b974660 100644 --- a/src/servo/layout/flow.rs +++ b/src/servo/layout/flow.rs @@ -41,6 +41,7 @@ use geom::point::Point2D; use geom::rect::Rect; use gfx::display_list::DisplayList; use gfx::geometry::Au; +use servo_util::tree::{TreeNode, TreeNodeRef, TreeUtils}; /// The type of the formatting context and data specific to each context, such as line box /// structures or float lists. @@ -64,6 +65,21 @@ pub enum FlowContextType { Flow_Table } +impl Clone for FlowContext { + fn clone(&self) -> FlowContext { + *self + } +} + +impl TreeNodeRef for FlowContext { + fn with_immutable_node(&self, callback: &fn(&FlowData) -> R) -> R { + self.with_common_imm_info(callback) + } + fn with_mutable_node(&self, callback: &fn(&mut FlowData) -> R) -> R { + self.with_common_info(callback) + } +} + /// Data common to all flows. /// /// FIXME: We need a naming convention for pseudo-inheritance like this. How about @@ -88,6 +104,48 @@ pub struct FlowData { position: Rect, } +impl TreeNode for FlowData { + fn parent_node(&self) -> Option { + self.parent + } + + fn first_child(&self) -> Option { + self.first_child + } + + fn last_child(&self) -> Option { + self.last_child + } + + fn prev_sibling(&self) -> Option { + self.prev_sibling + } + + fn next_sibling(&self) -> Option { + self.next_sibling + } + + fn set_parent_node(&mut self, new_parent_node: Option) { + self.parent = new_parent_node + } + + fn set_first_child(&mut self, new_first_child: Option) { + self.first_child = new_first_child + } + + fn set_last_child(&mut self, new_last_child: Option) { + self.last_child = new_last_child + } + + fn set_prev_sibling(&mut self, new_prev_sibling: Option) { + self.prev_sibling = new_prev_sibling + } + + fn set_next_sibling(&mut self, new_next_sibling: Option) { + self.next_sibling = new_next_sibling + } +} + impl FlowData { pub fn new(id: int, node: AbstractNode) -> FlowData { FlowData { @@ -109,6 +167,30 @@ impl FlowData { } impl<'self> FlowContext { + // FIXME: This method is a duplicate of `with_immutable_node`; fix this. + #[inline(always)] + pub fn with_common_imm_info(&self, block: &fn(&FlowData) -> R) -> R { + match *self { + AbsoluteFlow(info) => block(info), + BlockFlow(info) => { + let info = &*info; // FIXME: Borrow check workaround. + block(&info.common) + } + FloatFlow(info) => block(info), + InlineBlockFlow(info) => block(info), + InlineFlow(info) => { + let info = &*info; // FIXME: Borrow check workaround. + block(&info.common) + } + RootFlow(info) => { + let info = &*info; // FIXME: Borrow check workaround. + block(&info.common) + } + TableFlow(info) => block(info), + } + } + + // FIXME: This method is a duplicate of `with_mutable_node`; fix this. #[inline(always)] pub fn with_common_info(&self, block: &fn(&mut FlowData) -> R) -> R { match *self { @@ -145,87 +227,6 @@ impl<'self> FlowContext { } } - /// Iterates over the immediate children of this flow. - /// - /// TODO: Fold me into `util::tree`. - pub fn each_child(&self, f: &fn(FlowContext) -> bool) { - let mut current_opt = self.with_common_info(|info| info.first_child); - while !current_opt.is_none() { - let current = current_opt.get(); - if !f(current) { - break; - } - current_opt = current.with_common_info(|info| info.next_sibling); - } - } - - /// Adds the given flow to the end of the list of this flow's children. The new child must be - /// detached from the tree before calling this method. - /// - /// TODO: Fold me into `util::tree`. - pub fn add_child(&self, child: FlowContext) { - do self.with_common_info |self_info| { - do child.with_common_info |child_info| { - assert!(child_info.parent.is_none()); - assert!(child_info.prev_sibling.is_none()); - assert!(child_info.next_sibling.is_none()); - - match self_info.last_child { - None => { - self_info.first_child = Some(child); - } - Some(last_child) => { - do last_child.with_common_info |last_child_info| { - assert!(last_child_info.next_sibling.is_none()); - last_child_info.next_sibling = Some(child); - child_info.prev_sibling = Some(last_child); - } - } - } - - self_info.last_child = Some(child); - child_info.parent = Some(*self); - } - } - } - - /// Removes the given flow from the tree. - /// - /// TODO: Fold me into `util::tree`. - pub fn remove_child(&self, child: FlowContext) { - do self.with_common_info |self_info| { - do child.with_common_info |child_info| { - assert!(child_info.parent.is_some()); - - match child_info.prev_sibling { - None => self_info.first_child = child_info.next_sibling, - Some(prev_sibling) => { - do prev_sibling.with_common_info |prev_sibling_info| { - prev_sibling_info.next_sibling = child_info.next_sibling; - child_info.prev_sibling = None; - } - } - } - - match child_info.next_sibling { - None => { - do child.with_common_info |child_info| { - self_info.last_child = child_info.prev_sibling; - } - } - Some(next_sibling) => { - do next_sibling.with_common_info |next_sibling_info| { - next_sibling_info.prev_sibling = Some(next_sibling); - child_info.next_sibling = None; - } - } - } - - child_info.parent = None; - } - } - } - pub fn inline(&self) -> @mut InlineFlowData { match *self { InlineFlow(info) => info, diff --git a/src/servo/layout/layout_task.rs b/src/servo/layout/layout_task.rs index c8f7bf23708..c445c7fdbb2 100644 --- a/src/servo/layout/layout_task.rs +++ b/src/servo/layout/layout_task.rs @@ -15,7 +15,6 @@ use layout::context::LayoutContext; use layout::debug::{BoxedMutDebugMethods, DebugMethods}; use layout::display_list_builder::{DisplayListBuilder, FlowDisplayListBuilderMethods}; use layout::flow::FlowContext; -use layout::traverse::*; use resource::image_cache_task::{ImageCacheTask, ImageResponseMsg}; use resource::local_image_cache::LocalImageCache; use util::task::spawn_listener; @@ -35,6 +34,7 @@ use gfx::render_task::{RenderMsg, RenderTask}; use newcss::select::SelectCtx; use newcss::stylesheet::Stylesheet; use newcss::types::OriginAuthor; +use servo_util::tree::TreeUtils; use std::net::url::Url; pub type LayoutTask = SharedChan; @@ -165,11 +165,12 @@ impl Layout { self.css_select_ctx.append_sheet(sheet.take(), OriginAuthor); } + /// The high-level routine that performs layout tasks. fn handle_build(&mut self, data: &BuildData) { let node = &data.node; - // FIXME: Bad copy + // FIXME: Bad copy! let doc_url = copy data.url; - // FIXME: Bad clone + // FIXME: Bad clone! let dom_event_chan = data.dom_event_chan.clone(); debug!("layout: received layout request for: %s", doc_url.to_str()); @@ -177,12 +178,13 @@ impl Layout { debug!("layout: parsed Node tree"); debug!("%?", node.dump()); - // Reset the image cache + // Reset the image cache. self.local_image_cache.next_round(self.make_on_image_available_cb(dom_event_chan)); let screen_size = Size2D(Au::from_px(data.window_size.width as int), Au::from_px(data.window_size.height as int)); + // Create a layout context for use throughout the following passes. let mut layout_ctx = LayoutContext { image_cache: self.local_image_cache, font_ctx: self.font_ctx, @@ -190,8 +192,10 @@ impl Layout { screen_size: Rect(Point2D(Au(0), Au(0)), screen_size) }; + // Initialize layout data for each node. + // + // FIXME: This is inefficient. We don't need an entire traversal to do this! do time("layout: aux initialization") { - // TODO: this is dumb. we don't need an entire traversal to do this node.initialize_style_for_subtree(&mut self.layout_refs); } @@ -205,6 +209,7 @@ impl Layout { } } + // Construct the flow tree. let layout_root: FlowContext = do time("layout: tree construction") { let mut builder = LayoutTreeBuilder::new(); let layout_root: FlowContext = match builder.construct_trees(&layout_ctx, *node) { @@ -218,13 +223,21 @@ impl Layout { layout_root }; + // Perform the primary layout passes over the flow tree to compute the locations of all + // the boxes. do time("layout: main layout") { - /* perform layout passes over the flow tree */ - do layout_root.traverse_postorder |f| { f.bubble_widths(&mut layout_ctx) } - do layout_root.traverse_preorder |f| { f.assign_widths(&mut layout_ctx) } - do layout_root.traverse_postorder |f| { f.assign_height(&mut layout_ctx) } + for layout_root.traverse_postorder |flow| { + flow.bubble_widths(&mut layout_ctx); + }; + for layout_root.traverse_preorder |flow| { + flow.assign_widths(&mut layout_ctx); + }; + for layout_root.traverse_postorder |flow| { + flow.assign_height(&mut layout_ctx); + }; } + // Build the display list, and send it to the renderer. do time("layout: display list building") { let builder = DisplayListBuilder { ctx: &layout_ctx, @@ -249,13 +262,13 @@ impl Layout { self.render_task.send(RenderMsg(render_layer)); } // time(layout: display list building) - // Tell content we're done + // Tell content that we're done. data.content_join_chan.send(()); } - - fn handle_query(&self, query: LayoutQuery, - reply_chan: Chan) { + /// Handles a query from the script task. This is the main routine that DOM functions like + /// `getClientRects()` or `getBoundingClientRect()` ultimately invoke. + fn handle_query(&self, query: LayoutQuery, reply_chan: Chan) { match query { ContentBox(node) => { let response = match node.layout_data().flow { diff --git a/src/servo/layout/root.rs b/src/servo/layout/root.rs index d74d8bb345b..46abe8489c6 100644 --- a/src/servo/layout/root.rs +++ b/src/servo/layout/root.rs @@ -13,6 +13,8 @@ use layout::context::LayoutContext; use layout::flow::{FlowContext, FlowData, RootFlow}; use layout::display_list_builder::DisplayListBuilder; +use servo_util::tree::TreeUtils; + pub struct RootFlowData { /// Data common to all flows. common: FlowData, diff --git a/src/servo/layout/traverse.rs b/src/servo/layout/traverse.rs deleted file mode 100644 index d242bcae4f5..00000000000 --- a/src/servo/layout/traverse.rs +++ /dev/null @@ -1,27 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -use layout::flow::FlowContext; - -/// A trait for running tree-based traversals over flows contexts. -pub trait FlowContextTraversals { - fn traverse_preorder(&self, preorder_cb: &fn(FlowContext)); - fn traverse_postorder(&self, postorder_cb: &fn(FlowContext)); -} - -impl FlowContextTraversals for FlowContext { - fn traverse_preorder(&self, preorder_cb: &fn(FlowContext)) { - preorder_cb(*self); - for self.each_child |child| { - child.traverse_preorder(preorder_cb); - } - } - - fn traverse_postorder(&self, postorder_cb: &fn(FlowContext)) { - for self.each_child |child| { - child.traverse_postorder(postorder_cb); - } - postorder_cb(*self); - } -} diff --git a/src/servo/servo.rc b/src/servo/servo.rc index b2d6424cdae..d2d749d8c70 100755 --- a/src/servo/servo.rc +++ b/src/servo/servo.rc @@ -104,7 +104,6 @@ pub mod layout { pub mod inline; pub mod root; pub mod text; - pub mod traverse; mod aux; } From 3995d967da34d9cb0197974f931b697f730bd1a3 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 20:29:13 -0700 Subject: [PATCH 09/12] layout: Remove duplicate methods from flows --- src/servo/layout/block.rs | 56 ++++++------- src/servo/layout/box_builder.rs | 102 ++++++++++------------- src/servo/layout/display_list_builder.rs | 24 +++--- src/servo/layout/flow.rs | 98 ++++++++++------------ src/servo/layout/layout_task.rs | 13 +-- src/servo/layout/root.rs | 8 +- 6 files changed, 134 insertions(+), 167 deletions(-) diff --git a/src/servo/layout/block.rs b/src/servo/layout/block.rs index 43864617714..67886bd9960 100644 --- a/src/servo/layout/block.rs +++ b/src/servo/layout/block.rs @@ -16,7 +16,7 @@ use geom::point::Point2D; use geom::rect::Rect; use gfx::display_list::DisplayList; use gfx::geometry::Au; -use servo_util::tree::TreeUtils; +use servo_util::tree::{TreeNodeRef, TreeUtils}; pub struct BlockFlowData { /// Data common to all flows. @@ -99,9 +99,9 @@ impl BlockLayout for FlowContext { for self.each_child |child_ctx| { assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow()); - do child_ctx.with_common_info |child_info| { - min_width = au::max(min_width, child_info.min_width); - pref_width = au::max(pref_width, child_info.pref_width); + do child_ctx.with_immutable_node |child_node| { + min_width = au::max(min_width, child_node.min_width); + pref_width = au::max(pref_width, child_node.pref_width); } } @@ -112,39 +112,37 @@ impl BlockLayout for FlowContext { pref_width = pref_width.add(&box.get_pref_width(ctx)); } - do self.with_common_info |info| { - info.min_width = min_width; - info.pref_width = pref_width; + do self.with_mutable_node |this_node| { + this_node.min_width = min_width; + this_node.pref_width = pref_width; } } - /* Recursively (top-down) determines the actual width of child - contexts and boxes. When called on this context, the context has - had its width set by the parent context. - - Dual boxes consume some width first, and the remainder is assigned to - all child (block) contexts. */ - - fn assign_widths_block(&self, _ctx: &LayoutContext) { + /// Recursively (top-down) determines the actual width of child contexts and boxes. When called + /// on this context, the context has had its width set by the parent context. + /// + /// Dual boxes consume some width first, and the remainder is assigned to all child (block) + /// contexts. + fn assign_widths_block(&self, _: &LayoutContext) { assert!(self.starts_block_flow()); - let mut remaining_width = self.with_common_info(|info| info.position.size.width); + let mut remaining_width = self.with_immutable_node(|this| this.position.size.width); let mut _right_used = Au(0); let mut left_used = Au(0); - /* Let the box consume some width. It will return the amount remaining - for its children. */ + // Let the box consume some width. It will return the amount remaining for its children. do self.with_block_box |box| { box.d().position.size.width = remaining_width; let (left_used, right_used) = box.get_used_width(); remaining_width -= left_used.add(&right_used); } - for self.each_child |child_ctx| { - assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow()); - do child_ctx.with_common_info |child_info| { - child_info.position.origin.x = left_used; - child_info.position.size.width = remaining_width; + for self.each_child |kid| { + assert!(kid.starts_block_flow() || kid.starts_inline_flow()); + + do kid.with_mutable_node |child_node| { + child_node.position.origin.x = left_used; + child_node.position.size.width = remaining_width; } } } @@ -154,15 +152,15 @@ impl BlockLayout for FlowContext { let mut cur_y = Au(0); - for self.each_child |child_ctx| { - do child_ctx.with_common_info |child_info| { - child_info.position.origin.y = cur_y; - cur_y += child_info.position.size.height; + for self.each_child |kid| { + do kid.with_mutable_node |child_node| { + child_node.position.origin.y = cur_y; + cur_y += child_node.position.size.height; } } - do self.with_common_info |info| { - info.position.size.height = cur_y; + do self.with_mutable_node |this_node| { + this_node.position.size.height = cur_y; } let _used_top = Au(0); diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index f7638e07c5d..10620b533af 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -19,7 +19,7 @@ use gfx::image::holder::ImageHolder; use newcss::values::{CSSDisplay, CSSDisplayBlock, CSSDisplayInline, CSSDisplayInlineBlock}; use newcss::values::{CSSDisplayNone}; use servo_util::range::Range; -use servo_util::tree::TreeUtils; +use servo_util::tree::{TreeNodeRef, TreeUtils}; pub struct LayoutTreeBuilder { root_flow: Option, @@ -122,7 +122,6 @@ impl BoxGenerator { // depending on flow, make a box for this node. match self.flow { InlineFlow(inline) => { - let mut inline = &mut *inline; let node_range_start = inline.boxes.len(); self.range_stack.push(node_range_start); @@ -140,42 +139,31 @@ impl BoxGenerator { } // TODO: cases for inline-block, etc. }, - BlockFlow(*) => { - do self.flow.with_common_info |flow_info| { - debug!("BoxGenerator[f%d]: point b", flow_info.id); - let new_box = builder.make_box(ctx, box_type, node, self.flow); - debug!("BoxGenerator[f%d]: attaching box[b%d] to block flow (node: %s)", - flow_info.id, - new_box.d().id, - node.debug_str()); + BlockFlow(block) => { + debug!("BoxGenerator[f%d]: point b", block.common.id); + let new_box = builder.make_box(ctx, box_type, node, self.flow); - assert!(self.flow.block().box.is_none()); - //XXXjdm We segfault when returning without this temporary. - let block = self.flow.block(); - block.box = Some(new_box); - } - }, - RootFlow(*) => { - do self.flow.with_common_info |info| { - debug!("BoxGenerator[f%d]: point c", info.id); - let new_box = builder.make_box(ctx, box_type, node, self.flow); - debug!("BoxGenerator[f%d]: (node is: %s)", info.id, node.debug_str()); - debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)", - info.id, - new_box.d().id, - node.debug_str()); + debug!("BoxGenerator[f%d]: attaching box[b%d] to block flow (node: %s)", + block.common.id, + new_box.d().id, + node.debug_str()); - assert!(self.flow.root().box.is_none()); - //XXXjdm We segfault when returning without this temporary. - let root = self.flow.root(); - root.box = Some(new_box); - } + assert!(block.box.is_none()); + block.box = Some(new_box); }, - _ => { - do self.flow.with_common_info |flow_info| { - warn!("push_node() not implemented for flow f%d", flow_info.id) - } - } + RootFlow(root) => { + debug!("BoxGenerator[f%d]: point c", root.common.id); + let new_box = builder.make_box(ctx, box_type, node, self.flow); + debug!("BoxGenerator[f%d]: (node is: %s)", root.common.id, node.debug_str()); + debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)", + root.common.id, + new_box.d().id, + node.debug_str()); + + assert!(root.box.is_none()); + root.box = Some(new_box); + }, + _ => warn!("push_node() not implemented for flow f%d", self.flow.id()), } } @@ -236,14 +224,10 @@ impl BuilderContext { priv fn attach_child_flow(&self, child: FlowContext) { let default_collector = &mut *self.default_collector; - do default_collector.flow.with_common_info |flow_info| { - do child.with_common_info |child_flow_info| { - debug!("BuilderContext: Adding child flow f%? of f%?", - flow_info.id, - child_flow_info.id); - default_collector.flow.add_child(child); - } - } + debug!("BuilderContext: Adding child flow f%? of f%?", + default_collector.flow.id(), + child.id()); + default_collector.flow.add_child(child); } priv fn create_child_flow_of_type(&self, @@ -352,23 +336,22 @@ pub impl LayoutTreeBuilder { let flow = &mut this_ctx.default_collector.flow; let flow: &FlowContext = flow; for flow.each_child |child_flow| { - do child_flow.with_common_info |child_flow_info| { - let node = child_flow_info.node; - assert!(node.has_layout_data()); - node.layout_data().flow = Some(child_flow); + do child_flow.with_immutable_node |child_node| { + let dom_node = child_node.node; + assert!(dom_node.has_layout_data()); + dom_node.layout_data().flow = Some(child_flow); } } } - // Fixup any irregularities such as: - // - // * split inlines (CSS 2.1 Section 9.2.1.1) - // * elide non-preformatted whitespace-only text boxes and their - // flows (CSS 2.1 Section 9.2.2.1). - // - // The latter can only be done immediately adjacent to, or at the - // beginning or end of a block flow. Otherwise, the whitespace - // might affect whitespace collapsing with adjacent text. + /// Fix up any irregularities such as: + /// + /// * split inlines (CSS 2.1 Section 9.2.1.1) + /// * elide non-preformatted whitespace-only text boxes and their flows (CSS 2.1 Section + /// 9.2.2.1). + /// + /// The latter can only be done immediately adjacent to, or at the beginning or end of a block + /// flow. Otherwise, the whitespace might affect whitespace collapsing with adjacent text. fn simplify_children_of_flow(&self, _: &LayoutContext, parent_ctx: &BuilderContext) { match parent_ctx.default_collector.flow { InlineFlow(*) => { @@ -394,10 +377,9 @@ pub impl LayoutTreeBuilder { // of its RenderBox or FlowContext children, and possibly keep alive other junk let parent_flow = parent_ctx.default_collector.flow; - let (first_child, last_child) = - do parent_flow.with_common_info |parent_flow_info| { - (parent_flow_info.first_child, parent_flow_info.last_child) - }; + let (first_child, last_child) = do parent_flow.with_immutable_node |parent_node| { + (parent_node.first_child, parent_node.last_child) + }; // check first/last child for whitespace-ness for first_child.each |first_flow| { diff --git a/src/servo/layout/display_list_builder.rs b/src/servo/layout/display_list_builder.rs index 85eed2db274..55b26180f5e 100644 --- a/src/servo/layout/display_list_builder.rs +++ b/src/servo/layout/display_list_builder.rs @@ -16,14 +16,14 @@ use geom::rect::Rect; use gfx::display_list::DisplayList; use gfx::geometry::Au; use gfx; +use servo_util::tree::TreeNodeRef; -/** A builder object that manages display list builder should mainly - hold information about the initial request and desired result---for - example, whether the DisplayList to be used for painting or hit - testing. This can affect which boxes are created. - - Right now, the builder isn't used for much, but it establishes the - pattern we'll need once we support DL-based hit testing &c. */ +/// A builder object that manages display list builder should mainly hold information about the +/// initial request and desired result--for example, whether the `DisplayList` is to be used for +/// painting or hit testing. This can affect which boxes are created. +/// +/// Right now, the builder isn't used for much, but it establishes the pattern we'll need once we +/// support display-list-based hit testing and so forth. pub struct DisplayListBuilder<'self> { ctx: &'self LayoutContext, } @@ -53,13 +53,13 @@ impl FlowDisplayListBuilderMethods for FlowContext { dirty: &Rect, offset: &Point2D, list: &Cell) { - // adjust the dirty rect to child flow context coordinates - do child_flow.with_common_info |child_flow_info| { - let abs_flow_bounds = child_flow_info.position.translate(offset); - let adj_offset = offset.add(&child_flow_info.position.origin); + // Adjust the dirty rect to child flow context coordinates. + do child_flow.with_immutable_node |child_node| { + let abs_flow_bounds = child_node.position.translate(offset); + let adj_offset = offset.add(&child_node.position.origin); debug!("build_display_list_for_child: rel=%?, abs=%?", - child_flow_info.position, + child_node.position, abs_flow_bounds); debug!("build_display_list_for_child: dirty=%?, offset=%?", dirty, offset); diff --git a/src/servo/layout/flow.rs b/src/servo/layout/flow.rs index 6086b974660..facbd1de3d6 100644 --- a/src/servo/layout/flow.rs +++ b/src/servo/layout/flow.rs @@ -73,10 +73,44 @@ impl Clone for FlowContext { impl TreeNodeRef for FlowContext { fn with_immutable_node(&self, callback: &fn(&FlowData) -> R) -> R { - self.with_common_imm_info(callback) + match *self { + AbsoluteFlow(info) => callback(info), + BlockFlow(info) => { + let info = &*info; // FIXME: Borrow check workaround. + callback(&info.common) + } + FloatFlow(info) => callback(info), + InlineBlockFlow(info) => callback(info), + InlineFlow(info) => { + let info = &*info; // FIXME: Borrow check workaround. + callback(&info.common) + } + RootFlow(info) => { + let info = &*info; // FIXME: Borrow check workaround. + callback(&info.common) + } + TableFlow(info) => callback(info), + } } fn with_mutable_node(&self, callback: &fn(&mut FlowData) -> R) -> R { - self.with_common_info(callback) + match *self { + AbsoluteFlow(info) => callback(info), + BlockFlow(info) => { + let info = &mut *info; // FIXME: Borrow check workaround. + callback(&mut info.common) + } + FloatFlow(info) => callback(info), + InlineBlockFlow(info) => callback(info), + InlineFlow(info) => { + let info = &mut *info; // FIXME: Borrow check workaround. + callback(&mut info.common) + } + RootFlow(info) => { + let info = &mut *info; // FIXME: Borrow check workaround. + callback(&mut info.common) + } + TableFlow(info) => callback(info), + } } } @@ -167,62 +201,20 @@ impl FlowData { } impl<'self> FlowContext { - // FIXME: This method is a duplicate of `with_immutable_node`; fix this. + /// A convenience method to return the position of this flow. Fails if the flow is currently + /// being borrowed mutably. #[inline(always)] - pub fn with_common_imm_info(&self, block: &fn(&FlowData) -> R) -> R { - match *self { - AbsoluteFlow(info) => block(info), - BlockFlow(info) => { - let info = &*info; // FIXME: Borrow check workaround. - block(&info.common) - } - FloatFlow(info) => block(info), - InlineBlockFlow(info) => block(info), - InlineFlow(info) => { - let info = &*info; // FIXME: Borrow check workaround. - block(&info.common) - } - RootFlow(info) => { - let info = &*info; // FIXME: Borrow check workaround. - block(&info.common) - } - TableFlow(info) => block(info), - } - } - - // FIXME: This method is a duplicate of `with_mutable_node`; fix this. - #[inline(always)] - pub fn with_common_info(&self, block: &fn(&mut FlowData) -> R) -> R { - match *self { - AbsoluteFlow(info) => block(info), - BlockFlow(info) => { - let info = &mut *info; // FIXME: Borrow check workaround. - block(&mut info.common) - } - FloatFlow(info) => block(info), - InlineBlockFlow(info) => block(info), - InlineFlow(info) => { - let info = &mut *info; // FIXME: Borrow check workaround. - block(&mut info.common) - } - RootFlow(info) => { - let info = &mut *info; // FIXME: Borrow check workaround. - block(&mut info.common) - } - TableFlow(info) => block(info), - } - } - pub fn position(&self) -> Rect { - do self.with_common_info |common_info| { + do self.with_immutable_node |common_info| { common_info.position } } - /// Returns the ID of this flow. + /// A convenience method to return the ID of this flow. Fails if the flow is currently being + /// borrowed mutably. #[inline(always)] pub fn id(&self) -> int { - do self.with_common_info |info| { + do self.with_immutable_node |info| { info.id } } @@ -280,7 +272,7 @@ impl<'self> FlowContext { dirty: &Rect, offset: &Point2D, list: &Cell) { - do self.with_common_info |info| { + do self.with_immutable_node |info| { debug!("FlowContext::build_display_list at %?: %s", info.position, self.debug_str()); } @@ -414,8 +406,8 @@ impl DebugMethods for FlowContext { _ => ~"(Unknown flow)" }; - do self.with_common_info |info| { - fmt!("f%? %?", info.id, repr) + do self.with_immutable_node |this_node| { + fmt!("f%? %?", this_node.id, repr) } } } diff --git a/src/servo/layout/layout_task.rs b/src/servo/layout/layout_task.rs index c445c7fdbb2..d03ac503e67 100644 --- a/src/servo/layout/layout_task.rs +++ b/src/servo/layout/layout_task.rs @@ -245,18 +245,13 @@ impl Layout { let display_list = @Cell(DisplayList::new()); - // TODO: set options on the builder before building - // TODO: be smarter about what needs painting - do layout_root.with_common_info |layout_root_info| { - layout_root.build_display_list(&builder, - © layout_root_info.position, - display_list); - } + // TODO: Set options on the builder before building. + // TODO: Be smarter about what needs painting. + layout_root.build_display_list(&builder, &layout_root.position(), display_list); let render_layer = RenderLayer { display_list: display_list.take(), - size: Size2D(screen_size.width.to_px() as uint, - screen_size.height.to_px() as uint) + size: Size2D(screen_size.width.to_px() as uint, screen_size.height.to_px() as uint) }; self.render_task.send(RenderMsg(render_layer)); diff --git a/src/servo/layout/root.rs b/src/servo/layout/root.rs index 46abe8489c6..4a0150dda3a 100644 --- a/src/servo/layout/root.rs +++ b/src/servo/layout/root.rs @@ -13,7 +13,7 @@ use layout::context::LayoutContext; use layout::flow::{FlowContext, FlowData, RootFlow}; use layout::display_list_builder::DisplayListBuilder; -use servo_util::tree::TreeUtils; +use servo_util::tree::{TreeNodeRef, TreeUtils}; pub struct RootFlowData { /// Data common to all flows. @@ -64,9 +64,9 @@ impl RootFlowData { let mut cur_y = Au(0); for RootFlow(self).each_child |child_flow| { - do child_flow.with_common_info |child_common_info| { - child_common_info.position.origin.y = cur_y; - cur_y += child_common_info.position.size.height; + do child_flow.with_mutable_node |child_node| { + child_node.position.origin.y = cur_y; + cur_y += child_node.position.size.height; } } From 066d1bf1c839f0c09eaa448b5827264df43cda3a Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 6 May 2013 20:40:28 -0700 Subject: [PATCH 10/12] dom: Remove duplicate methods from the DOM node type --- src/servo/content/content_task.rs | 42 ++++++++++++++----------------- src/servo/dom/bindings/node.rs | 29 +++++++++++---------- src/servo/dom/document.rs | 6 ++--- src/servo/dom/node.rs | 41 ++++++++++++------------------ 4 files changed, 54 insertions(+), 64 deletions(-) diff --git a/src/servo/content/content_task.rs b/src/servo/content/content_task.rs index 84f9da7be04..6585aef70c8 100644 --- a/src/servo/content/content_task.rs +++ b/src/servo/content/content_task.rs @@ -2,31 +2,31 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/*! -The content task is the main task that runs JavaScript and spawns layout -tasks. -*/ +/// The content task (also called the script task) is the main task that owns the DOM in memory, +/// runs JavaScript, and spawns parsing and layout tasks. use dom::bindings::utils::GlobalStaticData; use dom::document::Document; -use dom::node::define_bindings; use dom::event::{Event, ResizeEvent, ReflowEvent}; +use dom::node::define_bindings; use dom::window::Window; -use layout::layout_task; use layout::layout_task::{AddStylesheet, BuildData, BuildMsg, Damage, LayoutTask}; use layout::layout_task::{MatchSelectorsDamage, NoDamage, ReflowDamage}; +use layout::layout_task; use core::cell::Cell; use core::comm::{Port, SharedChan}; -use core::pipes::select2i; use core::either; -use core::task::{SingleThreaded, task}; use core::io::{println, read_whole_file}; +use core::pipes::select2i; use core::ptr::null; +use core::task::{SingleThreaded, task}; use core::util::replace; +use dom; use geom::size::Size2D; use gfx::resource::image_cache_task::ImageCacheTask; use gfx::resource::resource_task::ResourceTask; +use html; use js::JSVAL_NULL; use js::global::{global_class, debug_fns}; use js::glue::bindgen::RUST_JSVAL_TO_OBJECT; @@ -34,10 +34,9 @@ use js::jsapi::JSContext; use js::jsapi::bindgen::{JS_CallFunctionValue, JS_GetContextPrivate}; use js::rust::{Compartment, Cx}; use jsrt = js::rust::rt; +use servo_util::tree::TreeNodeRef; use std::net::url::Url; use url_to_str = std::net::url::to_str; -use dom; -use html; pub enum ControlMsg { ParseMsg(Url), @@ -232,7 +231,7 @@ pub impl Content { ptr::to_mut_unsafe_ptr(&mut *self)); //FIXME store this safely let document = Document(root, Some(window)); - do root.with_mut_node |node| { + do root.with_mutable_node |node| { node.add_to_doc(document); } @@ -312,24 +311,20 @@ pub impl Content { } } - /** - This method will wait until the layout task has completed its current action, - join the layout task, and then request a new layout run. It won't wait for the - new layout computation to finish. - */ + /// This method will wait until the layout task has completed its current action, join the + /// layout task, and then request a new layout run. It won't wait for the new layout + /// computation to finish. fn relayout(&mut self, document: &Document, doc_url: &Url) { debug!("content: performing relayout"); - // Now, join the layout so that they will see the latest - // changes we have made. + // Now, join the layout so that they will see the latest changes we have made. self.join_layout(); - // Layout will let us know when it's done + // Layout will let us know when it's done. let (join_port, join_chan) = comm::stream(); self.layout_join_port = Some(join_port); - // Send new document and relevant styles to layout - + // Send new document and relevant styles to layout. let data = ~BuildData { node: document.root, url: copy *doc_url, @@ -344,7 +339,8 @@ pub impl Content { debug!("content: layout forked"); } - fn query_layout(&mut self, query: layout_task::LayoutQuery) -> layout_task::LayoutQueryResponse { + fn query_layout(&mut self, query: layout_task::LayoutQuery) + -> layout_task::LayoutQueryResponse { //self.relayout(self.document.get(), &(copy self.doc_url).get()); self.join_layout(); @@ -378,7 +374,7 @@ pub impl Content { ReflowEvent => { debug!("content got reflow event"); self.damage.add(MatchSelectorsDamage); - match copy self.document { + match /*bad*/ copy self.document { None => { // Nothing to do. } diff --git a/src/servo/dom/bindings/node.rs b/src/servo/dom/bindings/node.rs index d0228b05445..1ad90162c22 100644 --- a/src/servo/dom/bindings/node.rs +++ b/src/servo/dom/bindings/node.rs @@ -18,6 +18,7 @@ use js::jsval::{INT_TO_JSVAL}; use js::rust::{Compartment, jsobj}; use js::{JSPROP_ENUMERATE, JSPROP_SHARED, JSVAL_NULL}; use js::{JS_THIS_OBJECT, JSPROP_NATIVE_ACCESSORS}; +use servo_util::tree::TreeNodeRef; pub fn init(compartment: @mut Compartment) { let obj = utils::define_empty_prototype(~"Node", None, compartment); @@ -80,7 +81,7 @@ extern fn getFirstChild(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool } let node = unwrap(obj); - let rval = do node.with_mut_node |node| { + let rval = do node.with_mutable_node |node| { node.getFirstChild() }; match rval { @@ -102,7 +103,7 @@ extern fn getNextSibling(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBoo } let node = unwrap(obj); - let rval = do node.with_mut_node |node| { + let rval = do node.with_mutable_node |node| { node.getNextSibling() }; match rval { @@ -127,19 +128,19 @@ impl Node { fn getNextSibling(&mut self) -> Option<&mut AbstractNode> { match self.next_sibling { - // transmute because the compiler can't deduce that the reference - // is safe outside of with_mut_node blocks. - Some(ref mut n) => Some(unsafe { cast::transmute(n) }), - None => None + // transmute because the compiler can't deduce that the reference + // is safe outside of with_mutable_node blocks. + Some(ref mut n) => Some(unsafe { cast::transmute(n) }), + None => None } } fn getFirstChild(&mut self) -> Option<&mut AbstractNode> { match self.first_child { - // transmute because the compiler can't deduce that the reference - // is safe outside of with_mut_node blocks. - Some(ref mut n) => Some(unsafe { cast::transmute(n) }), - None => None + // transmute because the compiler can't deduce that the reference + // is safe outside of with_mutable_node blocks. + Some(ref mut n) => Some(unsafe { cast::transmute(n) }), + None => None } } } @@ -152,7 +153,7 @@ extern fn getNodeType(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool { } let node = unwrap(obj); - let rval = do node.with_imm_node |node| { + let rval = do node.with_immutable_node |node| { node.getNodeType() }; *vp = INT_TO_JSVAL(rval); @@ -162,8 +163,10 @@ extern fn getNodeType(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool { impl CacheableWrapper for AbstractNode { fn get_wrappercache(&mut self) -> &mut WrapperCache { - do self.with_mut_node |n| { - unsafe { cast::transmute(&n.wrapper) } + do self.with_mutable_node |node| { + unsafe { + cast::transmute(&node.wrapper) + } } } diff --git a/src/servo/dom/document.rs b/src/servo/dom/document.rs index fc841534718..99150968d4c 100644 --- a/src/servo/dom/document.rs +++ b/src/servo/dom/document.rs @@ -11,7 +11,7 @@ use dom::node::AbstractNode; use dom::window::Window; use js::jsapi::bindgen::{JS_AddObjectRoot, JS_RemoveObjectRoot}; -use servo_util::tree::TreeUtils; +use servo_util::tree::{TreeNodeRef, TreeUtils}; pub struct Document { root: AbstractNode, @@ -27,7 +27,7 @@ pub fn Document(root: AbstractNode, window: window }; let compartment = global_content().compartment.get(); - do root.with_imm_node |node| { + do root.with_immutable_node |node| { assert!(node.wrapper.get_wrapper().is_not_null()); let rootable = node.wrapper.get_rootable(); JS_AddObjectRoot(compartment.cx.ptr, rootable); @@ -40,7 +40,7 @@ pub fn Document(root: AbstractNode, impl Drop for Document { fn finalize(&self) { let compartment = global_content().compartment.get(); - do self.root.with_imm_node |node| { + do self.root.with_immutable_node |node| { assert!(node.wrapper.get_wrapper().is_not_null()); let rootable = node.wrapper.get_rootable(); JS_RemoveObjectRoot(compartment.cx.ptr, rootable); diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs index 5b97149aa1d..22ed52d49f2 100644 --- a/src/servo/dom/node.rs +++ b/src/servo/dom/node.rs @@ -205,13 +205,13 @@ impl TreeNode for Node { } impl TreeNodeRef for AbstractNode { - // FIXME: The duplication between `with_imm_node` and `with_immutable_node` is ugly. + // FIXME: The duplication between `with_immutable_node` and `with_immutable_node` is ugly. fn with_immutable_node(&self, callback: &fn(&Node) -> R) -> R { - self.with_imm_node(callback) + self.transmute(callback) } fn with_mutable_node(&self, callback: &fn(&mut Node) -> R) -> R { - self.with_mut_node(callback) + self.transmute_mut(callback) } } @@ -220,44 +220,44 @@ impl AbstractNode { /// Returns the type ID of this node. Fails if this node is borrowed mutably. pub fn type_id(self) -> NodeTypeId { - self.with_imm_node(|n| n.type_id) + self.with_immutable_node(|n| n.type_id) } /// Returns the parent node of this node. Fails if this node is borrowed mutably. pub fn parent_node(self) -> Option { - self.with_imm_node(|n| n.parent_node) + self.with_immutable_node(|n| n.parent_node) } /// Returns the first child of this node. Fails if this node is borrowed mutably. pub fn first_child(self) -> Option { - self.with_imm_node(|n| n.first_child) + self.with_immutable_node(|n| n.first_child) } /// Returns the last child of this node. Fails if this node is borrowed mutably. pub fn last_child(self) -> Option { - self.with_imm_node(|n| n.last_child) + self.with_immutable_node(|n| n.last_child) } /// Returns the previous sibling of this node. Fails if this node is borrowed mutably. pub fn prev_sibling(self) -> Option { - self.with_imm_node(|n| n.prev_sibling) + self.with_immutable_node(|n| n.prev_sibling) } /// Returns the next sibling of this node. Fails if this node is borrowed mutably. pub fn next_sibling(self) -> Option { - self.with_imm_node(|n| n.next_sibling) + self.with_immutable_node(|n| n.next_sibling) } // NB: You must not call these if you are not layout. We should do something with scoping to // ensure this. pub fn layout_data(self) -> @mut LayoutData { - self.with_imm_node(|n| n.layout_data.get()) + self.with_immutable_node(|n| n.layout_data.get()) } pub fn has_layout_data(self) -> bool { - self.with_imm_node(|n| n.layout_data.is_some()) + self.with_immutable_node(|n| n.layout_data.is_some()) } pub fn set_layout_data(self, data: @mut LayoutData) { - self.with_mut_node(|n| n.layout_data = Some(data)) + self.with_mutable_node(|n| n.layout_data = Some(data)) } // @@ -290,14 +290,6 @@ impl AbstractNode { } } - pub fn with_imm_node(self, f: &fn(&Node) -> R) -> R { - self.transmute(f) - } - - pub fn with_mut_node(self, f: &fn(&mut Node) -> R) -> R { - self.transmute_mut(f) - } - pub fn is_text(self) -> bool { self.type_id() == TextNodeTypeId } @@ -408,12 +400,11 @@ impl Node { self.owner_doc = Some(doc); let mut node = self.first_child; while node.is_some() { - node.get().traverse_preorder(|n| { - do n.with_mut_node |n| { - n.owner_doc = Some(doc); + for node.get().traverse_preorder |node| { + do node.with_mutable_node |node_data| { + node_data.owner_doc = Some(doc); } - true - }); + }; node = node.get().next_sibling(); } } From b7ac68813612f40223b8b0c0ffe85c56f642e0ed Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 7 May 2013 10:15:52 -0700 Subject: [PATCH 11/12] layout: Refactor inline layout a bit --- src/servo/layout/inline.rs | 198 ++++++++++++++++++++----------------- 1 file changed, 107 insertions(+), 91 deletions(-) diff --git a/src/servo/layout/inline.rs b/src/servo/layout/inline.rs index 57cbe17de56..706f2b1f8cc 100644 --- a/src/servo/layout/inline.rs +++ b/src/servo/layout/inline.rs @@ -61,30 +61,30 @@ struct ElementMapping { priv entries: ~[NodeRange], } -pub impl ElementMapping { - fn new() -> ElementMapping { +impl ElementMapping { + pub fn new() -> ElementMapping { ElementMapping { entries: ~[] } } - fn add_mapping(&mut self, node: AbstractNode, range: &Range) { + pub fn add_mapping(&mut self, node: AbstractNode, range: &Range) { self.entries.push(NodeRange::new(node, range)) } - fn each(&self, cb: &fn(nr: &NodeRange) -> bool) { + pub fn each(&self, cb: &fn(nr: &NodeRange) -> bool) { do self.entries.each |nr| { cb(nr) } } - fn eachi(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) { + pub fn eachi(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) { do self.entries.eachi |i, nr| { cb(i, nr) } } - fn eachi_mut(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) { + pub fn eachi_mut(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) { do self.entries.eachi |i, nr| { cb(i, nr) } } - fn repair_for_box_changes(&mut self, - old_boxes: &[@mut RenderBox], - new_boxes: &[@mut RenderBox]) { + pub fn repair_for_box_changes(&mut self, + old_boxes: &[@mut RenderBox], + new_boxes: &[@mut RenderBox]) { let entries = &mut self.entries; debug!("--- Old boxes: ---"); @@ -154,13 +154,12 @@ pub impl ElementMapping { } } -// stack-allocated object for scanning an inline flow into -// TextRun-containing TextBoxes. -priv struct TextRunScanner { +/// A stack-allocated object for scanning an inline flow into `TextRun`-containing `TextBox`es. +struct TextRunScanner { clump: Range, } -priv impl TextRunScanner { +impl TextRunScanner { fn new() -> TextRunScanner { TextRunScanner { clump: Range::empty(), @@ -168,7 +167,7 @@ priv impl TextRunScanner { } } -priv impl TextRunScanner { +impl TextRunScanner { fn scan_for_runs(&mut self, ctx: &mut LayoutContext, flow: FlowContext) { let inline = &mut *flow.inline(); assert!(inline.boxes.len() > 0); @@ -206,18 +205,16 @@ priv impl TextRunScanner { } } - // a 'clump' is a range of inline flow leaves that can be merged - // together into a single RenderBox. Adjacent text with the same - // style can be merged, and nothing else can. - // - // the flow keeps track of the RenderBoxes contained by all - // non-leaf DOM nodes. This is necessary for correct painting - // order. Since we compress several leaf RenderBoxes here, the - // mapping must be adjusted. - // - // N.B. in_boxes is passed by reference, since we cannot - // recursively borrow or swap the flow's dvec of boxes. When all - // boxes are appended, the caller swaps the flow's box list. + /// A "clump" is a range of inline flow leaves that can be merged together into a single + /// `RenderBox`. Adjacent text with the same style can be merged, and nothing else can. + /// + /// The flow keeps track of the `RenderBox`es contained by all non-leaf DOM nodes. This is + /// necessary for correct painting order. Since we compress several leaf `RenderBox`es here, + /// the mapping must be adjusted. + /// + /// N.B. `in_boxes` is passed by reference, since the old code used a `DVec`. The caller is + /// responsible for swapping out the list. It is not clear to me (pcwalton) that this is still + /// necessary. fn flush_clump_to_list(&mut self, ctx: &mut LayoutContext, flow: FlowContext, @@ -227,8 +224,8 @@ priv impl TextRunScanner { debug!("TextRunScanner: flushing boxes in range=%?", self.clump); let is_singleton = self.clump.length() == 1; - let is_text_clump = match in_boxes[self.clump.begin()] { - @UnscannedTextBox(*) => true, + let is_text_clump = match *in_boxes[self.clump.begin()] { + UnscannedTextBox(*) => true, _ => false }; @@ -244,14 +241,18 @@ priv impl TextRunScanner { let old_box = in_boxes[self.clump.begin()]; let text = old_box.raw_text(); let font_style = old_box.font_style(); - // TODO(Issue #115): use actual CSS 'white-space' property of relevant style. + + // TODO(#115): Use the actual CSS `white-space` property of the relevant style. let compression = CompressWhitespaceNewline; + let transformed_text = transform_text(text, compression); - // TODO(Issue #177): text run creation must account for text-renderability by fontgroup fonts. - // this is probably achieved by creating fontgroup above, and then letting FontGroup decide - // which Font to stick into the TextRun. + + // TODO(#177): Text run creation must account for the renderability of text by + // font group fonts. This is probably achieved by creating the font group above + // and then letting `FontGroup` decide which `Font` to stick into the text run. let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style); let run = @fontgroup.create_textrun(transformed_text); + debug!("TextRunScanner: pushing single text box in range: %?", self.clump); let new_box = adapt_textbox_with_range(old_box.d(), run, @@ -259,23 +260,23 @@ priv impl TextRunScanner { out_boxes.push(new_box); }, (false, true) => { - // TODO(Issue #115): use actual CSS 'white-space' property of relevant style. + // TODO(#115): Use the actual CSS `white-space` property of the relevant style. let compression = CompressWhitespaceNewline; - // first, transform/compress text of all the nodes - let transformed_strs : ~[~str] = vec::from_fn(self.clump.length(), |i| { - // TODO(Issue #113): we shoud be passing compression context - // between calls to transform_text, so that boxes - // starting/ending with whitespace &c can be - // compressed correctly w.r.t. the TextRun. + // First, transform/compress text of all the nodes. + let transformed_strs: ~[~str] = do vec::from_fn(self.clump.length()) |i| { + // TODO(#113): We should be passing the compression context between calls to + // `transform_text`, so that boxes starting and/or ending with whitespace can + // be compressed correctly with respect to the text run. let idx = i + self.clump.begin(); transform_text(in_boxes[idx].raw_text(), compression) - }); + }; - // next, concatenate all of the transformed strings together, saving the new char indices - let mut run_str : ~str = ~""; - let mut new_ranges : ~[Range] = ~[]; - let mut char_total = 0u; + // Next, concatenate all of the transformed strings together, saving the new + // character indices. + let mut run_str: ~str = ~""; + let mut new_ranges: ~[Range] = ~[]; + let mut char_total = 0; for uint::range(0, transformed_strs.len()) |i| { let added_chars = str::char_len(transformed_strs[i]); new_ranges.push(Range::new(char_total, added_chars)); @@ -283,28 +284,32 @@ priv impl TextRunScanner { char_total += added_chars; } - // create the run, then make new boxes with the run and adjusted text indices - - // TODO(Issue #177): text run creation must account for text-renderability by fontgroup fonts. - // this is probably achieved by creating fontgroup above, and then letting FontGroup decide - // which Font to stick into the TextRun. + // Now create the run. + // + // TODO(#177): Text run creation must account for the renderability of text by + // font group fonts. This is probably achieved by creating the font group above + // and then letting `FontGroup` decide which `Font` to stick into the text run. let font_style = in_boxes[self.clump.begin()].font_style(); let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style); let run = @TextRun::new(fontgroup.fonts[0], run_str); + + // Make new boxes with the run and adjusted text indices. debug!("TextRunScanner: pushing box(es) in range: %?", self.clump); let clump = self.clump; for clump.eachi |i| { let range = &new_ranges[i - self.clump.begin()]; if range.length() == 0 { - error!("Elided an UnscannedTextbox because it was zero-length after compression; %s", - in_boxes[i].debug_str()); + error!("Elided an `UnscannedTextbox` because it was zero-length after \ + compression; %s", + in_boxes[i].debug_str()); loop } + let new_box = adapt_textbox_with_range(in_boxes[i].d(), run, range); out_boxes.push(new_box); } } - } /* /match */ + } // End of match. debug!("--- In boxes: ---"); for in_boxes.eachi |i, box| { @@ -327,7 +332,7 @@ priv impl TextRunScanner { let end = self.clump.end(); // FIXME: borrow checker workaround self.clump.reset(end, 0); - } /* /fn flush_clump_to_list */ + } // End of `flush_clump_to_list`. } struct PendingLine { @@ -343,19 +348,19 @@ struct LineboxScanner { line_spans: ~[Range], } -fn LineboxScanner(inline: FlowContext) -> LineboxScanner { - assert!(inline.starts_inline_flow()); - - LineboxScanner { - flow: inline, - new_boxes: ~[], - work_list: @mut Deque::new(), - pending_line: PendingLine {mut range: Range::empty(), mut width: Au(0)}, - line_spans: ~[] - } -} - impl LineboxScanner { + fn new(inline: FlowContext) -> LineboxScanner { + assert!(inline.starts_inline_flow()); + + LineboxScanner { + flow: inline, + new_boxes: ~[], + work_list: @mut Deque::new(), + pending_line: PendingLine {mut range: Range::empty(), mut width: Au(0)}, + line_spans: ~[] + } + } + priv fn reset_scanner(&mut self) { debug!("Resetting line box scanner's state for flow f%d.", self.flow.id()); self.line_spans = ~[]; @@ -636,10 +641,11 @@ impl InlineFlowData { /// Recursively (top-down) determines the actual width of child contexts and boxes. When called /// on this context, the context has had its width set by the parent context. pub fn assign_widths_inline(@mut self, ctx: &mut LayoutContext) { + // Initialize content box widths if they haven't been initialized already. + // + // TODO: Combine this with `LineboxScanner`'s walk in the box list, or put this into + // `RenderBox`. { - // initialize (content) box widths, if they haven't been - // already. This could be combined with LineboxScanner's walk - // over the box list, and/or put into RenderBox. let this = &mut *self; for this.boxes.each |&box| { let box2 = &mut *box; @@ -652,17 +658,18 @@ impl InlineFlowData { // Text boxes are initialized with dimensions. box.d().position.size.width }, - // TODO(Issue #225): different cases for 'inline-block', other replaced content + // TODO(#225): There will be different cases here for `inline-block` and other + // replaced content. GenericBox(*) => Au::from_px(45), _ => fail!(fmt!("Tried to assign width to unknown Box variant: %?", box)) }; - } // for boxes.each |box| + } // End of for loop. } - let mut scanner = LineboxScanner(InlineFlow(self)); + let mut scanner = LineboxScanner::new(InlineFlow(self)); scanner.scan_for_lines(ctx); - /* There are no child contexts, so stop here. */ + // There are no child contexts, so stop here. // TODO(Issue #225): once there are 'inline-block' elements, this won't be // true. In that case, set the InlineBlockBox's width to the @@ -671,7 +678,7 @@ impl InlineFlowData { // 'inline-block' box that created this flow before recursing. } - pub fn assign_height_inline(&mut self, _ctx: &mut LayoutContext) { + pub fn assign_height_inline(&mut self, _: &mut LayoutContext) { // TODO(#226): Get the CSS `line-height` property from the containing block's style to // determine minimum linebox height. // @@ -683,16 +690,16 @@ impl InlineFlowData { for self.lines.eachi |i, line_span| { debug!("assign_height_inline: processing line %u with box span: %?", i, line_span); - // coords relative to left baseline + + // These coordinates are relative to the left baseline. let mut linebox_bounding_box = Au::zero_rect(); let boxes = &mut self.boxes; for line_span.eachi |box_i| { - let cur_box: &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround + let cur_box = boxes[box_i]; // FIXME: borrow checker workaround // Compute the height of each box. let d = cur_box.d(); // FIXME: borrow checker workaround - let cur_box: &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround - + let cur_box = &mut *cur_box; // FIXME: borrow checker workaround d.position.size.height = match *cur_box { ImageBox(_, ref img) => { Au::from_px(img.size().height) @@ -710,8 +717,9 @@ impl InlineFlowData { } }; - // Compute the bounding rect with the left baseline as origin. Linebox height is a - // matter of lining up ideal baselines and then using the union of all these rects. + // Compute the bounding rect with the left baseline as origin. Determining line box + // height is a matter of lining up ideal baselines and then taking the union of all + // these rects. let bounding_box = match *cur_box { // Adjust to baseline coordinates. // @@ -726,6 +734,7 @@ impl InlineFlowData { }, // Adjust the bounding box metric to the box's horizontal offset. + // // TODO: We can use font metrics directly instead of re-measuring for the // bounding box. TextBox(_, data) => { @@ -739,32 +748,39 @@ impl InlineFlowData { cur_box.debug_str())) } }; + debug!("assign_height_inline: bounding box for box b%d = %?", cur_box.d().id, bounding_box); + linebox_bounding_box = linebox_bounding_box.union(&bounding_box); + debug!("assign_height_inline: linebox bounding box = %?", linebox_bounding_box); } + let linebox_height = linebox_bounding_box.size.height; let baseline_offset = -linebox_bounding_box.origin.y; - // now go back and adjust y coordinates to match determined baseline + + // Now go back and adjust the Y coordinates to match the baseline we determined. for line_span.eachi |box_i| { let cur_box = boxes[box_i]; - // TODO(Issue #226): this is completely wrong. Need to use element's - // 'line-height' when calculating linebox height. Then, go back over - // and set y offsets according to 'vertical-align' property of containing block. - let halfleading = match cur_box { - @TextBox(_, data) => { + + // TODO(#226): This is completely wrong. We need to use the element's `line-height` + // when calculating line box height. Then we should go back over and set Y offsets + // according to the `vertical-align` property of the containing block. + let halfleading = match *cur_box { + TextBox(_, data) => { (data.run.font.metrics.em_size - line_height).scale_by(0.5f) }, _ => Au(0), }; + cur_box.d().position.origin.y = cur_y + halfleading + (baseline_offset - cur_box.d().position.size.height); } cur_y += Au::max(line_height, linebox_height); - } // /lines.each |line_span| + } // End of `lines.each` loop. self.common.position.size.height = cur_y; } @@ -774,8 +790,8 @@ impl InlineFlowData { dirty: &Rect, offset: &Point2D, list: &Cell) { - // TODO(Issue #228): once we form line boxes and have their cached bounds, we can be - // smarter and not recurse on a line if nothing in it can intersect dirty + // TODO(#228): Once we form line boxes and have their cached bounds, we can be smarter and + // not recurse on a line if nothing in it can intersect the dirty region. debug!("FlowContext[%d]: building display list for %u inline boxes", self.common.id, self.boxes.len()); @@ -784,8 +800,8 @@ impl InlineFlowData { box.build_display_list(builder, dirty, offset, list) } - // TODO(Issue #225): should inline-block elements have flows as children - // of the inline flow, or should the flow be nested inside the box somehow? + // TODO(#225): Should `inline-block` elements have flows as children of the inline flow or + // should the flow be nested inside the box somehow? } +} -} // @FlowContext : InlineLayout From e06b31a1f8904aa8469f2b4c2e5ce06afad7025d Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 7 May 2013 11:00:36 -0700 Subject: [PATCH 12/12] servo: Rename `as_immutable_node` to `as_imm_node` and `as_mutable_node` to `as_mut_node` --- src/servo-util/tree.rs | 24 +++++++++++----------- src/servo/content/content_task.rs | 2 +- src/servo/dom/bindings/node.rs | 12 +++++------ src/servo/dom/document.rs | 4 ++-- src/servo/dom/node.rs | 26 ++++++++++++------------ src/servo/layout/block.rs | 12 +++++------ src/servo/layout/box_builder.rs | 4 ++-- src/servo/layout/display_list_builder.rs | 2 +- src/servo/layout/flow.rs | 12 +++++------ src/servo/layout/root.rs | 2 +- 10 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/servo-util/tree.rs b/src/servo-util/tree.rs index 2e46ee541fa..49e30e3dcd5 100644 --- a/src/servo-util/tree.rs +++ b/src/servo-util/tree.rs @@ -7,10 +7,10 @@ /// The basic trait. This function is meant to encapsulate a clonable reference to a tree node. pub trait TreeNodeRef : Clone { /// Borrows this node as immutable. - fn with_immutable_node(&self, callback: &fn(&N) -> R) -> R; + fn with_imm_node(&self, callback: &fn(&N) -> R) -> R; /// Borrows this node as mutable. - fn with_mutable_node(&self, callback: &fn(&mut N) -> R) -> R; + fn with_mut_node(&self, callback: &fn(&mut N) -> R) -> R; } /// The contents of a tree node. @@ -73,14 +73,14 @@ pub trait TreeUtils { impl,N:TreeNode> TreeUtils for NR { fn is_leaf(&self) -> bool { - do self.with_immutable_node |this_node| { + do self.with_imm_node |this_node| { this_node.first_child().is_none() } } fn add_child(&self, new_child: NR) { - do self.with_mutable_node |this_node| { - do new_child.with_mutable_node |new_child_node| { + do self.with_mut_node |this_node| { + do new_child.with_mut_node |new_child_node| { assert!(new_child_node.parent_node().is_none()); assert!(new_child_node.prev_sibling().is_none()); assert!(new_child_node.next_sibling().is_none()); @@ -88,7 +88,7 @@ impl,N:TreeNode> TreeUtils for NR { match this_node.last_child() { None => this_node.set_first_child(Some(new_child.clone())), Some(last_child) => { - do last_child.with_mutable_node |last_child_node| { + do last_child.with_mut_node |last_child_node| { assert!(last_child_node.next_sibling().is_none()); last_child_node.set_next_sibling(Some(new_child.clone())); new_child_node.set_prev_sibling(Some(last_child.clone())); @@ -103,14 +103,14 @@ impl,N:TreeNode> TreeUtils for NR { } fn remove_child(&self, child: NR) { - do self.with_mutable_node |this_node| { - do child.with_mutable_node |child_node| { + do self.with_mut_node |this_node| { + do child.with_mut_node |child_node| { assert!(child_node.parent_node().is_some()); match child_node.prev_sibling() { None => this_node.set_first_child(child_node.next_sibling()), Some(prev_sibling) => { - do prev_sibling.with_mutable_node |prev_sibling_node| { + do prev_sibling.with_mut_node |prev_sibling_node| { prev_sibling_node.set_next_sibling(child_node.next_sibling()); } } @@ -119,7 +119,7 @@ impl,N:TreeNode> TreeUtils for NR { match child_node.next_sibling() { None => this_node.set_last_child(child_node.prev_sibling()), Some(next_sibling) => { - do next_sibling.with_mutable_node |next_sibling_node| { + do next_sibling.with_mut_node |next_sibling_node| { next_sibling_node.set_prev_sibling(child_node.prev_sibling()); } } @@ -133,14 +133,14 @@ impl,N:TreeNode> TreeUtils for NR { } fn each_child(&self, callback: &fn(NR) -> bool) { - let mut maybe_current = self.with_immutable_node(|n| n.first_child()); + let mut maybe_current = self.with_imm_node(|n| n.first_child()); while !maybe_current.is_none() { let current = maybe_current.get_ref().clone(); if !callback(current.clone()) { break; } - maybe_current = current.with_immutable_node(|n| n.next_sibling()); + maybe_current = current.with_imm_node(|n| n.next_sibling()); } } diff --git a/src/servo/content/content_task.rs b/src/servo/content/content_task.rs index 6585aef70c8..af9cd1aa2e1 100644 --- a/src/servo/content/content_task.rs +++ b/src/servo/content/content_task.rs @@ -231,7 +231,7 @@ pub impl Content { ptr::to_mut_unsafe_ptr(&mut *self)); //FIXME store this safely let document = Document(root, Some(window)); - do root.with_mutable_node |node| { + do root.with_mut_node |node| { node.add_to_doc(document); } diff --git a/src/servo/dom/bindings/node.rs b/src/servo/dom/bindings/node.rs index 1ad90162c22..c1be3694aa8 100644 --- a/src/servo/dom/bindings/node.rs +++ b/src/servo/dom/bindings/node.rs @@ -81,7 +81,7 @@ extern fn getFirstChild(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool } let node = unwrap(obj); - let rval = do node.with_mutable_node |node| { + let rval = do node.with_mut_node |node| { node.getFirstChild() }; match rval { @@ -103,7 +103,7 @@ extern fn getNextSibling(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBoo } let node = unwrap(obj); - let rval = do node.with_mutable_node |node| { + let rval = do node.with_mut_node |node| { node.getNextSibling() }; match rval { @@ -129,7 +129,7 @@ impl Node { fn getNextSibling(&mut self) -> Option<&mut AbstractNode> { match self.next_sibling { // transmute because the compiler can't deduce that the reference - // is safe outside of with_mutable_node blocks. + // is safe outside of with_mut_node blocks. Some(ref mut n) => Some(unsafe { cast::transmute(n) }), None => None } @@ -138,7 +138,7 @@ impl Node { fn getFirstChild(&mut self) -> Option<&mut AbstractNode> { match self.first_child { // transmute because the compiler can't deduce that the reference - // is safe outside of with_mutable_node blocks. + // is safe outside of with_mut_node blocks. Some(ref mut n) => Some(unsafe { cast::transmute(n) }), None => None } @@ -153,7 +153,7 @@ extern fn getNodeType(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool { } let node = unwrap(obj); - let rval = do node.with_immutable_node |node| { + let rval = do node.with_imm_node |node| { node.getNodeType() }; *vp = INT_TO_JSVAL(rval); @@ -163,7 +163,7 @@ extern fn getNodeType(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool { impl CacheableWrapper for AbstractNode { fn get_wrappercache(&mut self) -> &mut WrapperCache { - do self.with_mutable_node |node| { + do self.with_mut_node |node| { unsafe { cast::transmute(&node.wrapper) } diff --git a/src/servo/dom/document.rs b/src/servo/dom/document.rs index 99150968d4c..277727292c7 100644 --- a/src/servo/dom/document.rs +++ b/src/servo/dom/document.rs @@ -27,7 +27,7 @@ pub fn Document(root: AbstractNode, window: window }; let compartment = global_content().compartment.get(); - do root.with_immutable_node |node| { + do root.with_imm_node |node| { assert!(node.wrapper.get_wrapper().is_not_null()); let rootable = node.wrapper.get_rootable(); JS_AddObjectRoot(compartment.cx.ptr, rootable); @@ -40,7 +40,7 @@ pub fn Document(root: AbstractNode, impl Drop for Document { fn finalize(&self) { let compartment = global_content().compartment.get(); - do self.root.with_immutable_node |node| { + do self.root.with_imm_node |node| { assert!(node.wrapper.get_wrapper().is_not_null()); let rootable = node.wrapper.get_rootable(); JS_RemoveObjectRoot(compartment.cx.ptr, rootable); diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs index 22ed52d49f2..b24e50d6a3e 100644 --- a/src/servo/dom/node.rs +++ b/src/servo/dom/node.rs @@ -205,12 +205,12 @@ impl TreeNode for Node { } impl TreeNodeRef for AbstractNode { - // FIXME: The duplication between `with_immutable_node` and `with_immutable_node` is ugly. - fn with_immutable_node(&self, callback: &fn(&Node) -> R) -> R { + // FIXME: The duplication between `with_imm_node` and `with_imm_node` is ugly. + fn with_imm_node(&self, callback: &fn(&Node) -> R) -> R { self.transmute(callback) } - fn with_mutable_node(&self, callback: &fn(&mut Node) -> R) -> R { + fn with_mut_node(&self, callback: &fn(&mut Node) -> R) -> R { self.transmute_mut(callback) } } @@ -220,44 +220,44 @@ impl AbstractNode { /// Returns the type ID of this node. Fails if this node is borrowed mutably. pub fn type_id(self) -> NodeTypeId { - self.with_immutable_node(|n| n.type_id) + self.with_imm_node(|n| n.type_id) } /// Returns the parent node of this node. Fails if this node is borrowed mutably. pub fn parent_node(self) -> Option { - self.with_immutable_node(|n| n.parent_node) + self.with_imm_node(|n| n.parent_node) } /// Returns the first child of this node. Fails if this node is borrowed mutably. pub fn first_child(self) -> Option { - self.with_immutable_node(|n| n.first_child) + self.with_imm_node(|n| n.first_child) } /// Returns the last child of this node. Fails if this node is borrowed mutably. pub fn last_child(self) -> Option { - self.with_immutable_node(|n| n.last_child) + self.with_imm_node(|n| n.last_child) } /// Returns the previous sibling of this node. Fails if this node is borrowed mutably. pub fn prev_sibling(self) -> Option { - self.with_immutable_node(|n| n.prev_sibling) + self.with_imm_node(|n| n.prev_sibling) } /// Returns the next sibling of this node. Fails if this node is borrowed mutably. pub fn next_sibling(self) -> Option { - self.with_immutable_node(|n| n.next_sibling) + self.with_imm_node(|n| n.next_sibling) } // NB: You must not call these if you are not layout. We should do something with scoping to // ensure this. pub fn layout_data(self) -> @mut LayoutData { - self.with_immutable_node(|n| n.layout_data.get()) + self.with_imm_node(|n| n.layout_data.get()) } pub fn has_layout_data(self) -> bool { - self.with_immutable_node(|n| n.layout_data.is_some()) + self.with_imm_node(|n| n.layout_data.is_some()) } pub fn set_layout_data(self, data: @mut LayoutData) { - self.with_mutable_node(|n| n.layout_data = Some(data)) + self.with_mut_node(|n| n.layout_data = Some(data)) } // @@ -401,7 +401,7 @@ impl Node { let mut node = self.first_child; while node.is_some() { for node.get().traverse_preorder |node| { - do node.with_mutable_node |node_data| { + do node.with_mut_node |node_data| { node_data.owner_doc = Some(doc); } }; diff --git a/src/servo/layout/block.rs b/src/servo/layout/block.rs index 67886bd9960..8bb3a93429a 100644 --- a/src/servo/layout/block.rs +++ b/src/servo/layout/block.rs @@ -99,7 +99,7 @@ impl BlockLayout for FlowContext { for self.each_child |child_ctx| { assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow()); - do child_ctx.with_immutable_node |child_node| { + do child_ctx.with_imm_node |child_node| { min_width = au::max(min_width, child_node.min_width); pref_width = au::max(pref_width, child_node.pref_width); } @@ -112,7 +112,7 @@ impl BlockLayout for FlowContext { pref_width = pref_width.add(&box.get_pref_width(ctx)); } - do self.with_mutable_node |this_node| { + do self.with_mut_node |this_node| { this_node.min_width = min_width; this_node.pref_width = pref_width; } @@ -126,7 +126,7 @@ impl BlockLayout for FlowContext { fn assign_widths_block(&self, _: &LayoutContext) { assert!(self.starts_block_flow()); - let mut remaining_width = self.with_immutable_node(|this| this.position.size.width); + let mut remaining_width = self.with_imm_node(|this| this.position.size.width); let mut _right_used = Au(0); let mut left_used = Au(0); @@ -140,7 +140,7 @@ impl BlockLayout for FlowContext { for self.each_child |kid| { assert!(kid.starts_block_flow() || kid.starts_inline_flow()); - do kid.with_mutable_node |child_node| { + do kid.with_mut_node |child_node| { child_node.position.origin.x = left_used; child_node.position.size.width = remaining_width; } @@ -153,13 +153,13 @@ impl BlockLayout for FlowContext { let mut cur_y = Au(0); for self.each_child |kid| { - do kid.with_mutable_node |child_node| { + do kid.with_mut_node |child_node| { child_node.position.origin.y = cur_y; cur_y += child_node.position.size.height; } } - do self.with_mutable_node |this_node| { + do self.with_mut_node |this_node| { this_node.position.size.height = cur_y; } diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index 10620b533af..7c2bc3cecb6 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -336,7 +336,7 @@ pub impl LayoutTreeBuilder { let flow = &mut this_ctx.default_collector.flow; let flow: &FlowContext = flow; for flow.each_child |child_flow| { - do child_flow.with_immutable_node |child_node| { + do child_flow.with_imm_node |child_node| { let dom_node = child_node.node; assert!(dom_node.has_layout_data()); dom_node.layout_data().flow = Some(child_flow); @@ -377,7 +377,7 @@ pub impl LayoutTreeBuilder { // of its RenderBox or FlowContext children, and possibly keep alive other junk let parent_flow = parent_ctx.default_collector.flow; - let (first_child, last_child) = do parent_flow.with_immutable_node |parent_node| { + let (first_child, last_child) = do parent_flow.with_imm_node |parent_node| { (parent_node.first_child, parent_node.last_child) }; diff --git a/src/servo/layout/display_list_builder.rs b/src/servo/layout/display_list_builder.rs index 55b26180f5e..353f25561e7 100644 --- a/src/servo/layout/display_list_builder.rs +++ b/src/servo/layout/display_list_builder.rs @@ -54,7 +54,7 @@ impl FlowDisplayListBuilderMethods for FlowContext { offset: &Point2D, list: &Cell) { // Adjust the dirty rect to child flow context coordinates. - do child_flow.with_immutable_node |child_node| { + do child_flow.with_imm_node |child_node| { let abs_flow_bounds = child_node.position.translate(offset); let adj_offset = offset.add(&child_node.position.origin); diff --git a/src/servo/layout/flow.rs b/src/servo/layout/flow.rs index facbd1de3d6..a6d0c23626a 100644 --- a/src/servo/layout/flow.rs +++ b/src/servo/layout/flow.rs @@ -72,7 +72,7 @@ impl Clone for FlowContext { } impl TreeNodeRef for FlowContext { - fn with_immutable_node(&self, callback: &fn(&FlowData) -> R) -> R { + fn with_imm_node(&self, callback: &fn(&FlowData) -> R) -> R { match *self { AbsoluteFlow(info) => callback(info), BlockFlow(info) => { @@ -92,7 +92,7 @@ impl TreeNodeRef for FlowContext { TableFlow(info) => callback(info), } } - fn with_mutable_node(&self, callback: &fn(&mut FlowData) -> R) -> R { + fn with_mut_node(&self, callback: &fn(&mut FlowData) -> R) -> R { match *self { AbsoluteFlow(info) => callback(info), BlockFlow(info) => { @@ -205,7 +205,7 @@ impl<'self> FlowContext { /// being borrowed mutably. #[inline(always)] pub fn position(&self) -> Rect { - do self.with_immutable_node |common_info| { + do self.with_imm_node |common_info| { common_info.position } } @@ -214,7 +214,7 @@ impl<'self> FlowContext { /// borrowed mutably. #[inline(always)] pub fn id(&self) -> int { - do self.with_immutable_node |info| { + do self.with_imm_node |info| { info.id } } @@ -272,7 +272,7 @@ impl<'self> FlowContext { dirty: &Rect, offset: &Point2D, list: &Cell) { - do self.with_immutable_node |info| { + do self.with_imm_node |info| { debug!("FlowContext::build_display_list at %?: %s", info.position, self.debug_str()); } @@ -406,7 +406,7 @@ impl DebugMethods for FlowContext { _ => ~"(Unknown flow)" }; - do self.with_immutable_node |this_node| { + do self.with_imm_node |this_node| { fmt!("f%? %?", this_node.id, repr) } } diff --git a/src/servo/layout/root.rs b/src/servo/layout/root.rs index 4a0150dda3a..a67eb0b6b1c 100644 --- a/src/servo/layout/root.rs +++ b/src/servo/layout/root.rs @@ -64,7 +64,7 @@ impl RootFlowData { let mut cur_y = Au(0); for RootFlow(self).each_child |child_flow| { - do child_flow.with_mutable_node |child_node| { + do child_flow.with_mut_node |child_node| { child_node.position.origin.y = cur_y; cur_y += child_node.position.size.height; }