From 135d11684c4a0f9fb208272750d48dbba06b8d46 Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Mon, 20 May 2013 20:06:08 -0700 Subject: [PATCH 1/7] Add underline to layout and gfx. --- src/support/css/rust-css | 2 +- src/support/netsurfcss/rust-netsurfcss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/support/css/rust-css b/src/support/css/rust-css index 83ab4896148..e3057f02d48 160000 --- a/src/support/css/rust-css +++ b/src/support/css/rust-css @@ -1 +1 @@ -Subproject commit 83ab48961487dc65d8921cc5ea829430e4e46f5e +Subproject commit e3057f02d48bf43856a0c13ad17372647f3b934f diff --git a/src/support/netsurfcss/rust-netsurfcss b/src/support/netsurfcss/rust-netsurfcss index 01a90ed6771..36a651dd830 160000 --- a/src/support/netsurfcss/rust-netsurfcss +++ b/src/support/netsurfcss/rust-netsurfcss @@ -1 +1 @@ -Subproject commit 01a90ed6771560ef4a84c42c0419c3e7e3b21abb +Subproject commit 36a651dd83089c01da888ee9b5fff14437d4bcb8 From cd39d0931f6157b914b322dfe83166926531adda Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Thu, 23 May 2013 18:57:04 -0700 Subject: [PATCH 2/7] Added underline test --- src/test/test_underline.html | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 src/test/test_underline.html diff --git a/src/test/test_underline.html b/src/test/test_underline.html new file mode 100644 index 00000000000..812f86b02da --- /dev/null +++ b/src/test/test_underline.html @@ -0,0 +1,58 @@ + + + +The Book of Mozilla, 11:9 + + + + +

+Mammon slept. And the beast reborn spread over the earth and its numbers +grew legion. And they proclaimed the times and sacrificed crops unto the +fire, with the cunning of foxes. And they built a new world in their own +image as promised by the +sacred words, and spoke + of the beast with their children. Mammon awoke, and lo! it was +naught but a follower. +

+ +

+from The Book of Mozilla, 11:9
(10th Edition) +

+ + + From 1016e273a667d34c7f0d5d07be48d91080d0955d Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Wed, 22 May 2013 13:48:24 -0700 Subject: [PATCH 3/7] Fix rebase issues --- src/components/servo-gfx/display_list.rs | 12 ++++- src/components/servo-gfx/font.rs | 4 +- src/components/servo-gfx/text/text_run.rs | 7 ++- src/components/servo/layout/box.rs | 7 ++- src/components/servo/layout/inline.rs | 15 ++++++- src/test/test_italic_bold.html | 55 +++++++++++++++++++++++ src/test/test_underline.html | 2 - 7 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 src/test/test_italic_bold.html diff --git a/src/components/servo-gfx/display_list.rs b/src/components/servo-gfx/display_list.rs index e79dccea6c2..5fe2c419768 100644 --- a/src/components/servo-gfx/display_list.rs +++ b/src/components/servo-gfx/display_list.rs @@ -9,7 +9,7 @@ use text::SendableTextRun; use clone_arc = std::arc::clone; use geom::Rect; -use geom::Point2D; +use geom::{Point2D, Size2D}; use std::arc::ARC; use servo_net::image::base::Image; use servo_util::range::Range; @@ -56,6 +56,16 @@ pub impl<'self> DisplayItem { let origin = self.d().bounds.origin; let baseline_origin = Point2D(origin.x, origin.y + font.metrics.ascent); font.draw_text_into_context(ctx, new_run, range, baseline_origin, color); + if(new_run.underline){ + let width = self.d().bounds.size.width; + let offset = font.metrics.underline_offset; + let u_size = font.metrics.underline_size; + let u_bounds = Rect( + Point2D(baseline_origin.x, baseline_origin.y), + Size2D(width, u_size) + ); + ctx.draw_solid_color(&u_bounds, color); + } }, &Image(_, ref img) => { debug!("drawing image at %?", self.d().bounds); diff --git a/src/components/servo-gfx/font.rs b/src/components/servo-gfx/font.rs index fd200050de4..6405c0f34cb 100644 --- a/src/components/servo-gfx/font.rs +++ b/src/components/servo-gfx/font.rs @@ -181,11 +181,11 @@ pub impl FontGroup { self.fonts = ~[]; } - fn create_textrun(&self, text: ~str) -> TextRun { + fn create_textrun(&self, text: ~str, underline: bool) -> TextRun { assert!(self.fonts.len() > 0); // TODO(Issue #177): Actually fall back through the FontGroup when a font is unsuitable. - return TextRun::new(self.fonts[0], text); + return TextRun::new(self.fonts[0], text, underline); } } diff --git a/src/components/servo-gfx/text/text_run.rs b/src/components/servo-gfx/text/text_run.rs index d49c5ddd858..3bee023bd35 100644 --- a/src/components/servo-gfx/text/text_run.rs +++ b/src/components/servo-gfx/text/text_run.rs @@ -12,6 +12,7 @@ use servo_util::range::Range; pub struct TextRun { text: ~str, font: @mut Font, + underline: bool, glyphs: GlyphStore, } @@ -19,6 +20,7 @@ pub struct TextRun { pub struct SendableTextRun { text: ~str, font: FontDescriptor, + underline: bool, priv glyphs: GlyphStore, } @@ -32,13 +34,14 @@ impl SendableTextRun { TextRun { text: copy self.text, font: font, + underline: copy self.underline, glyphs: copy self.glyphs } } } pub impl<'self> TextRun { - fn new(font: @mut Font, text: ~str) -> TextRun { + fn new(font: @mut Font, text: ~str, underline: bool) -> TextRun { let mut glyph_store = GlyphStore::new(str::char_len(text)); TextRun::compute_potential_breaks(text, &mut glyph_store); font.shape_text(text, &mut glyph_store); @@ -46,6 +49,7 @@ pub impl<'self> TextRun { let run = TextRun { text: text, font: font, + underline: underline, glyphs: glyph_store, }; return run; @@ -101,6 +105,7 @@ pub impl<'self> TextRun { SendableTextRun { text: copy self.text, font: self.font.get_descriptor(), + underline: copy self.underline, glyphs: copy self.glyphs, } } diff --git a/src/components/servo/layout/box.rs b/src/components/servo/layout/box.rs index 129b79d1834..13112e53b08 100644 --- a/src/components/servo/layout/box.rs +++ b/src/components/servo/layout/box.rs @@ -26,7 +26,7 @@ use newcss::units::{Cursive, Em, Fantasy, Length, Monospace, Pt, Px, SansSerif, use newcss::values::{CSSBorderWidthLength, CSSBorderWidthMedium}; use newcss::values::{CSSFontFamilyFamilyName, CSSFontFamilyGenericFamily}; use newcss::values::{CSSFontSizeLength, CSSFontStyleItalic, CSSFontStyleNormal}; -use newcss::values::{CSSFontStyleOblique, CSSTextAlign}; +use newcss::values::{CSSFontStyleOblique, CSSTextAlign, CSSTextDecoration}; use servo_net::image::holder::ImageHolder; use servo_net::local_image_cache::LocalImageCache; use servo_util::range::*; @@ -759,6 +759,11 @@ pub impl RenderBox { fn text_align(&self) -> CSSTextAlign { self.nearest_ancestor_element().style().text_align() } + + /// Returns the text decoration of the computed style of the nearest `Element` node + fn text_decoration(&self) -> CSSTextDecoration { + self.nearest_ancestor_element().style().text_decoration() + } } impl DebugMethods for RenderBox { diff --git a/src/components/servo/layout/inline.rs b/src/components/servo/layout/inline.rs index b1b2c495029..89d9209319a 100644 --- a/src/components/servo/layout/inline.rs +++ b/src/components/servo/layout/inline.rs @@ -21,6 +21,8 @@ use gfx::text::text_run::TextRun; use gfx::text::util::*; use newcss::values::{CSSTextAlignCenter, CSSTextAlignJustify, CSSTextAlignLeft}; use newcss::values::{CSSTextAlignRight}; +use newcss::values::CSSTextDecorationUnderline; +use newcss::values::CSSTextDecoration; use servo_util::range::Range; use std::deque::Deque; @@ -243,6 +245,13 @@ impl TextRunScanner { let inline = &mut *flow.inline(); let in_boxes = &inline.boxes; + fn hasUnderline(decoration: CSSTextDecoration) -> bool{ + match decoration { + CSSTextDecorationUnderline => true, + _ => false + } + } + assert!(self.clump.length() > 0); debug!("TextRunScanner: flushing boxes in range=%?", self.clump); @@ -264,6 +273,7 @@ impl TextRunScanner { let old_box = in_boxes[self.clump.begin()]; let text = old_box.raw_text(); let font_style = old_box.font_style(); + let underline = hasUnderline(old_box.text_decoration()); // TODO(#115): Use the actual CSS `white-space` property of the relevant style. let compression = CompressWhitespaceNewline; @@ -274,7 +284,7 @@ impl TextRunScanner { // 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); + let run = @fontgroup.create_textrun(transformed_text, underline); debug!("TextRunScanner: pushing single text box in range: %?", self.clump); let new_box = do old_box.with_imm_base |old_box_base| { @@ -316,12 +326,13 @@ impl TextRunScanner { // 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 underline = hasUnderline(in_boxes[self.clump.begin()].text_decoration()); // TextRuns contain a cycle which is usually resolved by the teardown // sequence. If no clump takes ownership, however, it will leak. let clump = self.clump; let run = if clump.length() != 0 { - Some(@TextRun::new(fontgroup.fonts[0], run_str)) + Some(@TextRun::new(fontgroup.fonts[0], run_str, underline)) } else { None }; diff --git a/src/test/test_italic_bold.html b/src/test/test_italic_bold.html new file mode 100644 index 00000000000..6db60c6afcb --- /dev/null +++ b/src/test/test_italic_bold.html @@ -0,0 +1,55 @@ + + + +The Book of Mozilla, 11:9 + + + + +

+Mammon slept. And the beast reborn spread over the earth and its numbers +grew legion. And they proclaimed the times and sacrificed crops unto the +fire, with the cunning of foxes. And they built a new world in their own +image as promised by the +sacred words, and spoke + of the beast with their children. Mammon awoke, and lo! it was +naught but a follower. +

+ +

+from The Book of Mozilla, 11:9
(10th Edition) +

+ + + diff --git a/src/test/test_underline.html b/src/test/test_underline.html index 812f86b02da..eeafdcca8c4 100644 --- a/src/test/test_underline.html +++ b/src/test/test_underline.html @@ -18,8 +18,6 @@ html { text-decoration: underline; } -* { -} #from { font-size: 1.95em; From d4b837bba69df34c40ea8020be5ebc1327d67ada Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Thu, 23 May 2013 17:11:04 -0700 Subject: [PATCH 4/7] Clean up code a little --- src/components/servo-gfx/display_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/servo-gfx/display_list.rs b/src/components/servo-gfx/display_list.rs index 5fe2c419768..afda98af08e 100644 --- a/src/components/servo-gfx/display_list.rs +++ b/src/components/servo-gfx/display_list.rs @@ -57,8 +57,8 @@ pub impl<'self> DisplayItem { let baseline_origin = Point2D(origin.x, origin.y + font.metrics.ascent); font.draw_text_into_context(ctx, new_run, range, baseline_origin, color); if(new_run.underline){ + //TODO: Use the font metrics to properly position the underline bar let width = self.d().bounds.size.width; - let offset = font.metrics.underline_offset; let u_size = font.metrics.underline_size; let u_bounds = Rect( Point2D(baseline_origin.x, baseline_origin.y), From e45ae156408d450bdba4e8e5ee8c814219c26a70 Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Fri, 24 May 2013 10:34:16 -0700 Subject: [PATCH 5/7] Fix style issues --- src/components/servo-gfx/text/text_run.rs | 4 ++-- src/components/servo/layout/inline.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/servo-gfx/text/text_run.rs b/src/components/servo-gfx/text/text_run.rs index 3bee023bd35..3778e4353db 100644 --- a/src/components/servo-gfx/text/text_run.rs +++ b/src/components/servo-gfx/text/text_run.rs @@ -34,7 +34,7 @@ impl SendableTextRun { TextRun { text: copy self.text, font: font, - underline: copy self.underline, + underline: self.underline, glyphs: copy self.glyphs } } @@ -105,7 +105,7 @@ pub impl<'self> TextRun { SendableTextRun { text: copy self.text, font: self.font.get_descriptor(), - underline: copy self.underline, + underline: self.underline, glyphs: copy self.glyphs, } } diff --git a/src/components/servo/layout/inline.rs b/src/components/servo/layout/inline.rs index 89d9209319a..cc69c0dc31d 100644 --- a/src/components/servo/layout/inline.rs +++ b/src/components/servo/layout/inline.rs @@ -245,7 +245,7 @@ impl TextRunScanner { let inline = &mut *flow.inline(); let in_boxes = &inline.boxes; - fn hasUnderline(decoration: CSSTextDecoration) -> bool{ + fn has_underline(decoration: CSSTextDecoration) -> bool{ match decoration { CSSTextDecorationUnderline => true, _ => false @@ -273,7 +273,7 @@ impl TextRunScanner { let old_box = in_boxes[self.clump.begin()]; let text = old_box.raw_text(); let font_style = old_box.font_style(); - let underline = hasUnderline(old_box.text_decoration()); + let underline = has_underline(old_box.text_decoration()); // TODO(#115): Use the actual CSS `white-space` property of the relevant style. let compression = CompressWhitespaceNewline; @@ -326,7 +326,7 @@ impl TextRunScanner { // 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 underline = hasUnderline(in_boxes[self.clump.begin()].text_decoration()); + let underline = has_underline(in_boxes[self.clump.begin()].text_decoration()); // TextRuns contain a cycle which is usually resolved by the teardown // sequence. If no clump takes ownership, however, it will leak. From 656c741f21982064b4ab8c66dd0b8356ef3fd4bd Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Fri, 24 May 2013 14:39:03 -0700 Subject: [PATCH 6/7] Better propagation of text-decoration, though still not perfect. Also, boxes now cannot be merged if they have different text-decorations. --- src/components/servo/layout/box.rs | 44 +++++++++++++++++++++++-- src/test/test_underline.html | 4 +++ src/test/test_underline_helloworld.html | 17 ++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 src/test/test_underline_helloworld.html diff --git a/src/components/servo/layout/box.rs b/src/components/servo/layout/box.rs index 13112e53b08..13c76b79bcb 100644 --- a/src/components/servo/layout/box.rs +++ b/src/components/servo/layout/box.rs @@ -27,6 +27,8 @@ use newcss::values::{CSSBorderWidthLength, CSSBorderWidthMedium}; use newcss::values::{CSSFontFamilyFamilyName, CSSFontFamilyGenericFamily}; use newcss::values::{CSSFontSizeLength, CSSFontStyleItalic, CSSFontStyleNormal}; use newcss::values::{CSSFontStyleOblique, CSSTextAlign, CSSTextDecoration}; +use newcss::values::{CSSTextDecorationNone, CSSFloatNone, CSSPositionStatic}; +use newcss::values::{CSSDisplayInlineBlock, CSSDisplayInlineTable}; use servo_net::image::holder::ImageHolder; use servo_net::local_image_cache::LocalImageCache; use servo_util::range::*; @@ -256,7 +258,7 @@ pub impl RenderBox { fn can_merge_with_box(&self, other: RenderBox) -> bool { match (self, &other) { (&UnscannedTextRenderBoxClass(*), &UnscannedTextRenderBoxClass(*)) => { - self.font_style() == other.font_style() + self.font_style() == other.font_style() && self.text_decoration() == other.text_decoration() }, (&TextRenderBoxClass(text_box_a), &TextRenderBoxClass(text_box_b)) => { managed::ptr_eq(text_box_a.text_data.run, text_box_b.text_data.run) @@ -762,7 +764,45 @@ pub impl RenderBox { /// Returns the text decoration of the computed style of the nearest `Element` node fn text_decoration(&self) -> CSSTextDecoration { - self.nearest_ancestor_element().style().text_decoration() + /// Computes the propagated value of text-decoration, as specified in CSS 2.1 § 16.3.1 + /// TODO: make sure this works with anonymous box generation. + /// FIXME: this really needs to be done in rust-css. Doing it here means there is no way + /// of determining whether text-decoration was set to none or whether it defaulted to + /// none. + fn get_propagated_text_decoration(element: AbstractNode) -> CSSTextDecoration { + //Skip over non-element nodes in the DOM + if(!element.is_element()){ + return match element.parent_node() { + None => CSSTextDecorationNone, + Some(parent) => get_propagated_text_decoration(parent), + }; + } + + //FIXME: is the root param on display() important? + let display_in_flow = match element.style().display(false) { + CSSDisplayInlineTable | CSSDisplayInlineBlock => false, + _ => true, + }; + + let position = element.style().position(); + let float = element.style().float(); + + let in_flow = (position == CSSPositionStatic) && (float == CSSFloatNone) && + display_in_flow; + + let text_decoration = element.style().text_decoration(); + + if(text_decoration == CSSTextDecorationNone && in_flow){ + match element.parent_node() { + None => CSSTextDecorationNone, + Some(parent) => get_propagated_text_decoration(parent), + } + } + else { + text_decoration + } + } + get_propagated_text_decoration(self.nearest_ancestor_element()) } } diff --git a/src/test/test_underline.html b/src/test/test_underline.html index eeafdcca8c4..05db4e01358 100644 --- a/src/test/test_underline.html +++ b/src/test/test_underline.html @@ -34,6 +34,10 @@ a { text-decoration: none; color: white; } + +strong{ + text-decoration: underline; +} diff --git a/src/test/test_underline_helloworld.html b/src/test/test_underline_helloworld.html new file mode 100644 index 00000000000..e04408c09c9 --- /dev/null +++ b/src/test/test_underline_helloworld.html @@ -0,0 +1,17 @@ + + + +The Book of Mozilla, 11:9 + + + + +
+
Hello
+ +
World
+
+ + From 457008e1ee478269c9ec55c13a0fb924061e989e Mon Sep 17 00:00:00 2001 From: Eric Atkinson Date: Fri, 24 May 2013 14:57:01 -0700 Subject: [PATCH 7/7] Removed unnecessary FIXME --- src/components/servo/layout/box.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/servo/layout/box.rs b/src/components/servo/layout/box.rs index 13c76b79bcb..2c84ddc625e 100644 --- a/src/components/servo/layout/box.rs +++ b/src/components/servo/layout/box.rs @@ -766,9 +766,6 @@ pub impl RenderBox { fn text_decoration(&self) -> CSSTextDecoration { /// Computes the propagated value of text-decoration, as specified in CSS 2.1 § 16.3.1 /// TODO: make sure this works with anonymous box generation. - /// FIXME: this really needs to be done in rust-css. Doing it here means there is no way - /// of determining whether text-decoration was set to none or whether it defaulted to - /// none. fn get_propagated_text_decoration(element: AbstractNode) -> CSSTextDecoration { //Skip over non-element nodes in the DOM if(!element.is_element()){