From a351710004758824111fea572bad6d78282ccaea Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Fri, 20 Jun 2014 09:07:11 -0700 Subject: [PATCH] Use iterators to make some logic clearer --- src/components/gfx/text/text_run.rs | 15 ++--- src/components/main/layout/inline.rs | 88 ++++++++++++---------------- 2 files changed, 45 insertions(+), 58 deletions(-) diff --git a/src/components/gfx/text/text_run.rs b/src/components/gfx/text/text_run.rs index 3575686079e1..1f182572ac1a 100644 --- a/src/components/gfx/text/text_run.rs +++ b/src/components/gfx/text/text_run.rs @@ -206,10 +206,9 @@ impl<'a> TextRun { } pub fn range_is_trimmable_whitespace(&self, range: &Range) -> bool { - for (slice_glyphs, _, _) in self.iter_slices_for_range(range) { - if !slice_glyphs.is_whitespace() { return false; } - } - true + self.iter_slices_for_range(range).all(|(slice_glyphs, _, _)| { + slice_glyphs.is_whitespace() + }) } pub fn ascent(&self) -> Au { @@ -242,13 +241,11 @@ impl<'a> TextRun { } pub fn min_width_for_range(&self, range: &Range) -> Au { - let mut max_piece_width = Au(0); debug!("iterating outer range {:?}", range); - for (_, offset, slice_range) in self.iter_slices_for_range(range) { + self.iter_slices_for_range(range).fold(Au(0), |max_piece_width, (_, offset, slice_range)| { debug!("iterated on {:?}[{:?}]", offset, slice_range); - max_piece_width = Au::max(max_piece_width, self.advance_for_range(&slice_range)); - } - max_piece_width + Au::max(max_piece_width, self.advance_for_range(&slice_range)) + }) } /// Returns the index of the first glyph run containing the given character index. diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index f1911b472262..5f8158a0bea3 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -639,13 +639,9 @@ pub struct FragmentIterator<'a> { impl<'a> Iterator<(&'a Fragment, InlineFragmentContext<'a>)> for FragmentIterator<'a> { #[inline] fn next(&mut self) -> Option<(&'a Fragment, InlineFragmentContext<'a>)> { - match self.iter.next() { - None => None, - Some((i, fragment)) => Some(( - fragment, - InlineFragmentContext::new(self.ranges, FragmentIndex(i as int)), - )), - } + self.iter.next().map(|(i, fragment)| { + (fragment, InlineFragmentContext::new(self.ranges, FragmentIndex(i as int))) + }) } } @@ -658,13 +654,9 @@ pub struct MutFragmentIterator<'a> { impl<'a> Iterator<(&'a mut Fragment, InlineFragmentContext<'a>)> for MutFragmentIterator<'a> { #[inline] fn next(&mut self) -> Option<(&'a mut Fragment, InlineFragmentContext<'a>)> { - match self.iter.next() { - None => None, - Some((i, fragment)) => Some(( - fragment, - InlineFragmentContext::new(self.ranges, FragmentIndex(i as int)), - )), - } + self.iter.next().map(|(i, fragment)| { + (fragment, InlineFragmentContext::new(self.ranges, FragmentIndex(i as int))) + }) } } @@ -868,22 +860,15 @@ impl InlineFragments { /// Strips ignorable whitespace from the start of a list of fragments. pub fn strip_ignorable_whitespace_from_start(&mut self) { - if self.is_empty() { - return; - } - - // FIXME(#2264, pcwalton): This is slow because vector shift is broken. :( - let mut found_nonwhitespace = false; - let mut new_fragments = Vec::new(); - for fragment in self.fragments.iter() { - if !found_nonwhitespace && fragment.is_whitespace_only() { - debug!("stripping ignorable whitespace from start"); - continue; - } + if self.is_empty() { return }; // Fast path - found_nonwhitespace = true; - new_fragments.push(fragment.clone()) - } + let new_fragments = mem::replace(&mut self.fragments, vec![]) + .move_iter() + .skip_while(|fragment| { + if fragment.is_whitespace_only() { + debug!("stripping ignorable whitespace from start"); true + } else { false } + }).collect(); self.fixup(new_fragments); } @@ -1358,29 +1343,24 @@ struct InlineFragmentFixupWorkItem { pub struct RangeIterator<'a> { iter: Items<'a,InlineFragmentRange>, index: FragmentIndex, - seen_first: bool, + is_first: bool, } impl<'a> Iterator<&'a InlineFragmentRange> for RangeIterator<'a> { fn next(&mut self) -> Option<&'a InlineFragmentRange> { - if self.seen_first { - match self.iter.next() { - Some(fragment_range) if fragment_range.range.contains(self.index) => { - return Some(fragment_range) - } - Some(_) | None => return None - } - } - - loop { - match self.iter.next() { - None => return None, - Some(fragment_range) if fragment_range.range.contains(self.index) => { - self.seen_first = true; - return Some(fragment_range) - } - Some(_) => {} - } + if !self.is_first { + // Yield the next fragment range if it contains the index + self.iter.next().and_then(|frag_range| { + if frag_range.range.contains(self.index) { Some(frag_range) } else { None } + }) + } else { + // Find the first fragment range that contains the index if it exists + let index = self.index; + let first = self.iter.by_ref().find(|frag_range| { + frag_range.range.contains(index) + }); + self.is_first = false; // We have made our first iteration + first } } } @@ -1403,10 +1383,20 @@ impl<'a> InlineFragmentContext<'a> { /// Iterates over all ranges that contain the fragment at context's index, outermost first. #[inline(always)] pub fn ranges(&self) -> RangeIterator<'a> { + // TODO: It would be more straightforward to return an existing iterator + // rather defining our own `RangeIterator`, but this requires unboxed + // closures in order to satisfy the borrow checker: + // + // ~~~rust + // let index = self.index; + // self.ranges.iter() + // .skip_while(|fr| fr.range.contains(index)) + // .take_while(|fr| fr.range.contains(index)) + // ~~~ RangeIterator { iter: self.ranges.iter(), index: self.index, - seen_first: false, + is_first: true, } } }