Revert "auto merge of #2584 : pcwalton/servo/binary-search, r=SimonSa…

…pin"

This reverts commit d8483d2, reversing
changes made to 3dedbd2.

Wikipedia crashes on linux after this change by attempting to index bogus glyphs.

src/components/gfx/text/glyph.rs

@@ -270,7 +270,7 @@ impl DetailedGlyph {
     }
 }
 
-#[deriving(Eq, Clone, TotalEq, TotalOrd)]
+#[deriving(Eq, Clone)]
 struct DetailedGlyphRecord {
     // source string offset/GlyphEntry offset in the TextRun
     entry_offset: CharIndex,

src/components/gfx/text/text_run.rs

@@ -5,70 +5,50 @@
 use font::{Font, FontDescriptor, RunMetrics, FontStyle, FontMetrics};
 use servo_util::geometry::Au;
 use servo_util::range::Range;
-use servo_util::vec::{Comparator, FullBinarySearchMethods};
 use std::slice::Items;
 use style::computed_values::text_decoration;
 use sync::Arc;
 use text::glyph::{CharIndex, GlyphStore};
 
-/// A single "paragraph" of text in one font size and style.
+/// A text run.
 #[deriving(Clone)]
 pub struct TextRun {
     pub text: Arc<String>,
     pub font_descriptor: FontDescriptor,
     pub font_metrics: FontMetrics,
     pub font_style: FontStyle,
     pub decoration: text_decoration::T,
-    /// The glyph runs that make up this text run.
-    pub glyphs: Arc<Vec<GlyphRun>>,
-}
-
-/// A single series of glyphs within a text run.
-#[deriving(Clone)]
-pub struct GlyphRun {
-    /// The glyphs.
-    glyph_store: Arc<GlyphStore>,
-    /// The range of characters in the containing run.
-    range: Range<CharIndex>,
+    // An Arc pointing to a Vec of Arcs?! Wat.
+    pub glyphs: Arc<Vec<Arc<GlyphStore>>>,
 }
 
 pub struct SliceIterator<'a> {
-    glyph_iter: Items<'a, GlyphRun>,
+    glyph_iter: Items<'a, Arc<GlyphStore>>,
     range:      Range<CharIndex>,
-}
-
-struct CharIndexComparator;
-
-impl Comparator<CharIndex,GlyphRun> for CharIndexComparator {
-    fn compare(&self, key: &CharIndex, value: &GlyphRun) -> Ordering {
-        if *key < value.range.begin() {
-            Less
-        } else if *key >= value.range.end() {
-            Greater
-        } else {
-            Equal
-        }
-    }
+    offset:     CharIndex,
 }
 
 impl<'a> Iterator<(&'a GlyphStore, CharIndex, Range<CharIndex>)> for SliceIterator<'a> {
     // inline(always) due to the inefficient rt failures messing up inline heuristics, I think.
     #[inline(always)]
     fn next(&mut self) -> Option<(&'a GlyphStore, CharIndex, Range<CharIndex>)> {
-        let slice_glyphs = self.glyph_iter.next();
-        if slice_glyphs.is_none() {
-            return None;
-        }
-        let slice_glyphs = slice_glyphs.unwrap();
+        loop {
+            let slice_glyphs = self.glyph_iter.next();
+            if slice_glyphs.is_none() {
+                return None;
+            }
+            let slice_glyphs = slice_glyphs.unwrap();
 
-        let mut char_range = self.range.intersect(&slice_glyphs.range);
-        let slice_range_begin = slice_glyphs.range.begin();
-        char_range.shift_by(-slice_range_begin);
-        if !char_range.is_empty() {
-            return Some((&*slice_glyphs.glyph_store, slice_range_begin, char_range))
-        }
+            let slice_range = Range::new(self.offset, slice_glyphs.char_len());
+            let mut char_range = self.range.intersect(&slice_range);
+            char_range.shift_by(-self.offset);
 
-        return None;
+            let old_offset = self.offset;
+            self.offset = self.offset + slice_glyphs.char_len();
+            if !char_range.is_empty() {
+                return Some((&**slice_glyphs, old_offset, char_range))
+            }
+        }
     }
 }
 
@@ -131,13 +111,13 @@ impl<'a> TextRun {
         return run;
     }
 
-    pub fn break_and_shape(font: &mut Font, text: &str) -> Vec<GlyphRun> {
+    pub fn break_and_shape(font: &mut Font, text: &str) -> Vec<Arc<GlyphStore>> {
         // TODO(Issue #230): do a better job. See Gecko's LineBreaker.
 
         let mut glyphs = vec!();
-        let (mut byte_i, mut char_i) = (0u, CharIndex(0));
+        let mut byte_i = 0u;
         let mut cur_slice_is_whitespace = false;
-        let (mut byte_last_boundary, mut char_last_boundary) = (0, CharIndex(0));
+        let mut byte_last_boundary = 0;
         while byte_i < text.len() {
             let range = text.char_range_at(byte_i);
             let ch = range.ch;
@@ -168,40 +148,31 @@ impl<'a> TextRun {
                 let slice = text.slice(byte_last_boundary, byte_i).to_string();
                 debug!("creating glyph store for slice {} (ws? {}), {} - {} in run {}",
                         slice, !cur_slice_is_whitespace, byte_last_boundary, byte_i, text);
-                glyphs.push(GlyphRun {
-                    glyph_store: font.shape_text(slice, !cur_slice_is_whitespace),
-                    range: Range::new(char_last_boundary, char_i - char_last_boundary),
-                });
+                glyphs.push(font.shape_text(slice, !cur_slice_is_whitespace));
                 byte_last_boundary = byte_i;
-                char_last_boundary = char_i;
             }
 
             byte_i = next;
-            char_i = char_i + CharIndex(1);
         }
 
         // Create a glyph store for the final slice if it's nonempty.
         if byte_i > byte_last_boundary {
             let slice = text.slice_from(byte_last_boundary).to_string();
             debug!("creating glyph store for final slice {} (ws? {}), {} - {} in run {}",
                 slice, cur_slice_is_whitespace, byte_last_boundary, text.len(), text);
-            glyphs.push(GlyphRun {
-                glyph_store: font.shape_text(slice, cur_slice_is_whitespace),
-                range: Range::new(char_last_boundary, char_i - char_last_boundary),
-            });
+            glyphs.push(font.shape_text(slice, cur_slice_is_whitespace));
         }
 
         glyphs
     }
 
     pub fn char_len(&self) -> CharIndex {
-        match self.glyphs.last() {
-            None => CharIndex(0),
-            Some(ref glyph_run) => glyph_run.range.end(),
-        }
+        self.glyphs.iter().fold(CharIndex(0), |len, slice_glyphs| {
+            len + slice_glyphs.char_len()
+        })
     }
 
-    pub fn glyphs(&'a self) -> &'a Vec<GlyphRun> {
+    pub fn glyphs(&'a self) -> &'a Vec<Arc<GlyphStore>> {
         &*self.glyphs
     }
 
@@ -251,19 +222,11 @@ impl<'a> TextRun {
         max_piece_width
     }
 
-    /// Returns the index of the first glyph run containing the given character index.
-    fn index_of_first_glyph_run_containing(&self, index: CharIndex) -> Option<uint> {
-        self.glyphs.as_slice().binary_search_index_by(&index, CharIndexComparator)
-    }
-
     pub fn iter_slices_for_range(&'a self, range: &Range<CharIndex>) -> SliceIterator<'a> {
-        let index = match self.index_of_first_glyph_run_containing(range.begin()) {
-            None => self.glyphs.len(),
-            Some(index) => index,
-        };
         SliceIterator {
-            glyph_iter: self.glyphs.slice_from(index).iter(),
+            glyph_iter: self.glyphs.iter(),
             range:      *range,
+            offset:     CharIndex(0),
         }
     }
 

src/components/util/vec.rs

@@ -4,33 +4,17 @@
 
 use std::cmp::{Ord, Eq};
 
-/// FIXME(pcwalton): Workaround for lack of unboxed closures. This is called in
-/// performance-critical code, so a closure is insufficient.
-pub trait Comparator<K,T> {
-    fn compare(&self, key: &K, value: &T) -> Ordering;
-}
-
-pub trait BinarySearchMethods<'a, T: TotalOrd + Ord + Eq> {
+pub trait BinarySearchMethods<'a, T: Ord + Eq> {
     fn binary_search(&self, key: &T) -> Option<&'a T>;
     fn binary_search_index(&self, key: &T) -> Option<uint>;
 }
 
-pub trait FullBinarySearchMethods<T> {
-    fn binary_search_index_by<K,C:Comparator<K,T>>(&self, key: &K, cmp: C) -> Option<uint>;
-}
-
-impl<'a, T: TotalOrd + Ord + Eq> BinarySearchMethods<'a, T> for &'a [T] {
+impl<'a, T: Ord + Eq> BinarySearchMethods<'a, T> for &'a [T] {
     fn binary_search(&self, key: &T) -> Option<&'a T> {
         self.binary_search_index(key).map(|i| &self[i])
     }
 
     fn binary_search_index(&self, key: &T) -> Option<uint> {
-        self.binary_search_index_by(key, DefaultComparator)
-    }
-}
-
-impl<'a, T> FullBinarySearchMethods<T> for &'a [T] {
-    fn binary_search_index_by<K,C:Comparator<K,T>>(&self, key: &K, cmp: C) -> Option<uint> {
         if self.len() == 0 {
             return None;
         }
@@ -43,26 +27,20 @@ impl<'a, T> FullBinarySearchMethods<T> for &'a [T] {
             let mid = ((low as uint) + (high as uint)) >> 1;
             let midv = &self[mid];
 
-            match cmp.compare(key, midv) {
-                Greater => low = (mid as int) + 1,
-                Less => high = (mid as int) - 1,
-                Equal => return Some(mid),
+            if midv < key {
+                low = (mid as int) + 1;
+            } else if midv > key {
+                high = (mid as int) - 1;
+            } else {
+                return Some(mid);
             }
         }
         return None;
     }
 }
 
-struct DefaultComparator;
-
-impl<T:Eq + Ord + TotalOrd> Comparator<T,T> for DefaultComparator {
-    fn compare(&self, key: &T, value: &T) -> Ordering {
-        (*key).cmp(value)
-    }
-}
-
 #[cfg(test)]
-fn test_find_all_elems<T: Eq + Ord + TotalEq + TotalOrd>(arr: &[T]) {
+fn test_find_all_elems<T: Eq + Ord>(arr: &[T]) {
     let mut i = 0;
     while i < arr.len() {
         assert!(test_match(&arr[i], arr.binary_search(&arr[i])));
@@ -71,7 +49,7 @@ fn test_find_all_elems<T: Eq + Ord + TotalEq + TotalOrd>(arr: &[T]) {
 }
 
 #[cfg(test)]
-fn test_miss_all_elems<T: Eq + Ord + TotalEq + TotalOrd>(arr: &[T], misses: &[T]) {
+fn test_miss_all_elems<T: Eq + Ord>(arr: &[T], misses: &[T]) {
     let mut i = 0;
     while i < misses.len() {
         let res = arr.binary_search(&misses[i]);