Skip to content

Commit

Permalink
OOFs and the initial column balancing pass
Browse files Browse the repository at this point in the history
We lay out the OOFs during the initial column balancing pass once they
reach the CB rather than the fragmentation context root. As a result,
we may get different results in a non-initial column balancing pass.

Thus, miss the cache if the initial column balancing pass between the
new and old constraint spaces are different, and there is an OOF in
the fragmentainer subtree.

(cherry picked from commit 4e5ea40)

Bug: 1329877,1329424
Change-Id: I32e771579f333e268f87610548c72b2c6f3be4d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3682172
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Alison Maher <almaher@microsoft.com>
Cr-Original-Commit-Position: refs/heads/main@{#1010296}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3691376
Auto-Submit: Alison Maher <almaher@microsoft.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#605}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
alisonmaher authored and Chromium LUCI CQ committed Jun 6, 2022
1 parent 3a7bc38 commit c0e9f36
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 16 deletions.
6 changes: 4 additions & 2 deletions third_party/blink/renderer/core/layout/layout_box_hot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,11 @@ const NGLayoutResult* LayoutBox::CachedLayoutResult(
}

// Multi-cols behave differently between the initial column balancing
// pass, and the regular pass (specifically when forced breaks are
// present), we just miss the cache for these cases.
// pass, and the regular pass (specifically when forced breaks or OOFs
// are present), we just miss the cache for these cases.
if (old_space.IsInitialColumnBalancingPass()) {
if (physical_fragment.HasOutOfFlowInFragmentainerSubtree())
return nullptr;
if (auto* block = DynamicTo<LayoutBlock>(this)) {
if (block->IsFragmentationContextRoot())
return nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,12 @@ void NGBoxFragmentBuilder::PropagateBreakInfo(
} else {
DCHECK(!child_layout_result.ColumnSpannerPath());
}

if (!child_box_fragment->IsFragmentainerBox() &&
!HasOutOfFlowInFragmentainerSubtree()) {
SetHasOutOfFlowInFragmentainerSubtree(
child_box_fragment->HasOutOfFlowInFragmentainerSubtree());
}
}

void NGBoxFragmentBuilder::PropagateChildBreakValues(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,16 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGFragmentBuilder {
has_out_of_flow_fragment_child_ = has_out_of_flow_fragment_child;
}

bool HasOutOfFlowInFragmentainerSubtree() const {
return has_out_of_flow_in_fragmentainer_subtree_;
}

void SetHasOutOfFlowInFragmentainerSubtree(
bool has_out_of_flow_in_fragmentainer_subtree) {
has_out_of_flow_in_fragmentainer_subtree_ =
has_out_of_flow_in_fragmentainer_subtree;
}

void SwapOutOfFlowPositionedCandidates(
HeapVector<NGLogicalOutOfFlowPositionedNode>* candidates);

Expand Down Expand Up @@ -442,6 +452,7 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGFragmentBuilder {

bool has_oof_candidate_that_needs_block_offset_adjustment_ = false;
bool has_out_of_flow_fragment_child_ = false;
bool has_out_of_flow_in_fragmentainer_subtree_ = false;

#if DCHECK_IS_ON()
bool is_may_have_descendant_above_block_start_explicitly_set_ = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -771,20 +771,26 @@ void NGOutOfFlowLayoutPart::LayoutCandidates(
SaveStaticPositionOnPaintLayer(layout_box, candidate.static_position);
if (IsContainingBlockForCandidate(candidate) &&
(!only_layout || layout_box == only_layout)) {
if (has_block_fragmentation_ &&
!container_builder_->IsInitialColumnBalancingPass()) {
// As an optimization, only populate legacy positioned objects lists
// when inside a fragmentation context root, since otherwise we can
// just look at the children in the fragment tree.
if (layout_box != only_layout)
container_builder_->InsertLegacyPositionedObject(candidate.Node());
NGLogicalOOFNodeForFragmentation fragmentainer_descendant(candidate);
container_builder_->AdjustFragmentainerDescendant(
fragmentainer_descendant);
container_builder_->AdjustFixedposContainingBlockForInnerMulticols();
container_builder_->AddOutOfFlowFragmentainerDescendant(
fragmentainer_descendant);
continue;
if (has_block_fragmentation_) {
container_builder_->SetHasOutOfFlowInFragmentainerSubtree(true);
if (!container_builder_->IsInitialColumnBalancingPass()) {
// As an optimization, only populate legacy positioned objects lists
// when inside a fragmentation context root, since otherwise we can
// just look at the children in the fragment tree.
if (layout_box != only_layout) {
container_builder_->InsertLegacyPositionedObject(
candidate.Node());
}
NGLogicalOOFNodeForFragmentation fragmentainer_descendant(
candidate);
container_builder_->AdjustFragmentainerDescendant(
fragmentainer_descendant);
container_builder_
->AdjustFixedposContainingBlockForInnerMulticols();
container_builder_->AddOutOfFlowFragmentainerDescendant(
fragmentainer_descendant);
continue;
}
}
NodeInfo node_info = SetupNodeInfo(candidate);
NodeToLayout node_to_layout = {node_info,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ NGPhysicalFragment::NGPhysicalFragment(NGContainerFragmentBuilder* builder,
!builder->oof_positioned_fragmentainer_descendants_.IsEmpty() ||
!builder->multicols_with_pending_oofs_.IsEmpty()),
has_out_of_flow_fragment_child_(builder->HasOutOfFlowFragmentChild()),
has_out_of_flow_in_fragmentainer_subtree_(
builder->HasOutOfFlowInFragmentainerSubtree()),
break_token_(std::move(builder->break_token_)),
oof_data_(builder->oof_positioned_descendants_.IsEmpty() &&
!has_fragmented_out_of_flow_data_
Expand Down Expand Up @@ -433,6 +435,8 @@ NGPhysicalFragment::NGPhysicalFragment(const NGPhysicalFragment& other,
has_last_baseline_(other.has_last_baseline_),
has_fragmented_out_of_flow_data_(other.has_fragmented_out_of_flow_data_),
has_out_of_flow_fragment_child_(other.has_out_of_flow_fragment_child_),
has_out_of_flow_in_fragmentainer_subtree_(
other.has_out_of_flow_in_fragmentainer_subtree_),
base_direction_(other.base_direction_),
break_token_(other.break_token_),
oof_data_(other.oof_data_ ? other.CloneOutOfFlowData() : nullptr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,13 @@ class CORE_EXPORT NGPhysicalFragment
return has_out_of_flow_fragment_child_;
}

// If there is an OOF contained within a fragmentation context, this will
// return true for all fragments in the chain from the OOF's CB to the
// fragmentainer that the CB resides in.
bool HasOutOfFlowInFragmentainerSubtree() const {
return has_out_of_flow_in_fragmentainer_subtree_;
}

bool HasOutOfFlowPositionedDescendants() const {
return oof_data_ && !oof_data_->oof_positioned_descendants.IsEmpty();
}
Expand Down Expand Up @@ -709,6 +716,7 @@ class CORE_EXPORT NGPhysicalFragment
unsigned has_last_baseline_ : 1;
const unsigned has_fragmented_out_of_flow_data_ : 1;
const unsigned has_out_of_flow_fragment_child_ : 1;
const unsigned has_out_of_flow_in_fragmentainer_subtree_ : 1;

// The following are only used by NGPhysicalLineBoxFragment.
unsigned base_direction_ : 1; // TextDirection
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!DOCTYPE html>
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1329877">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1329424">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="column-count:1; column-fill:auto;">
<div style="width:100px; height:50px; background:green;"></div>
<div>
<div style="position:relative; width:100px; height:50px; background:red;">
<div style="width:100px; height:50px; position:absolute; background:green;"></div>
</div>
</div>
</div>

0 comments on commit c0e9f36

Please sign in to comment.