From 5dda97d1875562db0fd05201cbe2d8fe279aef2b Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 6 Feb 2024 17:33:22 +0100 Subject: [PATCH] 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 | 68 ++++++++++++--------------- components/layout_2020/flow/line.rs | 42 ++++------------- 2 files changed, 38 insertions(+), 72 deletions(-) diff --git a/components/layout_2020/flow/inline.rs b/components/layout_2020/flow/inline.rs index 6c36f009711..1873843f58f 100644 --- a/components/layout_2020/flow/inline.rs +++ b/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, - - /// 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::(), + ), + _ => 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 => { - if line_left_is_inline_start { - TextAlign::Start - } else { - TextAlign::End - } - }, - TextAlignKeyword::Right => { - if line_left_is_inline_start { - TextAlign::End - } else { - TextAlign::Start - } - }, + TextAlignKeyword::Left if line_left_is_inline_start => TextAlign::Start, + TextAlignKeyword::Left => TextAlign::End, + TextAlignKeyword::Right if line_left_is_inline_start => TextAlign::End, + TextAlignKeyword::Right => 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. => { - (available_space - line_length) / num_justification_opportunities + (TextAlignKeyword::Justify, _) => { + 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; diff --git a/components/layout_2020/flow/line.rs b/components/layout_2020/flow/line.rs index b36b9fdb4c0..73203ca890e 100644 --- a/components/layout_2020/flow/line.rs +++ b/components/layout_2020/flow/line.rs @@ -117,15 +117,9 @@ pub(super) enum LineItem { } impl LineItem { - pub(super) fn trim_whitespace_at_end( - &mut self, - whitespace_trimmed: &mut Length, - word_seperators_trimmed: &mut usize, - ) -> bool { + pub(super) fn trim_whitespace_at_end(&mut self, whitespace_trimmed: &mut Length) -> bool { match self { - LineItem::TextRun(ref mut item) => { - item.trim_whitespace_at_end(whitespace_trimmed, word_seperators_trimmed) - }, + LineItem::TextRun(ref mut item) => item.trim_whitespace_at_end(whitespace_trimmed), LineItem::StartInlineBox(_) => true, LineItem::EndInlineBox => true, LineItem::Atomic(_) => false, @@ -134,15 +128,9 @@ impl LineItem { } } - pub(super) fn trim_whitespace_at_start( - &mut self, - whitespace_trimmed: &mut Length, - word_separators_trimmed: &mut usize, - ) -> bool { + pub(super) fn trim_whitespace_at_start(&mut self, whitespace_trimmed: &mut Length) -> bool { match self { - LineItem::TextRun(ref mut item) => { - item.trim_whitespace_at_start(whitespace_trimmed, word_separators_trimmed) - }, + LineItem::TextRun(ref mut item) => item.trim_whitespace_at_start(whitespace_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( - &mut self, - whitespace_trimmed: &mut Length, - word_seperators_trimmed: &mut usize, - ) -> bool { + fn trim_whitespace_at_end(&mut self, whitespace_trimmed: &mut Length) -> bool { if self .parent_style .get_inherited_text() @@ -187,21 +171,14 @@ impl TextRunLineItem { *whitespace_trimmed += self .text .drain(first_whitespace_index..) - .map(|glyph| { - *word_seperators_trimmed += glyph.total_word_separators(); - Length::from(glyph.total_advance()) - }) + .map(|glyph| 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( - &mut self, - whitespace_trimmed: &mut Length, - word_separators_trimmed: &mut usize, - ) -> bool { + fn trim_whitespace_at_start(&mut self, whitespace_trimmed: &mut Length) -> 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| { - *word_separators_trimmed += glyph.total_word_separators(); - Length::from(glyph.total_advance()) - }) + .map(|glyph| Length::from(glyph.total_advance())) .sum(); // Only keep going if we only encountered whitespace.