Skip to content

Block/float: fix BFC-beside-floats edge cases (full-height overlap, negative margins, flow-root) #1060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
nicoburns wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Block/float: fix BFC-beside-floats edge cases (full-height overlap, negative margins, flow-root) #1060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Changed

- Block/float: `FloatContext::find_bfc_slot` (and `BlockContext::find_bfc_slot`) now takes the box's top border edge `y` and its `height` (instead of `min_y`, `clear` and `after`), unioning the float insets over all float segments the box vertically intersects. A new `next_bfc_candidate_y` method returns the next float-segment boundary below a given position, for iterating candidate positions

- `TaffyTree::compute_layout_with_measure`'s measure function now takes the full `LayoutInput` (plus `NodeId`, `Option<&mut NodeContext>` and `&Style`) and returns a `LayoutOutput` directly instead of a `Size<f32>`, allowing measure functions to set baselines (and other `LayoutOutput` fields) on leaf nodes. `compute_leaf_layout` is no longer called implicitly (#953)

Migration: to retain the previous behaviour, wrap your existing measure logic in an explicit call to `compute_leaf_layout` within the new-style measure function:
Expand All @@ -24,6 +26,12 @@

### Fixed

- Block/float: a box that establishes an independent formatting context must not overlap floats over its *entire* height, not just at its top edge (CSS2 §9.5). The box is now measured at each candidate position and moved down past float-segment boundaries until its full border box fits, so e.g. a tall BFC root beside a short-but-wide lower float no longer overlaps it

- Block/float: a BFC root pulled up by a negative top margin may sit (partially) above a float when it fits beside it, and moves below the float when it does not fit, matching browser behaviour

- Block: the height of a BFC root containing floats now includes its bottom padding and border below the bottom margin edge of its floated descendants

- `TaffyTree::remove` now marks the removed node's former parent as dirty, like `remove_child`, `remove_child_at_index` and `remove_children_range` already did. Previously the parent and its ancestors kept their stale cached layout, so recomputing the layout of an ancestor did not account for the removed node (#998)

- Grid: fixed a subtract-with-overflow panic (in debug builds) when resolving named lines for a template containing a repetition with fewer line name sets than tracks. Any template combining line names with a repetition created by the `repeat()` style helper (or parsed from CSS such as `[a] repeat(2, 10px) [c] 10px`) could trigger this. In release builds the same bug silently mis-numbered the lines following the repetition. The length of `GridTemplateRepetition::line_names` is now part of the API contract: it must either be empty (all lines unnamed) or contain exactly `tracks.len() + 1` line name sets, and any other length panics (in all builds) during layout
Expand Down
114 changes: 85 additions & 29 deletions src/compute/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,29 +180,27 @@ impl BlockContext<'_> {
slot
}

/// Search for a space suitable for laying out a box that establishes an independent
/// formatting context (whose border box must not overlap floats)
pub fn find_bfc_slot(
&self,
min_y: f32,
margins: [f32; 2],
direction: Direction,
clear: Clear,
after: Option<usize>,
) -> BfcSlot {
/// Compute a slot for a box that establishes an independent formatting context (whose
/// border box must not overlap floats), with its top border edge at `y`
pub fn find_bfc_slot(&self, y: f32, height: f32, margins: [f32; 2], direction: Direction) -> BfcSlot {
let mut slot = self.bfc.float_context.find_bfc_slot(
min_y + self.y_offset,
y + self.y_offset,
height,
self.content_box_insets,
margins,
direction,
clear,
after,
);
slot.y -= self.y_offset;
slot.x -= self.insets[0];
slot
}

/// The next candidate y position (below `y`) at which to try placing a box that must not
/// overlap floats: the next float-segment boundary strictly below `y`
pub fn next_bfc_candidate_y(&self, y: f32) -> Option<f32> {
self.bfc.float_context.next_bfc_candidate_y(y + self.y_offset).map(|candidate| candidate - self.y_offset)
}

/// Get the bottom of lowest relevant float for the specific clear property
pub fn cleared_threshold(&self, clear: Clear) -> Option<f32> {
self.bfc.float_context.cleared_threshold(clear).map(|threshold| threshold - self.y_offset)
Expand Down Expand Up @@ -590,10 +588,12 @@ fn compute_inner(
block_ctx,
);

// Root BFCs contain floats
// Root BFCs contain floats: the content height extends to include the bottom margin edge
// of any floated descendant, and the container's bottom padding/border sit below that
#[cfg(feature = "float_layout")]
if block_ctx.is_bfc_root() || establishes_new_bfc {
intrinsic_outer_height = intrinsic_outer_height.max(block_ctx.floated_content_height_contribution());
intrinsic_outer_height = intrinsic_outer_height
.max(block_ctx.floated_content_height_contribution() + resolved_content_box_inset.bottom);
}

let container_outer_height = known_dimensions
Expand Down Expand Up @@ -1099,21 +1099,77 @@ fn perform_final_layout_on_in_flow_children(
// (so that the margin box width is non-negative, per CSS2 §10.3.3)
let min_auto_width = -item_non_auto_x_margin_sum;

// Find the highest slot (at or below `min_y`) with enough horizontal space
// for the item's border box, which must not overlap any float
let mut slot_segment = None;
let slot = loop {
let slot = block_ctx.find_bfc_slot(min_y, x_margins, direction, item.clear, slot_segment);
let Some(segment_id) = slot.segment_id else { break slot };
let width = item
.size
.width
.unwrap_or(slot.stretch_width.max(min_auto_width))
.maybe_clamp(item.min_size.width, item.max_size.width);
if width <= slot.border_width + 0.001 {
break slot;
// Find the highest position (at or below `min_y`) at which the item's
// border box does not overlap any float over its entire height.
//
// Candidate positions are the item's natural position followed by
// successive float-segment boundaries below it. At each candidate the
// item is measured at the slot's width and the slot is recomputed with
// the measured height (the item must not overlap floats over its entire
// height, and its height depends on the width it is laid out at). Within
// a candidate the float insets only grow with height, so this converges.
let mut candidate_y = min_y;
if let Some(threshold) = block_ctx.cleared_threshold(item.clear) {
candidate_y = candidate_y.max(threshold);
}
let slot = 'candidate: loop {
let mut slot = block_ctx.find_bfc_slot(candidate_y, 0.0, x_margins, direction);
for _ in 0..8 {
let stretch_width = slot.stretch_width.max(min_auto_width);

// Measure the size the item would have when laid out in this slot.
// Tables and replaced elements resolve their own width, so they
// are measured with no known dimensions.
let known_dimensions = if item.is_table || item.is_replaced {
Size::NONE
} else {
item.size
.map_width(|width| {
Some(
width
.unwrap_or(stretch_width)
.maybe_clamp(item.min_size.width, item.max_size.width)
.max(0.0),
)
})
.maybe_clamp(item.min_size, item.max_size)
};
let measured_size = tree
.compute_child_layout(
item.node_id,
LayoutInput {
run_mode: RunMode::ComputeSize,
sizing_mode: SizingMode::InherentSize,
axis: RequestedAxis::Both,
known_dimensions,
known_dimensions_are_definite: Size { width: true, height: true },
parent_size,
available_space: available_space
.map_width(|_| AvailableSpace::Definite(stretch_width)),
vertical_margins_are_collapsible: Line::FALSE,
},
)
.size;

// The item doesn't fit at this position: try the next one down
if measured_size.width > slot.border_width + 0.001 {
break;
}

// Recompute the slot with the measured height. If the insets are
// unchanged (the width is not narrower) then the slot is stable.
let full_height_slot =
block_ctx.find_bfc_slot(candidate_y, measured_size.height, x_margins, direction);
if full_height_slot.border_width >= slot.border_width - 0.001 {
break 'candidate full_height_slot;
}
slot = full_height_slot;
}
match block_ctx.next_bfc_candidate_y(candidate_y) {
Some(next_y) => candidate_y = next_y,
// No float boundaries below: place below all floats
None => break block_ctx.find_bfc_slot(candidate_y, 0.0, x_margins, direction),
}
slot_segment = Some(segment_id);
};

// If the item had to move down to avoid floats then it "separates from the
Expand Down
134 changes: 78 additions & 56 deletions src/compute/float.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,14 @@ impl FloatContext {
}
}

/// Search for a space suitable for laying out a box that establishes an independent
/// formatting context (whose border box must not overlap floats).
/// Compute a slot for a box that establishes an independent formatting context (whose
/// border box must not overlap floats), with its top border edge at `y`.
///
/// The float insets are unioned over all segments that the box (of the given `height`)
/// vertically intersects: the box fits in the slot if its border box width does not exceed
/// `border_width`. Since the box's height generally depends on the width it is laid out at,
/// callers should iterate: measure the box at the slot's width, then recompute the slot with
/// the measured height until it stabilises (insets only grow with height, so this converges).
///
/// The box's margins are resolved against the containing block's content edges. When there
/// are floats beside the box:
Expand All @@ -659,80 +665,96 @@ impl FloatContext {
/// When there are no floats beside the box, its (possibly negative) margins apply as usual.
pub fn find_bfc_slot(
&self,
min_y: f32,
y: f32,
height: f32,
containing_block_insets: [f32; 2],
margins: [f32; 2],
direction: Direction,
clear: Clear,
after: Option<usize>,
) -> BfcSlot {
let margin_insets = [containing_block_insets[0] + margins[0], containing_block_insets[1] + margins[1]];
let no_float_width = self.available_width - margin_insets[0] - margin_insets[1];
let no_float_slot = BfcSlot {
segment_id: None,
x: margin_insets[0],
y: min_y,
y,
border_width: no_float_width,
stretch_width: no_float_width,
};

if !self.has_active_floats(min_y) {
if !self.has_active_floats(y) {
return no_float_slot;
}

// Clearance clears past the bottom of floats on the relevant side, including
// zero-sized floats which occupy no segment
let min_y = min_y.max(self.cleared_threshold(clear).unwrap_or(f32::NEG_INFINITY));
// Union the float insets (and float presence per side) of all segments that the box
// vertically intersects
let end_y = y + height.max(0.0);
let mut float_insets: Option<[f32; 2]> = None;
let mut has_float = [false; 2];
let mut segment_id = None;
for (idx, segment) in self.segments.iter().enumerate() {
if segment.y.end <= y {
continue;
}
if segment_id.is_none() {
segment_id = Some(idx);
}
if segment.y.start > end_y || (segment.y.start == end_y && segment.y.start > y) {
break;
}
float_insets = Some(match float_insets {
Some(insets) => [insets[0].max(segment.insets[0]), insets[1].max(segment.insets[1])],
None => segment.insets,
});
has_float = [has_float[0] || segment.has_float[0], has_float[1] || segment.has_float[1]];
}

// The min starting segment index
let at_least = after.map(|idx| idx + 1).unwrap_or(0);
let hwm = at_least.max(self.cleared_segment(clear).map(|idx| idx + 1).unwrap_or(0));
// The box does not vertically intersect any float segment
let Some(float_insets) = float_insets else {
return BfcSlot { segment_id, ..no_float_slot };
};

let start_idx = self
.segments
.get(hwm..)
.and_then(|segments| segments.iter().position(|segment| segment.y.end > min_y).map(|idx| idx + hwm));
let start_idx = start_idx.unwrap_or(self.segments.len());
match self.segments.get(start_idx) {
Some(segment) => {
let lead = match direction {
Direction::Ltr => 0,
Direction::Rtl => 1,
};
let trail = 1 - lead;
let has_lead_float = segment.has_float[lead];
let has_trail_float = segment.has_float[trail];
let mut fit_insets = [0.0; 2];
let mut stretch_insets = [0.0; 2];
fit_insets[lead] =
if has_lead_float { segment.insets[lead].max(margin_insets[lead]) } else { margin_insets[lead] };
stretch_insets[lead] = fit_insets[lead];
fit_insets[trail] = if has_trail_float {
segment.insets[trail].max(containing_block_insets[trail])
} else {
// A positive trailing margin may overflow the containing block edge (it does
// not affect fit), but a negative one widens the space for the border box
margin_insets[trail].min(containing_block_insets[trail])
};
stretch_insets[trail] = if has_trail_float {
segment.insets[trail].max(margin_insets[trail])
} else {
margin_insets[trail]
};
BfcSlot {
segment_id: Some(start_idx),
x: fit_insets[0],
y: segment.y.start.max(min_y),
border_width: self.available_width - fit_insets[0] - fit_insets[1],
stretch_width: self.available_width - stretch_insets[0] - stretch_insets[1],
}
let lead = match direction {
Direction::Ltr => 0,
Direction::Rtl => 1,
};
let trail = 1 - lead;
let has_lead_float = has_float[lead];
let has_trail_float = has_float[trail];
let mut fit_insets = [0.0; 2];
let mut stretch_insets = [0.0; 2];
fit_insets[lead] =
if has_lead_float { float_insets[lead].max(margin_insets[lead]) } else { margin_insets[lead] };
stretch_insets[lead] = fit_insets[lead];
fit_insets[trail] = if has_trail_float {
float_insets[trail].max(containing_block_insets[trail])
} else {
// A positive trailing margin may overflow the containing block edge (it does
// not affect fit), but a negative one widens the space for the border box
margin_insets[trail].min(containing_block_insets[trail])
};
stretch_insets[trail] =
if has_trail_float { float_insets[trail].max(margin_insets[trail]) } else { margin_insets[trail] };
BfcSlot {
segment_id,
x: fit_insets[0],
y,
border_width: self.available_width - fit_insets[0] - fit_insets[1],
stretch_width: self.available_width - stretch_insets[0] - stretch_insets[1],
}
}

/// The next candidate y position (below `y`) at which to try placing a box that must not
/// overlap floats: the next float-segment boundary strictly below `y`.
///
/// Returns `None` if there are no float boundaries below `y` (in which case moving the box
/// further down cannot change which floats it is beside).
pub fn next_bfc_candidate_y(&self, y: f32) -> Option<f32> {
for segment in &self.segments {
if segment.y.start > y {
return Some(segment.y.start);
}
// Below all floats
None => BfcSlot {
y: self.segments.last().map(|segment| segment.y.end).unwrap_or(min_y).max(min_y),
..no_float_slot
},
}
self.segments.last().map(|segment| segment.y.end).filter(|end| *end > y)
}
}

Expand Down
22 changes: 22 additions & 0 deletions test_fixtures/float/float_bfc_beside_adjoining_float_in_wide_parent.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html lang="en">
<head>
<script src="../../scripts/gentest/test_helper.js"></script>
<link rel="stylesheet" type="text/css" href="../../scripts/gentest/test_base_style.css">
<title>
A BFC root in a parent wider than its BFC fits beside an adjoining float and pulls it down with its top margin
</title>
</head>
<body>

<div id="test-root" style="display: block; width: 200px; overflow: hidden;">
<div style="display: block; width: 300px; margin-top: 50px;">
<div style="display: block;">
<div style="float: left; width: 200px; height: 10px;"></div>
</div>
<div style="display: flow-root; width: 100px; height: 10px; margin-top: 190px;"></div>
</div>
</div>

</body>
</html>
Loading