Skip to content

Commit

Permalink
Fix collapse through for floats in IFCs
Browse files Browse the repository at this point in the history
  • Loading branch information
mrobinson committed Oct 17, 2023
1 parent ba814e1 commit da4ffa9
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 10 deletions.
17 changes: 15 additions & 2 deletions components/layout_2020/flow/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use style::computed_values::white_space::T as WhiteSpace;
use style::properties::longhands::list_style_position::computed_value::T as ListStylePosition;
use style::properties::ComputedValues;
use style::selector_parser::PseudoElement;
use style::values::generics::length::{GenericLengthOrNumber, GenericLengthPercentageOrAuto};
use style::values::specified::text::TextDecorationLine;

use crate::cell::ArcRefCell;
Expand All @@ -21,6 +22,7 @@ use crate::flow::float::FloatBox;
use crate::flow::inline::{InlineBox, InlineFormattingContext, InlineLevelBox, TextRun};
use crate::flow::{BlockContainer, BlockFormattingContext, BlockLevelBox};
use crate::formatting_contexts::IndependentFormattingContext;
use crate::geom::LengthOrAuto;
use crate::positioned::AbsolutelyPositionedBox;
use crate::style_ext::{ComputedValuesExt, DisplayGeneratingBox, DisplayInside, DisplayOutside};

Expand Down Expand Up @@ -65,6 +67,7 @@ impl BlockFormattingContext {
has_first_formatted_line: true,
contains_floats: false,
ends_with_whitespace: false,
has_inline_content_or_nonzero_border_padding_margin: false,
};
let contents = BlockContainer::InlineFormattingContext(ifc);
let bfc = Self {
Expand Down Expand Up @@ -189,7 +192,6 @@ impl BlockContainer {
ongoing_inline_formatting_context: InlineFormattingContext::new(
text_decoration_line,
/* has_first_formatted_line = */ true,
/* ends_with_whitespace */ false,
),
ongoing_inline_boxes_stack: Vec::new(),
anonymous_style: None,
Expand Down Expand Up @@ -290,6 +292,9 @@ where
return;
}

self.ongoing_inline_formatting_context
.has_inline_content_or_nonzero_border_padding_margin &= !has_uncollapsible_content;

self.ongoing_inline_formatting_context.ends_with_whitespace =
output.chars().last().unwrap().is_ascii_whitespace();

Expand Down Expand Up @@ -494,6 +499,13 @@ where
.unwrap()
.traverse(self.context, info, self);

let writing_mode = self.info.style.writing_mode;
let has_inline_pbm = info
.style
.inline_padding_border_margin_is_definitely_zero(writing_mode);
self.ongoing_inline_formatting_context
.has_inline_content_or_nonzero_border_padding_margin &= has_inline_pbm;

let mut inline_box = self
.ongoing_inline_boxes_stack
.pop()
Expand All @@ -502,6 +514,8 @@ where
ArcRefCell::new(InlineLevelBox::InlineBox(inline_box))
} else {
self.ongoing_inline_formatting_context.ends_with_whitespace = false;
self.ongoing_inline_formatting_context
.has_inline_content_or_nonzero_border_padding_margin = false;
ArcRefCell::new(InlineLevelBox::Atomic(
IndependentFormattingContext::construct(
self.context,
Expand Down Expand Up @@ -692,7 +706,6 @@ where
let mut ifc = InlineFormattingContext::new(
self.ongoing_inline_formatting_context.text_decoration_line,
/* has_first_formatted_line = */ false,
/* ends_with_whitespace */ false,
);
std::mem::swap(&mut self.ongoing_inline_formatting_context, &mut ifc);
let kind = BlockLevelCreator::SameFormattingContextBlock(
Expand Down
24 changes: 16 additions & 8 deletions components/layout_2020/flow/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ pub(crate) struct InlineFormattingContext {
/// > within the same inline formatting context—is collapsed to have zero advance width.
/// > (It is invisible, but retains its soft wrap opportunity, if any.)
pub(super) ends_with_whitespace: bool,
/// Whether or not this [`InlineFormattingContext`] will definitely collapse through.
/// Note: This is only a heuristic for now, so even if this is true, the IFC may
/// indeed collapse through.
pub(super) has_inline_content_or_nonzero_border_padding_margin: bool,
}

#[derive(Debug, Serialize)]
Expand Down Expand Up @@ -624,14 +628,14 @@ impl InlineFormattingContext {
pub(super) fn new(
text_decoration_line: TextDecorationLine,
has_first_formatted_line: bool,
ends_with_whitespace: bool,
) -> InlineFormattingContext {
InlineFormattingContext {
inline_level_boxes: Default::default(),
text_decoration_line,
has_first_formatted_line,
contains_floats: false,
ends_with_whitespace,
ends_with_whitespace: false,
has_inline_content_or_nonzero_border_padding_margin: true,
}
}

Expand Down Expand Up @@ -821,12 +825,13 @@ impl InlineFormattingContext {
inline_box_state_stack: Vec::new(),
};

// FIXME(pcwalton): This assumes that margins never collapse through inline formatting
// contexts (i.e. that inline formatting contexts are never empty). Is that right?
// FIXME(mrobinson): This should not happen if the IFC collapses through.
if let Some(ref mut sequential_layout_state) = ifc.sequential_layout_state {
sequential_layout_state.collapse_margins();
// FIXME(mrobinson): Collapse margins in the containing block offsets as well??
// FIXME(mrobinson): This is only a heuristic, but we need to be able to
// rewind layout if we find out that an IFC is going to have a zero block
// size. We can't do this yet.
if !self.has_inline_content_or_nonzero_border_padding_margin {
sequential_layout_state.collapse_margins();
}
}

let mut iterator = InlineBoxChildIter::from_formatting_context(self);
Expand Down Expand Up @@ -879,8 +884,11 @@ impl InlineFormattingContext {

let mut collapsible_margins_in_children = CollapsedBlockMargins::zero();
let content_block_size = ifc.current_line.start_position.block;
let zero_block_size = content_block_size == Length::zero();
assert!(!self.has_inline_content_or_nonzero_border_padding_margin || zero_block_size);

collapsible_margins_in_children.collapsed_through =
content_block_size == Length::zero() && collapsible_with_parent_start_margin.0;
zero_block_size && collapsible_with_parent_start_margin.0;

return FlowLayout {
fragments: ifc.fragments,
Expand Down
21 changes: 21 additions & 0 deletions components/layout_2020/style_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ pub(crate) trait ComputedValuesExt {
containing_block: &ContainingBlock,
pbm: &PaddingBorderMargin,
) -> LogicalVec2<Option<Length>>;
fn inline_padding_border_margin_is_definitely_zero(&self, writing_mode: WritingMode) -> bool;
fn padding_border_margin(&self, containing_block: &ContainingBlock) -> PaddingBorderMargin;
fn padding(
&self,
Expand Down Expand Up @@ -281,11 +282,31 @@ impl ComputedValuesExt for ComputedValues {
}
}

fn inline_padding_border_margin_is_definitely_zero(&self, writing_mode: WritingMode) -> bool {
let padding = self.padding(writing_mode);
let border = self.border_width(writing_mode);
let margin = self.margin(writing_mode);
padding.inline_start.is_definitely_zero() &&
padding.inline_end.is_definitely_zero() &&
border.inline_sum().is_zero() &&
margin
.inline_start
.non_auto()
.map(|value| value.is_definitely_zero())
.unwrap_or(true) &&
margin
.inline_end
.non_auto()
.map(|value| value.is_definitely_zero())
.unwrap_or(true)
}

fn padding_border_margin(&self, containing_block: &ContainingBlock) -> PaddingBorderMargin {
let cbis = containing_block.inline_size;
let padding = self
.padding(containing_block.style.writing_mode)
.percentages_relative_to(cbis);

let border = self.border_width(containing_block.style.writing_mode);
PaddingBorderMargin {
padding_border_sums: LogicalVec2 {
Expand Down

0 comments on commit da4ffa9

Please sign in to comment.