Improve line box block size calculation (#30519)

Improve the calculation of the block size of line boxes and all their
component elements. Even empty spans can increase the size of the line
based on their font-size. Elements that have a line-height should
increase the block size of the line, but that setting should not effect
their own size.

In addition to the new passes there are some new failures

Failing because a progression exposes the real issue these tests are
testing:

- css/css-color/t32-opacity-offscreen-multiple-boxes-1-c.xht
- css/css-color/t32-opacity-offscreen-multiple-boxes-2-c.xht

Likely failing because of vertical-align and another sizing issue:

- css/css-transforms/perspective-untransformable-no-stacking-context.html

Failing because a progression reveals another failure:

 - html/rendering/non-replaced-elements/hidden-elements.html

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
This commit is contained in:
Martin Robinson 2023-10-18 11:35:19 +02:00 committed by GitHub
parent b21952b6c4
commit 8a12b4c957
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 173 additions and 116 deletions

View file

@ -108,14 +108,11 @@ struct LineUnderConstruction {
/// is trimmed from the end.
trailing_whitespace_advance: Length,
/// The currently calculated block size of this line, taking into account all inline
/// content already laid out into [`LineItem`]s. Later content may increase the block
/// size.
block_size: Length,
/// The maximum block size of all boxes that ended and are in progress in this line.
max_block_size: Length,
/// Whether any active linebox has added a glyph, border, margin, or padding
/// to this line, which indicates that the next run that exceeds the line length
/// can cause a line break.
/// Whether any active linebox has added a glyph or atomic element to this line, which
/// indicates that the next run that exceeds the line length can cause a line break.
has_content: bool,
/// Whether or not there are floats that did not fit on the current line. Before
@ -136,7 +133,7 @@ impl LineUnderConstruction {
inline_position: start_position.inline.clone(),
trailing_whitespace_advance: Length::zero(),
start_position: start_position,
block_size: Length::zero(),
max_block_size: Length::zero(),
has_content: false,
has_floats_waiting_to_be_placed: false,
placement_among_floats: OnceCell::new(),
@ -170,6 +167,12 @@ struct InlineContainerState {
// "When specified on or propagated to a block container that establishes
// an IFC..."
text_decoration_line: TextDecorationLine,
/// The block size of this inline container maxed with the block sizes of all inline
/// container ancestors. This isn't the block size of this container, but if this
/// container adds content to a line, this is the block size necessary for that new
/// content.
nested_block_size: Length,
}
struct InlineBoxContainerState {
@ -197,6 +200,7 @@ struct InlineFormattingContextState<'a, 'b> {
positioning_context: &'a mut PositioningContext,
containing_block: &'b ContainingBlock<'b>,
sequential_layout_state: Option<&'a mut SequentialLayoutState>,
layout_context: &'b LayoutContext<'b>,
/// A vector of fragment that are laid out. This includes one [`Fragment::Anonymous`]
/// per line that is currently laid out plus fragments for all floats, which
@ -207,6 +211,26 @@ struct InlineFormattingContextState<'a, 'b> {
/// [`LineItem`]s themselves are stored in the nesting state.
current_line: LineUnderConstruction,
/// After a forced line break (for instance from a `<br>` element) we wait to actually
/// break the line until seeing more content. This allows ongoing inline boxes to finish,
/// since in the case where they have no more content they should not be on the next
/// line.
///
/// For instance:
///
/// ``` html
/// <span style="border-right: 30px solid blue;">
/// first line<br>
/// </span>
/// second line
/// ```
///
/// In this case, the `<span>` should not extend to the second line. If we linebreak
/// as soon as we encounter the `<br>` the `<span>`'s ending inline borders would be
/// placed on the second line, because we add those borders in
/// [`InlineFormattingContextState::finish_inline_box()`].
linebreak_before_new_content: bool,
/// The line breaking state for this inline formatting context.
linebreaker: Option<LineBreakLeafIter>,
@ -238,9 +262,10 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
self.current_line.has_content = true;
self.current_line.inline_position += inline_size;
self.current_line.trailing_whitespace_advance = last_whitespace_advance;
self.current_line
.block_size
.max_assign(line_item.block_size());
self.current_line.max_block_size.max_assign(
self.current_line_max_block_size()
.max(line_item.block_size()),
);
let current_nesting_level = self.current_inline_container_state_mut();
current_nesting_level.line_items_so_far.push(line_item);
@ -262,6 +287,12 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
}
}
fn current_line_max_block_size(&self) -> Length {
self.current_inline_container_state()
.nested_block_size
.max(self.current_line.max_block_size)
}
fn propagate_current_nesting_level_white_space_style(&mut self) {
let style = match self.inline_box_state_stack.last() {
Some(inline_box_state) => &inline_box_state.style,
@ -273,13 +304,19 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
/// Start laying out a particular [`InlineBox`] into line items. This will push
/// a new [`InlineBoxContainerState`] onto [`Self::inline_box_state_stack`].
fn start_inline_box(&mut self, inline_box: &InlineBox) {
let text_decoration_of_parent = self.current_inline_container_state().text_decoration_line;
let (text_decoration_of_parent, nested_block_size_of_parent) = {
let parent = self.current_inline_container_state();
(parent.text_decoration_line, parent.nested_block_size)
};
self.inline_box_state_stack
.push(InlineBoxContainerState::new(
inline_box,
&mut self.current_line.inline_position,
&self.containing_block,
text_decoration_of_parent,
nested_block_size_of_parent,
self.layout_context,
));
}
@ -295,10 +332,14 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
// We reached the end of the remaining boxes in this nesting level, so we finish it and
// start working on the parent nesting level again.
let line_item = inline_box_state.layout_into_line_item(
self.layout_context,
&mut self.current_line.inline_position,
true, /* at_end_of_inline_element */
);
self.current_line
.max_block_size
.max_assign(inline_box_state.base.nested_block_size);
self.current_inline_container_state_mut()
.line_items_so_far
.push(LineItem::InlineBox(line_item));
@ -316,6 +357,8 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
/// [`LineItem`]s and turn them into [`Fragment`]s, then reset the
/// [`InlineFormattingContextState`] preparing it for laying out a new line.
fn finish_current_line_and_reset(&mut self, layout_context: &LayoutContext) {
self.linebreak_before_new_content = false;
let mut line_item_from_child = None;
for inline_box_state in self.inline_box_state_stack.iter_mut().rev() {
if let Some(line_item_from_child) = line_item_from_child {
@ -325,10 +368,11 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
.push(LineItem::InlineBox(line_item_from_child));
}
line_item_from_child = Some(
inline_box_state
.layout_into_line_item(&mut self.current_line.inline_position, false),
);
line_item_from_child = Some(inline_box_state.layout_into_line_item(
self.layout_context,
&mut self.current_line.inline_position,
false,
));
}
if let Some(line_item_from_child) = line_item_from_child {
@ -354,7 +398,15 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
let block_start_position = self
.current_line
.line_block_start_considering_placement_among_floats();
let block_end_position = block_start_position + self.current_line.block_size;
let had_inline_advance =
self.current_line.inline_position != self.current_line.start_position.inline;
let effective_block_advance = if self.current_line.has_content || had_inline_advance {
self.current_line_max_block_size()
} else {
Length::zero()
};
let block_end_position = block_start_position + effective_block_advance;
if let Some(sequential_layout_state) = self.sequential_layout_state.as_mut() {
// This amount includes both the block size of the line and any extra space
@ -369,7 +421,6 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
let mut state = LineItemLayoutState {
inline_position: inline_start_position,
max_block_size: Length::zero(),
inline_start_of_parent: Length::zero(),
ifc_containing_block: self.containing_block,
positioning_context: &mut self.positioning_context,
@ -381,7 +432,7 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
let size = LogicalVec2 {
inline: self.containing_block.inline_size,
block: state.max_block_size,
block: effective_block_advance,
};
// The inline part of this start offset was taken into account when determining
@ -806,15 +857,18 @@ impl InlineFormattingContext {
positioning_context,
containing_block,
sequential_layout_state,
layout_context,
fragments: Vec::new(),
current_line: LineUnderConstruction::new(LogicalVec2 {
inline: first_line_inline_start,
block: Length::zero(),
}),
linebreak_before_new_content: false,
white_space: containing_block.style.get_inherited_text().white_space,
linebreaker: None,
root_nesting_level: InlineContainerState {
line_items_so_far: Vec::with_capacity(self.inline_level_boxes.len()),
nested_block_size: line_height_from_style(layout_context, &containing_block.style),
has_content: false,
text_decoration_line: self.text_decoration_line,
},
@ -832,7 +886,13 @@ impl InlineFormattingContext {
let mut iterator = InlineBoxChildIter::from_formatting_context(self);
let mut parent_iterators = Vec::new();
loop {
match iterator.next() {
let next = iterator.next();
if next.is_some() {
if ifc.linebreak_before_new_content {
ifc.finish_current_line_and_reset(layout_context);
}
}
match next {
Some(child) => match &mut *child.borrow_mut() {
InlineLevelBox::InlineBox(inline_box) => {
ifc.start_inline_box(inline_box);
@ -919,6 +979,8 @@ impl InlineBoxContainerState {
inline_position: &mut Length,
containing_block: &ContainingBlock,
text_decoration_of_parent: TextDecorationLine,
nested_block_size_of_parent: Length,
layout_context: &LayoutContext,
) -> Self {
let style = inline_box.style.clone();
let mut pbm = style.padding_border_margin(containing_block);
@ -939,6 +1001,8 @@ impl InlineBoxContainerState {
line_items_so_far: Vec::with_capacity(inline_box.children.len()),
has_content: false,
text_decoration_line,
nested_block_size: nested_block_size_of_parent
.max(line_height_from_style(layout_context, &style)),
},
style,
base_fragment_info: inline_box.base_fragment_info,
@ -949,6 +1013,7 @@ impl InlineBoxContainerState {
fn layout_into_line_item(
&mut self,
layout_context: &LayoutContext,
inline_position: &mut Length,
at_end_of_inline_element: bool,
) -> InlineBoxLineItem {
@ -968,6 +1033,7 @@ impl InlineBoxContainerState {
let new_line_item = InlineBoxLineItem {
base_fragment_info: self.base_fragment_info,
style: self.style.clone(),
block_size: line_gap_from_style(layout_context, &self.style),
pbm,
children: std::mem::take(&mut self.base.line_items_so_far),
always_make_fragment: !self.was_part_of_previous_line,
@ -1112,7 +1178,7 @@ impl IndependentFormattingContext {
let size = &pbm_sums.sum() + &fragment.content_rect.size;
let new_potential_line_size = LogicalVec2 {
inline: ifc.current_line.inline_position + size.inline,
block: ifc.current_line.block_size.max(size.block),
block: ifc.current_line_max_block_size().max(size.block),
};
if ifc.new_potential_line_size_causes_line_break(&new_potential_line_size) {
@ -1255,58 +1321,61 @@ impl TextRun {
} = self.break_and_shape(layout_context, &mut ifc.linebreaker);
let white_space = self.parent_style.get_inherited_text().white_space;
let add_glyphs_to_current_line =
|ifc: &mut InlineFormattingContextState,
glyphs: Vec<std::sync::Arc<GlyphStore>>,
inline_advance,
force_text_run_creation: bool| {
if !force_text_run_creation && glyphs.is_empty() {
return;
}
let add_glyphs_to_current_line = |ifc: &mut InlineFormattingContextState,
glyphs: Vec<std::sync::Arc<GlyphStore>>,
inline_advance| {
if glyphs.is_empty() {
return;
}
let last_whitespace_advance = match (white_space.preserve_spaces(), glyphs.last()) {
(false, Some(last_glyph)) if last_glyph.is_whitespace() => {
last_glyph.total_advance()
},
_ => Au::zero(),
};
ifc.push_line_item(
inline_advance,
LineItem::TextRun(TextRunLineItem {
text: glyphs,
base_fragment_info: self.base_fragment_info.into(),
parent_style: self.parent_style.clone(),
font_metrics,
font_key,
text_decoration_line: ifc
.current_inline_container_state()
.text_decoration_line,
}),
Length::from(last_whitespace_advance),
);
let last_whitespace_advance = match (white_space.preserve_spaces(), glyphs.last()) {
(false, Some(last_glyph)) if last_glyph.is_whitespace() => {
last_glyph.total_advance()
},
_ => Au::zero(),
};
ifc.push_line_item(
inline_advance,
LineItem::TextRun(TextRunLineItem {
text: glyphs,
base_fragment_info: self.base_fragment_info.into(),
parent_style: self.parent_style.clone(),
font_metrics,
font_key,
text_decoration_line: ifc.current_inline_container_state().text_decoration_line,
}),
Length::from(last_whitespace_advance),
);
};
let line_height = line_height(&self.parent_style, &font_metrics);
let new_max_height_of_line = ifc.current_line.block_size.max(line_height);
let new_max_height_of_line = ifc.current_line_max_block_size().max(line_height);
let mut glyphs = vec![];
let mut text_run_inline_size = Length::zero();
let mut iterator = runs.iter().enumerate();
while let Some((run_index, run)) = iterator.next() {
// If this whitespace forces a line break, finish the line and reset everything.
if self.glyph_run_is_whitespace_ending_with_preserved_newline(run) {
// TODO: We shouldn't need to force the creation of a TextRun here, but only TextRuns are
// influencing line height calculation of lineboxes (and not all inline boxes on a line).
// Once that is fixed, we can avoid adding an empty TextRun here.
add_glyphs_to_current_line(
ifc,
glyphs.drain(..).collect(),
text_run_inline_size,
true,
);
if ifc.linebreak_before_new_content {
ifc.finish_current_line_and_reset(layout_context);
text_run_inline_size = Length::zero();
}
// If this whitespace forces a line break, finish the line and reset everything.
if self.glyph_run_is_whitespace_ending_with_preserved_newline(run) {
add_glyphs_to_current_line(ifc, glyphs.drain(..).collect(), text_run_inline_size);
// We need to ensure that the appropriate space for a linebox is created even if there
// was no other content on this line. We mark the line as having content (needing a
// advance) and having at least the height associated with this nesting of inline boxes.
ifc.current_line.has_content = true;
ifc.current_line
.max_block_size
.max_assign(new_max_height_of_line);
// Defer the actual line break until we've cleared all ending inline boxes.
ifc.linebreak_before_new_content = true;
continue;
}
@ -1353,7 +1422,6 @@ impl TextRun {
ifc,
glyphs.drain(..).collect(),
text_run_inline_size,
true,
);
ifc.finish_current_line_and_reset(layout_context);
text_run_inline_size = Length::zero();
@ -1375,7 +1443,7 @@ impl TextRun {
ifc.propagate_current_nesting_level_white_space_style();
}
add_glyphs_to_current_line(ifc, glyphs.drain(..).collect(), text_run_inline_size, false);
add_glyphs_to_current_line(ifc, glyphs.drain(..).collect(), text_run_inline_size);
}
}
@ -1420,7 +1488,7 @@ impl FloatBox {
// start position.
let new_placement = ifc.place_line_among_floats(&LogicalVec2 {
inline: ifc.current_line.inline_position,
block: ifc.current_line.block_size,
block: ifc.current_line.max_block_size,
});
ifc.current_line
.replace_placement_among_floats(new_placement);
@ -1490,7 +1558,6 @@ impl<'box_tree> Iterator for InlineBoxChildIter<'box_tree> {
/// laid out.
struct LineItemLayoutState<'a> {
inline_position: Length,
max_block_size: Length,
/// The inline start position of the parent (the inline box that established this state)
/// relative to the edge of the containing block of this [`InlineFormattingCotnext`].
@ -1600,15 +1667,39 @@ struct TextRunLineItem {
text_decoration_line: TextDecorationLine,
}
fn line_height(parent_style: &Arc<ComputedValues>, font_metrics: &FontMetrics) -> Length {
fn line_height(parent_style: &ComputedValues, font_metrics: &FontMetrics) -> Length {
let font_size = parent_style.get_font().font_size.size.0;
match parent_style.get_inherited_text().line_height {
LineHeight::Normal => font_metrics.line_gap,
LineHeight::Number(n) => font_size * n.0,
LineHeight::Length(l) => l.0,
LineHeight::Number(number) => font_size * number.0,
LineHeight::Length(length) => length.0,
}
}
fn line_gap_from_style(layout_context: &LayoutContext, style: &ComputedValues) -> Length {
crate::context::with_thread_local_font_context(layout_context, |font_context| {
let font_group = font_context.font_group(style.clone_font());
let font = font_group
.borrow_mut()
.first(font_context)
.expect("could not find font");
let font_metrics: FontMetrics = (&font.borrow().metrics).into();
font_metrics.line_gap
})
}
fn line_height_from_style(layout_context: &LayoutContext, style: &ComputedValues) -> Length {
crate::context::with_thread_local_font_context(layout_context, |font_context| {
let font_group = font_context.font_group(style.clone_font());
let font = font_group
.borrow_mut()
.first(font_context)
.expect("could not find font");
let font_metrics: FontMetrics = (&font.borrow().metrics).into();
line_height(style, &font_metrics)
})
}
impl TextRunLineItem {
fn trim_whitespace_at_end(&mut self, whitespace_trimmed: &mut Length) -> bool {
if self
@ -1643,8 +1734,6 @@ impl TextRunLineItem {
}
fn layout(self, state: &mut LineItemLayoutState) -> Option<TextFragment> {
state.max_block_size.max_assign(self.line_height());
// This happens after updating the `max_block_size`, because even trimmed newlines
// should affect the height of the line.
if self.text.is_empty() {
@ -1684,6 +1773,7 @@ struct InlineBoxLineItem {
base_fragment_info: BaseFragmentInfo,
style: Arc<ComputedValues>,
pbm: PaddingBorderMargin,
block_size: Length,
children: Vec<LineItem>,
always_make_fragment: bool,
}
@ -1712,7 +1802,6 @@ impl InlineBoxLineItem {
let mut nested_state = LineItemLayoutState {
inline_position: state.inline_position,
max_block_size: Length::zero(),
inline_start_of_parent: state.inline_position,
ifc_containing_block: state.ifc_containing_block,
positioning_context: nested_positioning_context,
@ -1725,7 +1814,6 @@ impl InlineBoxLineItem {
let box_had_absolutes =
original_nested_positioning_context_length != nested_state.positioning_context.len();
if !self.always_make_fragment &&
nested_state.max_block_size.is_zero() &&
fragments.is_empty() &&
!box_has_padding_border_or_margin &&
!box_had_absolutes
@ -1740,12 +1828,11 @@ impl InlineBoxLineItem {
},
size: LogicalVec2 {
inline: nested_state.inline_position - state.inline_position,
block: nested_state.max_block_size,
block: self.block_size,
},
};
state.inline_position = nested_state.inline_position + pbm_sums.inline_end;
state.max_block_size.max_assign(content_rect.size.block);
// Relative adjustment should not affect the rest of line layout, so we can
// do it right before creating the Fragment.
@ -1807,7 +1894,6 @@ impl AtomicLineItem {
}
state.inline_position += self.size.inline;
state.max_block_size.max_assign(self.size.block);
if let Some(mut positioning_context) = self.positioning_context {
positioning_context.adjust_static_position_of_hoisted_fragments_with_offset(

View file

@ -0,0 +1,2 @@
[font-applies-to-017.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[empty-inline-002.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[empty-inline-003.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[inline-formatting-context-002.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[inline-formatting-context-003.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[line-breaking-font-size-zero-001.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[padding-left-applies-to-008.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[inlines-020.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[content-height-001.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[content-height-002.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[content-height-003.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[content-height-005.html]
expected: FAIL

View file

@ -0,0 +1,2 @@
[t32-opacity-offscreen-multiple-boxes-1-c.xht]
expected: FAIL

View file

@ -0,0 +1,2 @@
[t32-opacity-offscreen-multiple-boxes-2-c.xht]
expected: FAIL

View file

@ -1,2 +0,0 @@
[pseudo-element-inline-box.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[flexbox_flex-none-wrappable-content.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[font-size-adjust-zero-2.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[font-size-zero-2.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[line-edge-white-space-collapse-001.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[line-edge-white-space-collapse-002.html]
expected: FAIL

View file

@ -0,0 +1,2 @@
[perspective-untransformable-no-stacking-context.html]
expected: FAIL

View file

@ -7,3 +7,6 @@
[embed[hidden='until-found'\] element should be inline 0x0]
expected: FAIL
[embed[hidden=''\] element should be inline 0x0]
expected: FAIL

View file

@ -1,2 +0,0 @@
[select-1-block-size-001.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[border_inline_split.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[list_style_position_a.html]
expected: FAIL