Skip to content

Commit

Permalink
Better handling of OOFs inside clipped containers.
Browse files Browse the repository at this point in the history 
If we're inside a fragmentation context, and the containing block of an
out-of-flow positioned element is inside a container that clips block
direction overflow, keep track of the offset to the clipping container.

This is used by printing, as an attempt to prevent the OOF from ending
up in a fragmentainer that precedes the fragmentainer that contains the
clipping container. This helps pre-paint set the correct clip rectangle.
If the OOF ends up in an earlier fragmentainer, we would have no idea
what clip rectangle to use, since the clipping container isn't
represented there.

This eliminates the need for the is_fragmented_inside_clipped_container
flag, since the new approach provides the same information, and more.
Besides, we failed to update is_fragmented_inside_clipped_container in
nested OOF situations, so that the inner OOF wouldn't see any clipped
container established by the containing block (or an ancestor) of the
outer OOF. Rather than fixing the redundant flag, remove it.

Some of the printing tests look sillier than I'd like, but this is due
to the fact that when generating print output, content that should have
been hidden entirely by clipping still bleeds through.

This behavior change is limited to printing. It doesn't really make
sense to do the same for multicol, since columns are laid out in the
inline direction. The previous column isn't visually above the next
column, so it wouldn't look right. For printing the previous page is
just above the next page, visually, though. The new multicol tests in
this CL are there just to illustrate that we don't want this behavior
in multicol. They also pass without this CL.

(cherry picked from commit c52a430)

Bug: 1400739
Change-Id: Ie05a9427dab454e7b1481bab1e05286f9c64d943
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4136240
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1089157}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4147198
Auto-Submit: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#174}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
@mstensho
mstensho authored and Chromium LUCI CQ committed Jan 9, 2023
1 parent bb08da7 commit bb614df
Show file tree
Hide file tree
Showing 32 changed files with 540 additions and 56 deletions.
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/layout/ng/ng_column_layout_algorithm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ const NGLayoutResult* NGColumnLayoutAlgorithm::LayoutRow(
container_builder_.PropagateOOFFragmentainerDescendants(
new_column.Fragment(), new_column.offset,
/* relative_offset */ LogicalOffset(), containing_block_adjustment,
/* containing_block */ nullptr,
/* fixedpos_containing_block */ nullptr,
&column_balancing_info.out_of_flow_fragmentainer_descendants);
}
Expand Down
74 changes: 58 additions & 16 deletions third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ void NGContainerFragmentBuilder::PropagateOOFPositionedInfo(
LogicalOffset offset_adjustment,
const NGInlineContainer<LogicalOffset>* inline_container,
LayoutUnit containing_block_adjustment,
const NGContainingBlock<LogicalOffset>* containing_block,
const NGContainingBlock<LogicalOffset>* fixedpos_containing_block,
const NGInlineContainer<LogicalOffset>* fixedpos_inline_container,
LogicalOffset additional_fixedpos_offset) {
Expand Down Expand Up @@ -532,32 +533,38 @@ void NGContainerFragmentBuilder::PropagateOOFPositionedInfo(
} else {
multicol_offset += adjusted_offset;
}

// TODO(layout-dev): Adjust any clipped container block-offset. For now,
// just reset it, rather than passing an incorrect value.
absl::optional<LayoutUnit> fixedpos_clipped_container_block_offset;

AddMulticolWithPendingOOFs(
NGBlockNode(multicol.key),
MakeGarbageCollected<NGMulticolWithPendingOOFs<LogicalOffset>>(
multicol_offset,
NGContainingBlock<LogicalOffset>(
fixedpos_containing_block_offset,
fixedpos_containing_block_rel_offset,
fixedpos_containing_block_fragment, is_inside_column_spanner,
multicol_info->fixedpos_containing_block
.RequiresContentBeforeBreaking(),
fixedpos_containing_block_fragment,
fixedpos_clipped_container_block_offset,
is_inside_column_spanner,
multicol_info->fixedpos_containing_block
.IsFragmentedInsideClippedContainer()),
.RequiresContentBeforeBreaking()),
new_fixedpos_inline_container));
}
}

PropagateOOFFragmentainerDescendants(fragment, offset, relative_offset,
containing_block_adjustment,
fixedpos_containing_block);
PropagateOOFFragmentainerDescendants(
fragment, offset, relative_offset, containing_block_adjustment,
containing_block, fixedpos_containing_block);
}

void NGContainerFragmentBuilder::PropagateOOFFragmentainerDescendants(
const NGPhysicalFragment& fragment,
LogicalOffset offset,
LogicalOffset relative_offset,
LayoutUnit containing_block_adjustment,
const NGContainingBlock<LogicalOffset>* containing_block,
const NGContainingBlock<LogicalOffset>* fixedpos_containing_block,
HeapVector<NGLogicalOOFNodeForFragmentation>* out_list) {
NGFragmentedOutOfFlowData* oof_data = fragment.FragmentedOutOfFlowData();
Expand Down Expand Up @@ -621,6 +628,40 @@ void NGContainerFragmentBuilder::PropagateOOFFragmentainerDescendants(
containing_block_offset += offset;
containing_block_offset.block_offset += containing_block_adjustment;

// If the containing block of the OOF is inside a clipped container, update
// this offset.
auto UpdatedClippedContainerBlockOffset =
[&containing_block, &offset, &fragment,
&containing_block_adjustment](const NGContainingBlock<PhysicalOffset>&
descendant_containing_block) {
absl::optional<LayoutUnit> clipped_container_offset =
descendant_containing_block.ClippedContainerBlockOffset();
if (!clipped_container_offset &&
fragment.HasNonVisibleBlockOverflow()) {
// We just found a clipped container.
clipped_container_offset.emplace();
}
if (clipped_container_offset) {
// We're inside a clipped container. Adjust the offset.
if (!fragment.IsFragmentainerBox()) {
*clipped_container_offset += offset.block_offset;
}
*clipped_container_offset += containing_block_adjustment;
}
if (!clipped_container_offset && containing_block &&
containing_block->ClippedContainerBlockOffset()) {
// We were not inside a clipped container, but we're contained by an
// OOF which is inside one. E.g.: <clipped><relpos><abspos><abspos>
// This happens when we're at the inner abspos in this example.
clipped_container_offset =
containing_block->ClippedContainerBlockOffset();
}
return clipped_container_offset;
};

absl::optional<LayoutUnit> clipped_container_block_offset =
UpdatedClippedContainerBlockOffset(descendant.containing_block);

LogicalOffset inline_relative_offset = converter.ToLogical(
descendant.inline_container.relative_offset, PhysicalSize());
NGInlineContainer<LogicalOffset> new_inline_container(
Expand Down Expand Up @@ -655,6 +696,7 @@ void NGContainerFragmentBuilder::PropagateOOFFragmentainerDescendants(

LogicalOffset fixedpos_containing_block_offset;
LogicalOffset fixedpos_containing_block_rel_offset;
absl::optional<LayoutUnit> fixedpos_clipped_container_block_offset;
if (fixedpos_containing_block_fragment) {
fixedpos_containing_block_offset =
converter.ToLogical(descendant.fixedpos_containing_block.Offset(),
Expand All @@ -668,6 +710,10 @@ void NGContainerFragmentBuilder::PropagateOOFFragmentainerDescendants(
fixedpos_containing_block_offset.block_offset +=
containing_block_adjustment;

fixedpos_clipped_container_block_offset =
UpdatedClippedContainerBlockOffset(
descendant.fixedpos_containing_block);

if (is_column_spanner)
fixedpos_container_inside_column_spanner = true;
}
Expand All @@ -679,25 +725,21 @@ void NGContainerFragmentBuilder::PropagateOOFFragmentainerDescendants(
fixedpos_containing_block_rel_offset =
fixedpos_containing_block->RelativeOffset();
}
bool is_clipped_container = fragment.HasNonVisibleBlockOverflow();
NGLogicalOOFNodeForFragmentation oof_node(
descendant.Node(), static_position, new_inline_container,
NGContainingBlock<LogicalOffset>(
containing_block_offset, containing_block_rel_offset,
containing_block_fragment, container_inside_column_spanner,
descendant.containing_block.RequiresContentBeforeBreaking(),
descendant.containing_block.IsFragmentedInsideClippedContainer() ||
is_clipped_container),
containing_block_fragment, clipped_container_block_offset,
container_inside_column_spanner,
descendant.containing_block.RequiresContentBeforeBreaking()),
NGContainingBlock<LogicalOffset>(
fixedpos_containing_block_offset,
fixedpos_containing_block_rel_offset,
fixedpos_containing_block_fragment,
fixedpos_clipped_container_block_offset,
fixedpos_container_inside_column_spanner,
descendant.fixedpos_containing_block
.RequiresContentBeforeBreaking(),
descendant.fixedpos_containing_block
.IsFragmentedInsideClippedContainer() ||
is_clipped_container),
.RequiresContentBeforeBreaking()),
new_fixedpos_inline_container);

if (out_list) {
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGFragmentBuilder {
LogicalOffset offset_adjustment = LogicalOffset(),
const NGInlineContainer<LogicalOffset>* inline_container = nullptr,
LayoutUnit containing_block_adjustment = LayoutUnit(),
const NGContainingBlock<LogicalOffset>* containing_block = nullptr,
const NGContainingBlock<LogicalOffset>* fixedpos_containing_block =
nullptr,
const NGInlineContainer<LogicalOffset>* fixedpos_inline_container =
Expand All @@ -269,6 +270,7 @@ class CORE_EXPORT NGContainerFragmentBuilder : public NGFragmentBuilder {
LogicalOffset offset,
LogicalOffset relative_offset,
LayoutUnit containing_block_adjustment,
const NGContainingBlock<LogicalOffset>* containing_block,
const NGContainingBlock<LogicalOffset>* fixedpos_containing_block,
HeapVector<NGLogicalOOFNodeForFragmentation>* out_list = nullptr);

Expand Down
62 changes: 41 additions & 21 deletions third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,18 +1047,18 @@ void NGOutOfFlowLayoutPart::LayoutOOFsInMulticol(
NGContainingBlock<LogicalOffset>(
containing_block_offset, containing_block_rel_offset,
containing_block_fragment,
descendant.containing_block.ClippedContainerBlockOffset(),
descendant.containing_block.IsInsideColumnSpanner(),
descendant.containing_block.RequiresContentBeforeBreaking(),
descendant.containing_block.IsFragmentedInsideClippedContainer()),
descendant.containing_block.RequiresContentBeforeBreaking()),
NGContainingBlock<LogicalOffset>(
fixedpos_containing_block_offset,
fixedpos_containing_block_rel_offset,
fixedpos_containing_block_fragment,
descendant.fixedpos_containing_block.IsInsideColumnSpanner(),
descendant.fixedpos_containing_block
.RequiresContentBeforeBreaking(),
.ClippedContainerBlockOffset(),
descendant.fixedpos_containing_block.IsInsideColumnSpanner(),
descendant.fixedpos_containing_block
.IsFragmentedInsideClippedContainer()),
.RequiresContentBeforeBreaking()),
fixedpos_inline_container};
oof_nodes_to_layout.push_back(node);
}
Expand Down Expand Up @@ -1308,8 +1308,9 @@ void NGOutOfFlowLayoutPart::LayoutFragmentainerDescendants(
wtf_size_t start_index = 0;
ComputeStartFragmentIndexAndRelativeOffset(
node_info.default_writing_direction.GetWritingMode(),
*node_to_layout.offset_info.block_estimate, &start_index,
&node_to_layout.offset_info.offset);
*node_to_layout.offset_info.block_estimate,
node_info.containing_block.ClippedContainerBlockOffset(),
&start_index, &node_to_layout.offset_info.offset);
if (start_index >= descendants_to_layout.size())
descendants_to_layout.resize(start_index + 1);
descendants_to_layout[start_index].emplace_back(node_to_layout);
Expand Down Expand Up @@ -1437,15 +1438,12 @@ NGOutOfFlowLayoutPart::NodeInfo NGOutOfFlowLayoutPart::SetupNodeInfo(
// case, we also need to adjust the offset to account for this.
NGLogicalStaticPosition static_position = oof_node.static_position;
static_position.offset -= container_info.rect.offset;
bool is_fragmented_inside_clipped_container = false;
if (containing_block_fragment) {
const auto& containing_block_for_fragmentation =
To<NGLogicalOOFNodeForFragmentation>(oof_node).containing_block;
static_position.offset += containing_block_for_fragmentation.Offset();
requires_content_before_breaking =
containing_block_for_fragmentation.RequiresContentBeforeBreaking();
is_fragmented_inside_clipped_container =
containing_block_for_fragmentation.IsFragmentedInsideClippedContainer();
}

NGLogicalStaticPosition oof_static_position =
Expand All @@ -1471,23 +1469,25 @@ NGOutOfFlowLayoutPart::NodeInfo NGOutOfFlowLayoutPart::SetupNodeInfo(
/* requires_content_before_breaking */ false);
}

NGContainingBlock<LogicalOffset> containing_block;
NGContainingBlock<LogicalOffset> fixedpos_containing_block;
NGInlineContainer<LogicalOffset> fixedpos_inline_container;
if (containing_block_fragment) {
containing_block =
To<NGLogicalOOFNodeForFragmentation>(oof_node).containing_block;
fixedpos_containing_block = To<NGLogicalOOFNodeForFragmentation>(oof_node)
.fixedpos_containing_block;
fixedpos_inline_container = To<NGLogicalOOFNodeForFragmentation>(oof_node)
.fixedpos_inline_container;
}

return NodeInfo(node, builder.ToConstraintSpace(), oof_static_position,
container_physical_content_size, container_info,
ConstraintSpace().GetWritingDirection(),
/* is_fragmentainer_descendant */ containing_block_fragment,
fixedpos_containing_block, fixedpos_inline_container,
oof_node.inline_container.container,
requires_content_before_breaking,
is_fragmented_inside_clipped_container);
return NodeInfo(
node, builder.ToConstraintSpace(), oof_static_position,
container_physical_content_size, container_info,
ConstraintSpace().GetWritingDirection(),
/* is_fragmentainer_descendant */ containing_block_fragment,
containing_block, fixedpos_containing_block, fixedpos_inline_container,
oof_node.inline_container.container, requires_content_before_breaking);
}

const NGLayoutResult* NGOutOfFlowLayoutPart::LayoutOOFNode(
Expand Down Expand Up @@ -1978,7 +1978,7 @@ const NGLayoutResult* NGOutOfFlowLayoutPart::GenerateFragment(
// that might be. But as long as the OOF doesn't contribute to any
// additional fragmentainers, we should be (pretty) good.
if (is_last_fragmentainer_so_far &&
node_info.is_fragmented_inside_clipped_container) {
node_info.containing_block.IsFragmentedInsideClippedContainer()) {
builder.DisableFurtherFragmentation();
}
}
Expand Down Expand Up @@ -2232,6 +2232,7 @@ void NGOutOfFlowLayoutPart::AddOOFToFragmentainer(
result->PhysicalFragment(), oof_offset, relative_offset,
offset_adjustment,
/* inline_container */ nullptr, containing_block_adjustment,
&descendant.node_info.containing_block,
&descendant.node_info.fixedpos_containing_block,
&descendant.node_info.fixedpos_inline_container,
additional_fixedpos_offset);
Expand Down Expand Up @@ -2395,6 +2396,7 @@ NGConstraintSpace NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace(
void NGOutOfFlowLayoutPart::ComputeStartFragmentIndexAndRelativeOffset(
WritingMode default_writing_mode,
LayoutUnit block_estimate,
absl::optional<LayoutUnit> clipped_container_block_offset,
wtf_size_t* start_index,
LogicalOffset* offset) const {
wtf_size_t child_index = 0;
Expand All @@ -2404,6 +2406,23 @@ void NGOutOfFlowLayoutPart::ComputeStartFragmentIndexAndRelativeOffset(
LayoutUnit current_max_block_size;
// The block size for the last fragmentainer we encountered.
LayoutUnit fragmentainer_block_size;

LayoutUnit target_block_offset = offset->block_offset;
if (clipped_container_block_offset &&
container_builder_->Node().IsPaginatedRoot()) {
// If we're printing, and we have an OOF inside a clipped container, prevent
// the start fragmentainer from preceding that of the clipped container.
// This way we increase the likelihood of luring the OOF into the same
// fragmentainer as the clipped container, so that we get the correct clip
// rectangle during pre-paint.
//
// TODO(crbug.com/1371426): We might be able to get rid of this, if we
// either get pre-paint to handle missing ancestor fragments better, or if
// we rewrite OOF layout to always generate the necessary ancestor
// fragments.
target_block_offset =
std::max(target_block_offset, *clipped_container_block_offset);
}
auto& children = FragmentationContextChildren();
// TODO(bebeaudr): There is a possible performance improvement here as we'll
// repeat this for each abspos in a same fragmentainer.
Expand All @@ -2423,8 +2442,8 @@ void NGOutOfFlowLayoutPart::ComputeStartFragmentIndexAndRelativeOffset(
// |container_builder_| when we have a nested abspos. Because we use that
// value to position the nested abspos, its start offset would be off by
// exactly one fragmentainer block size.
if (offset->block_offset < current_max_block_size ||
(offset->block_offset == current_max_block_size &&
if (target_block_offset < current_max_block_size ||
(target_block_offset == current_max_block_size &&
block_estimate == 0)) {
*start_index = child_index;
offset->block_offset -= used_block_size;
Expand Down Expand Up @@ -2599,6 +2618,7 @@ void NGOutOfFlowLayoutPart::MulticolChildInfo::Trace(Visitor* visitor) const {

void NGOutOfFlowLayoutPart::NodeInfo::Trace(Visitor* visitor) const {
visitor->Trace(node);
visitor->Trace(containing_block);
visitor->Trace(fixedpos_containing_block);
visitor->Trace(fixedpos_inline_container);
}
Expand Down
12 changes: 6 additions & 6 deletions third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,11 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
PhysicalSize container_physical_content_size;
const ContainingBlockInfo container_info;
const WritingDirectionMode default_writing_direction;
const NGContainingBlock<LogicalOffset> containing_block;
const NGContainingBlock<LogicalOffset> fixedpos_containing_block;
const NGInlineContainer<LogicalOffset> fixedpos_inline_container;
bool inline_container = false;
bool requires_content_before_breaking = false;
bool is_fragmented_inside_clipped_container = false;

NodeInfo(NGBlockNode node,
const NGConstraintSpace constraint_space,
Expand All @@ -189,23 +189,22 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
const ContainingBlockInfo container_info,
const WritingDirectionMode default_writing_direction,
bool is_fragmentainer_descendant,
const NGContainingBlock<LogicalOffset>& containing_block,
const NGContainingBlock<LogicalOffset>& fixedpos_containing_block,
const NGInlineContainer<LogicalOffset>& fixedpos_inline_container,
bool inline_container,
bool requires_content_before_breaking,
bool is_fragmented_inside_clipped_container)
bool requires_content_before_breaking)
: node(node),
constraint_space(constraint_space),
static_position(static_position),
container_physical_content_size(container_physical_content_size),
container_info(container_info),
default_writing_direction(default_writing_direction),
containing_block(containing_block),
fixedpos_containing_block(fixedpos_containing_block),
fixedpos_inline_container(fixedpos_inline_container),
inline_container(inline_container),
requires_content_before_breaking(requires_content_before_breaking),
is_fragmented_inside_clipped_container(
is_fragmented_inside_clipped_container) {}
requires_content_before_breaking(requires_content_before_breaking) {}

void Trace(Visitor* visitor) const;
};
Expand Down Expand Up @@ -402,6 +401,7 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
void ComputeStartFragmentIndexAndRelativeOffset(
WritingMode default_writing_mode,
LayoutUnit block_estimate,
absl::optional<LayoutUnit> clipped_container_block_offset,
wtf_size_t* start_index,
LogicalOffset* offset) const;

Expand Down

0 comments on commit bb614df

Please sign in to comment.