diff options
| -rw-r--r-- | components/layout/construct.rs | 14 | ||||
| -rw-r--r-- | components/layout/display_list_builder.rs | 4 | ||||
| -rw-r--r-- | components/layout/fragment.rs | 142 | ||||
| -rw-r--r-- | components/layout/incremental.rs | 2 | ||||
| -rw-r--r-- | components/layout/inline.rs | 34 | ||||
| -rw-r--r-- | components/layout/layout_debug.rs | 4 | ||||
| -rw-r--r-- | components/layout/table_colgroup.rs | 3 | ||||
| -rw-r--r-- | components/layout/text.rs | 13 |
8 files changed, 128 insertions, 88 deletions
diff --git a/components/layout/construct.rs b/components/layout/construct.rs index ca97d7e36b7..424826f5d8c 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -238,12 +238,12 @@ impl<'a> FlowConstructor<'a> { Some(url) => { // FIXME(pcwalton): The fact that image fragments store the cache within them makes // little sense to me. - ImageFragment(ImageFragmentInfo::new(node, - url, - self.layout_context - .shared - .image_cache - .clone())) + ImageFragment(box ImageFragmentInfo::new(node, + url, + self.layout_context + .shared + .image_cache + .clone())) } } } @@ -258,7 +258,7 @@ impl<'a> FlowConstructor<'a> { -> SpecificFragmentInfo { match node.type_id() { Some(ElementNodeTypeId(HTMLIFrameElementTypeId)) => { - IframeFragment(IframeFragmentInfo::new(node)) + IframeFragment(box IframeFragmentInfo::new(node)) } Some(ElementNodeTypeId(HTMLImageElementTypeId)) => { self.build_fragment_info_for_image(node, node.image_url()) diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 3f298a92686..48d6f706fef 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -502,7 +502,7 @@ impl FragmentDisplayListBuilding for Fragment { if opts::get().show_debug_fragment_borders { self.build_debug_borders_around_text_fragments(display_list, flow_origin, - text_fragment, + &**text_fragment, clip_rect); } } @@ -559,7 +559,7 @@ impl FragmentDisplayListBuilding for Fragment { // the iframe is actually going to be displayed. match self.specific { IframeFragment(ref iframe_fragment) => { - self.finalize_position_and_size_of_iframe(iframe_fragment, + self.finalize_position_and_size_of_iframe(&**iframe_fragment, absolute_fragment_bounds.origin, layout_context) } diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 44a4c2669d8..23fd1b41dff 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -70,7 +70,11 @@ use url::Url; /// Different types of fragments may also contain custom data; for example, text fragments contain /// text. /// -/// FIXME(#2260, pcwalton): This can be slimmed down some. +/// Do not add fields to this structure unless they're really really mega necessary! Fragments get +/// moved around a lot and thus their size impacts performance of layout quite a bit. +/// +/// FIXME(#2260, pcwalton): This can be slimmed down some by (at least) moving `inline_context` +/// to be on `InlineFlow` only. #[deriving(Clone)] pub struct Fragment { /// An opaque reference to the DOM node that this `Fragment` originates from. @@ -79,9 +83,6 @@ pub struct Fragment { /// The CSS style of this fragment. pub style: Arc<ComputedValues>, - /// How damaged this fragment is since last reflow. - pub restyle_damage: RestyleDamage, - /// The position of this fragment relative to its owning flow. /// The size includes padding and border, but not margin. pub border_box: LogicalRect<Au>, @@ -96,18 +97,16 @@ pub struct Fragment { /// Info specific to the kind of fragment. Keep this enum small. pub specific: SpecificFragmentInfo, - /// New-line chracter(\n)'s positions(relative, not absolute) - /// - /// FIXME(#2260, pcwalton): This is very inefficient; remove. - pub new_line_pos: Vec<CharIndex>, - /// Holds the style context information for fragments /// that are part of an inline formatting context. pub inline_context: Option<InlineFragmentContext>, /// A debug ID that is consistent for the life of /// this fragment (via transform etc). - pub debug_id: uint, + pub debug_id: u16, + + /// How damaged this fragment is since last reflow. + pub restyle_damage: RestyleDamage, } impl<E, S: Encoder<E>> Encodable<S, E> for Fragment { @@ -120,14 +119,14 @@ impl<E, S: Encoder<E>> Encodable<S, E> for Fragment { } } -/// Info specific to the kind of fragment. Keep this enum small. +/// Info specific to the kind of fragment. /// -/// FIXME(pcwalton): We have completely failed at keeping this enum small. +/// Keep this enum small. As in, no more than one word. Or pcwalton will yell at you. #[deriving(Clone)] pub enum SpecificFragmentInfo { GenericFragment, - IframeFragment(IframeFragmentInfo), - ImageFragment(ImageFragmentInfo), + IframeFragment(Box<IframeFragmentInfo>), + ImageFragment(Box<ImageFragmentInfo>), /// A hypothetical box (see CSS 2.1 § 10.3.7) for an absolutely-positioned block that was /// declared with `display: inline;`. @@ -135,7 +134,7 @@ pub enum SpecificFragmentInfo { InlineBlockFragment(InlineBlockFragmentInfo), InputFragment, - ScannedTextFragment(ScannedTextFragmentInfo), + ScannedTextFragment(Box<ScannedTextFragmentInfo>), TableFragment, TableCellFragment, TableColumnFragment(TableColumnFragmentInfo), @@ -368,6 +367,12 @@ pub struct ScannedTextFragmentInfo { /// The range within the above text run that this represents. pub range: Range<CharIndex>, + /// The positions of newlines within this scanned text fragment. + /// + /// FIXME(#2260, pcwalton): Can't this go somewhere else, like in the text run or something? + /// Or can we just remove it? + pub new_line_pos: Vec<CharIndex>, + /// The new_line_pos is eaten during line breaking. If we need to re-merge /// fragments, it will have to be restored. pub original_new_line_pos: Option<Vec<CharIndex>>, @@ -378,11 +383,15 @@ pub struct ScannedTextFragmentInfo { impl ScannedTextFragmentInfo { /// Creates the information specific to a scanned text fragment from a range and a text run. - pub fn new(run: Arc<Box<TextRun>>, range: Range<CharIndex>, content_size: LogicalSize<Au>) + pub fn new(run: Arc<Box<TextRun>>, + range: Range<CharIndex>, + new_line_positions: Vec<CharIndex>, + content_size: LogicalSize<Au>) -> ScannedTextFragmentInfo { ScannedTextFragmentInfo { run: run, range: range, + new_line_pos: new_line_positions, original_new_line_pos: None, content_size: content_size, } @@ -406,12 +415,15 @@ impl SplitInfo { } } -/// Data for an unscanned text fragment. Unscanned text fragments are the results of flow construction that -/// have not yet had their inline-size determined. +/// Data for an unscanned text fragment. Unscanned text fragments are the results of flow +/// construction that have not yet had their inline-size determined. #[deriving(Clone)] pub struct UnscannedTextFragmentInfo { /// The text inside the fragment. - pub text: String, + /// + /// FIXME(pcwalton): Is there something more clever we can do here that avoids the double + /// indirection while not penalizing all fragments? + pub text: Box<String>, } impl UnscannedTextFragmentInfo { @@ -419,7 +431,7 @@ impl UnscannedTextFragmentInfo { pub fn new(node: &ThreadSafeLayoutNode) -> UnscannedTextFragmentInfo { // FIXME(pcwalton): Don't copy text; atomically reference count it instead. UnscannedTextFragmentInfo { - text: node.text(), + text: box node.text(), } } @@ -427,7 +439,7 @@ impl UnscannedTextFragmentInfo { #[inline] pub fn from_text(text: String) -> UnscannedTextFragmentInfo { UnscannedTextFragmentInfo { - text: text, + text: box text, } } } @@ -436,7 +448,7 @@ impl UnscannedTextFragmentInfo { #[deriving(Clone)] pub struct TableColumnFragmentInfo { /// the number of columns a <col> element should span - pub span: Option<int>, + pub span: int, } impl TableColumnFragmentInfo { @@ -447,7 +459,7 @@ impl TableColumnFragmentInfo { element.get_attr(&ns!(""), &atom!("span")).and_then(|string| { let n: Option<int> = FromStr::from_str(string); n - }) + }).unwrap_or(0) }; TableColumnFragmentInfo { span: span, @@ -476,7 +488,6 @@ impl Fragment { border_padding: LogicalMargin::zero(writing_mode), margin: LogicalMargin::zero(writing_mode), specific: constructor.build_specific_fragment_info_for_node(node), - new_line_pos: vec!(), inline_context: None, debug_id: layout_debug::generate_unique_debug_id(), } @@ -495,7 +506,6 @@ impl Fragment { border_padding: LogicalMargin::zero(writing_mode), margin: LogicalMargin::zero(writing_mode), specific: specific, - new_line_pos: vec!(), inline_context: None, debug_id: layout_debug::generate_unique_debug_id(), } @@ -525,7 +535,6 @@ impl Fragment { border_padding: LogicalMargin::zero(writing_mode), margin: LogicalMargin::zero(writing_mode), specific: specific, - new_line_pos: vec!(), inline_context: None, debug_id: layout_debug::generate_unique_debug_id(), } @@ -546,7 +555,6 @@ impl Fragment { border_padding: LogicalMargin::zero(writing_mode), margin: LogicalMargin::zero(writing_mode), specific: specific, - new_line_pos: vec!(), inline_context: None, debug_id: layout_debug::generate_unique_debug_id(), } @@ -562,8 +570,8 @@ impl Fragment { pub fn save_new_line_pos(&mut self) { match &mut self.specific { &ScannedTextFragment(ref mut info) => { - if !self.new_line_pos.is_empty() { - info.original_new_line_pos = Some(self.new_line_pos.clone()); + if !info.new_line_pos.is_empty() { + info.original_new_line_pos = Some(info.new_line_pos.clone()); } } _ => {} @@ -575,7 +583,7 @@ impl Fragment { &ScannedTextFragment(ref mut info) => { match info.original_new_line_pos.take() { None => {} - Some(new_line_pos) => self.new_line_pos = new_line_pos, + Some(new_line_pos) => info.new_line_pos = new_line_pos, } return } @@ -585,15 +593,17 @@ impl Fragment { /// Returns a debug ID of this fragment. This ID should not be considered stable across /// multiple layouts or fragment manipulations. - pub fn debug_id(&self) -> uint { + pub fn debug_id(&self) -> u16 { self.debug_id } /// Transforms this fragment into another fragment of the given type, with the given size, /// preserving all the other data. - pub fn transform(&self, size: LogicalSize<Au>, mut info: ScannedTextFragmentInfo) -> Fragment { - let new_border_box = - LogicalRect::from_point_size(self.style.writing_mode, self.border_box.start, size); + pub fn transform(&self, size: LogicalSize<Au>, mut info: Box<ScannedTextFragmentInfo>) + -> Fragment { + let new_border_box = LogicalRect::from_point_size(self.style.writing_mode, + self.border_box.start, + size); info.content_size = size.clone(); @@ -605,7 +615,6 @@ impl Fragment { border_padding: self.border_padding, margin: self.margin, specific: ScannedTextFragment(info), - new_line_pos: self.new_line_pos.clone(), inline_context: self.inline_context.clone(), debug_id: self.debug_id, } @@ -918,6 +927,22 @@ impl Fragment { self.is_scanned_text_fragment() } + /// Returns the newline positions of this fragment, if it's a scanned text fragment. + pub fn newline_positions(&self) -> Option<&Vec<CharIndex>> { + match self.specific { + ScannedTextFragment(ref info) => Some(&info.new_line_pos), + _ => None, + } + } + + /// Returns the newline positions of this fragment, if it's a scanned text fragment. + pub fn newline_positions_mut(&mut self) -> Option<&mut Vec<CharIndex>> { + match self.specific { + ScannedTextFragment(ref mut info) => Some(&mut info.new_line_pos), + _ => None, + } + } + /// Returns true if and only if this is a scanned text fragment. fn is_scanned_text_fragment(&self) -> bool { match self.specific { @@ -1049,19 +1074,22 @@ impl Fragment { fail!("Inline blocks or inline absolute hypothetical fragments do not get split") } ScannedTextFragment(ref text_fragment_info) => { - let mut new_line_pos = self.new_line_pos.clone(); + let mut new_line_pos = text_fragment_info.new_line_pos.clone(); let cur_new_line_pos = new_line_pos.remove(0).unwrap(); - let inline_start_range = Range::new(text_fragment_info.range.begin(), cur_new_line_pos); - let inline_end_range = Range::new(text_fragment_info.range.begin() + cur_new_line_pos + CharIndex(1), - text_fragment_info.range.length() - (cur_new_line_pos + CharIndex(1))); + let inline_start_range = Range::new(text_fragment_info.range.begin(), + cur_new_line_pos); + let inline_end_range = Range::new( + text_fragment_info.range.begin() + cur_new_line_pos + CharIndex(1), + text_fragment_info.range.length() - (cur_new_line_pos + CharIndex(1))); // Left fragment is for inline-start text of first founded new-line character. - let inline_start_fragment = SplitInfo::new(inline_start_range, text_fragment_info); + let inline_start_fragment = SplitInfo::new(inline_start_range, + &**text_fragment_info); // Right fragment is for inline-end text of first founded new-line character. let inline_end_fragment = if inline_end_range.length() > CharIndex(0) { - Some(SplitInfo::new(inline_end_range, text_fragment_info)) + Some(SplitInfo::new(inline_end_range, &**text_fragment_info)) } else { None }; @@ -1078,24 +1106,30 @@ impl Fragment { /// Otherwise the information pertaining to the split is returned. The inline-start /// and inline-end split information are both optional due to the possibility of /// them being whitespace. - // - // TODO(bjz): The text run should be removed in the future, but it is currently needed for - // the current method of fragment splitting in the `inline::try_append_*` functions. - pub fn find_split_info_for_inline_size(&self, start: CharIndex, max_inline_size: Au, starts_line: bool) - -> Option<(Option<SplitInfo>, Option<SplitInfo>, Arc<Box<TextRun>> /* TODO(bjz): remove */)> { + pub fn find_split_info_for_inline_size(&self, + start: CharIndex, + max_inline_size: Au, + starts_line: bool) + -> Option<(Option<SplitInfo>, + Option<SplitInfo>, + Arc<Box<TextRun>>)> { match self.specific { - GenericFragment | IframeFragment(_) | ImageFragment(_) | TableFragment | TableCellFragment | - TableRowFragment | TableWrapperFragment | InlineBlockFragment(_) | InputFragment | - InlineAbsoluteHypotheticalFragment(_) => None, + GenericFragment | IframeFragment(_) | ImageFragment(_) | TableFragment | + TableCellFragment | TableRowFragment | TableWrapperFragment | InlineBlockFragment(_) | + InputFragment | InlineAbsoluteHypotheticalFragment(_) => None, TableColumnFragment(_) => fail!("Table column fragments do not have inline_size"), - UnscannedTextFragment(_) => fail!("Unscanned text fragments should have been scanned by now!"), + UnscannedTextFragment(_) => { + fail!("Unscanned text fragments should have been scanned by now!") + } ScannedTextFragment(ref text_fragment_info) => { let mut pieces_processed_count: uint = 0; let mut remaining_inline_size: Au = max_inline_size; - let mut inline_start_range = Range::new(text_fragment_info.range.begin() + start, CharIndex(0)); + let mut inline_start_range = Range::new(text_fragment_info.range.begin() + start, + CharIndex(0)); let mut inline_end_range: Option<Range<CharIndex>> = None; - debug!("split_to_inline_size: splitting text fragment (strlen={}, range={}, avail_inline_size={})", + debug!("split_to_inline_size: splitting text fragment \ + (strlen={}, range={}, avail_inline_size={})", text_fragment_info.run.text.len(), text_fragment_info.range, max_inline_size); @@ -1151,12 +1185,12 @@ impl Fragment { None } else { let inline_start = if inline_start_is_some { - Some(SplitInfo::new(inline_start_range, text_fragment_info)) + Some(SplitInfo::new(inline_start_range, &**text_fragment_info)) } else { None }; let inline_end = inline_end_range.map(|inline_end_range| { - SplitInfo::new(inline_end_range, text_fragment_info) + SplitInfo::new(inline_end_range, &**text_fragment_info) }); Some((inline_start, inline_end, text_fragment_info.run.clone())) diff --git a/components/layout/incremental.rs b/components/layout/incremental.rs index 959647df85a..6f23756442f 100644 --- a/components/layout/incremental.rs +++ b/components/layout/incremental.rs @@ -8,7 +8,7 @@ use style::ComputedValues; bitflags! { #[doc = "Individual layout actions that may be necessary after restyling."] - flags RestyleDamage: int { + flags RestyleDamage: u8 { #[doc = "Repaint the node itself."] #[doc = "Currently unused; need to decide how this propagates."] static Repaint = 0x01, diff --git a/components/layout/inline.rs b/components/layout/inline.rs index b95eb847888..3a186920399 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -391,7 +391,11 @@ impl LineBreaker { } fn try_append_to_line_by_new_line(&mut self, in_fragment: Fragment) -> bool { - if in_fragment.new_line_pos.len() == 0 { + let no_newline_positions = match in_fragment.newline_positions() { + None => true, + Some(ref positions) => positions.is_empty(), + }; + if no_newline_positions { debug!("LineBreaker: Did not find a new-line character, so pushing the fragment to \ the line without splitting."); self.push_fragment_to_line(in_fragment); @@ -406,13 +410,14 @@ impl LineBreaker { let writing_mode = self.floats.writing_mode; let split_fragment = |split: SplitInfo| { - let info = - ScannedTextFragmentInfo::new( - run.clone(), - split.range, - in_fragment.border_box.size); - let size = LogicalSize::new( - writing_mode, split.inline_size, in_fragment.border_box.size.block); + let info = box ScannedTextFragmentInfo::new(run.clone(), + split.range, + (*in_fragment.newline_positions() + .unwrap()).clone(), + in_fragment.border_box.size); + let size = LogicalSize::new(writing_mode, + split.inline_size, + in_fragment.border_box.size.block); in_fragment.transform(size, info) }; @@ -420,14 +425,14 @@ impl LineBreaker { to the line."); let mut inline_start = split_fragment(inline_start); inline_start.save_new_line_pos(); - inline_start.new_line_pos = vec![]; + *inline_start.newline_positions_mut().unwrap() = vec![]; self.push_fragment_to_line(inline_start); for inline_end in inline_end.into_iter() { debug!("LineBreaker: Deferring the fragment to the inline_end of the new-line \ character to the line."); let mut inline_end = split_fragment(inline_end); - inline_end.new_line_pos.remove(0); + inline_end.newline_positions_mut().unwrap().remove(0); self.work_list.push_front(inline_end); } @@ -497,11 +502,10 @@ impl LineBreaker { line_is_empty); match split.map(|(inline_start, inline_end, run)| { let split_fragment = |split: SplitInfo| { - let info = - ScannedTextFragmentInfo::new( - run.clone(), - split.range, - in_fragment.border_box.size); + let info = box ScannedTextFragmentInfo::new(run.clone(), + split.range, + Vec::new(), + in_fragment.border_box.size); let size = LogicalSize::new(self.floats.writing_mode, split.inline_size, in_fragment.border_box.size.block); diff --git a/components/layout/layout_debug.rs b/components/layout/layout_debug.rs index 21e2e59324c..5f3051a9610 100644 --- a/components/layout/layout_debug.rs +++ b/components/layout/layout_debug.rs @@ -92,8 +92,8 @@ impl Drop for Scope { /// Generate a unique ID. This is used for items such as Fragment /// which are often reallocated but represent essentially the /// same data. -pub fn generate_unique_debug_id() -> uint { - unsafe { DEBUG_ID_COUNTER.fetch_add(1, SeqCst) } +pub fn generate_unique_debug_id() -> u16 { + unsafe { DEBUG_ID_COUNTER.fetch_add(1, SeqCst) as u16 } } /// Begin a layout debug trace. If this has not been called, diff --git a/components/layout/table_colgroup.rs b/components/layout/table_colgroup.rs index 96cec13c5aa..deb1268426c 100644 --- a/components/layout/table_colgroup.rs +++ b/components/layout/table_colgroup.rs @@ -13,6 +13,7 @@ use layout_debug; use wrapper::ThreadSafeLayoutNode; use servo_util::geometry::Au; +use std::cmp::max; use std::fmt; use style::computed_values::LengthOrPercentageOrAuto; @@ -64,7 +65,7 @@ impl Flow for TableColGroupFlow { // Retrieve the specified value from the appropriate CSS property. let inline_size = fragment.style().content_inline_size(); let span: int = match fragment.specific { - TableColumnFragment(col_fragment) => col_fragment.span.unwrap_or(1), + TableColumnFragment(col_fragment) => max(col_fragment.span, 1), _ => fail!("non-table-column fragment inside table column?!"), }; for _ in range(0, span) { diff --git a/components/layout/text.rs b/components/layout/text.rs index 7889c8a18bc..db0380615df 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -160,16 +160,17 @@ impl TextRunScanner { } let text_size = old_fragment.border_box.size; + let &NewLinePositions(ref mut new_line_positions) = + new_line_positions.get_mut(logical_offset); let new_text_fragment_info = - ScannedTextFragmentInfo::new(run.clone(), range, text_size); + box ScannedTextFragmentInfo::new(run.clone(), + range, + mem::replace(new_line_positions, Vec::new()), + text_size); let new_metrics = new_text_fragment_info.run.metrics_for_range(&range); let bounding_box_size = bounding_box_for_run_metrics(&new_metrics, old_fragment.style.writing_mode); - let mut new_fragment = old_fragment.transform(bounding_box_size, - new_text_fragment_info); - let &NewLinePositions(ref mut new_line_positions) = - new_line_positions.get_mut(logical_offset); - new_fragment.new_line_pos = mem::replace(new_line_positions, Vec::new()); + let new_fragment = old_fragment.transform(bounding_box_size, new_text_fragment_info); out_fragments.push(new_fragment) } |