layout: Wait to count justification opportunities until justification (#31236)

Instead of tracking justification opportunities during line layout, wait
until the line is about to be laid out and justification is about
happen. This makes the logic for tracking justification opportunities
simpler. In particular, we no longer have to carefully adjust them when
trimming whitespace. Additionally, this avoids a bit of work unless
justification is turned on.

This also includes a small cleanup of the justification code.

components/layout_2020/flow/inline.rs

@@ -124,9 +124,6 @@ struct LineUnderConstruction {
     /// The LineItems for the current line under construction that have already
     /// been committed to this line.
     line_items: Vec<LineItem>,
-
-    /// The number of justification opportunities in this line.
-    justification_opportunities: usize,
 }
 
 impl LineUnderConstruction {
@@ -139,7 +136,6 @@ impl LineUnderConstruction {
             has_floats_waiting_to_be_placed: false,
             placement_among_floats: OnceCell::new(),
             line_items: Vec::new(),
-            justification_opportunities: 0,
         }
     }
 
@@ -162,16 +158,31 @@ impl LineUnderConstruction {
         // >    as well as any trailing U+1680   OGHAM SPACE MARK whose white-space
         // >    property is normal, nowrap, or pre-line.
         let mut whitespace_trimmed = Length::zero();
-        let mut word_seperators_trimmed = 0;
         for item in self.line_items.iter_mut().rev() {
-            if !item.trim_whitespace_at_end(&mut whitespace_trimmed, &mut word_seperators_trimmed) {
+            if !item.trim_whitespace_at_end(&mut whitespace_trimmed) {
                 break;
             }
         }
 
-        self.justification_opportunities -= word_seperators_trimmed;
         whitespace_trimmed
     }
+
+    /// Count the number of justification opportunities in this line.
+    fn count_justification_opportunities(&self) -> usize {
+        self.line_items
+            .iter()
+            .filter_map(|item| match item {
+                LineItem::TextRun(text_run) => Some(
+                    text_run
+                        .text
+                        .iter()
+                        .map(|glyph_store| glyph_store.total_word_separators())
+                        .sum::<usize>(),
+                ),
+                _ => None,
+            })
+            .sum()
+    }
 }
 
 /// A block size relative to a line's final baseline. This is to track the size
@@ -326,9 +337,6 @@ struct UnbreakableSegmentUnderConstruction {
 
     /// The inline size of any trailing whitespace in this segment.
     trailing_whitespace_size: Length,
-
-    /// The number of justification opportunities in this unbreakable segment.
-    justification_opportunities: usize,
 }
 
 impl UnbreakableSegmentUnderConstruction {
@@ -344,7 +352,6 @@ impl UnbreakableSegmentUnderConstruction {
             inline_box_hierarchy_depth: None,
             has_content: false,
             trailing_whitespace_size: Length::zero(),
-            justification_opportunities: 0,
         }
     }
 
@@ -356,7 +363,6 @@ impl UnbreakableSegmentUnderConstruction {
         self.inline_box_hierarchy_depth = None;
         self.has_content = false;
         self.trailing_whitespace_size = Length::zero();
-        self.justification_opportunities = 0;
     }
 
     /// Push a single line item to this segment. In addition, record the inline box
@@ -382,15 +388,12 @@ impl UnbreakableSegmentUnderConstruction {
     /// This prevents whitespace from being added to the beginning of a line.
     fn trim_leading_whitespace(&mut self) {
         let mut whitespace_trimmed = Length::zero();
-        let mut word_seperators_trimmed = 0;
         for item in self.line_items.iter_mut() {
-            if !item.trim_whitespace_at_start(&mut whitespace_trimmed, &mut word_seperators_trimmed)
+            if !item.trim_whitespace_at_start(&mut whitespace_trimmed) {
-            {
                 break;
             }
         }
         self.inline_size -= whitespace_trimmed;
-        self.justification_opportunities -= word_seperators_trimmed;
     }
 
     /// Prepare this segment for placement on a new and empty line. This happens when the
@@ -829,20 +832,10 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
             TextAlignKeyword::Start => TextAlign::Start,
             TextAlignKeyword::Center => TextAlign::Center,
             TextAlignKeyword::End => TextAlign::End,
-            TextAlignKeyword::Left => {
+            TextAlignKeyword::Left if line_left_is_inline_start => TextAlign::Start,
-                if line_left_is_inline_start {
+            TextAlignKeyword::Left => TextAlign::End,
-                    TextAlign::Start
+            TextAlignKeyword::Right if line_left_is_inline_start => TextAlign::End,
-                } else {
+            TextAlignKeyword::Right => TextAlign::Start,
-                    TextAlign::End
-                }
-            },
-            TextAlignKeyword::Right => {
-                if line_left_is_inline_start {
-                    TextAlign::End
-                } else {
-                    TextAlign::Start
-                }
-            },
             TextAlignKeyword::Justify => TextAlign::Start,
             TextAlignKeyword::ServoCenter |
             TextAlignKeyword::ServoLeft |
@@ -878,14 +871,18 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
         // Calculate the justification adjustment. This is simply the remaining space on the line,
         // dividided by the number of justficiation opportunities that we recorded when building
         // the line.
-        let num_justification_opportunities = self.current_line.justification_opportunities as f32;
         let text_justify = self.containing_block.style.clone_text_justify();
         let justification_adjustment = match (text_align_keyword, text_justify) {
             // `text-justify: none` should disable text justification.
             // TODO: Handle more `text-justify` values.
             (TextAlignKeyword::Justify, TextJustify::None) => Length::zero(),
-            (TextAlignKeyword::Justify, _) if num_justification_opportunities > 0. => {
+            (TextAlignKeyword::Justify, _) => {
-                (available_space - line_length) / num_justification_opportunities
+                match self.current_line.count_justification_opportunities() {
+                    0 => Length::zero(),
+                    num_justification_opportunities => {
+                        (available_space - line_length) / (num_justification_opportunities as f32)
+                    },
+                }
             },
             _ => Length::zero(),
         };
@@ -1129,9 +1126,6 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
         font_metrics: &FontMetrics,
         font_key: FontInstanceKey,
     ) {
-        self.current_line_segment.justification_opportunities +=
-            glyph_store.total_word_separators();
-
         let inline_advance = Length::from(glyph_store.total_advance());
         let preserve_spaces = parent_style
             .get_inherited_text()
@@ -1262,8 +1256,6 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
         self.current_line.max_block_size = self
             .current_line_max_block_size_including_nested_containers()
             .max(&self.current_line_segment.max_block_size);
-        self.current_line.justification_opportunities +=
-            self.current_line_segment.justification_opportunities;
         let line_inline_size_without_trailing_whitespace =
             self.current_line.inline_position - self.current_line_segment.trailing_whitespace_size;
 

components/layout_2020/flow/line.rs

@@ -117,15 +117,9 @@ pub(super) enum LineItem {
 }
 
 impl LineItem {
-    pub(super) fn trim_whitespace_at_end(
+    pub(super) fn trim_whitespace_at_end(&mut self, whitespace_trimmed: &mut Length) -> bool {
-        &mut self,
-        whitespace_trimmed: &mut Length,
-        word_seperators_trimmed: &mut usize,
-    ) -> bool {
         match self {
-            LineItem::TextRun(ref mut item) => {
+            LineItem::TextRun(ref mut item) => item.trim_whitespace_at_end(whitespace_trimmed),
-                item.trim_whitespace_at_end(whitespace_trimmed, word_seperators_trimmed)
-            },
             LineItem::StartInlineBox(_) => true,
             LineItem::EndInlineBox => true,
             LineItem::Atomic(_) => false,
@@ -134,15 +128,9 @@ impl LineItem {
         }
     }
 
-    pub(super) fn trim_whitespace_at_start(
+    pub(super) fn trim_whitespace_at_start(&mut self, whitespace_trimmed: &mut Length) -> bool {
-        &mut self,
-        whitespace_trimmed: &mut Length,
-        word_separators_trimmed: &mut usize,
-    ) -> bool {
         match self {
-            LineItem::TextRun(ref mut item) => {
+            LineItem::TextRun(ref mut item) => item.trim_whitespace_at_start(whitespace_trimmed),
-                item.trim_whitespace_at_start(whitespace_trimmed, word_separators_trimmed)
-            },
             LineItem::StartInlineBox(_) => true,
             LineItem::EndInlineBox => true,
             LineItem::Atomic(_) => false,
@@ -162,11 +150,7 @@ pub(super) struct TextRunLineItem {
 }
 
 impl TextRunLineItem {
-    fn trim_whitespace_at_end(
+    fn trim_whitespace_at_end(&mut self, whitespace_trimmed: &mut Length) -> bool {
-        &mut self,
-        whitespace_trimmed: &mut Length,
-        word_seperators_trimmed: &mut usize,
-    ) -> bool {
         if self
             .parent_style
             .get_inherited_text()
@@ -187,21 +171,14 @@ impl TextRunLineItem {
         *whitespace_trimmed += self
             .text
             .drain(first_whitespace_index..)
-            .map(|glyph| {
+            .map(|glyph| Length::from(glyph.total_advance()))
-                *word_seperators_trimmed += glyph.total_word_separators();
-                Length::from(glyph.total_advance())
-            })
             .sum();
 
         // Only keep going if we only encountered whitespace.
         index_of_last_non_whitespace.is_none()
     }
 
-    fn trim_whitespace_at_start(
+    fn trim_whitespace_at_start(&mut self, whitespace_trimmed: &mut Length) -> bool {
-        &mut self,
-        whitespace_trimmed: &mut Length,
-        word_separators_trimmed: &mut usize,
-    ) -> bool {
         if self
             .parent_style
             .get_inherited_text()
@@ -220,10 +197,7 @@ impl TextRunLineItem {
         *whitespace_trimmed += self
             .text
             .drain(0..index_of_first_non_whitespace)
-            .map(|glyph| {
+            .map(|glyph| Length::from(glyph.total_advance()))
-                *word_separators_trimmed += glyph.total_word_separators();
-                Length::from(glyph.total_advance())
-            })
             .sum();
 
         // Only keep going if we only encountered whitespace.