Skip to content

Commit a4c4550

Browse files
committed
Layout 2020: Optimize collapsible margin lookahead
Every time that we would lay out a block box that could collapse its top margin with its contents, we would do a lookahead to compute the resulting margin in order to place floats correctly. The problem was that this lookahead could iterate several descendants, but then when laying these we would run the lookahead again. This patch restricts the lookahead to boxes that either aren't collapsing their top margin with their parent, or that have 'clear' different than 'none' (since clearance prevents collapsing margins with the parent). Since the lookahead stops iterating when it finds a box that doesn't collapse its top margin with its parent, or whose 'clear' isn't 'none', this should ensure that lookahead never handles the same box twice.
1 parent a1dfadc commit a4c4550

File tree

1 file changed

+25
-5
lines changed
  • components/layout_2020/flow

1 file changed

+25
-5
lines changed

components/layout_2020/flow/mod.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ impl BlockLevelBox {
6767
collected_margin: &mut CollapsedMargin,
6868
containing_block: &ContainingBlock,
6969
) -> bool {
70-
// TODO(mrobinson,Loirooriol): Cache margins here so that we don't constantly
71-
// have to keep looking forward when dealing with sequences of floats.
7270
let style = match self {
7371
BlockLevelBox::SameFormattingContextBlock { ref style, .. } => &style,
7472
BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(_) |
@@ -399,6 +397,7 @@ fn layout_block_level_children_in_parallel(
399397
&mut child_positioning_context,
400398
containing_block,
401399
/* sequential_layout_state = */ None,
400+
/* collapsible_with_parent_start_margin = */ None,
402401
);
403402
(fragment, child_positioning_context)
404403
})
@@ -448,6 +447,9 @@ fn layout_block_level_children_sequentially(
448447
&mut child_positioning_context,
449448
containing_block,
450449
Some(&mut *sequential_layout_state),
450+
Some(CollapsibleWithParentStartMargin(
451+
placement_state.next_in_flow_margin_collapses_with_parent_start_margin,
452+
)),
451453
);
452454

453455
placement_state.place_fragment(&mut fragment, Some(sequential_layout_state));
@@ -474,6 +476,7 @@ impl BlockLevelBox {
474476
positioning_context: &mut PositioningContext,
475477
containing_block: &ContainingBlock,
476478
sequential_layout_state: Option<&mut SequentialLayoutState>,
479+
collapsible_with_parent_start_margin: Option<CollapsibleWithParentStartMargin>,
477480
) -> Fragment {
478481
match self {
479482
BlockLevelBox::SameFormattingContextBlock {
@@ -493,6 +496,7 @@ impl BlockLevelBox {
493496
style,
494497
NonReplacedContents::SameFormattingContextBlock(contents),
495498
sequential_layout_state,
499+
collapsible_with_parent_start_margin,
496500
)
497501
},
498502
)),
@@ -529,6 +533,7 @@ impl BlockLevelBox {
529533
non_replaced,
530534
),
531535
sequential_layout_state,
536+
collapsible_with_parent_start_margin,
532537
)
533538
},
534539
))
@@ -591,6 +596,7 @@ fn layout_in_flow_non_replaced_block_level(
591596
style: &Arc<ComputedValues>,
592597
block_level_kind: NonReplacedContents,
593598
mut sequential_layout_state: Option<&mut SequentialLayoutState>,
599+
collapsible_with_parent_start_margin: Option<CollapsibleWithParentStartMargin>,
594600
) -> BoxFragment {
595601
let pbm = style.padding_border_margin(containing_block);
596602
let box_size = style.content_box_size(containing_block, &pbm);
@@ -671,13 +677,27 @@ fn layout_in_flow_non_replaced_block_level(
671677
None => parent_containing_block_position_info = None,
672678
Some(ref mut sequential_layout_state) => {
673679
let mut block_start_margin = CollapsedMargin::new(margin.block_start);
674-
if start_margin_can_collapse_with_children {
680+
681+
// The block start margin may collapse with content margins,
682+
// compute the resulting one in order to place floats correctly.
683+
// Only need to do this if the element isn't also collapsing with its parent,
684+
// otherwise we should have already included the margin in an ancestor.
685+
// Note this lookahead stops when finding a descendant whose `clear` isn't `none`
686+
// (since clearance prevents collapsing margins with the parent).
687+
// But then we have to decide whether to actually add clearance or not,
688+
// so look forward again regardless of `collapsible_with_parent_start_margin`.
689+
// TODO: This isn't completely right: if we don't add actual clearance,
690+
// the margin should have been included in the parent (or some ancestor).
691+
// The lookahead should stop for actual clearance, not just for `clear`.
692+
let collapsible_with_parent_start_margin = collapsible_with_parent_start_margin.expect(
693+
"We should know whether we are collapsing the block start margin with the parent \
694+
when laying out sequentially",
695+
).0 && style.get_box().clear == Clear::None;
696+
if !collapsible_with_parent_start_margin && start_margin_can_collapse_with_children {
675697
if let NonReplacedContents::SameFormattingContextBlock(
676698
BlockContainer::BlockLevelBoxes(child_boxes),
677699
) = block_level_kind
678700
{
679-
// The block start margin may collapse with content margins,
680-
// compute the resulting one in order to place floats correctly.
681701
BlockLevelBox::find_block_margin_collapsing_with_parent_from_slice(
682702
child_boxes,
683703
&mut block_start_margin,

0 commit comments

Comments
 (0)