From 17552aae37e220c6d234425c63233a21f22c16b8 Mon Sep 17 00:00:00 2001 From: "Brian J. Burg" Date: Thu, 6 Sep 2012 16:37:12 -0700 Subject: [PATCH] Fill in remaining DisplayType values, replace uses, and make box-building return and option'd box since display:none nodes generate no boxes. --- src/servo/css/parser.rs | 4 +- src/servo/css/parser_util.rs | 15 +++--- src/servo/css/styles.rs | 16 +++---- src/servo/css/values.rs | 18 ++++++-- src/servo/layout/box_builder.rs | 81 +++++++++++++++++---------------- src/servo/layout/layout_task.rs | 16 +++++-- 6 files changed, 87 insertions(+), 63 deletions(-) diff --git a/src/servo/css/parser.rs b/src/servo/css/parser.rs index 9f2dfcde4bd..2e02d154306 100644 --- a/src/servo/css/parser.rs +++ b/src/servo/css/parser.rs @@ -3,8 +3,8 @@ // TODO: fail according to the css spec instead of failing when things // are not as expected -use css::values::{DisInline, DisBlock, DisNone, Display, TextColor, BackgroundColor, FontSize, - Height, Width, StyleDeclaration}; +use css::values::{TextColor, BackgroundColor, FontSize, Height, Width, + Display, StyleDeclaration}; // Disambiguate parsed Selector, Rule values from tokens use css = css::values; use tok = lexer; diff --git a/src/servo/css/parser_util.rs b/src/servo/css/parser_util.rs index 3806f7b34a8..0d45f001e23 100644 --- a/src/servo/css/parser_util.rs +++ b/src/servo/css/parser_util.rs @@ -1,9 +1,10 @@ #[doc = "Helper functions to parse values of specific attributes."] -import css::values::*; -import str::{pop_char, from_chars}; -import float::from_str; -import option::map; +use css::values::{DisplayType, Inline, Block, DisplayNone}; +use css::values::{Unit, Pt, Mm, Px, Percent, Auto}; +use str::{pop_char, from_chars}; +use float::from_str; +use option::map; export parse_font_size; export parse_size; @@ -53,9 +54,9 @@ fn parse_size(str : ~str) -> Option { fn parse_display_type(str : ~str) -> Option { match str { - ~"inline" => Some(DisInline), - ~"block" => Some(DisBlock), - ~"none" => Some(DisNone), + ~"inline" => Some(Inline), + ~"block" => Some(Block), + ~"none" => Some(DisplayNone), _ => { #debug["Recieved unknown display value '%s'", str]; None } } } diff --git a/src/servo/css/styles.rs b/src/servo/css/styles.rs index 64b8757746c..63368e3d522 100644 --- a/src/servo/css/styles.rs +++ b/src/servo/css/styles.rs @@ -2,7 +2,7 @@ import std::arc::{ARC, get, clone}; -import css::values::{DisplayType, DisBlock, DisInline, DisNone, Unit, Auto}; +import css::values::{DisplayType, DisplayNone, Inline, Block, Unit, Auto}; import css::values::Stylesheet; import dom::base::{HTMLDivElement, HTMLHeadElement, HTMLImageElement, UnknownElement, HTMLScriptElement}; import dom::base::{Comment, Doctype, Element, Node, NodeKind, Text}; @@ -37,17 +37,17 @@ impl NodeKind : DefaultStyleMethods { fn default_display_type() -> DisplayType { match self { - Text(*) => { DisInline } + Text(*) => { Inline } Element(element) => { match *element.kind { - HTMLDivElement => DisBlock, - HTMLHeadElement => DisNone, - HTMLImageElement(*) => DisInline, - HTMLScriptElement => DisNone, - UnknownElement => DisInline, + HTMLDivElement => Block, + HTMLHeadElement => DisplayNone, + HTMLImageElement(*) => Inline, + HTMLScriptElement => DisplayNone, + UnknownElement => Inline, } }, - Comment(*) | Doctype(*) => DisNone + Comment(*) | Doctype(*) => DisplayNone } } diff --git a/src/servo/css/values.rs b/src/servo/css/values.rs index c391a054ab6..98d79a84864 100644 --- a/src/servo/css/values.rs +++ b/src/servo/css/values.rs @@ -7,9 +7,21 @@ import util::color::Color; "] enum DisplayType { - DisBlock, - DisInline, - DisNone + Inline, + Block, + ListItem, + InlineBlock, + Table, + InlineTable, + TableRowGroup, + TableHeaderGroup, + TableFooterGroup, + TableRow, + TableColumnGroup, + TableColumn, + TableCell, + TableCaption, + DisplayNone } enum Unit { diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index b98aa1f122c..a2d26bb16a4 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -1,6 +1,6 @@ #[doc="Creates CSS boxes from a DOM."] -import css::values::{DisplayType, DisBlock, DisInline, DisNone}; +import css::values::{DisplayType, Block, Inline, DisplayNone}; import dom::base::{ElementData, HTMLDivElement, HTMLImageElement, Element, Text, Node}; import gfx::geometry::zero_size_au; import layout::base::{Appearance, BTree, BlockBox, Box, BoxKind, InlineBox, IntrinsicBox, NTree}; @@ -43,17 +43,20 @@ impl ctxt { // Create boxes for the child. Get its primary box. let kid_box = kid.construct_boxes(); + if (kid_box.is_none()) { + again; + } // Determine the child's display. let disp = kid.get_specified_style().display_type; - if disp != Some(DisInline) { + if disp != Some(Inline) { self.finish_anonymous_box_if_necessary(); } // Add the child's box to the current enclosing box or the current anonymous box. match kid.get_specified_style().display_type { - Some(DisBlock) => BTree.add_child(self.parent_box, kid_box), - Some(DisInline) => { + Some(Block) => BTree.add_child(self.parent_box, kid_box.get()), + Some(Inline) => { let anon_box = match self.anon_box { None => { // @@ -70,9 +73,9 @@ impl ctxt { } Some(b) => b }; - BTree.add_child(anon_box, kid_box); + BTree.add_child(anon_box, kid_box.get()); } - Some(DisNone) => { + Some(DisplayNone) => { // Nothing to do. } _ => { //hack for now @@ -93,21 +96,21 @@ impl ctxt { // Determine the child's display. let disp = kid.get_specified_style().display_type; - if disp != Some(DisInline) { + if disp != Some(Inline) { // TODO } // Add the child's box to the current enclosing box. match kid.get_specified_style().display_type { - Some(DisBlock) => { + Some(Block) => { // TODO #warn("TODO: non-inline display found inside inline box"); - BTree.add_child(self.parent_box, kid_box); + BTree.add_child(self.parent_box, kid_box.get()); } - Some(DisInline) => { - BTree.add_child(self.parent_box, kid_box); + Some(Inline) => { + BTree.add_child(self.parent_box, kid_box.get()); } - Some(DisNone) => { + Some(DisplayNone) => { // Nothing to do. } _ => { //hack for now @@ -122,9 +125,9 @@ impl ctxt { self.parent_node.dump(); match self.parent_node.get_specified_style().display_type { - Some(DisBlock) => self.construct_boxes_for_block_children(), - Some(DisInline) => self.construct_boxes_for_inline_children(), - Some(DisNone) => { /* Nothing to do. */ } + Some(Block) => self.construct_boxes_for_block_children(), + Some(Inline) => self.construct_boxes_for_inline_children(), + Some(DisplayNone) => { /* Nothing to do. */ } _ => { //hack for now } } @@ -147,7 +150,7 @@ impl ctxt { } trait PrivBoxBuilder { - fn determine_box_kind() -> BoxKind; + fn determine_box_kind() -> Option; } impl Node : PrivBoxBuilder { @@ -155,18 +158,16 @@ impl Node : PrivBoxBuilder { Determines the kind of box that this node needs. Also, for images, computes the intrinsic size. "] - fn determine_box_kind() -> BoxKind { + fn determine_box_kind() -> Option { match self.read(|n| copy n.kind) { - ~Text(string) => TextBoxKind(@TextBox(copy string)), + ~Text(string) => Some(TextBoxKind(@TextBox(copy string))), ~Element(element) => { match (copy *element.kind, self.get_specified_style().display_type) { - (HTMLImageElement({size}), _) => IntrinsicBox(@size), - (_, Some(DisBlock)) => BlockBox, - (_, Some(DisInline)) => InlineBox, - (_, Some(DisNone)) => { - // TODO: don't have a box here at all? - IntrinsicBox(@zero_size_au()) - } + (HTMLImageElement({size}), _) => Some(IntrinsicBox(@size)), + (_, Some(Block)) => Some(BlockBox), + (_, Some(Inline)) => Some(InlineBox), + (_, Some(DisplayNone)) => None, + (_, Some(_)) => Some(InlineBox), (_, None) => { fail ~"The specified display style should be a default instead of none" } @@ -178,24 +179,28 @@ impl Node : PrivBoxBuilder { } trait BoxBuilder { - fn construct_boxes() -> @Box; + fn construct_boxes() -> Option<@Box>; } impl Node : BoxBuilder { #[doc="Creates boxes for this node. This is the entry point."] - fn construct_boxes() -> @Box { - let box_kind = self.determine_box_kind(); - let my_box = @Box(self, box_kind); - match box_kind { - BlockBox | InlineBox => { - let cx = create_context(self, my_box); - cx.construct_boxes_for_children(); - } - _ => { - // Nothing to do. - } + fn construct_boxes() -> Option<@Box> { + match self.determine_box_kind() { + None => None, + Some(kind) => { + let my_box = @Box(self, kind); + match kind { + BlockBox | InlineBox => { + let cx = create_context(self, my_box); + cx.construct_boxes_for_children(); + } + _ => { + // Nothing to do. + } + } + Some(my_box) + } } - return my_box; } } diff --git a/src/servo/layout/layout_task.rs b/src/servo/layout/layout_task.rs index db6744b7218..941662229aa 100644 --- a/src/servo/layout/layout_task.rs +++ b/src/servo/layout/layout_task.rs @@ -10,6 +10,7 @@ import css::values::Stylesheet; import gfx::geometry::px_to_au; import gfx::render_task; import render_task::RenderTask; +import layout::base::Box; import resource::image_cache_task::ImageCacheTask; import std::net::url::Url; import css::resolve::apply::apply_style; @@ -48,16 +49,21 @@ fn LayoutTask(render_task: RenderTask, image_cache_task: ImageCacheTask) -> Layo layout_data_refs += node.initialize_style_for_subtree(); node.recompute_style_for_subtree(styles); - let this_box = node.construct_boxes(); - this_box.dump(); + let root_box: @Box; + match node.construct_boxes() { + None => fail ~"Root node should always exist; did it get 'display: none' somehow?", + Some(root) => root_box = root + } + + root_box.dump(); let reflow: fn~() = || event_chan.send(ReflowEvent); - apply_style(this_box, &doc_url, image_cache_task, reflow); + apply_style(root_box, &doc_url, image_cache_task, reflow); - this_box.reflow_subtree(px_to_au(800)); + root_box.reflow_subtree(px_to_au(800)); - let dlist = build_display_list(this_box); + let dlist = build_display_list(root_box); render_task.send(render_task::RenderMsg(dlist)); } }