Convert linebox scanner and other bits to use character indices instead of string byte offsets. Fix some bugs in set_char_break_before, and remove dead inline tests.

src/servo-gfx/font.rs

@@ -416,7 +416,7 @@ pub impl Font : FontMethods {
         let azglyphs = DVec();
         azglyphs.reserve(range.length());
 
-        for run.glyphs.iter_glyphs_for_byte_range(range) |_i, glyph| {
+        for run.glyphs.iter_glyphs_for_char_range(range) |_i, glyph| {
             let glyph_advance = glyph.advance();
             let glyph_offset = glyph.offset().get_default(Au::zero_point());
 

src/servo-gfx/text/glyph.rs

@@ -214,10 +214,12 @@ impl GlyphEntry {
     }
 
     // setter methods
+    #[inline(always)]
     pure fn set_char_is_space() -> GlyphEntry {
         GlyphEntry(self.value | FLAG_CHAR_IS_SPACE)
     }
 
+    #[inline(always)]
     pure fn set_char_is_tab() -> GlyphEntry {
         assert !self.is_simple();
         GlyphEntry(self.value | FLAG_CHAR_IS_TAB)
@@ -229,16 +231,10 @@ impl GlyphEntry {
         GlyphEntry(self.value | FLAG_CHAR_IS_NEWLINE)
     }
 
-    // returns a glyph entry only if the setting had changed.
+    #[inline(always)]
-    pure fn set_can_break_before(e: BreakType) -> Option<GlyphEntry> {
+    pure fn set_can_break_before(e: BreakType) -> GlyphEntry {
-        let flag = break_enum_to_flag(e);
+        let flag = (break_enum_to_flag(e) as u32) << FLAG_CAN_BREAK_SHIFT;
-        let mask = (flag as u32) << FLAG_CAN_BREAK_SHIFT;
+        GlyphEntry(self.value | flag)
-        let toggle = mask ^ (self.value & FLAG_CAN_BREAK_MASK);
-
-        match (toggle as bool) {
-            true  => Some(GlyphEntry(self.value ^ toggle)),
-            false => None
-        }
     }
 
     // helper methods
@@ -599,12 +595,10 @@ impl GlyphStore {
                 }
             }
         }
-
 	return true;
     }
 
-    pure fn iter_glyphs_for_byte_range(range: &const Range, cb: fn&(uint, GlyphInfo/&) -> bool) {
+    pure fn iter_glyphs_for_char_range(range: &const Range, cb: fn&(uint, GlyphInfo/&) -> bool) {
-        warn!("Using deprecated iter_glyphs_for_byte_range API!");
         if range.begin() >= self.entry_buffer.len() {
             error!("iter_glyphs_for_range: range.begin beyond length!");
             return;
@@ -614,10 +608,6 @@ impl GlyphStore {
             return;
         }
 
-        self.iter_glyphs_for_char_range(range, cb)
-    }
-
-    pure fn iter_glyphs_for_char_range(range: &const Range, cb: fn&(uint, GlyphInfo/&) -> bool) {
         for range.eachi |i| {
 	    if !self.iter_glyphs_for_char_index(i, cb) { break; }
 	}
@@ -686,9 +676,6 @@ impl GlyphStore {
     fn set_can_break_before(&mut self, i: uint, t: BreakType) {
         assert i < self.entry_buffer.len();
         let entry = self.entry_buffer[i];
-        match entry.set_can_break_before(t) {
+        self.entry_buffer[i] = entry.set_can_break_before(t);
-            Some(e) => self.entry_buffer[i] = e,
-            None => {}
-        };
     }
 }

src/servo-gfx/text/text_run.rs

@@ -123,8 +123,6 @@ impl TextRun {
     }
 
     fn min_width_for_range(&self, range: &const Range) -> Au {
-        assert range.is_valid_for_string(self.text);
-
         let mut max_piece_width = Au(0);
         for self.iter_indivisible_pieces_for_range(range) |piece_range| {
             let metrics = self.font.measure_text(self, piece_range);
@@ -134,8 +132,6 @@ impl TextRun {
     }
 
     fn iter_natural_lines_for_range(&self, range: &const Range, f: fn(&const Range) -> bool) {
-        assert range.is_valid_for_string(self.text);
-
         let mut clump = Range::new(range.begin(), 0);
         let mut in_clump = false;
 
@@ -147,8 +143,7 @@ impl TextRun {
                 (true, false) => { /* chomp whitespace */ }
                 (true, true)  => {
                     in_clump = false;
-                    // don't include the linebreak 'glyph'
+                    // don't include the linebreak character itself in the clump.
-                    // (we assume there's one GlyphEntry for a newline, and no actual glyphs)
                     if !f(&const clump) { break }
                 }
             }
@@ -162,86 +157,21 @@ impl TextRun {
     }
 
     fn iter_indivisible_pieces_for_range(&self, range: &const Range, f: fn(&const Range) -> bool) {
-        assert range.is_valid_for_string(self.text);
-
         let mut clump = Range::new(range.begin(), 0);
+
         loop {
-            // find next non-whitespace byte index, then clump all whitespace before it.
+            // extend clump to non-break-before characters.
-            match str::find_between(self.text, clump.begin(), range.end(), |c| !char::is_whitespace(c)) {
+            while clump.end() < range.end() 
-                Some(nonws_char_offset) => {
+                && self.glyphs.can_break_before(clump.end()) != BreakTypeNormal {
-                    clump.extend_to(nonws_char_offset);
-                    if !f(&const clump) { break }
-                    clump.reset(clump.end(), 0);
-                },
-                None => {
-                    // nothing left, flush last piece containing only whitespace
-                    if clump.end() < range.end() {
-                        clump.extend_to(range.end());
-                        f(&const clump);
-                        break;
-                    }
-                }
-            };
 
-            // find next whitespace byte index, then clump all non-whitespace before it.
+                clump.extend_by(1);
-            match str::find_between(self.text, clump.begin(), range.end(), |c| char::is_whitespace(c)) {
-                Some(ws_char_offset) => {
-                    clump.extend_to(ws_char_offset);
-                    if !f(&const clump) { break }
-                    clump.reset(clump.end(), 0);
-                }
-                None => {
-                    // nothing left, flush last piece containing only non-whitespaces
-                    if clump.end() < range.end() {
-                        clump.extend_to(range.end());
-                        f(&const clump);
-                        break;
-                    }
-                }
             }
+
+            // now clump.end() is break-before or range.end()
+            if !f(&const clump) || clump.end() == range.end() { break; }
+
+            // now clump includes one break-before character, or starts from range.end()
+            clump.reset(clump.end(), 1);
         }
     }
 }
- 
-// this test can't run until LayoutContext is removed as an argument
-// to min_width_for_range.
-/*
-#[test]
-fn test_calc_min_break_width() {
-
-    fn test_min_width_for_run(text: ~str, width: Au) {
-        let flib = FontCache();
-        let font = flib.get_test_font();
-        let run = TextRun(font, text);
-        run.min_width_for_range(0, text.len())
-    }
-
-    test_min_width_for_run(~"firecracker", au::from_px(84));
-    test_min_width_for_run(~"firecracker yumyum", au::from_px(84));
-    test_min_width_for_run(~"yumyum firecracker", au::from_px(84));
-    test_min_width_for_run(~"yumyum firecracker yumyum", au::from_px(84));
-}
-*/
-
-/*#[test]
-#[ignore]
-fn test_iter_indivisible_pieces() {
-    fn test_pieces(text: ~str, res: ~[~str]) {
-        let flib = FontCache();
-        let font = flib.get_test_font();
-        let run = TextRun::new(font, copy text);
-        let mut slices : ~[~str] = ~[];
-        for run.iter_indivisible_pieces_for_range(Range(0, text.len())) |subrange| {
-            slices.push(str::slice(text, subrange.begin(), subrange.length()));
-        }
-        assert slices == res;
-    }
-
-    test_pieces(~"firecracker yumyum woopwoop", ~[~"firecracker", ~" ", ~"yumyum", ~" ", ~"woopwoop"]);
-    test_pieces(~"firecracker yumyum     ", ~[~"firecracker", ~" ", ~"yumyum", ~"     "]);
-    test_pieces(~"     firecracker yumyum", ~[~"     ", ~"firecracker", ~" ", ~"yumyum"]);
-    test_pieces(~"  ", ~[~"  "]);
-    test_pieces(~"", ~[]);
-}
-
-*/

src/servo/layout/box.rs

@@ -295,7 +295,7 @@ impl RenderBox : RenderBoxMethods {
                 let mut max_line_width: Au = Au(0);
                 for d.run.iter_natural_lines_for_range(&const d.range) |line_range| {
                     let mut line_width: Au = Au(0);
-                    for d.run.glyphs.iter_glyphs_for_byte_range(line_range) |_char_i, glyph| {
+                    for d.run.glyphs.iter_glyphs_for_char_range(line_range) |_char_i, glyph| {
                         line_width += glyph.advance()
                     }
                     max_line_width = Au::max(max_line_width, line_width);