Invert DisplayItem enum variant data nesting, and add some TextRun static new's. Fixes #96.

src/servo/gfx/display_list.rs

@@ -13,91 +13,70 @@ use dvec::DVec;
 
 pub use layout::display_list_builder::DisplayListBuilder;
 
-// TODO: invert this so common data is nested inside each variant as first arg.
+struct DisplayItemData {
-struct DisplayItem {
-    draw: ~fn((&DisplayItem), (&RenderContext)),
     bounds : Rect<Au>, // TODO: whose coordinate system should this use?
-    data : DisplayItemData
 }
 
-pub enum DisplayItemData {
+impl DisplayItemData {
-    SolidColorData(u8, u8, u8),
+    static pure fn new(bounds: &Rect<Au>) -> DisplayItemData {
+        DisplayItemData { bounds: copy *bounds }
+    }
+}
+
+pub enum DisplayItem {
+    SolidColor(DisplayItemData, u8, u8, u8),
     // TODO: need to provide spacing data for text run.
     // (i.e, to support rendering of CSS 'word-spacing' and 'letter-spacing')
     // TODO: don't copy text runs, ever.
-    TextData(~SendableTextRun, Range),
+    Text(DisplayItemData, ~SendableTextRun, Range),
-    ImageData(ARC<~image::base::Image>),
+    Image(DisplayItemData, ARC<~image::base::Image>),
-    BorderData(Au, u8, u8, u8)
+    Border(DisplayItemData, Au, u8, u8, u8)
 }
 
-fn draw_SolidColor(self: &DisplayItem, ctx: &RenderContext) {
+impl DisplayItem {
-    match self.data {
+    pure fn d(&self) -> &self/DisplayItemData {
-        SolidColorData(r,g,b) => ctx.draw_solid_color(&self.bounds, r, g, b),
+        match *self {
-        _ => fail
+            SolidColor(ref d, _, _, _) => d,
-    }        
+            Text(ref d, _, _) => d,
-}
+            Image(ref d, _) => d,
-
+            Border(ref d, _, _, _, _) => d
-fn draw_Text(self: &DisplayItem, ctx: &RenderContext) {
+        }
-    match self.data {
-        TextData(run, range) => {
-            let new_run = text_run::deserialize(ctx.font_cache, run);
-            ctx.draw_text(self.bounds, new_run, range)
-        },
-        _ => fail
-    }        
-}
-
-fn draw_Image(self: &DisplayItem, ctx: &RenderContext) {
-    match self.data {
-        ImageData(ref img) => ctx.draw_image(self.bounds, clone_arc(img)),
-        _ => fail
-    }        
-}
-
-fn draw_Border(self: &DisplayItem, ctx: &RenderContext) {
-    match self.data {
-        BorderData(width, r, g, b) => ctx.draw_border(&self.bounds, width, r, g, b),
-        _ => fail
     }
-}
+    
-
+    fn draw_into_context(&self, ctx: &RenderContext) {
-pub fn SolidColor(bounds: Rect<Au>, r: u8, g: u8, b: u8) -> DisplayItem {
+        match *self {
-    DisplayItem { 
+            SolidColor(_, r,g,b) => ctx.draw_solid_color(&self.d().bounds, r, g, b),
-        draw: |self, ctx| draw_SolidColor(self, ctx),
+            Text(_, run, range) => {
-        bounds: bounds,
+                let new_run = @run.deserialize(ctx.font_cache);
-        data: SolidColorData(r, g, b)
+                ctx.draw_text(self.d().bounds, new_run, range)
+            },
+            Image(_, ref img) => ctx.draw_image(self.d().bounds, clone_arc(img)),
+            Border(_, width, r, g, b) => ctx.draw_border(&self.d().bounds, width, r, g, b),
+        }
     }
-}
 
-pub fn Border(bounds: Rect<Au>, width: Au, r: u8, g: u8, b: u8) -> DisplayItem {
+    static pure fn new_SolidColor(bounds: &Rect<Au>, r: u8, g: u8, b: u8) -> DisplayItem {
-    DisplayItem {
+        SolidColor(DisplayItemData::new(bounds), r, g, b)
-        draw: |self, ctx| draw_Border(self, ctx),
-        bounds: bounds,
-        data: BorderData(width, r, g, b)
     }
-}
 
-pub fn Text(bounds: Rect<Au>, run: ~SendableTextRun, range: Range) -> DisplayItem {
+    static pure fn new_Border(bounds: &Rect<Au>, width: Au, r: u8, g: u8, b: u8) -> DisplayItem {
-    DisplayItem {
+        Border(DisplayItemData::new(bounds), width, r, g, b)
-        draw: |self, ctx| draw_Text(self, ctx),
-        bounds: bounds,
-        data: TextData(move run, move range)
     }
-}
 
-// ARC should be cloned into ImageData, but Images are not sendable
+    static pure fn new_Text(bounds: &Rect<Au>, run: ~SendableTextRun, range: Range) -> DisplayItem {
-pub fn Image(bounds: Rect<Au>, image: ARC<~image::base::Image>) -> DisplayItem {
+        Text(DisplayItemData::new(bounds), move run, range)
-    DisplayItem {
+    }
-        draw: |self, ctx| draw_Image(self, ctx),
+
-        bounds: bounds,
+    // ARC should be cloned into ImageData, but Images are not sendable
-        data: ImageData(move image)
+    static pure fn new_Image(bounds: &Rect<Au>, image: ARC<~image::base::Image>) -> DisplayItem {
+        Image(DisplayItemData::new(bounds), move image)
     }
 }
 
 pub type DisplayList = DVec<~DisplayItem>;
 
 trait DisplayListMethods {
-    fn draw(ctx: &RenderContext);
+    fn draw_into_context(ctx: &RenderContext);
 }
 
 impl DisplayList : DisplayListMethods {
@@ -106,11 +85,11 @@ impl DisplayList : DisplayListMethods {
         self.push(move item);
     }
 
-    fn draw(ctx: &RenderContext) {
+    fn draw_into_context(ctx: &RenderContext) {
         debug!("beginning display list");
         for self.each |item| {
             debug!("drawing %?", *item);
-            item.draw(*item, ctx);
+            item.draw_into_context(ctx);
         }
         debug!("ending display list");
     }

src/servo/gfx/render_task.rs

@@ -92,7 +92,7 @@ impl<C: Compositor Send> Renderer<C> {
                 };
 
                 ctx.clear();
-                render_layer.display_list.draw(&ctx);
+                render_layer.display_list.draw_into_context(&ctx);
             };
 
             #debug("renderer: returning surface");

src/servo/layout/box.rs

@@ -11,6 +11,7 @@ use core::rand;
 use css::styles::SpecifiedStyle;
 use css::values::{BoxSizing, Length, Px, CSSDisplay, Specified, BgColor, BgColorTransparent, BdrColor, PosAbsolute};
 use dl = gfx::display_list;
+use dl::DisplayItem;
 use dom::element::{ElementKind, HTMLDivElement, HTMLImageElement};
 use dom::node::{Element, Node, NodeData, NodeKind, NodeTree};
 use geom::rect::Rect;
@@ -417,22 +418,23 @@ impl RenderBox : RenderBoxMethods {
         match *self {
             UnscannedTextBox(*) => fail ~"Shouldn't see unscanned boxes here.",
             TextBox(_,d) => {
-                list.append_item(~dl::Text(copy abs_box_bounds, text_run::serialize(builder.ctx.font_cache, d.run),
+                list.append_item(~DisplayItem::new_Text(&abs_box_bounds, 
-                                           d.range))
+                                                        ~d.run.serialize(builder.ctx.font_cache),
+                                                        d.range))
             },
             // TODO: items for background, border, outline
             GenericBox(_) => {
             },
             ImageBox(_,i) => {
                 match i.get_image() {
-                    Some(image) => list.append_item(~dl::Image(copy abs_box_bounds, arc::clone(&image))),
+                    Some(image) => list.append_item(~DisplayItem::new_Image(&abs_box_bounds, arc::clone(&image))),
                     /* No image data at all? Okay, add some fallback content instead. */
                     None => ()
                 }
             }
         }
 
-        self.add_border_to_list(list, abs_box_bounds);
+        self.add_border_to_list(list, &abs_box_bounds);
     }
 
     fn add_bgcolor_to_list(list: &dl::DisplayList, abs_bounds: &Rect<Au>) {
@@ -444,11 +446,11 @@ impl RenderBox : RenderBoxMethods {
             Specified(BgColorTransparent) | _ => util::color::rgba(0,0,0,0.0)
         };
         if !bgcolor.alpha.fuzzy_eq(&0.0) {
-            list.append_item(~dl::SolidColor(copy *abs_bounds, bgcolor.red, bgcolor.green, bgcolor.blue));
+            list.append_item(~DisplayItem::new_SolidColor(abs_bounds, bgcolor.red, bgcolor.green, bgcolor.blue));
         }
     }
 
-    fn add_border_to_list(list: &dl::DisplayList, abs_bounds: Rect<Au>) {
+    fn add_border_to_list(list: &dl::DisplayList, abs_bounds: &Rect<Au>) {
         let style = self.d().node.style();
         match style.border_width {
             Specified(Px(px)) => {
@@ -468,7 +470,7 @@ impl RenderBox : RenderBoxMethods {
                     Specified(BdrColor(color)) => color,
                     _ => rgb(0, 0, 0) // FIXME
                 };
-                list.push(~dl::Border(abs_bounds, border_width, color.red, color.green, color.blue));
+                list.push(~DisplayItem::new_Border(&abs_bounds, border_width, color.red, color.green, color.blue));
             }
             _ => () // TODO
         }

src/servo/layout/inline.rs

@@ -237,7 +237,7 @@ impl TextRunScanner {
                 let compression = CompressWhitespaceNewline;
                 let transformed_text = transform_text(text, compression);
                 // TODO(Issue #116): use actual font for corresponding DOM node to create text run.
-                let run = @TextRun(ctx.font_cache.get_test_font(), move transformed_text);
+                let run = @TextRun::new(ctx.font_cache.get_test_font(), move transformed_text);
                 debug!("TextRunScanner: pushing single text box in range: %?", self.clump);
                 let new_box = layout::text::adapt_textbox_with_range(in_boxes[self.clump.begin()].d(), run,
                                                                      Range(0, run.text.len()));
@@ -270,7 +270,7 @@ impl TextRunScanner {
                 // create the run, then make new boxes with the run and adjusted text indices
 
                 // TODO(Issue #116): use actual font for corresponding DOM node to create text run.
-                let run = @TextRun(ctx.font_cache.get_test_font(), move run_str);
+                let run = @TextRun::new(ctx.font_cache.get_test_font(), move run_str);
                 debug!("TextRunScanner: pushing box(es) in range: %?", self.clump);
                 for self.clump.eachi |i| {
                     let range = new_ranges[i - self.clump.begin()];

src/servo/text/text_run.rs

@@ -28,37 +28,39 @@ pub struct SendableTextRun {
     priv glyphs: GlyphStore,
 }
 
-pub fn serialize(_cache: @FontCache, run: &TextRun) -> ~SendableTextRun {
+impl SendableTextRun {
-    ~SendableTextRun {
+    pub fn deserialize(&self, cache: @FontCache) -> TextRun {
-        text: copy run.text,
+        TextRun {
-        // TODO: actually serialize a font descriptor thingy
+            text: copy self.text,
-        font_descriptor: (),
+            // TODO: actually deserialize a font descriptor thingy
-        glyphs: copy run.glyphs,
+            font: cache.get_test_font(),
+            glyphs: copy self.glyphs
+        }
     }
 }
 
-pub fn deserialize(cache: @FontCache, run: &SendableTextRun) -> @TextRun {
+impl TextRun {
-    @TextRun {
+    pub static fn new(font: @Font, text: ~str) -> TextRun {
-        text: copy run.text,
+        let glyph_store = GlyphStore(text.len());
-        // TODO: actually deserialize a font descriptor thingy
+        let run = TextRun {
-        font: cache.get_test_font(),
+            text: move text,
-        glyphs: copy run.glyphs
+            font: font,
+            glyphs: move glyph_store,
+        };
+
+        shape_textrun(&run);
+        return move run;
     }
-}
 
-trait TextRunMethods {
+    pub pure fn serialize(&self, _cache: @FontCache) -> SendableTextRun {
-    pure fn glyphs(&self) -> &self/GlyphStore;
+        SendableTextRun {
-    fn iter_indivisible_pieces_for_range(&self, range: Range, f: fn&(Range) -> bool);
+            text: copy self.text,
-    // TODO: needs to take box style as argument, or move to TextBox.
+            // TODO: actually serialize a font descriptor thingy
-    // see Gecko's IsTrimmableSpace methods for details.
+            font_descriptor: (),
-    pure fn range_is_trimmable_whitespace(&self, range: Range) -> bool;
+            glyphs: copy self.glyphs,
+        }
+    }
 
-    fn metrics_for_range(&self, range: Range) -> RunMetrics;
-    fn min_width_for_range(&self, range: Range) -> Au;
-    fn iter_natural_lines_for_range(&self, range: Range, f: fn&(Range) -> bool);
-}
-
-impl TextRun : TextRunMethods {
     pure fn glyphs(&self) -> &self/GlyphStore { &self.glyphs }
 
     pure fn range_is_trimmable_whitespace(&self, range: Range) -> bool {
@@ -160,18 +162,6 @@ impl TextRun : TextRunMethods {
     }
 }
  
-fn TextRun(font: @Font, text: ~str) -> TextRun {
-    let glyph_store = GlyphStore(text.len());
-    let run = TextRun {
-        text: move text,
-        font: font,
-        glyphs: move glyph_store,
-    };
-
-    shape_textrun(&run);
-    return move run;
-}
-
 // this test can't run until LayoutContext is removed as an argument
 // to min_width_for_range.
 /*