Reapply "Add initial support for the line-height property. Line heigh…

…t is only based on the tallest box in each line and does not factor in the vertical-alignment. Improves whitespace handling by passing the whitespace state between function invocations."

This reverts commit 0bb3fbd.

src/components/gfx/geometry.rs

@@ -112,9 +112,8 @@ pub impl Au {
         Rect(Point2D(z, z), Size2D(z, z))
     }
 
-    // assumes 72 points per inch, and 96 px per inch
     pub fn from_pt(f: float) -> Au {
-        from_px((f / 72f * 96f) as int)
+        from_px(pt_to_px(f) as int)
     }
 
     pub fn from_frac_px(f: float) -> Au {
@@ -125,6 +124,16 @@ pub impl Au {
     pub fn max(x: Au, y: Au) -> Au { if *x > *y { x } else { y } }
 }
 
+// assumes 72 points per inch, and 96 px per inch
+pub fn pt_to_px(f: float) -> float {
+    f / 72f * 96f
+}
+
+// assumes 72 points per inch, and 96 px per inch
+pub fn px_to_pt(f: float) -> float {
+    f / 96f * 72f
+}
+
 pub fn zero_rect() -> Rect<Au> {
     let z = Au(0);
     Rect(Point2D(z, z), Size2D(z, z))

src/components/gfx/platform/linux/font.rs

@@ -219,7 +219,7 @@ impl FontHandleMethods for FontHandle {
             x_height:         geometry::from_pt(0.0), //FIXME
             em_size:          em_size,
             ascent:           ascent,
-            descent:          descent,
+            descent:          -descent, // linux font's seem to use the opposite sign from mac
             max_advance:      max_advance
         }
     }

src/components/gfx/platform/macos/font.rs

@@ -12,7 +12,7 @@ use font::{CSSFontWeight, FontHandleMethods, FontMetrics, FontTableMethods};
 use font::{FontTableTag, FontWeight100, FontWeight200, FontWeight300, FontWeight400};
 use font::{FontWeight500, FontWeight600, FontWeight700, FontWeight800, FontWeight900};
 use font::{FractionalPixel, SpecifiedFontStyle};
-use geometry::Au;
+use geometry::{Au, px_to_pt};
 use platform::macos::font_context::FontContextHandle;
 use text::glyph::GlyphIndex;
 
@@ -157,6 +157,9 @@ impl FontHandleMethods for FontHandle {
         let bounding_rect: CGRect = self.ctfont.bounding_box();
         let ascent = Au::from_pt(self.ctfont.ascent() as float);
         let descent = Au::from_pt(self.ctfont.descent() as float);
+        let em_size = Au::from_frac_px(self.ctfont.pt_size() as float);
+
+        let scale = px_to_pt(self.ctfont.pt_size() as float) / (self.ctfont.ascent() as float + self.ctfont.descent() as float);
 
         let metrics =  FontMetrics {
             underline_size:   Au::from_pt(self.ctfont.underline_thickness() as float),
@@ -168,9 +171,9 @@ impl FontHandleMethods for FontHandle {
             underline_offset: Au::from_pt(self.ctfont.underline_position() as float),
             leading:          Au::from_pt(self.ctfont.leading() as float),
             x_height:         Au::from_pt(self.ctfont.x_height() as float),
-            em_size:          ascent + descent,
-            ascent:           ascent,
-            descent:          descent,
+            em_size:          em_size,
+            ascent:           ascent.scale_by(scale),
+            descent:          descent.scale_by(scale),
             max_advance:      Au::from_pt(bounding_rect.size.width as float)
         };
 

src/components/gfx/text/util.rs

@@ -28,15 +28,15 @@ impl Eq for CompressionMode {
 // 
 // High level TODOs:
 //
-// * Issue #113: consider incoming text state (preceding spaces, arabic, etc)
+// * Issue #113: consider incoming text state (arabic, etc)
 //               and propogate outgoing text state (dual of above) 
 //
 // * Issue #114: record skipped and kept chars for mapping original to new text
 //
 // * Untracked: various edge cases for bidi, CJK, etc.
-pub fn transform_text(text: &str, mode: CompressionMode) -> ~str {
+pub fn transform_text(text: &str, mode: CompressionMode, incoming_whitespace: bool) -> (~str, bool) {
     let mut out_str: ~str = ~"";
-    match mode {
+    let out_whitespace = match mode {
         CompressNone | DiscardNewline => {
             for str::each_char(text) |ch: char| {
                 if is_discardable_char(ch, mode) {
@@ -49,18 +49,14 @@ pub fn transform_text(text: &str, mode: CompressionMode) -> ~str {
                     str::push_char(&mut out_str, ch);
                 }
             }
+            text.len() > 0 && is_in_whitespace(text.char_at_reverse(0), mode)
         },
 
         CompressWhitespace | CompressWhitespaceNewline => {
-            let mut in_whitespace: bool = false;
+            let mut in_whitespace: bool = incoming_whitespace;
             for str::each_char(text) |ch: char| {
                 // TODO: discard newlines between CJK chars
-                let mut next_in_whitespace: bool = match (ch, mode) {
-                    (' ', _)  => true,
-                    ('\t', _) => true,
-                    ('\n', CompressWhitespaceNewline) => true,
-                    (_, _)    => false
-                };
+                let mut next_in_whitespace: bool = is_in_whitespace(ch, mode);
 
                 if !next_in_whitespace {
                     if is_always_discardable_char(ch) {
@@ -82,10 +78,20 @@ pub fn transform_text(text: &str, mode: CompressionMode) -> ~str {
                 // save whitespace context for next char
                 in_whitespace = next_in_whitespace;
             } /* /for str::each_char */
+            in_whitespace
         } 
-    }
+    };
+
+    return (out_str, out_whitespace);
 
-    return out_str;
+    fn is_in_whitespace(ch: char, mode: CompressionMode) -> bool {
+        match (ch, mode) {
+            (' ', _)  => true,
+            ('\t', _) => true,
+            ('\n', CompressWhitespaceNewline) => true,
+            (_, _)    => false
+        }
+    }
 
     fn is_discardable_char(ch: char, mode: CompressionMode) -> bool {
         if is_always_discardable_char(ch) {
@@ -143,7 +149,8 @@ fn test_transform_compress_none() {
     let mode = CompressNone;
 
     for uint::range(0, test_strs.len()) |i| {
-        assert!(transform_text(test_strs[i], mode) == test_strs[i]);
+        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(trimmed_str == test_strs[i])
     }
 }
 
@@ -170,7 +177,8 @@ fn test_transform_discard_newline() {
     let mode = DiscardNewline;
 
     for uint::range(0, test_strs.len()) |i| {
-        assert!(transform_text(test_strs[i], mode) == oracle_strs[i]);
+        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(trimmed_str == oracle_strs[i])
     }
 }
 
@@ -196,13 +204,42 @@ fn test_transform_compress_whitespace() {
     let mode = CompressWhitespace;
 
     for uint::range(0, test_strs.len()) |i| {
-        assert!(transform_text(test_strs[i], mode) == oracle_strs[i]);
+        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(trimmed_str == oracle_strs[i])
+    }
+}
+
+#[test]
+fn test_transform_compress_whitespace_newline() {
+    let  test_strs : ~[~str] = ~[~"  foo bar",
+                                 ~"foo bar  ",
+                                 ~"foo\n bar",
+                                 ~"foo \nbar",
+                                 ~"  foo  bar  \nbaz",
+                                 ~"foo bar baz",
+                                 ~"foobarbaz\n\n"];
+
+    let oracle_strs : ~[~str] = ~[~"foo bar",
+                                 ~"foo bar ",
+                                 ~"foo bar",
+                                 ~"foo bar",
+                                 ~" foo bar baz",
+                                 ~"foo bar baz",
+                                 ~"foobarbaz "];
+
+    assert!(test_strs.len() == oracle_strs.len());
+    let mode = CompressWhitespaceNewline;
+
+    for uint::range(0, test_strs.len()) |i| {
+        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(trimmed_str == oracle_strs[i])
     }
 }
 
 #[test]
 fn test_transform_compress_whitespace_newline() {
     let  test_strs : ~[~str] = ~[~"  foo bar",
+                                 ~"\nfoo bar",
                                  ~"foo bar  ",
                                  ~"foo\n bar",
                                  ~"foo \nbar",
@@ -211,6 +248,7 @@ fn test_transform_compress_whitespace_newline() {
                                  ~"foobarbaz\n\n"];
 
     let oracle_strs : ~[~str] = ~[~" foo bar",
+                                 ~" foo bar",
                                  ~"foo bar ",
                                  ~"foo bar",
                                  ~"foo bar",
@@ -222,6 +260,7 @@ fn test_transform_compress_whitespace_newline() {
     let mode = CompressWhitespaceNewline;
 
     for uint::range(0, test_strs.len()) |i| {
-        assert!(transform_text(test_strs[i], mode) == oracle_strs[i]);
+        (trimmed_str, _out) = transform_text(test_strs[i], mode, false);
+        assert!(trimmed_str == oracle_strs[i])
     }
 }

src/components/main/layout/box.rs

@@ -21,14 +21,14 @@ use gfx::display_list::{DisplayList, ImageDisplayItem, ImageDisplayItemClass};
 use gfx::display_list::{SolidColorDisplayItem, SolidColorDisplayItemClass, TextDisplayItem};
 use gfx::display_list::{TextDisplayItemClass};
 use gfx::font::{FontStyle, FontWeight300};
-use gfx::geometry::Au;
+use gfx::geometry::{Au, pt_to_px};
 use gfx::text::text_run::TextRun;
 use newcss::color::rgb;
 use newcss::complete::CompleteStyle;
 use newcss::units::{Cursive, Em, Fantasy, Monospace, Pt, Px, SansSerif, Serif};
 use newcss::values::{CSSFontFamilyFamilyName, CSSFontFamilyGenericFamily};
 use newcss::values::{CSSFontSizeLength, CSSFontStyleItalic, CSSFontStyleNormal};
-use newcss::values::{CSSFontStyleOblique, CSSTextAlign, CSSTextDecoration};
+use newcss::values::{CSSFontStyleOblique, CSSTextAlign, CSSTextDecoration, CSSLineHeight};
 use newcss::values::{CSSTextDecorationNone, CSSFloatNone, CSSPositionStatic};
 use newcss::values::{CSSDisplayInlineBlock, CSSDisplayInlineTable};
 use script::dom::node::{AbstractNode, LayoutView};
@@ -559,9 +559,6 @@ pub impl RenderBox {
             return;
         }
 
-        // Add the background to the list, if applicable.
-        self.paint_background_if_applicable(list, &absolute_box_bounds);
-
         match *self {
             UnscannedTextRenderBoxClass(*) => fail!(~"Shouldn't see unscanned boxes here."),
             TextRenderBoxClass(text_box) => {
@@ -625,26 +622,35 @@ pub impl RenderBox {
                     ()
                 });
             },
-
             GenericRenderBoxClass(_) => {
-                // FIXME(pcwalton): This is somewhat of an abuse of the logging system.
-                debug!("%?", { 
-                    // Compute the text box bounds and draw a border surrounding them.
+
+                // Add the background to the list, if applicable.
+                self.paint_background_if_applicable(list, &absolute_box_bounds);
+
+                // FIXME(pcwalton): This is a bit of an abuse of the logging infrastructure. We
+                // should have a real `SERVO_DEBUG` system.
+                debug!("%?", {
                     do list.with_mut_ref |list| {
                         let border_display_item = ~BorderDisplayItem {
                             base: BaseDisplayItem {
-                                    bounds: absolute_box_bounds,
-                                    extra: ExtraDisplayListData::new(*self),
+                                bounds: absolute_box_bounds,
+                                extra: ExtraDisplayListData::new(*self),
                             },
                             width: Au::from_px(1),
-                            color: rgb(0, 0, 0).to_gfx_color(),
+                            color: rgb(0, 0, 200).to_gfx_color(),
                         };
                         list.append_item(BorderDisplayItemClass(border_display_item))
                     }
+
+                    ()
                 });
-            }
 
+            },
             ImageRenderBoxClass(image_box) => {
+
+                // Add the background to the list, if applicable.
+                self.paint_background_if_applicable(list, &absolute_box_bounds);
+
                 match image_box.image.get_image() {
                     Some(image) => {
                         debug!("(building display list) building image box");
@@ -708,6 +714,9 @@ pub impl RenderBox {
     fn font_style(&self) -> FontStyle {
         let my_style = self.nearest_ancestor_element().style();
 
+        debug!("(font style) start: %?", self.nearest_ancestor_element().type_id());
+        self.dump();
+
         // FIXME: Too much allocation here.
         let font_families = do my_style.font_family().map |family| {
             match *family {
@@ -723,12 +732,13 @@ pub impl RenderBox {
         debug!("(font style) font families: `%s`", font_families);
 
         let font_size = match my_style.font_size() {
-            CSSFontSizeLength(Px(length)) |
-            CSSFontSizeLength(Pt(length)) |
-            CSSFontSizeLength(Em(length)) => length,
-            _ => 16.0
+            CSSFontSizeLength(Px(length)) => length,
+            CSSFontSizeLength(Pt(length)) => pt_to_px(length),
+            // todo: this is based on a hard coded font size, should be the parent element's font size
+            CSSFontSizeLength(Em(length)) => length * 16f, 
+            _ => 16f // px units
         };
-        debug!("(font style) font size: `%f`", font_size);
+        debug!("(font style) font size: `%fpx`", font_size);
 
         let (italic, oblique) = match my_style.font_style() {
             CSSFontStyleNormal => (false, false),
@@ -751,6 +761,10 @@ pub impl RenderBox {
         self.nearest_ancestor_element().style().text_align()
     }
 
+    fn line_height(&self) -> CSSLineHeight {
+        self.nearest_ancestor_element().style().line_height()
+    }
+
     /// Returns the text decoration of the computed style of the nearest `Element` node
     fn text_decoration(&self) -> CSSTextDecoration {
         /// Computes the propagated value of text-decoration, as specified in CSS 2.1 § 16.3.1