From 2e37b5e8fe979e719f55bbae779266b20942e920 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Wed, 14 May 2014 15:41:00 -0700 Subject: [PATCH 1/4] Make RangeIndex trait more general --- src/components/gfx/text/glyph.rs | 4 +- src/components/main/layout/inline.rs | 6 +- src/components/util/range.rs | 108 ++++++++++++++------------- 3 files changed, 63 insertions(+), 55 deletions(-) diff --git a/src/components/gfx/text/glyph.rs b/src/components/gfx/text/glyph.rs index 62bf4fb9f05..09e4c773d00 100644 --- a/src/components/gfx/text/glyph.rs +++ b/src/components/gfx/text/glyph.rs @@ -4,7 +4,7 @@ use servo_util::vec::*; use servo_util::range; -use servo_util::range::{Range, RangeIndex, EachIndex}; +use servo_util::range::{Range, RangeIndex, IntRangeIndex, EachIndex}; use servo_util::geometry::Au; use std::cmp::{Ord, Eq}; @@ -526,7 +526,7 @@ pub struct GlyphStore { is_whitespace: bool, } -range_index! { +int_range_index! { #[doc = "An index that refers to a character in a text run. This could \ point to the middle of a glyph."] struct CharIndex(int) diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index e9ee308a3e4..9099d5b5ce2 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -20,7 +20,7 @@ use gfx::font::FontMetrics; use gfx::font_context::FontContext; use servo_util::geometry::Au; use servo_util::geometry; -use servo_util::range::{Range, RangeIndex}; +use servo_util::range::{Range, RangeIndex, IntRangeIndex}; use std::iter::Enumerate; use std::fmt; use std::mem; @@ -61,7 +61,7 @@ pub struct LineBox { pub green_zone: Size2D } -range_index! { +int_range_index! { struct BoxIndex(int) } @@ -981,7 +981,7 @@ impl fmt::Show for InlineFlow { } } -range_index! { +int_range_index! { #[doc = "The index of a DOM element into the flat list of fragments."] struct FragmentIndex(int) } diff --git a/src/components/util/range.rs b/src/components/util/range.rs index 05421d9af87..13baf86d2e0 100644 --- a/src/components/util/range.rs +++ b/src/components/util/range.rs @@ -9,21 +9,26 @@ use std::num; use std::num::{Bounded, Zero}; /// An index type to be used by a `Range` -pub trait RangeIndex: Eq + Ord - + Clone - + Copy - + Zero - + TotalEq - + TotalOrd - + Add - + Sub - + Neg - + fmt::Show { +pub trait RangeIndex: Copy + + Clone + + fmt::Show + + Eq + + Ord + + TotalEq + + TotalOrd + + Add + + Sub + + Neg + + Zero {} + +pub trait IntRangeIndex: RangeIndex + Copy { fn new(x: T) -> Self; fn get(self) -> T; } -impl RangeIndex for int { +impl RangeIndex for int {} + +impl IntRangeIndex for int { #[inline] fn new(x: int) -> int { x } @@ -33,7 +38,7 @@ impl RangeIndex for int { /// Implements a range index type with operator overloads #[macro_export] -macro_rules! range_index { +macro_rules! int_range_index { ($(#[$attr:meta])* struct $Self:ident($T:ty)) => ( #[deriving(Clone, Eq, Ord, TotalEq, TotalOrd, Show, Zero)] $(#[$attr])* @@ -46,7 +51,9 @@ macro_rules! range_index { } } - impl RangeIndex<$T> for $Self { + impl RangeIndex for $Self {} + + impl IntRangeIndex<$T> for $Self { #[inline] fn new(x: $T) -> $Self { $Self(x) @@ -99,7 +106,7 @@ pub struct Range { len: I, } -impl> fmt::Show for Range { +impl fmt::Show for Range { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f.buf, "[{} .. {})", self.begin(), self.end()) } @@ -110,16 +117,14 @@ pub struct EachIndex { it: iter::Range, } -pub fn each_index>(start: I, stop: I) -> EachIndex { - EachIndex { - it: iter::range(start.get(), stop.get()) - } +pub fn each_index>(start: I, stop: I) -> EachIndex { + EachIndex { it: iter::range(start.get(), stop.get()) } } -impl> Iterator for EachIndex { +impl> Iterator for EachIndex { #[inline] fn next(&mut self) -> Option { - self.it.next().map(|i| RangeIndex::new(i)) + self.it.next().map(|i| IntRangeIndex::new(i)) } #[inline] @@ -128,7 +133,7 @@ impl> Iterator for EachIndex { } } -impl> Range { +impl Range { #[inline] pub fn new(off: I, len: I) -> Range { Range { off: off, len: len } @@ -146,39 +151,11 @@ impl> Range { #[inline] pub fn end(&self) -> I { self.off + self.len } - #[inline] - pub fn each_index(&self) -> EachIndex { - each_index(self.off, self.off + self.len) - } - #[inline] pub fn contains(&self, i: I) -> bool { i >= self.begin() && i < self.end() } - #[inline] - pub fn is_valid_for_string(&self, s: &str) -> bool { - let s_len = s.len(); - match num::cast::(s_len) { - Some(len) => { - let len = RangeIndex::new(len); - self.begin() < len - && self.end() <= len - && self.length() <= len - }, - None => { - debug!("Range::is_valid_for_string: string length (len={}) is longer than the \ - max value for the range index (max={})", s_len, - { - let max: T = Bounded::max_value(); - let val: I = RangeIndex::new(max); - val - }); - false - }, - } - } - #[inline] pub fn is_empty(&self) -> bool { self.len.is_zero() @@ -254,6 +231,37 @@ impl> Range { fail!("relation_to_range(): didn't classify self={}, other={}", self, other); } +} + +/// Methods for `Range`s with indices based on integer values +impl> Range { + #[inline] + pub fn each_index(&self) -> EachIndex { + each_index(self.off, self.off + self.len) + } + + #[inline] + pub fn is_valid_for_string(&self, s: &str) -> bool { + let s_len = s.len(); + match num::cast::(s_len) { + Some(len) => { + let len = IntRangeIndex::new(len); + self.begin() < len + && self.end() <= len + && self.length() <= len + }, + None => { + debug!("Range::is_valid_for_string: string length (len={}) is longer than the \ + max value for the range index (max={})", s_len, + { + let max: T = Bounded::max_value(); + let val: I = IntRangeIndex::new(max); + val + }); + false + }, + } + } #[inline] pub fn repair_after_coalesced_range(&mut self, other: &Range) { @@ -261,7 +269,7 @@ impl> Range { debug!("repair_after_coalesced_range: possibly repairing range {}", *self); debug!("repair_after_coalesced_range: relation of original range and coalesced range {}: {}", *other, relation); - let _1: I = RangeIndex::new(num::one::()); + let _1: I = IntRangeIndex::new(num::one::()); match relation { EntirelyBefore => {}, EntirelyAfter => { self.shift_by(-other.length()); }, From 26e580bbdc7340ccb0155599e20d9e345b1c1be4 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Thu, 15 May 2014 10:48:48 -0700 Subject: [PATCH 2/4] Improve Range method documentation and field and parameter names --- src/components/util/range.rs | 115 ++++++++++++++++++++++++++++------- 1 file changed, 92 insertions(+), 23 deletions(-) diff --git a/src/components/util/range.rs b/src/components/util/range.rs index 13baf86d2e0..f5e0925c04b 100644 --- a/src/components/util/range.rs +++ b/src/components/util/range.rs @@ -102,8 +102,8 @@ pub enum RangeRelation { /// A range of indices #[deriving(Clone)] pub struct Range { - off: I, - len: I, + begin: I, + length: I, } impl fmt::Show for Range { @@ -134,9 +134,17 @@ impl> Iterator for EachIndex { } impl Range { + /// Create a new range from beginning and length offsets. This could be + /// denoted as `[begin, begin + length)`. + /// + /// ~~~ + /// |-- begin ->|-- length ->| + /// | | | + /// <- o - - - - - +============+ - - - -> + /// ~~~ #[inline] - pub fn new(off: I, len: I) -> Range { - Range { off: off, len: len } + pub fn new(begin: I, length: I) -> Range { + Range { begin: begin, length: length } } #[inline] @@ -144,48 +152,108 @@ impl Range { Range::new(num::zero(), num::zero()) } + /// The index offset to the beginning of the range. + /// + /// ~~~ + /// |-- begin ->| + /// | | + /// <- o - - - - - +============+ - - - -> + /// ~~~ #[inline] - pub fn begin(&self) -> I { self.off } - #[inline] - pub fn length(&self) -> I { self.len } - #[inline] - pub fn end(&self) -> I { self.off + self.len } + pub fn begin(&self) -> I { self.begin } + /// The index offset from the beginning to the end of the range. + /// + /// ~~~ + /// |-- length ->| + /// | | + /// <- o - - - - - +============+ - - - -> + /// ~~~ + #[inline] + pub fn length(&self) -> I { self.length } + + /// The index offset to the end of the range. + /// + /// ~~~ + /// |--------- end --------->| + /// | | + /// <- o - - - - - +============+ - - - -> + /// ~~~ + #[inline] + pub fn end(&self) -> I { self.begin + self.length } + + /// `true` if the index is between the beginning and the end of the range. + /// + /// ~~~ + /// false true false + /// | | | + /// <- o - - + - - +=====+======+ - + - -> + /// ~~~ #[inline] pub fn contains(&self, i: I) -> bool { i >= self.begin() && i < self.end() } + /// `true` if the offset from the beginning to the end of the range is zero. #[inline] pub fn is_empty(&self) -> bool { - self.len.is_zero() + self.length().is_zero() } + /// Shift the entire range by the supplied index delta. + /// + /// ~~~ + /// |-- delta ->| + /// | | + /// <- o - +============+ - - - - - | - - - -> + /// | + /// <- o - - - - - - - +============+ - - - -> + /// ~~~ #[inline] - pub fn shift_by(&mut self, i: I) { - self.off = self.off + i; + pub fn shift_by(&mut self, delta: I) { + self.begin = self.begin + delta; } + /// Extend the end of the range by the supplied index delta. + /// + /// ~~~ + /// |-- delta ->| + /// | | + /// <- o - - - - - +====+ - - - - - | - - - -> + /// | + /// <- o - - - - - +================+ - - - -> + /// ~~~ #[inline] - pub fn extend_by(&mut self, i: I) { - self.len = self.len + i; + pub fn extend_by(&mut self, delta: I) { + self.length = self.length + delta; } + /// Move the end of the range to the target index. + /// + /// ~~~ + /// target + /// | + /// <- o - - - - - +====+ - - - - - | - - - -> + /// | + /// <- o - - - - - +================+ - - - -> + /// ~~~ #[inline] - pub fn extend_to(&mut self, i: I) { - self.len = i - self.off; + pub fn extend_to(&mut self, target: I) { + self.length = target - self.begin; } + /// Adjust the beginning offset and the length by the supplied deltas. #[inline] - pub fn adjust_by(&mut self, off_i: I, len_i: I) { - self.off = self.off + off_i; - self.len = self.len + len_i; + pub fn adjust_by(&mut self, begin_delta: I, length_delta: I) { + self.begin = self.begin + begin_delta; + self.length = self.length + length_delta; } + /// Set the begin and length values. #[inline] - pub fn reset(&mut self, off_i: I, len_i: I) { - self.off = off_i; - self.len = len_i; + pub fn reset(&mut self, begin: I, length: I) { + self.begin = begin; + self.length = length; } #[inline] @@ -235,9 +303,10 @@ impl Range { /// Methods for `Range`s with indices based on integer values impl> Range { + /// Returns an iterater that increments over `[begin, end)`. #[inline] pub fn each_index(&self) -> EachIndex { - each_index(self.off, self.off + self.len) + each_index(self.begin(), self.end()) } #[inline] From f883c6238e46a07b8e9e2e4e2e64d35a6ce6970c Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Thu, 15 May 2014 11:03:01 -0700 Subject: [PATCH 3/4] Use a tuple of fragment and glyph indices as the index type for LineBoxes Glyph indices are currently not tracked, once they are, they will allow us to implement faster line breaks in the future. --- src/components/main/layout/inline.rs | 153 +++++++++++++++++++++++---- 1 file changed, 135 insertions(+), 18 deletions(-) diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 9099d5b5ce2..5d4d17b6164 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -20,10 +20,12 @@ use gfx::font::FontMetrics; use gfx::font_context::FontContext; use servo_util::geometry::Au; use servo_util::geometry; -use servo_util::range::{Range, RangeIndex, IntRangeIndex}; +use servo_util::range; +use servo_util::range::{EachIndex, Range, RangeIndex, IntRangeIndex}; use std::iter::Enumerate; use std::fmt; use std::mem; +use std::num; use std::slice::{Items, MutItems}; use std::u16; use style::computed_values::{text_align, vertical_align, white_space}; @@ -56,13 +58,124 @@ use sync::Arc; /// left corner of the green zone is the same as that of the line, but /// the green zone can be taller and wider than the line itself. pub struct LineBox { - pub range: Range, + /// Consider the following HTML and rendered element with linebreaks: + /// + /// ~~~html + /// I like truffles, yes I do. + /// ~~~ + /// + /// ~~~ + /// +-----------+ + /// | I like | + /// | truffles, | + /// | yes I do. | + /// +-----------+ + /// ~~~ + /// + /// The ranges that describe these lines would be: + /// + /// ~~~ + /// | [0.0, 1.4) | [1.5, 2.0) | [2.1, 3.0) | + /// |------------|-------------|-------------| + /// | 'I like' | 'truffles,' | 'yes I do.' | + /// ~~~ + pub range: Range, pub bounds: Rect, pub green_zone: Size2D } int_range_index! { - struct BoxIndex(int) + #[doc = "The index of a box fragment into the flattened vector of DOM"] + #[doc = "elements."] + #[doc = ""] + #[doc = "For example, given the HTML below:"] + #[doc = ""] + #[doc = "~~~"] + #[doc = "I like truffles, yes I do."] + #[doc = "~~~"] + #[doc = ""] + #[doc = "The fragments would be indexed as follows:"] + #[doc = ""] + #[doc = "~~~"] + #[doc = "| 0 | 1 | 2 |"] + #[doc = "|------|------------------|--------------|"] + #[doc = "| 'I ' | 'like truffles,' | ' yes I do.' |"] + #[doc = "~~~"] + struct FragmentIndex(int) +} + +int_range_index! { + #[doc = "The index of a glyph in a single DOM fragment. Ligatures and"] + #[doc = "continuous runs of whitespace are treated as single glyphs."] + #[doc = "Non-breakable DOM fragments such as images are treated as"] + #[doc = "having a range length of `1`."] + #[doc = ""] + #[doc = "For example, given the HTML below:"] + #[doc = ""] + #[doc = "~~~"] + #[doc = "like truffles,"] + #[doc = "~~~"] + #[doc = ""] + #[doc = "The glyphs would be indexed as follows:"] + #[doc = ""] + #[doc = "~~~"] + #[doc = "| 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 |"] + #[doc = "|---|---|---|---|---|---|---|---|-----|---|----|----|"] + #[doc = "| l | i | k | e | | t | r | u | ffl | e | s | , |"] + #[doc = "~~~"] + struct GlyphIndex(int) +} + +/// A line index consists of two indices: a fragment index that refers to the +/// index of a DOM fragment within a flattened inline element; and a glyph index +/// where the 0th glyph refers to the first glyph of that fragment. +#[deriving(Clone, Eq, Ord, TotalEq, TotalOrd, Zero)] +pub struct LineIndices { + pub fragment_index: FragmentIndex, + pub glyph_index: GlyphIndex, +} + +impl RangeIndex for LineIndices {} + +impl Add for LineIndices { + fn add(&self, other: &LineIndices) -> LineIndices { + LineIndices { + fragment_index: self.fragment_index + other.fragment_index, + glyph_index: self.glyph_index + other.glyph_index, + } + } +} + +impl Sub for LineIndices { + fn sub(&self, other: &LineIndices) -> LineIndices { + LineIndices { + fragment_index: self.fragment_index - other.fragment_index, + glyph_index: self.glyph_index - other.glyph_index, + } + } +} + +impl Neg for LineIndices { + fn neg(&self) -> LineIndices { + LineIndices { + fragment_index: -self.fragment_index, + glyph_index: -self.glyph_index, + } + } +} + +impl fmt::Show for LineIndices { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f.buf, "{}.{}", self.fragment_index, self.glyph_index) + } +} + +pub fn each_fragment_index(range: &Range) -> EachIndex { + range::each_index(range.begin().fragment_index, range.length().fragment_index) +} + +pub fn each_glyph_index(range: &Range) -> EachIndex { + range::each_index(range.begin().glyph_index, range.length().glyph_index) } struct LineboxScanner { @@ -103,7 +216,7 @@ impl LineboxScanner { } fn reset_linebox(&mut self) { - self.pending_line.range.reset(BoxIndex(0), BoxIndex(0)); + self.pending_line.range.reset(num::zero(), num::zero()); self.pending_line.bounds = Rect(Point2D(Au::new(0), self.cur_y), Size2D(Au::new(0), Au::new(0))); self.pending_line.green_zone = Size2D(Au::new(0), Au::new(0)) } @@ -150,7 +263,7 @@ impl LineboxScanner { } } - if self.pending_line.range.length() > BoxIndex(0) { + if self.pending_line.range.length() > num::zero() { debug!("LineboxScanner: Partially full linebox {:u} left at end of scanning.", self.lines.len()); self.flush_current_line(); @@ -197,7 +310,7 @@ impl LineboxScanner { let first_box_size = first_box.border_box.size; let splittable = first_box.can_split(); debug!("LineboxScanner: box size: {}, splittable: {}", first_box_size, splittable); - let line_is_empty: bool = self.pending_line.range.length() == BoxIndex(0); + let line_is_empty: bool = self.pending_line.range.length() == num::zero(); // Initally, pretend a splittable box has 0 width. // We will move it later if it has nonzero width @@ -353,7 +466,7 @@ impl LineboxScanner { /// Tries to append the given box to the line, splitting it if necessary. Returns false only if /// we should break the line. fn try_append_to_line(&mut self, in_box: Box, flow: &mut InlineFlow) -> bool { - let line_is_empty = self.pending_line.range.length() == BoxIndex(0); + let line_is_empty = self.pending_line.range.length() == num::zero(); if line_is_empty { let (line_bounds, _) = self.initial_line_placement(&in_box, self.cur_y, flow); self.pending_line.bounds.origin = line_bounds.origin; @@ -448,11 +561,20 @@ impl LineboxScanner { fn push_box_to_line(&mut self, box_: Box) { debug!("LineboxScanner: Pushing box {} to line {:u}", box_.debug_id(), self.lines.len()); - if self.pending_line.range.length() == BoxIndex(0) { + if self.pending_line.range.length() == num::zero() { assert!(self.new_boxes.len() <= (u16::MAX as uint)); - self.pending_line.range.reset(BoxIndex(self.new_boxes.len() as int), BoxIndex(0)); + self.pending_line.range.reset( + LineIndices { + fragment_index: FragmentIndex(self.new_boxes.len() as int), + glyph_index: GlyphIndex(0) /* unused for now */, + }, + num::zero() + ); } - self.pending_line.range.extend_by(BoxIndex(1)); + self.pending_line.range.extend_by(LineIndices { + fragment_index: FragmentIndex(1), + glyph_index: GlyphIndex(0) /* unused for now */ , + }); self.pending_line.bounds.size.width = self.pending_line.bounds.size.width + box_.border_box.size.width; self.pending_line.bounds.size.height = Au::max(self.pending_line.bounds.size.height, @@ -723,7 +845,7 @@ impl InlineFlow { text_align::right => slack_width, }; - for i in line.range.each_index() { + for i in each_fragment_index(&line.range) { let box_ = boxes.get_mut(i.to_uint()); let size = box_.border_box.size; box_.border_box = Rect(Point2D(offset_x, box_.border_box.origin.y), size); @@ -854,7 +976,7 @@ impl Flow for InlineFlow { let (mut largest_height_for_top_fragments, mut largest_height_for_bottom_fragments) = (Au(0), Au(0)); - for box_i in line.range.each_index() { + for box_i in each_fragment_index(&line.range) { let fragment = self.boxes.boxes.get_mut(box_i.to_uint()); let InlineMetrics { @@ -930,7 +1052,7 @@ impl Flow for InlineFlow { // Compute the final positions in the block direction of each fragment. Recall that // `fragment.border_box.origin.y` was set to the distance from the baseline above. - for box_i in line.range.each_index() { + for box_i in each_fragment_index(&line.range) { let fragment = self.boxes.get_mut(box_i.to_uint()); match fragment.vertical_align() { vertical_align::top => { @@ -981,11 +1103,6 @@ impl fmt::Show for InlineFlow { } } -int_range_index! { - #[doc = "The index of a DOM element into the flat list of fragments."] - struct FragmentIndex(int) -} - /// Information that inline flows keep about a single nested element. This is used to recover the /// DOM structure from the flat box list when it's needed. pub struct FragmentRange { From fccc5d2071bcb8030f490debda07aa0f9a029af0 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Thu, 15 May 2014 13:57:32 -0700 Subject: [PATCH 4/4] Add some debug assertions --- src/components/main/layout/inline.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 5d4d17b6164..112a10b5bc3 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -139,6 +139,13 @@ impl RangeIndex for LineIndices {} impl Add for LineIndices { fn add(&self, other: &LineIndices) -> LineIndices { + // TODO: use debug_assert! after rustc upgrade + if cfg!(not(ndebug)) { + assert!(other.fragment_index == num::zero() || other.glyph_index == num::zero(), + "Attempted to add {} to {}. Both the fragment_index and \ + glyph_index of the RHS are non-zero. This probably \ + was a mistake!", self, other); + } LineIndices { fragment_index: self.fragment_index + other.fragment_index, glyph_index: self.glyph_index + other.glyph_index, @@ -148,6 +155,13 @@ impl Add for LineIndices { impl Sub for LineIndices { fn sub(&self, other: &LineIndices) -> LineIndices { + // TODO: use debug_assert! after rustc upgrade + if cfg!(not(ndebug)) { + assert!(other.fragment_index == num::zero() || other.glyph_index == num::zero(), + "Attempted to subtract {} from {}. Both the \ + fragment_index and glyph_index of the RHS are non-zero. \ + This probably was a mistake!", self, other); + } LineIndices { fragment_index: self.fragment_index - other.fragment_index, glyph_index: self.glyph_index - other.glyph_index, @@ -157,6 +171,13 @@ impl Sub for LineIndices { impl Neg for LineIndices { fn neg(&self) -> LineIndices { + // TODO: use debug_assert! after rustc upgrade + if cfg!(not(ndebug)) { + assert!(self.fragment_index == num::zero() || self.glyph_index == num::zero(), + "Attempted to negate {}. Both the fragment_index and \ + glyph_index are non-zero. This probably was a mistake!", + self); + } LineIndices { fragment_index: -self.fragment_index, glyph_index: -self.glyph_index,