Skip to content
Permalink
Browse files

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.
  • Loading branch information
brson committed Jun 6, 2014
1 parent eae9b94 commit 679baab734db0d7898ef236602c1afaef545880a
Showing with 42 additions and 101 deletions.
  1. +1 −1 src/components/gfx/text/glyph.rs
  2. +31 −68 src/components/gfx/text/text_run.rs
  3. +10 −32 src/components/util/vec.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,
@@ -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),
}
}

@@ -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]);

0 comments on commit 679baab

Please sign in to comment.
You can’t perform that action at this time.