Commit ab4ef147 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

[LayoutNG] Avoid creating unnecessary block break tokens.

We only need break tokens when doing block fragmentation. Long-term,
even when we do have block fragmentation, we might not need to create
break tokens when we're finished with a node, but that will require
some more work, in any case. We currently use break tokens to figure
out how much block space has been used in total by all previous
fragments generated for a node, for instance.

We now have to stop cheating in the paged overflow layout algorithm. If
height was auto, we used to just disable block fragmentation, but since
block fragmentation is what now triggers creation of break tokens, and
we still have a flow thread to feed, we need to always enable block
fragmentation for paged overflow (this is the correct solution
long-term, anyway). We'll now fail all tests that have paged overflow
and auto height, but only when LayoutNGBlockFragmentation is enabled, so
no big deal.

This should give both performance improvements and memory usage
reduction.

Some tests started to pass and some started to fail.

Three new failures: Two of them are an obvious result of no longer
supporting auto-height paged overflow containers. The last one
(wpt/css/css-multicol/filter-with-abspos.html for the
LayoutNGBlockFragmentation virtual test suite) I'm not quite sure about,
but it probably has something to do with us no longer creating break
tokens where we previously used to do it, and that upsets
someone. Nothing to worry about for now, since it only affects NG block
fragmentation, which isn't shipping in this phase.

Some new tests also start to pass. Didn't do a thorough analysis, but we
now only create break tokens from fragments generated by
NGBlockLayoutAlgorithm, so e.g. nodes laid out by the legacy engine and
nested column algorithms will no longer create break tokens. Which
apparently is a good thing in some cases. Or it could be bugs canceling
out other bugs. Anyway, since all the tests in question are also only
for the LayoutNGBlockFragmentation virtual test suite, we don't have to
worry about it yet.

Finally, one unit test had to be updated, because orthogonal flow roots
disable block fragmentation, and that suddenly means that there'll be no
break tokens generated.

Change-Id: Ief3d354e7ae0741ee239f530811ac9d9b3576ce3
Reviewed-on: https://chromium-review.googlesource.com/c/1296535Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602304}
parent 702055f4
...@@ -647,6 +647,7 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-re ...@@ -647,6 +647,7 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-re
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-relative-034.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-relative-034.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-relative-036.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-relative-036.xht [ Skip ]
crbug.com/829028 virtual/layout_ng_experimental/external/wpt/css/css-multicol/filter-with-abspos.html [ Failure ]
crbug.com/829028 virtual/layout_ng_experimental/external/wpt/css/css-multicol/going-out-of-flow-after-spanner.html [ Failure ] crbug.com/829028 virtual/layout_ng_experimental/external/wpt/css/css-multicol/going-out-of-flow-after-spanner.html [ Failure ]
crbug.com/829028 virtual/layout_ng_experimental/external/wpt/css/css-multicol/inline-block-and-column-span-all.html [ Failure ] crbug.com/829028 virtual/layout_ng_experimental/external/wpt/css/css-multicol/inline-block-and-column-span-all.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-basic-001.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-basic-001.html [ Failure ]
...@@ -877,8 +878,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/first-line-in-floa ...@@ -877,8 +878,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/first-line-in-floa
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/first-line-in-float-with-margin.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/first-line-in-float-with-margin.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/fixedpos-child-becomes-static.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/fixedpos-child-becomes-static.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox-starts-at-column-boundary.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox-starts-at-column-boundary-with-block.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox-with-overflow-auto-child-crash.html [ Crash ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox-with-overflow-auto-child-crash.html [ Crash ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flipped-blocks-border-after.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flipped-blocks-border-after.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flipped-blocks-hit-test.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flipped-blocks-hit-test.html [ Failure ]
...@@ -888,7 +887,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-big-line.htm ...@@ -888,7 +887,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-big-line.htm
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-break.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-break.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-content-break.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-content-break.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-edge.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-edge.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-margin-at-row-boundary-fixed-multicol-height.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-margin-at-row-boundary.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-margin-at-row-boundary.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-moved-by-child-line-and-unbreakable.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-moved-by-child-line-and-unbreakable.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-paginate-empty-lines.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-paginate-empty-lines.html [ Failure ]
...@@ -929,6 +927,7 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/mixed-positioning- ...@@ -929,6 +927,7 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/mixed-positioning-
crbug.com/714962 virtual/layout_ng_experimental/fast/multicol/multicol-becomes-abspos-crash.html [ Failure ] crbug.com/714962 virtual/layout_ng_experimental/fast/multicol/multicol-becomes-abspos-crash.html [ Failure ]
crbug.com/626703 virtual/layout_ng_experimental/fast/multicol/multicol-becomes-paged-auto-height.html [ Crash Pass ] crbug.com/626703 virtual/layout_ng_experimental/fast/multicol/multicol-becomes-paged-auto-height.html [ Crash Pass ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/multicol-with-child-renderLayer-for-input.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/multicol-with-child-renderLayer-for-input.html [ Failure ]
crbug.com/829028 virtual/layout_ng_experimental/fast/multicol/multicol-with-spanner-becomes-paged.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/nested-3-multicols-fixed-height.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/nested-3-multicols-fixed-height.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/nested-auto-height-extra-block-inbetween.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/nested-auto-height-extra-block-inbetween.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/nested-auto-height.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/nested-auto-height.html [ Failure ]
...@@ -1070,9 +1069,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/span/vertical-rl.h ...@@ -1070,9 +1069,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/span/vertical-rl.h
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/span/with-border.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/span/with-border.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/static-child-becomes-fixedpos.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/static-child-becomes-fixedpos.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/svg-change-column-crash.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/svg-change-column-crash.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-caption-and-cells-fixed-width.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-caption-and-cells.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-caption-with-block.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-cell-content-change.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-cell-content-change.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-cell-content-change-with-decorations.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-cell-content-change-with-decorations.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-margin-collapse.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/table-margin-collapse.html [ Failure ]
...@@ -1133,7 +1129,7 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/vertical-rl/offset ...@@ -1133,7 +1129,7 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/vertical-rl/offset
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/vertical-rl/unsplittable-inline-block.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/vertical-rl/unsplittable-inline-block.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/widows-and-orphans.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/widows-and-orphans.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/widows.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/widows.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/pagination/auto-height.html [ Crash Pass ] crbug.com/591099 virtual/layout_ng_experimental/fast/pagination/auto-height.html [ Crash Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fast/pagination/auto-height-with-break.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/pagination/auto-height-with-break.html [ Failure ]
crbug.com/879467 virtual/layout_ng_experimental/fast/pagination/body-make-paginated-table-abspos-crash.html [ Crash ] crbug.com/879467 virtual/layout_ng_experimental/fast/pagination/body-make-paginated-table-abspos-crash.html [ Crash ]
crbug.com/591099 virtual/layout_ng_experimental/fast/pagination/break-in-paged-overflow.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fast/pagination/break-in-paged-overflow.html [ Failure ]
...@@ -1182,7 +1178,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fragmentation/float-after-forced ...@@ -1182,7 +1178,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fragmentation/float-after-forced
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/float-margin-top.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fragmentation/float-margin-top.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/float-pushed-to-next-fragmentainer-by-floats.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fragmentation/float-pushed-to-next-fragmentainer-by-floats.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/forced-break-clearance-unsplittable-content.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fragmentation/forced-break-clearance-unsplittable-content.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/foreignobject-no-pagination.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/fragmented-rowspan-alignment.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fragmentation/fragmented-rowspan-alignment.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/fragmented-rowspan.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fragmentation/fragmented-rowspan.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/fragmentation/fragmented-table-cell.html [ Failure ] crbug.com/591099 virtual/layout_ng_experimental/fragmentation/fragmented-table-cell.html [ Failure ]
......
...@@ -404,6 +404,8 @@ scoped_refptr<NGLayoutResult> NGBlockLayoutAlgorithm::Layout() { ...@@ -404,6 +404,8 @@ scoped_refptr<NGLayoutResult> NGBlockLayoutAlgorithm::Layout() {
ComputeIntrinsicPadding(ConstraintSpace(), Node()); ComputeIntrinsicPadding(ConstraintSpace(), Node());
border_scrollbar_padding_ += intrinsic_padding; border_scrollbar_padding_ += intrinsic_padding;
if (ConstraintSpace().HasBlockFragmentation())
container_builder_.SetNeedsFinishedBreakToken();
container_builder_.SetInlineSize(border_box_size.inline_size); container_builder_.SetInlineSize(border_box_size.inline_size);
container_builder_.SetBfcLineOffset( container_builder_.SetBfcLineOffset(
ConstraintSpace().BfcOffset().line_offset); ConstraintSpace().BfcOffset().line_offset);
......
...@@ -2026,7 +2026,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, FloatFragmentationOrthogonalFlows) { ...@@ -2026,7 +2026,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, FloatFragmentationOrthogonalFlows) {
scoped_refptr<const NGPhysicalFragment> fragment = scoped_refptr<const NGPhysicalFragment> fragment =
NGBlockLayoutAlgorithm(node, space).Layout()->PhysicalFragment(); NGBlockLayoutAlgorithm(node, space).Layout()->PhysicalFragment();
EXPECT_EQ(NGPhysicalSize(LayoutUnit(150), LayoutUnit(60)), fragment->Size()); EXPECT_EQ(NGPhysicalSize(LayoutUnit(150), LayoutUnit(60)), fragment->Size());
ASSERT_TRUE(fragment->BreakToken()->IsFinished()); ASSERT_TRUE(!fragment->BreakToken() || fragment->BreakToken()->IsFinished());
// float2 should only have one fragment. // float2 should only have one fragment.
FragmentChildIterator iterator(ToNGPhysicalBoxFragment(fragment.get())); FragmentChildIterator iterator(ToNGPhysicalBoxFragment(fragment.get()));
...@@ -2035,7 +2035,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, FloatFragmentationOrthogonalFlows) { ...@@ -2035,7 +2035,7 @@ TEST_F(NGBlockLayoutAlgorithmTest, FloatFragmentationOrthogonalFlows) {
child = iterator.NextChild(&offset); child = iterator.NextChild(&offset);
EXPECT_EQ(NGPhysicalSize(LayoutUnit(60), LayoutUnit(200)), child->Size()); EXPECT_EQ(NGPhysicalSize(LayoutUnit(60), LayoutUnit(200)), child->Size());
EXPECT_EQ(NGPhysicalOffset(LayoutUnit(90), LayoutUnit(50)), offset); EXPECT_EQ(NGPhysicalOffset(LayoutUnit(90), LayoutUnit(50)), offset);
ASSERT_TRUE(child->BreakToken()->IsFinished()); ASSERT_TRUE(!child->BreakToken() || child->BreakToken()->IsFinished());
} }
// Tests that a float child inside a zero height block fragments correctly. // Tests that a float child inside a zero height block fragments correctly.
......
...@@ -220,7 +220,7 @@ scoped_refptr<NGLayoutResult> NGBoxFragmentBuilder::ToBoxFragment( ...@@ -220,7 +220,7 @@ scoped_refptr<NGLayoutResult> NGBoxFragmentBuilder::ToBoxFragment(
if (did_break_) { if (did_break_) {
break_token_ = NGBlockBreakToken::Create( break_token_ = NGBlockBreakToken::Create(
node_, used_block_size_, child_break_tokens_, has_last_resort_break_); node_, used_block_size_, child_break_tokens_, has_last_resort_break_);
} else { } else if (needs_finished_break_token_) {
break_token_ = NGBlockBreakToken::Create(node_, used_block_size_, break_token_ = NGBlockBreakToken::Create(node_, used_block_size_,
has_last_resort_break_); has_last_resort_break_);
} }
......
...@@ -105,6 +105,11 @@ class CORE_EXPORT NGBoxFragmentBuilder final ...@@ -105,6 +105,11 @@ class CORE_EXPORT NGBoxFragmentBuilder final
return *this; return *this;
} }
NGBoxFragmentBuilder& SetNeedsFinishedBreakToken() {
needs_finished_break_token_ = true;
return *this;
}
// Specify that we broke. // Specify that we broke.
// //
// This will result in a fragment which has an unfinished break token. // This will result in a fragment which has an unfinished break token.
...@@ -238,6 +243,7 @@ class CORE_EXPORT NGBoxFragmentBuilder final ...@@ -238,6 +243,7 @@ class CORE_EXPORT NGBoxFragmentBuilder final
NGPhysicalFragment::NGBoxType box_type_; NGPhysicalFragment::NGBoxType box_type_;
bool is_fieldset_container_ = false; bool is_fieldset_container_ = false;
bool is_old_layout_root_; bool is_old_layout_root_;
bool needs_finished_break_token_ = false;
bool did_break_; bool did_break_;
bool has_forced_break_ = false; bool has_forced_break_ = false;
LayoutUnit used_block_size_; LayoutUnit used_block_size_;
......
...@@ -216,7 +216,7 @@ LayoutUnit ComputeMarginBoxInlineSizeForUnpositionedFloat( ...@@ -216,7 +216,7 @@ LayoutUnit ComputeMarginBoxInlineSizeForUnpositionedFloat(
const auto& fragment = unpositioned_float->layout_result->PhysicalFragment(); const auto& fragment = unpositioned_float->layout_result->PhysicalFragment();
DCHECK(fragment); DCHECK(fragment);
DCHECK(fragment->BreakToken()->IsFinished()); DCHECK(!fragment->BreakToken() || fragment->BreakToken()->IsFinished());
return (NGFragment(parent_space.GetWritingMode(), *fragment).InlineSize() + return (NGFragment(parent_space.GetWritingMode(), *fragment).InlineSize() +
unpositioned_float->margins.InlineSum()) unpositioned_float->margins.InlineSum())
...@@ -266,9 +266,12 @@ NGPositionedFloat PositionFloat( ...@@ -266,9 +266,12 @@ NGPositionedFloat PositionFloat(
if (ShouldIgnoreBlockStartMargin(parent_space, unpositioned_float->node, if (ShouldIgnoreBlockStartMargin(parent_space, unpositioned_float->node,
unpositioned_float->token.get())) unpositioned_float->token.get()))
fragment_margins.block_start = LayoutUnit(); fragment_margins.block_start = LayoutUnit();
if (!layout_result->PhysicalFragment()->BreakToken()->IsFinished()) if (const NGBreakToken* break_token =
layout_result->PhysicalFragment()->BreakToken()) {
if (!break_token->IsFinished())
fragment_margins.block_end = LayoutUnit(); fragment_margins.block_end = LayoutUnit();
} }
}
DCHECK(layout_result->PhysicalFragment()); DCHECK(layout_result->PhysicalFragment());
NGFragment float_fragment(parent_space.GetWritingMode(), NGFragment float_fragment(parent_space.GetWritingMode(),
......
...@@ -100,14 +100,10 @@ NGConstraintSpace NGPageLayoutAlgorithm::CreateConstraintSpaceForPages( ...@@ -100,14 +100,10 @@ NGConstraintSpace NGPageLayoutAlgorithm::CreateConstraintSpaceForPages(
if (NGBaseline::ShouldPropagateBaselines(Node())) if (NGBaseline::ShouldPropagateBaselines(Node()))
space_builder.AddBaselineRequests(ConstraintSpace().BaselineRequests()); space_builder.AddBaselineRequests(ConstraintSpace().BaselineRequests());
// TODO(mstensho): Handle auto block size. For now just disable fragmentation // TODO(mstensho): Handle auto block size.
// if block size is auto. With the current approach, we'll just end up with
// one tall page. This is only correct if there are no explicit page breaks.
if (page_size.block_size != NGSizeIndefinite) {
space_builder.SetFragmentationType(kFragmentPage); space_builder.SetFragmentationType(kFragmentPage);
space_builder.SetFragmentainerBlockSize(page_size.block_size); space_builder.SetFragmentainerBlockSize(page_size.block_size);
space_builder.SetFragmentainerSpaceAtBfcStart(page_size.block_size); space_builder.SetFragmentainerSpaceAtBfcStart(page_size.block_size);
}
space_builder.SetIsNewFormattingContext(true); space_builder.SetIsNewFormattingContext(true);
space_builder.SetIsAnonymous(true); space_builder.SetIsAnonymous(true);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment