From 3d757cd9ceb172309d612f725773dfda0ebba00d Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Mon, 19 May 2014 11:53:17 -0700 Subject: [PATCH 1/5] Replace usages of SplitBoxResult with option types --- src/components/main/layout/box_.rs | 28 ++++++----------- src/components/main/layout/inline.rs | 45 +++++++++------------------- 2 files changed, 23 insertions(+), 50 deletions(-) diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index a9f1289871f..559050c291e 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -266,16 +266,6 @@ impl UnscannedTextBoxInfo { } } -/// Represents the outcome of attempting to split a box. -pub enum SplitBoxResult { - CannotSplit, - // in general, when splitting the left or right side can - // be zero length, due to leading/trailing trimmable whitespace - SplitDidFit(Option, Option), - SplitDidNotFit(Option, Option) -} - - /// A box that represents a table column. #[deriving(Clone)] pub struct TableColumnBoxInfo { @@ -1097,10 +1087,10 @@ impl Box { } /// Split box which includes new-line character - pub fn split_by_new_line(&self) -> SplitBoxResult { + pub fn split_by_new_line(&self) -> Option<(Box, Option)> { match self.specific { GenericBox | IframeBox(_) | ImageBox(_) | TableBox | TableCellBox | - TableRowBox | TableWrapperBox => CannotSplit, + TableRowBox | TableWrapperBox => None, TableColumnBox(_) => fail!("Table column boxes do not need to split"), UnscannedTextBox(_) => fail!("Unscanned text boxes should have been scanned by now!"), ScannedTextBox(ref text_box_info) => { @@ -1117,7 +1107,7 @@ impl Box { let new_metrics = new_text_box_info.run.metrics_for_range(&left_range); let mut new_box = self.transform(new_metrics.bounding_box.size, ScannedTextBox(new_text_box_info)); new_box.new_line_pos = vec!(); - Some(new_box) + new_box }; // Right box is for right text of first founded new-line character. @@ -1131,16 +1121,16 @@ impl Box { None }; - SplitDidFit(left_box, right_box) + Some((left_box, right_box)) } } } /// Attempts to split this box so that its width is no more than `max_width`. - pub fn split_to_width(&self, max_width: Au, starts_line: bool) -> SplitBoxResult { + pub fn split_to_width(&self, max_width: Au, starts_line: bool) -> Option<(Option, Option)> { match self.specific { GenericBox | IframeBox(_) | ImageBox(_) | TableBox | TableCellBox | - TableRowBox | TableWrapperBox => CannotSplit, + TableRowBox | TableWrapperBox => None, TableColumnBox(_) => fail!("Table column boxes do not have width"), UnscannedTextBox(_) => fail!("Unscanned text boxes should have been scanned by now!"), ScannedTextBox(ref text_box_info) => { @@ -1231,10 +1221,10 @@ impl Box { Some(self.transform(size, ScannedTextBox(new_text_box_info))) }); - if pieces_processed_count == 1 || left_box.is_none() { - SplitDidNotFit(left_box, right_box) + if (pieces_processed_count == 1 || left_box.is_none()) && !starts_line { + None } else { - SplitDidFit(left_box, right_box) + Some((left_box, right_box)) } } } diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 5b0557c2c4d..6743f9ee452 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use css::node_style::StyledNode; -use layout::box_::{Box, CannotSplit, SplitDidFit, SplitDidNotFit}; +use layout::box_::Box; use layout::context::LayoutContext; use layout::floats::{FloatLeft, Floats, PlacementInfo}; use layout::flow::{BaseFlow, FlowClass, Flow, InlineFlowClass}; @@ -425,19 +425,16 @@ impl LineboxScanner { } else { // In case of box includes new-line character match in_box.split_by_new_line() { - SplitDidFit(left, right) => { - match (left, right) { - (Some(left_box), Some(right_box)) => { - self.push_box_to_line(left_box); - self.work_list.push_front(right_box); - } - (Some(left_box), None) => { - self.push_box_to_line(left_box); - } - _ => error!("LineboxScanner: This split case makes no sense!"), - } - } - _ => {} + Some((left_box, Some(right_box))) => { + self.push_box_to_line(left_box); + self.work_list.push_front(right_box); + }, + Some((left_box, None)) => { + self.push_box_to_line(left_box); + }, + None => { + error!("LineboxScanner: This split case makes no sense!") + }, } false } @@ -496,30 +493,16 @@ impl LineboxScanner { let available_width = green_zone.width - self.pending_line.bounds.size.width; let split = in_box.split_to_width(available_width, line_is_empty); - let (left, right) = match (split, line_is_empty) { - (CannotSplit, _) => { + let (left, right) = match split { + None => { debug!("LineboxScanner: Tried to split unsplittable render box! {}", in_box); self.work_list.push_front(in_box); return false } - (SplitDidNotFit(_, _), false) => { - debug!("LineboxScanner: case=split box didn't fit, not appending and deferring \ - original box."); - self.work_list.push_front(in_box); - return false - } - (SplitDidFit(left, right), _) => { + Some((left, right)) => { debug!("LineboxScanner: case=split box did fit; deferring remainder box."); (left, right) - // Fall through to push boxes to the line. - } - (SplitDidNotFit(left, right), true) => { - // TODO(eatkinson, issue #224): Signal that horizontal overflow happened? - debug!("LineboxScanner: case=split box didn't fit and line {:u} is empty, so \ - overflowing and deferring remainder box.", - self.lines.len()); - (left, right) // Fall though to push boxes to the line. } }; From fe28e20bca9c6b3ba7d6b214ed27f908d07a7f6a Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Wed, 21 May 2014 09:10:28 -0700 Subject: [PATCH 2/5] Flatten some pattern matching --- src/components/main/layout/inline.rs | 45 +++++++++++++++------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs index 6743f9ee452..58e21329f95 100644 --- a/src/components/main/layout/inline.rs +++ b/src/components/main/layout/inline.rs @@ -492,32 +492,35 @@ impl LineboxScanner { } let available_width = green_zone.width - self.pending_line.bounds.size.width; - let split = in_box.split_to_width(available_width, line_is_empty); - let (left, right) = match split { + match in_box.split_to_width(available_width, line_is_empty) { None => { - debug!("LineboxScanner: Tried to split unsplittable render box! {}", - in_box); + debug!("LineboxScanner: Tried to split unsplittable render box! Deferring to next \ + line. {}", in_box); self.work_list.push_front(in_box); - return false - } - Some((left, right)) => { - debug!("LineboxScanner: case=split box did fit; deferring remainder box."); - (left, right) - // Fall though to push boxes to the line. - } - }; - - match (left, right) { - (Some(left_box), Some(right_box)) => { + false + }, + Some((Some(left_box), Some(right_box))) => { + debug!("LineboxScanner: Line break found! Pushing left box to line and deferring \ + right box to next line."); self.push_box_to_line(left_box); self.work_list.push_front(right_box); - } - (Some(left_box), None) => self.push_box_to_line(left_box), - (None, Some(right_box)) => self.push_box_to_line(right_box), - (None, None) => error!("LineboxScanner: This split case makes no sense!"), + true + }, + Some((Some(left_box), None)) => { + debug!("LineboxScanner: Pushing left box to line."); + self.push_box_to_line(left_box); + true + }, + Some((None, Some(right_box))) => { + debug!("LineboxScanner: Pushing right box to line."); + self.push_box_to_line(right_box); + true + }, + Some((None, None)) => { + error!("LineboxScanner: This split case makes no sense!"); + true + }, } - - true } // An unconditional push From d6d36997c6673df7ddca06742fe1193a5311fc22 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Wed, 21 May 2014 09:53:23 -0700 Subject: [PATCH 3/5] Add ref test for nested inline elements --- src/test/ref/basic.list | 1 + src/test/ref/linebreak_inline_span_a.html | 24 +++++++++++++++++++++++ src/test/ref/linebreak_inline_span_b.html | 23 ++++++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 src/test/ref/linebreak_inline_span_a.html create mode 100644 src/test/ref/linebreak_inline_span_b.html diff --git a/src/test/ref/basic.list b/src/test/ref/basic.list index 43def47ea67..11cb7c01519 100644 --- a/src/test/ref/basic.list +++ b/src/test/ref/basic.list @@ -71,3 +71,4 @@ == setattribute_id_restyle_a.html setattribute_id_restyle_b.html == pseudo_element_a.html pseudo_element_b.html == linebreak_simple_a.html linebreak_simple_b.html +== linebreak_inline_span_a.html linebreak_inline_span_b.html diff --git a/src/test/ref/linebreak_inline_span_a.html b/src/test/ref/linebreak_inline_span_a.html new file mode 100644 index 00000000000..ddaeabcb653 --- /dev/null +++ b/src/test/ref/linebreak_inline_span_a.html @@ -0,0 +1,24 @@ + + + + + +
+ The most terrifying fact about the universe is not that + it is hostile but that it is indifferent; but if we can come to terms + with this indifference and accept the challenges of life + within the boundaries of death - however + mutable man may be able to make them - our existence as a species can have + genuine meaning and fulfillment. However vast the darkness, we must supply + our own light. +
+ Stanley Kubrick +
+
+ + diff --git a/src/test/ref/linebreak_inline_span_b.html b/src/test/ref/linebreak_inline_span_b.html new file mode 100644 index 00000000000..ea1df5cc545 --- /dev/null +++ b/src/test/ref/linebreak_inline_span_b.html @@ -0,0 +1,23 @@ + + + + + +
+ The most terrifying fact about the universe is not that it is hostile but + that it is indifferent; but if we can come to terms with this indifference + and accept the challenges of life within the boundaries of death - however + mutable man may be able to make them - our existence as a species can have + genuine meaning and fulfillment. However vast the darkness, we must supply + our own light. +
+ Stanley Kubrick +
+
+ + From 9efaf831fc759961f2be5442c12e0bd71f4c6c47 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Wed, 21 May 2014 13:53:40 -0700 Subject: [PATCH 4/5] Avoid splitting boxes if possible --- src/components/main/layout/box_.rs | 38 ++++++++++++++++-------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index 559050c291e..2e2e1309638 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -1203,27 +1203,29 @@ impl Box { } } - let left_box = if left_range.length() > CharIndex(0) { - let new_text_box_info = ScannedTextBoxInfo::new(text_box_info.run.clone(), left_range); - let width = new_text_box_info.run.advance_for_range(&left_range); - let height = self.border_box.size.height; - let size = Size2D(width, height); - Some(self.transform(size, ScannedTextBox(new_text_box_info))) - } else { - None - }; + let left_is_some = left_range.length() > CharIndex(0); - let right_box = right_range.map_or(None, |right_range: Range| { - let new_text_box_info = ScannedTextBoxInfo::new(text_box_info.run.clone(), right_range); - let width = new_text_box_info.run.advance_for_range(&right_range); - let height = self.border_box.size.height; - let size = Size2D(width, height); - Some(self.transform(size, ScannedTextBox(new_text_box_info))) - }); - - if (pieces_processed_count == 1 || left_box.is_none()) && !starts_line { + if (pieces_processed_count == 1 || !left_is_some) && !starts_line { None } else { + let left_box = if left_is_some { + let new_text_box_info = ScannedTextBoxInfo::new(text_box_info.run.clone(), left_range); + let width = new_text_box_info.run.advance_for_range(&left_range); + let height = self.border_box.size.height; + let size = Size2D(width, height); + Some(self.transform(size, ScannedTextBox(new_text_box_info))) + } else { + None + }; + + let right_box = right_range.map(|right_range| { + let new_text_box_info = ScannedTextBoxInfo::new(text_box_info.run.clone(), right_range); + let width = new_text_box_info.run.advance_for_range(&right_range); + let height = self.border_box.size.height; + let size = Size2D(width, height); + (self.transform(size, ScannedTextBox(new_text_box_info))) + }); + Some((left_box, right_box)) } } From 60f21b12b2661c960514b5864e0d78d6a1e3ecd8 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Wed, 21 May 2014 15:51:38 -0700 Subject: [PATCH 5/5] Add doc comments explaining some return types --- src/components/main/layout/box_.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/main/layout/box_.rs b/src/components/main/layout/box_.rs index 2e2e1309638..e55a4ed2793 100644 --- a/src/components/main/layout/box_.rs +++ b/src/components/main/layout/box_.rs @@ -1086,7 +1086,11 @@ impl Box { } } - /// Split box which includes new-line character + /// Split box which includes new-line character. + /// + /// A return value of `None` indicates that the box could not be split. + /// Otherwise the split boxes are returned. The right boxe is optional due + /// to the possibility of it being whitespace. pub fn split_by_new_line(&self) -> Option<(Box, Option)> { match self.specific { GenericBox | IframeBox(_) | ImageBox(_) | TableBox | TableCellBox | @@ -1127,6 +1131,10 @@ impl Box { } /// Attempts to split this box so that its width is no more than `max_width`. + /// + /// A return value of `None` indicates that the box could not be split. + /// Otherwise the split boxes are returned. The left and right boxes are + /// optional due to the possibility of them being whitespace. pub fn split_to_width(&self, max_width: Au, starts_line: bool) -> Option<(Option, Option)> { match self.specific { GenericBox | IframeBox(_) | ImageBox(_) | TableBox | TableCellBox |