Remove RenderBox tree and traversals; refactor box builder to pass BuilderContext around; add and thread through a stub BoxConsumer implementation.

src/servo/css/resolve/apply.rs

@@ -6,7 +6,6 @@ use au = gfx::geometry;
 use css::styles::SpecifiedStyle;
 use dom::node::{Node, NodeTree};
 use dom::element::*;
-use layout::box::{RenderBox, RenderBoxTree};
 use layout::context::LayoutContext;
 use image::ImageHolder;
 use resource::image_cache_task::ImageCacheTask;

src/servo/layout/box.rs

@@ -60,18 +60,13 @@ padding, backgrounds. It is analogous to a CSS nonreplaced content box.
 
 */
 
-
 /* A box's kind influences how its styles are interpreted during
    layout.  For example, replaced content such as images are resized
    differently than tables, text, or other content.
 
    It also holds data specific to different box types, such as text.
 */
-
-
 struct RenderBoxData {
-    /* references to children, parent inline flow boxes  */
-    tree : tree::Tree<@RenderBox>,
     /* originating DOM node */
     node : Node,
     /* reference to containing flow context, which this box
@@ -116,8 +111,6 @@ trait RenderBoxMethods {
 
 fn RenderBoxData(node: Node, ctx: @FlowContext, id: int) -> RenderBoxData {
     RenderBoxData {
-        /* will be set if box is parented */
-        tree : tree::empty(),
         node : node,
         mut ctx  : ctx,
         mut position : au::zero_rect(),
@@ -328,34 +321,6 @@ impl RenderBox : RenderBoxMethods {
     }
 }
 
-/**
- * The tree holding render box relations. These are only defined for
- * nested CSS boxes that are nested in an otherwise inline flow
- * context.
-*/
-pub enum RenderBoxTree { RenderBoxTree }
-
-impl RenderBoxTree : tree::ReadMethods<@RenderBox> {
-    fn each_child(node: @RenderBox, f: fn(box: @RenderBox) -> bool) {
-        tree::each_child(&self, &node, |box| f(*box) )
-    }
-
-    fn with_tree_fields<R>(b: &@RenderBox, f: fn(&tree::Tree<@RenderBox>) -> R) -> R {
-        f(&b.d().tree)
-    }
-}
-
-impl RenderBoxTree : tree::WriteMethods<@RenderBox> {
-    fn add_child(parent: @RenderBox, child: @RenderBox) {
-        assert !core::box::ptr_eq(parent, child);
-        tree::add_child(&self, parent, child)
-    }
-
-    fn with_tree_fields<R>(b: &@RenderBox, f: fn(&tree::Tree<@RenderBox>) -> R) -> R {
-        f(&b.d().tree)
-    }
-}
-
 impl RenderBox : BoxedDebugMethods {
     fn dump(@self) {
         self.dump_indent(0u);
@@ -370,10 +335,6 @@ impl RenderBox : BoxedDebugMethods {
 
         s += self.debug_str();
         debug!("%s", s);
-
-        for RenderBoxTree.each_child(self) |kid| {
-            kid.dump_indent(indent + 1u) 
-        }
     }
 
     fn debug_str(@self) -> ~str {
@@ -387,40 +348,3 @@ impl RenderBox : BoxedDebugMethods {
         fmt!("box b%?: %?", self.d().id, repr)
     }
 }
-
-#[cfg(test)]
-mod test {
-    use dom::element::{ElementData, HTMLDivElement, HTMLImageElement};
-    use dom::node::{Element, NodeScope, Node, NodeKind};
-    use dom::rcu::Scope;
-
-    /*
-    use sdl;
-    use sdl::video;
-
-    fn with_screen(f: fn(*sdl::surface)) {
-        let screen = video::set_video_mode(
-            320, 200, 32,
-            ~[video::hwsurface], ~[video::doublebuf]);
-        assert screen != ptr::null();
-
-        f(screen);
-
-        video::free_surface(screen);
-    }
-    */
-
-    fn flat_bounds(root: @RenderBox) -> ~[Rect<au>] {
-        let mut r = ~[];
-        for tree::each_child(&RenderBoxTree, &root) |c| {
-            push_all(&mut r, flat_bounds(*c));
-        }
-
-        push(&mut r, copy root.d().position);
-
-        return r;
-    }
-
-    // TODO: redo tests here, but probably is part of box_builder.rs
-}
-

src/servo/layout/box_builder.rs

@@ -19,27 +19,29 @@ use util::tree;
 export LayoutTreeBuilder;
 
 struct LayoutTreeBuilder {
-    mut root_ctx: Option<@FlowContext>,
+    mut root_flow: Option<@FlowContext>,
     mut next_bid: int,
     mut next_cid: int
 }
 
 fn LayoutTreeBuilder() -> LayoutTreeBuilder {
     LayoutTreeBuilder {
-        root_ctx: None,
+        root_flow: None,
         next_bid: -1,
         next_cid: -1
     }
 }
 
+type BuilderContext = { flow: @FlowContext, consumer: BoxConsumer };
+
 impl LayoutTreeBuilder {
     /* Debug-only ids */
     fn next_box_id() -> int { self.next_bid += 1; self.next_bid }
-    fn next_ctx_id() -> int { self.next_cid += 1; self.next_cid }
+    fn next_flow_id() -> int { self.next_cid += 1; self.next_cid }
 
     /** Creates necessary box(es) and flow context(s) for the current DOM node,
     and recurses on its children. */
-    fn construct_recursively(layout_ctx: &LayoutContext, cur_node: Node, parent_ctx: @FlowContext) {
+    fn construct_recursively(layout_ctx: &LayoutContext, cur_node: Node, parent_ctx: &BuilderContext) {
         let style = cur_node.style();
         // DEBUG
         let n_str = fmt!("%?", cur_node.read(|n| copy n.kind ));
@@ -56,14 +58,14 @@ impl LayoutTreeBuilder {
         let box_type = self.decide_box_type(cur_node, simulated_display);
 
         // then, figure out its proper context, possibly reorganizing.
-        let next_ctx: @FlowContext = match box_type {
+        let next_flow: @FlowContext = match box_type {
             /* Text box is always an inline flow. create implicit inline
-            flow ctx if we aren't inside one already. */
+            flow if we aren't inside one already. */
             RenderBox_Text => {
-                if (parent_ctx.starts_inline_flow()) {
+                if (parent_ctx.flow.starts_inline_flow()) {
-                    parent_ctx
+                    parent_ctx.flow
                 } else {
-                    self.make_ctx(Flow_Inline)
+                    self.make_flow(Flow_Inline)
                 }
             },
             RenderBox_Image | RenderBox_Generic => {
@@ -71,51 +73,55 @@ impl LayoutTreeBuilder {
                     DisplayInline | DisplayInlineBlock => {
                         /* if inline, try to put into inline context,
                         making a new one if necessary */
-                        if (parent_ctx.starts_inline_flow()) {
+                        if (parent_ctx.flow.starts_inline_flow()) {
-                            parent_ctx
+                            parent_ctx.flow
                         } else {
-                            self.make_ctx(Flow_Inline)
+                            self.make_flow(Flow_Inline)
                         }
                     },
                     /* block boxes always create a new context */
                     DisplayBlock => {
-                        self.make_ctx(Flow_Block)
+                        self.make_flow(Flow_Block)
                     },
                     _ => fail fmt!("unsupported display type in box generation: %?", simulated_display)
                 }
             }
         };
 
+        let mut builder_ctx : BuilderContext = copy *parent_ctx;
+
+        // if this is a new flow, attach to parent flow and make a new BuilderContext.
+        if !core::box::ptr_eq(next_flow, parent_ctx.flow) {
+            debug!("Adding child flow f%? of f%?",
+                   parent_ctx.flow.d().id, next_flow.d().id);
+            FlowTree.add_child(parent_ctx.flow, next_flow);
+
+            builder_ctx = { flow: next_flow, consumer: BoxConsumer(next_flow) };
+        }
+
         // store reference to the flow context which contains any
         // boxes that correspond to cur_node. These boxes may
         // eventually be elided or split, but the mapping between
         // nodes and FlowContexts should not change during layout.
         assert cur_node.has_aux();
-        do cur_node.aux |data| { data.flow = Some(next_ctx) }
+        do cur_node.aux |data| { data.flow = Some(builder_ctx.flow) }
 
-        // make box, add box to any context-specific list.
-        let new_box = self.make_box(layout_ctx, box_type, cur_node, parent_ctx);
-        debug!("Assign ^box to flow: %?", next_ctx.debug_str());
-        next_ctx.accept_new_box(layout_ctx, new_box);
 
-        // if this is a new flow, attach to parent flow.
+        let new_box = self.make_box(layout_ctx, box_type, cur_node, builder_ctx.flow);
-        if !core::box::ptr_eq(next_ctx, parent_ctx) {
+        debug!("Assign ^box to flow: %?", builder_ctx.flow.debug_str());
-            debug!("Adding child flow f%? of f%?",
+        builder_ctx.consumer.accept_box(layout_ctx, new_box);
-                   parent_ctx.d().id, next_ctx.d().id);
-            FlowTree.add_child(parent_ctx, next_ctx);
-        }
 
         // recurse on child nodes.
         do NodeTree.each_child(&cur_node) |child_node| {
-            self.construct_recursively(layout_ctx, *child_node, next_ctx); true
+            self.construct_recursively(layout_ctx, *child_node, &builder_ctx); true
         }
 
         // Fixup any irregularities, such as split inlines (CSS 2.1 Section 9.2.1.1)
-        if (next_ctx.starts_inline_flow()) {
+        if (builder_ctx.flow.starts_inline_flow()) {
             let mut found_child_inline = false;
             let mut found_child_block = false;
 
-            do FlowTree.each_child(next_ctx) |child_ctx| {
+            do FlowTree.each_child(builder_ctx.flow) |child_ctx| {
                 match *child_ctx {
                     InlineFlow(*) | InlineBlockFlow(*) => found_child_inline = true,
                     BlockFlow(*) => found_child_block = true,
@@ -124,7 +130,7 @@ impl LayoutTreeBuilder {
             }
 
             if found_child_block && found_child_inline {
-                self.fixup_split_inline(next_ctx)
+                self.fixup_split_inline(builder_ctx.flow)
             }
         }
     }
@@ -158,13 +164,15 @@ impl LayoutTreeBuilder {
     /** entry point for box creation. Should only be 
     called on root DOM element. */
     fn construct_trees(layout_ctx: &LayoutContext, root: Node) -> Result<@FlowContext, ()> {
-        self.root_ctx = Some(self.make_ctx(Flow_Root));
+        let new_flow = self.make_flow(Flow_Root);
-        self.construct_recursively(layout_ctx, root, self.root_ctx.get());
+        self.root_flow = Some(new_flow);
-        return Ok(self.root_ctx.get())
+        let builder_ctx = { flow: new_flow, consumer: BoxConsumer(new_flow) };
+        self.construct_recursively(layout_ctx, root, &builder_ctx);
+        return Ok(new_flow)
     }
 
-    fn make_ctx(ty : FlowContextType) -> @FlowContext {
+    fn make_flow(ty : FlowContextType) -> @FlowContext {
-        let data = FlowData(self.next_ctx_id());
+        let data = FlowData(self.next_flow_id());
         let ret = match ty {
             Flow_Absolute    => @AbsoluteFlow(data),
             Flow_Block       => @BlockFlow(data, BlockFlowData()),

src/servo/layout/display_list_builder.rs

@@ -11,7 +11,7 @@ use either::{Left, Right};
 use geom::point::Point2D;
 use geom::rect::Rect;
 use geom::size::Size2D;
-use layout::box::{RenderBox, RenderBoxTree, TextBox};
+use layout::box::{RenderBox, TextBox};
 use layout::context::LayoutContext;
 use layout::flow::FlowContext;
 use layout::text::TextBoxData;

src/servo/layout/flow.rs

@@ -106,6 +106,35 @@ fn FlowData(id: int) -> FlowData {
     }
 }
 
+// helper object for building the initial box list and making the
+// mapping between DOM nodes and boxes.
+struct BoxConsumer {
+    flow: @FlowContext,
+}
+
+fn BoxConsumer(flow: @FlowContext) -> BoxConsumer {
+    BoxConsumer {
+        flow: flow,
+    }
+}
+
+impl BoxConsumer {
+    pub fn accept_box(_ctx: &LayoutContext, box: @RenderBox) {
+        match self.flow {
+            @InlineFlow(*) => self.flow.inline().boxes.push(box),
+            @BlockFlow(*) => {
+                assert self.flow.block().box.is_none();
+                self.flow.block().box = Some(box);
+            },
+            @RootFlow(*) => {
+                assert self.flow.block().box.is_none();
+                self.flow.block().box = Some(box);
+            },
+            _ => { warn!("accept_box not implemented for flow %?", self.flow.d().id) }
+        }
+    }
+}
+
 impl FlowContext : FlowContextMethods {
     pure fn d(&self) -> &self/FlowData {
         match *self {

src/servo/layout/inline.rs

@@ -8,7 +8,7 @@ use geom::point::Point2D;
 use geom::rect::Rect;
 use geom::size::Size2D;
 use gfx::geometry::au;
-use layout::box::{RenderBox, RenderBoxTree, ImageBox, TextBox, GenericBox, UnscannedTextBox};
+use layout::box::{RenderBox, ImageBox, TextBox, GenericBox, UnscannedTextBox};
 use layout::context::LayoutContext;
 use layout::flow::{FlowContext, InlineFlow};
 use layout::text::TextBoxData;
@@ -181,7 +181,7 @@ fn InlineFlowData() -> InlineFlowData {
     InlineFlowData {
         boxes: DVec(),
         lines: DVec(),
-        elems: DVec()
+        elems: DVec(),
     }
 }
 

src/servo/layout/traverse.rs

@@ -1,20 +1,6 @@
-/** Interface for running tree-based traversals over layout boxes and contextsg */
-
-use layout::box::{RenderBox, RenderBoxTree};
 use layout::flow::{FlowContext, FlowTree};
 
-/* TODO: we shouldn't need render box traversals  */
+/** Trait for running tree-based traversals over layout contexts */
-trait RenderBoxTraversals {
-    fn traverse_preorder(preorder_cb: &fn(@RenderBox));
-}
-
-impl @RenderBox : RenderBoxTraversals {
-    fn traverse_preorder(preorder_cb: &fn(@RenderBox)) {
-        preorder_cb(self);
-        do RenderBoxTree.each_child(self) |child| { child.traverse_preorder(preorder_cb); true }
-    }
-}
-
 trait FlowContextTraversals {
     fn traverse_preorder(preorder_cb: &fn(@FlowContext));
     fn traverse_postorder(postorder_cb: &fn(@FlowContext));