Share line breaking state across text runs

Fixes #874

Cargo.lock

@@ -1432,6 +1432,7 @@ dependencies = [
  "unicode-bidi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "unicode-script 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "webrender_api 0.56.1 (git+https://github.com/servo/webrender)",
+ "xi-unicode 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]

components/gfx/text/text_run.rs

@@ -15,7 +15,7 @@ use style::str::char_is_whitespace;
 use text::glyph::{ByteIndex, GlyphStore};
 use unicode_bidi as bidi;
 use webrender_api;
-use xi_unicode::LineBreakIterator;
+use xi_unicode::LineBreakLeafIter;
 
 thread_local! {
     static INDEX_OF_FIRST_GLYPH_RUN_CACHE: Cell<Option<(*const TextRun, ByteIndex, usize)>> =
@@ -177,9 +177,11 @@ impl<'a> Iterator for CharacterSliceIterator<'a> {
 }
 
 impl<'a> TextRun {
-    pub fn new(font: &mut Font, text: String, options: &ShapingOptions, bidi_level: bidi::Level) -> TextRun {
+    /// Constructs a new text run. Also returns if there is a line break at the beginning
-        let glyphs = TextRun::break_and_shape(font, &text, options);
+    pub fn new(font: &mut Font, text: String, options: &ShapingOptions,
-        TextRun {
+               bidi_level: bidi::Level, breaker: &mut Option<LineBreakLeafIter>) -> (TextRun, bool) {
+        let (glyphs, break_at_zero) = TextRun::break_and_shape(font, &text, options, breaker);
+        (TextRun {
             text: Arc::new(text),
             font_metrics: font.metrics.clone(),
             font_template: font.handle.template(),
@@ -188,15 +190,35 @@ impl<'a> TextRun {
             glyphs: Arc::new(glyphs),
             bidi_level: bidi_level,
             extra_word_spacing: Au(0),
-        }
+        }, break_at_zero)
     }
 
-    pub fn break_and_shape(font: &mut Font, text: &str, options: &ShapingOptions)
+    pub fn break_and_shape(font: &mut Font, text: &str, options: &ShapingOptions,
-                           -> Vec<GlyphRun> {
+                           breaker: &mut Option<LineBreakLeafIter>) -> (Vec<GlyphRun>, bool) {
         let mut glyphs = vec!();
         let mut slice = 0..0;
 
-        for (idx, _is_hard_break) in LineBreakIterator::new(text) {
+        let mut finished = false;
+        let mut break_at_zero = false;
+
+        if breaker.is_none() {
+            if text.len() == 0 {
+                return (glyphs, true)
+            }
+            *breaker = Some(LineBreakLeafIter::new(&text, 0));
+        }
+
+        let breaker = breaker.as_mut().unwrap();
+
+        while !finished {
+            let (idx, _is_hard_break) = breaker.next(text);
+            if idx == text.len() {
+                finished = true;
+            }
+            if idx == 0 {
+                break_at_zero = true;
+            }
+
             // Extend the slice to the next UAX#14 line break opportunity.
             slice.end = idx;
             let word = &text[slice.clone()];
@@ -230,7 +252,7 @@ impl<'a> TextRun {
             }
             slice.start = whitespace.end;
         }
-        glyphs
+        (glyphs, break_at_zero)
     }
 
     pub fn ascent(&self) -> Au {
@@ -302,6 +324,14 @@ impl<'a> TextRun {
         })
     }
 
+    pub fn on_glyph_run_boundary(&self, index: ByteIndex) -> bool {
+        if let Some(glyph_index) = self.index_of_first_glyph_run_containing(index) {
+            self.glyphs[glyph_index].range.begin() == index
+        } else {
+            true
+        }
+    }
+
     /// Returns the index in the range of the first glyph advancing over given advance
     pub fn range_index_of_advance(&self, range: &Range<ByteIndex>, advance: Au) -> usize {
         // TODO(Issue #199): alter advance direction for RTL

components/layout/Cargo.toml

@@ -48,6 +48,8 @@ style_traits = {path = "../style_traits"}
 unicode-bidi = {version = "0.3", features = ["with_serde"]}
 unicode-script = {version = "0.1", features = ["harfbuzz"]}
 webrender_api = {git = "https://github.com/servo/webrender", features = ["ipc"]}
+xi-unicode = "0.1.0"
 
 [dev-dependencies]
 size_of_test = {path = "../size_of_test"}
+

components/layout/fragment.rs

@@ -479,6 +479,11 @@ bitflags! {
 
         /// Is this fragment selected?
         const SELECTED = 0x02;
+
+        /// Suppress line breaking between this and the previous fragment
+        ///
+        /// This handles cases like Foo<span>bar</span>
+        const SUPPRESS_LINE_BREAK_BEFORE = 0x04;
     }
 }
 
@@ -1421,6 +1426,14 @@ impl Fragment {
         }
     }
 
+    pub fn suppress_line_break_before(&self) -> bool {
+        match self.specific {
+            SpecificFragmentInfo::ScannedText(ref st) =>
+                st.flags.contains(ScannedTextFlags::SUPPRESS_LINE_BREAK_BEFORE),
+            _ => false,
+        }
+    }
+
     /// Computes the intrinsic inline-sizes of this fragment.
     pub fn compute_intrinsic_inline_sizes(&mut self) -> IntrinsicISizesContribution {
         let mut result = self.style_specified_intrinsic_inline_size();
@@ -1621,6 +1634,16 @@ impl Fragment {
         }
     }
 
+    /// Does this fragment start on a glyph run boundary?
+    pub fn is_on_glyph_run_boundary(&self) -> bool {
+        let text_fragment_info = match self.specific {
+            SpecificFragmentInfo::ScannedText(ref text_fragment_info)
+                => text_fragment_info,
+            _   => return true,
+        };
+        text_fragment_info.run.on_glyph_run_boundary(text_fragment_info.range.begin())
+    }
+
     /// Truncates this fragment to the given `max_inline_size`, using a character-based breaking
     /// strategy. The resulting fragment will have `SpecificFragmentInfo::TruncatedFragment`,
     /// preserving the original fragment for use in incremental reflow.

components/layout/inline.rs

@@ -554,7 +554,6 @@ impl LineBreaker {
                        layout_context: &LayoutContext) {
         // Undo any whitespace stripping from previous reflows.
         fragment.reset_text_range_and_inline_size();
-
         // Determine initial placement for the fragment if we need to.
         //
         // Also, determine whether we can legally break the line before, or
@@ -566,7 +565,21 @@ impl LineBreaker {
             self.pending_line.green_zone = line_bounds.size;
             false
         } else {
-            fragment.white_space().allow_wrap()
+            // In case of Foo<span style="...">bar</span>, the line breaker will
+            // set the "suppress line break before" flag for the second fragment.
+            //
+            // In case of Foo<span>bar</span> the second fragment ("bar") will
+            // start _within_ a glyph run, so we also avoid breaking there
+            //
+            // is_on_glyph_run_boundary does a binary search, but this is ok
+            // because the result will be cached and reused in
+            // `calculate_split_position` later
+            if fragment.suppress_line_break_before() ||
+               !fragment.is_on_glyph_run_boundary() {
+                false
+            } else {
+                fragment.white_space().allow_wrap()
+            }
         };
 
         debug!("LineBreaker: trying to append to line {} \

components/layout/lib.rs

@@ -42,6 +42,7 @@ extern crate style_traits;
 extern crate unicode_bidi;
 extern crate unicode_script;
 extern crate webrender_api;
+extern crate xi_unicode;
 
 #[macro_use]
 pub mod layout_debug;

components/layout/text.rs

@@ -32,6 +32,7 @@ use style::properties::style_structs;
 use style::values::generics::text::LineHeight;
 use unicode_bidi as bidi;
 use unicode_script::{Script, get_script};
+use xi_unicode::LineBreakLeafIter;
 
 /// Returns the concatenated text of a list of unscanned text fragments.
 fn text(fragments: &LinkedList<Fragment>) -> String {
@@ -91,6 +92,15 @@ impl TextRunScanner {
         let mut last_whitespace = false;
         let mut paragraph_bytes_processed = 0;
 
+        // The first time we process a text run we will set this
+        // linebreaker. There is no way for the linebreaker to start
+        // with an empty state; you must give it its first input immediately.
+        //
+        // This linebreaker is shared across text runs, so we can know if
+        // there is a break at the beginning of a text run or clump, e.g.
+        // in the case of FooBar<span>Baz</span>
+        let mut linebreaker = None;
+
         while !fragments.is_empty() {
             // Create a clump.
             split_first_fragment_at_newline_if_necessary(&mut fragments);
@@ -109,7 +119,8 @@ impl TextRunScanner {
                                                        &mut new_fragments,
                                                        &mut paragraph_bytes_processed,
                                                        bidi_levels,
-                                                       last_whitespace);
+                                                       last_whitespace,
+                                                       &mut linebreaker);
         }
 
         debug!("TextRunScanner: complete.");
@@ -129,7 +140,8 @@ impl TextRunScanner {
                            out_fragments: &mut Vec<Fragment>,
                            paragraph_bytes_processed: &mut usize,
                            bidi_levels: Option<&[bidi::Level]>,
-                           mut last_whitespace: bool)
+                           mut last_whitespace: bool,
+                           linebreaker: &mut Option<LineBreakLeafIter>)
                            -> bool {
         debug!("TextRunScanner: flushing {} fragments in range", self.clump.len());
 
@@ -309,22 +321,26 @@ impl TextRunScanner {
                 flags: flags,
             };
 
-            // FIXME(https://github.com/rust-lang/rust/issues/23338)
+            let mut result = Vec::with_capacity(run_info_list.len());
-            run_info_list.into_iter().map(|run_info| {
+            for run_info in run_info_list {
                 let mut options = options;
                 options.script = run_info.script;
                 if run_info.bidi_level.is_rtl() {
                     options.flags.insert(ShapingFlags::RTL_FLAG);
                 }
                 let mut font = fontgroup.fonts.get(run_info.font_index).unwrap().borrow_mut();
-                ScannedTextRun {
+
-                    run: Arc::new(TextRun::new(&mut *font,
+                let (run, break_at_zero) = TextRun::new(&mut *font,
-                                               run_info.text,
+                                                        run_info.text,
-                                               &options,
+                                                        &options,
-                                               run_info.bidi_level)),
+                                                        run_info.bidi_level,
+                                                        linebreaker);
+                result.push((ScannedTextRun {
+                    run: Arc::new(run),
                     insertion_point: run_info.insertion_point,
-                }
+                }, break_at_zero))
-            }).collect::<Vec<_>>()
+            }
+            result
         };
 
         // Make new fragments with the runs and adjusted text indices.
@@ -351,12 +367,17 @@ impl TextRunScanner {
                     }
                 };
                 let mapping = mappings.next().unwrap();
-                let scanned_run = runs[mapping.text_run_index].clone();
+                let (scanned_run, break_at_zero) = runs[mapping.text_run_index].clone();
 
                 let mut byte_range = Range::new(ByteIndex(mapping.byte_range.begin() as isize),
                                                 ByteIndex(mapping.byte_range.length() as isize));
 
                 let mut flags = ScannedTextFlags::empty();
+                if !break_at_zero && mapping.byte_range.begin() == 0 {
+                    // If this is the first segment of the text run,
+                    // and the text run doesn't break at zero, suppress line breaks
+                    flags.insert(ScannedTextFlags::SUPPRESS_LINE_BREAK_BEFORE)
+                }
                 let text_size = old_fragment.border_box.size;
 
                 let requires_line_break_afterward_if_wrapping_on_newlines =

tests/wpt/metadata/css/CSS2/box-display/insert-inline-in-blocks-n-inlines-end-003.xht.ini

@@ -0,0 +1,3 @@
+[insert-inline-in-blocks-n-inlines-end-003.xht]
+  type: reftest
+  expected: FAIL

tests/wpt/metadata/css/CSS2/linebox/inline-formatting-context-012.xht.ini

@@ -0,0 +1,3 @@
+[inline-formatting-context-012.xht]
+  type: reftest
+  expected: FAIL

tests/wpt/metadata/css/CSS2/linebox/vertical-align-baseline-009.xht.ini

@@ -1,3 +0,0 @@
-[vertical-align-baseline-009.xht]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata/css/CSS2/normal-flow/inline-block-width-002a.xht.ini

@@ -1,3 +1,3 @@
-[line-breaking-bidi-001.xht]
+[inline-block-width-002a.xht]
   type: reftest
   expected: FAIL

tests/wpt/metadata/css/CSS2/normal-flow/inline-block-width-002b.xht.ini

@@ -1,3 +1,3 @@
-[line-breaking-bidi-002.xht]
+[inline-block-width-002b.xht]
   type: reftest
   expected: FAIL

tests/wpt/metadata/css/CSS2/text/text-align-white-space-001.xht.ini

@@ -1,3 +0,0 @@
-[text-align-white-space-001.xht]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata/css/CSS2/text/text-align-white-space-005.xht.ini

@@ -1,3 +0,0 @@
-[text-align-white-space-005.xht]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata/css/CSS2/ui/overflow-applies-to-009.xht.ini

@@ -1,3 +0,0 @@
-[overflow-applies-to-009.xht]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata/css/CSS2/ui/overflow-applies-to-012.xht.ini

@@ -1,3 +0,0 @@
-[overflow-applies-to-012.xht]
-  type: reftest
-  expected: FAIL

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-baspglwj-083.html.ini

@@ -1,5 +1,4 @@
-[css3-text-line-break-baspglwj-122.html]
+[css3-text-line-break-baspglwj-083.html]
   type: testharness
   [ ]
     expected: FAIL
-

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-baspglwj-094.html.ini

@@ -0,0 +1,4 @@
+[css3-text-line-break-baspglwj-094.html]
+  type: testharness
+  [ ]
+    expected: FAIL

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-baspglwj-126.html.ini

@@ -1,5 +0,0 @@
-[css3-text-line-break-baspglwj-126.html]
-  type: testharness
-  [ ]
-    expected: FAIL
-

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-baspglwj-127.html.ini

@@ -1,5 +0,0 @@
-[css3-text-line-break-baspglwj-127.html]
-  type: testharness
-  [ ]
-    expected: FAIL
-

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-baspglwj-128.html.ini

@@ -1,5 +0,0 @@
-[css3-text-line-break-baspglwj-128.html]
-  type: testharness
-  [ ]
-    expected: FAIL
-

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-jazh-159.html.ini

@@ -1,4 +1,3 @@
 [css3-text-line-break-jazh-159.html]
   type: reftest
-  expected:
+  expected: FAIL
-    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-jazh-377.html.ini

@@ -1,4 +1,3 @@
 [css3-text-line-break-jazh-377.html]
   type: reftest
-  expected:
+  expected: FAIL
-    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-opclns-005.html.ini

@@ -1,3 +1,4 @@
 [css3-text-line-break-opclns-005.html]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "mac": FAIL

tests/wpt/metadata/css/css-text/i18n/css3-text-line-break-opclns-006.html.ini

@@ -1,3 +1,4 @@
 [css3-text-line-break-opclns-006.html]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-normal-021.xht.ini

@@ -1,3 +1,4 @@
 [line-break-normal-021.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-normal-022.xht.ini

@@ -1,3 +1,4 @@
 [line-break-normal-022.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-normal-023.xht.ini

@@ -1,3 +1,4 @@
 [line-break-normal-023.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-normal-024.xht.ini

@@ -1,3 +1,4 @@
 [line-break-normal-024.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-011.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-011.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-012.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-012.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-013.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-013.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-014.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-014.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-015.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-015.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-016.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-016.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-break/line-break-strict-017.xht.ini

@@ -1,3 +1,4 @@
 [line-break-strict-017.xht]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL

tests/wpt/metadata/css/css-text/line-breaking/line-breaking-009.html.ini

@@ -1,3 +1,3 @@
-[vertical-align-103.xht]
+[line-breaking-009.html]
   type: reftest
   expected: FAIL

tests/wpt/metadata/css/css-text/line-breaking/line-breaking-011.html.ini

@@ -1,3 +1,3 @@
-[vertical-align-104.xht]
+[line-breaking-011.html]
   type: reftest
   expected: FAIL

tests/wpt/mozilla/meta/css/whitespace_nowrap_line_breaking_a.html.ini

@@ -0,0 +1,3 @@
+[whitespace_nowrap_line_breaking_a.html]
+  type: reftest
+  expected: FAIL