auto merge of #1362 : pcwalton/servo/rm-display-boxes, r=larsbergstrom

`@`-free layout

r? @larsbergstrom

src/components/gfx/display_list.rs

@@ -21,6 +21,7 @@ use render_context::RenderContext;
 use text::TextRun;
 
 use std::cast::transmute_region;
+use std::vec::VecIterator;
 use geom::{Point2D, Rect, Size2D, SideOffsets2D};
 use servo_net::image::base::Image;
 use servo_util::range::Range;
@@ -56,6 +57,11 @@ impl<E> DisplayList<E> {
         }
         debug!("Ending display list.")
     }
+
+    /// Returns a preorder iterator over the given display list.
+    pub fn iter<'a>(&'a self) -> DisplayItemIterator<'a,E> {
+        ParentDisplayItemIterator(self.list.iter())
+    }
 }
 
 /// One drawing command in the list.
@@ -118,6 +124,21 @@ pub struct ClipDisplayItem<E> {
     need_clip: bool
 }
 
+pub enum DisplayItemIterator<'self,E> {
+    EmptyDisplayItemIterator,
+    ParentDisplayItemIterator(VecIterator<'self,DisplayItem<E>>),
+}
+
+impl<'self,E> Iterator<&'self DisplayItem<E>> for DisplayItemIterator<'self,E> {
+    #[inline]
+    fn next(&mut self) -> Option<&'self DisplayItem<E>> {
+        match *self {
+            EmptyDisplayItemIterator => None,
+            ParentDisplayItemIterator(ref mut subiterator) => subiterator.next(),
+        }
+    }
+}
+
 impl<E> DisplayItem<E> {
     /// Renders this display item into the given render context.
     fn draw_into_context(&self, render_context: &mut RenderContext) {
@@ -212,5 +233,30 @@ impl<E> DisplayItem<E> {
     pub fn bounds(&self) -> Rect<Au> {
         self.base().bounds
     }
+
+    pub fn children<'a>(&'a self) -> DisplayItemIterator<'a,E> {
+        match *self {
+            ClipDisplayItemClass(ref clip) => ParentDisplayItemIterator(clip.child_list.iter()),
+            SolidColorDisplayItemClass(*) |
+            TextDisplayItemClass(*) |
+            ImageDisplayItemClass(*) |
+            BorderDisplayItemClass(*) => EmptyDisplayItemIterator,
+        }
+    }
+
+    pub fn debug_str(&self) -> ~str {
+        let class = match *self {
+            SolidColorDisplayItemClass(_) => "SolidColor",
+            TextDisplayItemClass(_) => "Text",
+            ImageDisplayItemClass(_) => "Image",
+            BorderDisplayItemClass(_) => "Border",
+            ClipDisplayItemClass(_) => "Clip",
+        };
+        let mut string = format!("{} @ {:?}", class, self.base().bounds);
+        for child in self.children() {
+            string = format!("{}\n  {}", string, child.debug_str());
+        }
+        string
+    }
 }
 

src/components/main/layout/display_list_builder.rs

@@ -35,12 +35,6 @@ impl ExtraDisplayListData for Nothing {
     }
 }
 
-impl ExtraDisplayListData for @Box {
-    fn new(box: &@Box) -> @Box {
-        *box
-    }
-}
-
 /// 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.

src/components/main/layout/extra.rs

@@ -4,7 +4,7 @@
 
 //! Code for managing the layout data in the DOM.
 
-use layout::util::{DisplayBoxes, LayoutData, LayoutDataAccess};
+use layout::util::{LayoutData, LayoutDataAccess};
 
 use script::dom::node::{AbstractNode, LayoutView};
 use servo_util::tree::TreeNodeRef;
@@ -17,11 +17,13 @@ pub trait LayoutAuxMethods {
 
 impl LayoutAuxMethods for AbstractNode<LayoutView> {
     /// Resets layout data and styles for the node.
+    ///
+    /// FIXME(pcwalton): Do this as part of box building instead of in a traversal.
     fn initialize_layout_data(self) {
         let layout_data_handle = self.mutate_layout_data();
         match *layout_data_handle.ptr {
             None => *layout_data_handle.ptr = Some(~LayoutData::new()),
-            Some(ref mut layout_data) => layout_data.boxes = DisplayBoxes::init(),
+            Some(_) => {}
         }
     }
 

src/components/main/layout/layout_task.rs

@@ -22,7 +22,7 @@ use extra::arc::{Arc, RWArc, MutexArc};
 use geom::point::Point2D;
 use geom::rect::Rect;
 use geom::size::Size2D;
-use gfx::display_list::{DisplayList,DisplayItem,ClipDisplayItemClass};
+use gfx::display_list::{ClipDisplayItemClass, DisplayItem, DisplayItemIterator, DisplayList};
 use gfx::font_context::FontContext;
 use gfx::opts::Opts;
 use gfx::render_task::{RenderMsg, RenderChan, RenderLayer};
@@ -473,10 +473,6 @@ impl LayoutTask {
 
                 let display_list = Arc::new(display_list.take());
 
-                for i in range(0,display_list.get().list.len()) {
-                    self.display_item_bound_to_node(&display_list.get().list[i]);
-                }
-
                     let mut color = color::rgba(255.0, 255.0, 255.0, 255.0);
 
                     for child in node.traverse_preorder() {
@@ -516,111 +512,62 @@ impl LayoutTask {
         data.script_chan.send(ReflowCompleteMsg(self.id, data.id));
     }
 
-    fn display_item_bound_to_node(&mut self,item: &DisplayItem<AbstractNode<()>>) {
-        let node: AbstractNode<LayoutView> = unsafe {
-            transmute(item.base().extra)
-        };
-
-        match *node.mutate_layout_data().ptr {
-            Some(ref mut layout_data) => {
-                let boxes = &mut layout_data.boxes;
-
-                if boxes.display_bound_list.is_none() {
-                    boxes.display_bound_list = Some(~[]);
-                }
-                match boxes.display_bound_list {
-                    Some(ref mut list) => list.push(item.base().bounds),
-                    None => {}
-                }
-
-                if boxes.display_bound.is_none() {
-                    boxes.display_bound = Some(item.base().bounds);
-                } else {
-                    boxes.display_bound = Some(boxes.display_bound.unwrap().union(&item.base().bounds));
-                }
-            }
-            None => fail!("no layout data"),
-        }
-
-        match *item {
-            ClipDisplayItemClass(ref cc) => {
-                for item in cc.child_list.iter() {
-                    self.display_item_bound_to_node(item);
-                }
-            }
-            _ => {}
-        }
-    }
-
     /// 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) {
         match query {
             ContentBoxQuery(node, reply_chan) => {
                 // FIXME: Isolate this transmutation into a single "bridge" module.
-                let node: AbstractNode<LayoutView> = unsafe {
+                let node: AbstractNode<()> = unsafe {
                     transmute(node)
                 };
 
-                fn box_for_node(node: AbstractNode<LayoutView>) -> Option<Rect<Au>> {
+                fn union_boxes_for_node<'a>(
-                    // FIXME(pcwalton): Why are we cloning the display list here?!
+                                        accumulator: &mut Option<Rect<Au>>,
-                    let layout_data = node.borrow_layout_data();
+                                        mut iter: DisplayItemIterator<'a,AbstractNode<()>>,
-                    let boxes = &layout_data.ptr.as_ref().unwrap().boxes;
+                                        node: AbstractNode<()>) {
-                    match boxes.display_bound {
+                    for item in iter {
-                        Some(_) => boxes.display_bound,
+                        union_boxes_for_node(accumulator, item.children(), node);
-                        _ => {
+                        if item.base().extra == node {
-                            let mut acc: Option<Rect<Au>> = None;
+                            match *accumulator {
-                            for child in node.children() {
+                                None => *accumulator = Some(item.base().bounds),
-                                let rect = box_for_node(child);
+                                Some(ref mut acc) => *acc = acc.union(&item.base().bounds),
-                                match rect {
-                                    None => continue,
-                                    Some(rect) => acc = match acc {
-                                        Some(acc) =>  Some(acc.union(&rect)),
-                                        None => Some(rect)
-                                    }
-                                }
                             }
-                            acc
                         }
                     }
                 }
 
-                let rect = box_for_node(node).unwrap_or(Rect(Point2D(Au(0), Au(0)),
+                let mut rect = None;
-                                                             Size2D(Au(0), Au(0))));
+                let display_list = self.display_list.as_ref().unwrap().get();
-                reply_chan.send(ContentBoxResponse(rect))
+                union_boxes_for_node(&mut rect, display_list.iter(), node);
+                reply_chan.send(ContentBoxResponse(rect.unwrap_or(Au::zero_rect())))
             }
             ContentBoxesQuery(node, reply_chan) => {
                 // FIXME: Isolate this transmutation into a single "bridge" module.
-                let node: AbstractNode<LayoutView> = unsafe {
+                let node: AbstractNode<()> = unsafe {
                     transmute(node)
                 };
 
-                fn boxes_for_node(node: AbstractNode<LayoutView>, mut box_accumulator: ~[Rect<Au>])
+                fn add_boxes_for_node<'a>(
-                                  -> ~[Rect<Au>] {
+                                      accumulator: &mut ~[Rect<Au>],
-                    let layout_data = node.borrow_layout_data();
+                                      mut iter: DisplayItemIterator<'a,AbstractNode<()>>,
-                    let boxes = &layout_data.ptr.as_ref().unwrap().boxes;
+                                      node: AbstractNode<()>) {
-                    match boxes.display_bound_list {
+                    for item in iter {
-                        Some(ref display_bound_list) => {
+                        add_boxes_for_node(accumulator, item.children(), node);
-                            for item in display_bound_list.iter() {
+                        if item.base().extra == node {
-                                box_accumulator.push(*item);
+                            accumulator.push(item.base().bounds)
-                            }
-                        }
-                        _ => {
-                            for child in node.children() {
-                                box_accumulator = boxes_for_node(child, box_accumulator);
-                            }
                         }
                     }
-                    box_accumulator
                 }
 
                 let mut boxes = ~[];
-                boxes = boxes_for_node(node, boxes);
+                let display_list = self.display_list.as_ref().unwrap().get();
+                add_boxes_for_node(&mut boxes, display_list.iter(), node);
                 reply_chan.send(ContentBoxesResponse(boxes))
             }
             HitTestQuery(_, point, reply_chan) => {
-                fn hit_test(x:Au, y:Au, list: &[DisplayItem<AbstractNode<()>>]) -> Option<HitTestResponse> {
+                fn hit_test(x: Au, y: Au, list: &[DisplayItem<AbstractNode<()>>])
-
+                            -> Option<HitTestResponse> {
                     for item in list.rev_iter() {
                         match *item {
                             ClipDisplayItemClass(ref cc) => {

src/components/main/layout/util.rs

@@ -6,7 +6,6 @@ use layout::box::Box;
 use layout::construct::{ConstructionResult, NoConstructionResult};
 
 use extra::arc::Arc;
-use gfx::display_list::DisplayList;
 use script::dom::node::{AbstractNode, LayoutView};
 use servo_util::range::Range;
 use servo_util::slot::{MutSlotRef, SlotRef};
@@ -15,25 +14,6 @@ use std::cast;
 use std::iter::Enumerate;
 use std::vec::VecIterator;
 use style::{ComputedValues, PropertyDeclaration};
-use geom::rect::Rect;
-use servo_util::geometry::Au;
-
-/// The boxes associated with a node.
-pub struct DisplayBoxes {
-    display_list: Option<Arc<DisplayList<AbstractNode<()>>>>,
-    display_bound_list: Option<~[Rect<Au>]>,
-    display_bound: Option<Rect<Au>>
-}
-
-impl DisplayBoxes {
-    pub fn init() -> DisplayBoxes {
-        DisplayBoxes {
-            display_list: None,
-            display_bound_list: None,
-            display_bound: None,
-        }
-    }
-}
 
 /// A range of nodes.
 pub struct NodeRange {
@@ -152,10 +132,6 @@ pub struct LayoutData {
     /// Description of how to account for recent style changes.
     restyle_damage: Option<int>,
 
-    /// The boxes assosiated with this flow.
-    /// Used for getBoundingClientRect and friends.
-    boxes: DisplayBoxes,
-
     /// The current results of flow construction for this node. This is either a flow or a
     /// `ConstructionItem`. See comments in `construct.rs` for more details.
     flow_construction_result: ConstructionResult,
@@ -168,7 +144,6 @@ impl LayoutData {
             applicable_declarations: ~[],
             style: None,
             restyle_damage: None,
-            boxes: DisplayBoxes::init(),
             flow_construction_result: NoConstructionResult,
         }
     }