Make text-align: justify incremental layout safe

components/gfx/paint_context.rs

@@ -1799,7 +1799,11 @@ impl ScaledFontExtensionMethods for ScaledFont {
 
         for slice in run.natural_word_slices_in_visual_order(range) {
             for glyph in slice.glyphs.iter_glyphs_for_byte_range(&slice.range) {
-                let glyph_advance = glyph.advance();
+                let glyph_advance = if glyph.char_is_space() {
+                    glyph.advance() + run.extra_word_spacing
+                } else {
+                    glyph.advance()
+                };
                 if !slice.glyphs.is_whitespace() {
                     let glyph_offset = glyph.offset().unwrap_or(Point2D::zero());
                     let azglyph = struct__AzGlyph {

components/gfx/text/glyph.rs

@@ -71,13 +71,14 @@ pub type GlyphId = u32;
 
 // TODO: make this more type-safe.
 
-const FLAG_CHAR_IS_SPACE: u32      = 0x40000000;
-const FLAG_IS_SIMPLE_GLYPH: u32    = 0x80000000;
+const FLAG_CHAR_IS_SPACE: u32       = 0x40000000;
+const FLAG_CHAR_IS_SPACE_SHIFT: u32 = 30;
+const FLAG_IS_SIMPLE_GLYPH: u32     = 0x80000000;
 
 // glyph advance; in Au's.
-const GLYPH_ADVANCE_MASK: u32      = 0x3FFF0000;
-const GLYPH_ADVANCE_SHIFT: u32     = 16;
-const GLYPH_ID_MASK: u32           = 0x0000FFFF;
+const GLYPH_ADVANCE_MASK: u32       = 0x3FFF0000;
+const GLYPH_ADVANCE_SHIFT: u32      = 16;
+const GLYPH_ID_MASK: u32            = 0x0000FFFF;
 
 // Non-simple glyphs (more than one glyph per char; missing glyph,
 // newline, tab, large advance, or nonzero x/y offsets) may have one
@@ -371,6 +372,15 @@ impl<'a> GlyphInfo<'a> {
             }
         }
     }
+
+    pub fn char_is_space(self) -> bool {
+        let (store, entry_i) = match self {
+            GlyphInfo::Simple(store, entry_i) => (store, entry_i),
+            GlyphInfo::Detail(store, entry_i, _) => (store, entry_i),
+        };
+
+        store.char_is_space(entry_i)
+    }
 }
 
 /// Stores the glyph data belonging to a text run.
@@ -406,6 +416,8 @@ pub struct GlyphStore {
 
     /// A cache of the advance of the entire glyph store.
     total_advance: Au,
+    /// A cache of the number of spaces in the entire glyph store.
+    total_spaces: i32,
 
     /// Used to check if fast path should be used in glyph iteration.
     has_detailed_glyphs: bool,
@@ -432,6 +444,7 @@ impl<'a> GlyphStore {
             entry_buffer: vec![GlyphEntry::initial(); length],
             detail_store: DetailedGlyphStore::new(),
             total_advance: Au(0),
+            total_spaces: 0,
             has_detailed_glyphs: false,
             is_whitespace: is_whitespace,
             is_rtl: is_rtl,
@@ -450,20 +463,21 @@ impl<'a> GlyphStore {
 
     pub fn finalize_changes(&mut self) {
         self.detail_store.ensure_sorted();
-        self.cache_total_advance()
+        self.cache_total_advance_and_spaces()
     }
 
     #[inline(never)]
-    fn cache_total_advance(&mut self) {
+    fn cache_total_advance_and_spaces(&mut self) {
         let mut total_advance = Au(0);
+        let mut total_spaces = 0;
         for glyph in self.iter_glyphs_for_byte_range(&Range::new(ByteIndex(0), self.len())) {
-            total_advance = total_advance + glyph.advance()
+            total_advance = total_advance + glyph.advance();
+            if glyph.char_is_space() {
+                total_spaces += 1;
+            }
         }
-        self.total_advance = total_advance
-    }
-
-    pub fn total_advance(&self) -> Au {
-        self.total_advance
+        self.total_advance = total_advance;
+        self.total_spaces = total_spaces;
     }
 
     /// Adds a single glyph.
@@ -538,57 +552,75 @@ impl<'a> GlyphStore {
     }
 
     #[inline]
-    pub fn advance_for_byte_range(&self, range: &Range<ByteIndex>) -> Au {
+    pub fn advance_for_byte_range(&self, range: &Range<ByteIndex>, extra_word_spacing: Au) -> Au {
         if range.begin() == ByteIndex(0) && range.end() == self.len() {
-            self.total_advance
+            self.total_advance + extra_word_spacing * self.total_spaces
         } else if !self.has_detailed_glyphs {
-            self.advance_for_byte_range_simple_glyphs(range)
+            self.advance_for_byte_range_simple_glyphs(range, extra_word_spacing)
         } else {
-            self.advance_for_byte_range_slow_path(range)
+            self.advance_for_byte_range_slow_path(range, extra_word_spacing)
         }
     }
 
     #[inline]
-    pub fn advance_for_byte_range_slow_path(&self, range: &Range<ByteIndex>) -> Au {
+    pub fn advance_for_byte_range_slow_path(&self, range: &Range<ByteIndex>, extra_word_spacing: Au) -> Au {
         self.iter_glyphs_for_byte_range(range)
-            .fold(Au(0), |advance, glyph| advance + glyph.advance())
+            .fold(Au(0), |advance, glyph| {
+                if glyph.char_is_space() {
+                    advance + glyph.advance() + extra_word_spacing
+                } else {
+                    advance + glyph.advance()
+                }
+            })
     }
 
     #[inline]
     #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
-    fn advance_for_byte_range_simple_glyphs(&self, range: &Range<ByteIndex>) -> Au {
-        let mask = u32x4::splat(GLYPH_ADVANCE_MASK);
+    fn advance_for_byte_range_simple_glyphs(&self, range: &Range<ByteIndex>, extra_word_spacing: Au) -> Au {
+        let advance_mask = u32x4::splat(GLYPH_ADVANCE_MASK);
+        let space_flag_mask = u32x4::splat(FLAG_CHAR_IS_SPACE);
         let mut simd_advance = u32x4::splat(0);
+        let mut simd_spaces = u32x4::splat(0);
         let begin = range.begin().to_usize();
         let len = range.length().to_usize();
         let num_simd_iterations = len / 4;
         let leftover_entries = range.end().to_usize() - (len - num_simd_iterations * 4);
         let buf = self.transmute_entry_buffer_to_u32_buffer();
 
         for i in 0..num_simd_iterations {
-            let mut v = u32x4::load(buf, begin + i * 4);
-            v = v & mask;
-            v = v >> GLYPH_ADVANCE_SHIFT;
-            simd_advance = simd_advance + v;
+            let v = u32x4::load(buf, begin + i * 4);
+            let advance = (v & advance_mask) >> GLYPH_ADVANCE_SHIFT;
+            let spaces = (v & space_flag_mask) >> FLAG_CHAR_IS_SPACE_SHIFT;
+            simd_advance = simd_advance + advance;
+            simd_spaces = simd_spaces + spaces;
         }
 
         let advance =
             (simd_advance.extract(0) +
              simd_advance.extract(1) +
              simd_advance.extract(2) +
              simd_advance.extract(3)) as i32;
-        let mut leftover = Au(0);
+        let spaces =
+            (simd_spaces.extract(0) +
+             simd_spaces.extract(1) +
+             simd_spaces.extract(2) +
+             simd_spaces.extract(3)) as i32;
+        let mut leftover_advance = Au(0);
+        let mut leftover_spaces = 0;
         for i in leftover_entries..range.end().to_usize() {
-            leftover = leftover + self.entry_buffer[i].advance();
+            leftover_advance = leftover_advance + self.entry_buffer[i].advance();
+            if self.entry_buffer[i].char_is_space() {
+                leftover_spaces += 1;
+            }
         }
-        Au(advance) + leftover
+        Au(advance) + leftover_advance + extra_word_spacing * (spaces + leftover_spaces)
     }
 
     /// When SIMD isn't available (non-x86_x64/aarch64), fallback to the slow path.
     #[inline]
     #[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))]
-    fn advance_for_byte_range_simple_glyphs(&self, range: &Range<ByteIndex>) -> Au {
-        self.advance_for_byte_range_slow_path(range)
+    fn advance_for_byte_range_simple_glyphs(&self, range: &Range<ByteIndex>, extra_word_spacing: Au) -> Au {
+        self.advance_for_byte_range_slow_path(range, extra_word_spacing)
     }
 
     /// Used for SIMD.
@@ -612,26 +644,6 @@ impl<'a> GlyphStore {
         }
         spaces
     }
-
-    pub fn distribute_extra_space_in_range(&mut self, range: &Range<ByteIndex>, space: f64) {
-        debug_assert!(space >= 0.0);
-        if range.is_empty() {
-            return
-        }
-        for index in range.each_index() {
-            // TODO(pcwalton): Handle spaces that are detailed glyphs -- these are uncommon but
-            // possible.
-            let entry = &mut self.entry_buffer[index.to_usize()];
-            if entry.is_simple() && entry.char_is_space() {
-                // FIXME(pcwalton): This can overflow for very large font-sizes.
-                let advance =
-                    ((entry.value & GLYPH_ADVANCE_MASK) >> GLYPH_ADVANCE_SHIFT) +
-                    Au::from_f64_px(space).0 as u32;
-                entry.value = (entry.value & !GLYPH_ADVANCE_MASK) |
-                    (advance << GLYPH_ADVANCE_SHIFT);
-            }
-        }
-    }
 }
 
 impl fmt::Debug for GlyphStore {

components/gfx/text/text_run.rs

@@ -33,6 +33,7 @@ pub struct TextRun {
     /// The glyph runs that make up this text run.
     pub glyphs: Arc<Vec<GlyphRun>>,
     pub bidi_level: u8,
+    pub extra_word_spacing: Au,
 }
 
 impl Drop for TextRun {
@@ -188,6 +189,7 @@ impl<'a> TextRun {
             actual_pt_size: font.actual_pt_size,
             glyphs: Arc::new(glyphs),
             bidi_level: bidi_level,
+            extra_word_spacing: Au(0),
         }
     }
 
@@ -247,7 +249,7 @@ impl<'a> TextRun {
         // TODO(Issue #98): using inter-char and inter-word spacing settings when measuring text
         self.natural_word_slices_in_range(range)
             .fold(Au(0), |advance, slice| {
-                advance + slice.glyphs.advance_for_byte_range(&slice.range)
+                advance + slice.glyphs.advance_for_byte_range(&slice.range, self.extra_word_spacing)
             })
     }
 
@@ -259,7 +261,7 @@ impl<'a> TextRun {
 
     pub fn metrics_for_slice(&self, glyphs: &GlyphStore, slice_range: &Range<ByteIndex>)
                              -> RunMetrics {
-        RunMetrics::new(glyphs.advance_for_byte_range(slice_range),
+        RunMetrics::new(glyphs.advance_for_byte_range(slice_range, self.extra_word_spacing),
                         self.font_metrics.ascent,
                         self.font_metrics.descent)
     }

components/layout/fragment.rs

@@ -1751,23 +1751,23 @@ impl Fragment {
     /// Restore any whitespace that was stripped from a text fragment, and recompute inline metrics
     /// if necessary.
     pub fn reset_text_range_and_inline_size(&mut self) {
-        match &mut self.specific {
-            &mut SpecificFragmentInfo::ScannedText(ref mut info) => {
-                // FIXME (mbrubeck): Do we need to restore leading too?
-                let range_end = info.range_end_including_stripped_whitespace;
-                if info.range.end() == range_end {
-                    return
-                }
-                info.range.extend_to(range_end);
-                info.content_size.inline = info.run.metrics_for_range(&info.range).advance_width;
-                self.border_box.size.inline = info.content_size.inline +
-                    self.border_padding.inline_start_end();
+        if let SpecificFragmentInfo::ScannedText(ref mut info) = self.specific {
+            if info.run.extra_word_spacing != Au(0) {
+                Arc::make_mut(&mut info.run).extra_word_spacing = Au(0);
             }
-            _ => {}
+
+            // FIXME (mbrubeck): Do we need to restore leading too?
+            let range_end = info.range_end_including_stripped_whitespace;
+            if info.range.end() == range_end {
+                return
+            }
+            info.range.extend_to(range_end);
+            info.content_size.inline = info.run.metrics_for_range(&info.range).advance_width;
+            self.border_box.size.inline = info.content_size.inline +
+                self.border_padding.inline_start_end();
         }
     }
 
-
     /// Assigns replaced inline-size, padding, and margins for this fragment only if it is replaced
     /// content per CSS 2.1 § 10.3.2.
     pub fn assign_replaced_inline_size_if_necessary(&mut self, container_inline_size: Au) {

components/layout/inline.rs

@@ -961,7 +961,7 @@ impl InlineFlow {
         }
 
         // First, calculate the number of expansion opportunities (spaces, normally).
-        let mut expansion_opportunities = 0i32;
+        let mut expansion_opportunities = 0;
         for fragment_index in line.range.each_index() {
             let fragment = fragments.get(fragment_index.to_usize());
             let scanned_text_fragment_info = match fragment.specific {
@@ -971,39 +971,25 @@ impl InlineFlow {
             let fragment_range = scanned_text_fragment_info.range;
 
             for slice in scanned_text_fragment_info.run.character_slices_in_range(&fragment_range) {
-                expansion_opportunities += slice.glyphs.space_count_in_range(&slice.range) as i32
+                expansion_opportunities += slice.glyphs.space_count_in_range(&slice.range)
             }
         }
 
+        if expansion_opportunities == 0 {
+            return
+        }
+
         // Then distribute all the space across the expansion opportunities.
-        let space_per_expansion_opportunity = slack_inline_size.to_f64_px() /
-            (expansion_opportunities as f64);
+        let space_per_expansion_opportunity = slack_inline_size / expansion_opportunities as i32;
         for fragment_index in line.range.each_index() {
             let fragment = fragments.get_mut(fragment_index.to_usize());
             let mut scanned_text_fragment_info = match fragment.specific {
                 SpecificFragmentInfo::ScannedText(ref mut info) if !info.range.is_empty() => info,
                 _ => continue
             };
             let fragment_range = scanned_text_fragment_info.range;
-
-            // FIXME(pcwalton): This is an awful lot of uniqueness making. I don't see any easy way
-            // to get rid of it without regressing the performance of the non-justified case,
-            // though.
             let run = Arc::make_mut(&mut scanned_text_fragment_info.run);
-            {
-                let glyph_runs = Arc::make_mut(&mut run.glyphs);
-                for mut glyph_run in &mut *glyph_runs {
-                    let mut range = glyph_run.range.intersect(&fragment_range);
-                    if range.is_empty() {
-                        continue
-                    }
-                    range.shift_by(-glyph_run.range.begin());
-
-                    let glyph_store = Arc::make_mut(&mut glyph_run.glyph_store);
-                    glyph_store.distribute_extra_space_in_range(&range,
-                                                                space_per_expansion_opportunity);
-                }
-            }
+            run.extra_word_spacing = space_per_expansion_opportunity;
 
             // Recompute the fragment's border box size.
             let new_inline_size = run.advance_for_range(&fragment_range);

components/layout/webrender_helpers.rs

@@ -393,7 +393,11 @@ impl WebRenderDisplayItemConverter for DisplayItem {
 
                 for slice in item.text_run.natural_word_slices_in_visual_order(&item.range) {
                     for glyph in slice.glyphs.iter_glyphs_for_byte_range(&slice.range) {
-                        let glyph_advance = glyph.advance();
+                        let glyph_advance = if glyph.char_is_space() {
+                            glyph.advance() + item.text_run.extra_word_spacing
+                        } else {
+                            glyph.advance()
+                        };
                         if !slice.glyphs.is_whitespace() {
                             let glyph_offset = glyph.offset().unwrap_or(Point2D::zero());
                             let glyph = webrender_traits::GlyphInstance {