Commit df684fb8 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[TablesNG] Implement PhysicalFragment::IsTableNGPart

Landing TablesNG paint caused an 8% perf regression in
blink_perf.paint large-table-background-change.html

In my pinpoint measurements, I saw a 6% regression.
https://pinpoint-dot-chromeperf.appspot.com/job/16ba906e520000

This patch brings the regression down to ~3%.

The regression was caused by queries for fragment type.
IsTableNGPart boolean has been added to PhysicalFragment,
allowing quicker response to IsTableSection/Row queries.

Bug: 1143139, 1143168
Change-Id: Ifa02a75119bfcef657106807c304dcd4a39ac3b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510830
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822886}
parent 3cdffc30
...@@ -2165,6 +2165,8 @@ void NGBlockLayoutAlgorithm::FinalizeForTableCell( ...@@ -2165,6 +2165,8 @@ void NGBlockLayoutAlgorithm::FinalizeForTableCell(
if (ConstraintSpace().IsLegacyTableCell()) if (ConstraintSpace().IsLegacyTableCell())
return; return;
container_builder_.SetIsTableNGPart();
container_builder_.SetTableCellColumnIndex( container_builder_.SetTableCellColumnIndex(
ConstraintSpace().TableCellColumnIndex()); ConstraintSpace().TableCellColumnIndex());
......
...@@ -405,6 +405,7 @@ class CORE_EXPORT NGBoxFragmentBuilder final ...@@ -405,6 +405,7 @@ class CORE_EXPORT NGBoxFragmentBuilder final
return BoxType() == NGPhysicalFragment::kColumnBox; return BoxType() == NGPhysicalFragment::kColumnBox;
} }
void SetIsFieldsetContainer() { is_fieldset_container_ = true; } void SetIsFieldsetContainer() { is_fieldset_container_ = true; }
void SetIsTableNGPart() { is_table_ng_part_ = true; }
void SetIsLegacyLayoutRoot() { is_legacy_layout_root_ = true; } void SetIsLegacyLayoutRoot() { is_legacy_layout_root_ = true; }
void SetIsInlineFormattingContext(bool is_inline_formatting_context) { void SetIsInlineFormattingContext(bool is_inline_formatting_context) {
...@@ -576,6 +577,7 @@ class CORE_EXPORT NGBoxFragmentBuilder final ...@@ -576,6 +577,7 @@ class CORE_EXPORT NGBoxFragmentBuilder final
NGPhysicalFragment::NGBoxType box_type_; NGPhysicalFragment::NGBoxType box_type_;
bool may_have_descendant_above_block_start_ = false; bool may_have_descendant_above_block_start_ = false;
bool is_fieldset_container_ = false; bool is_fieldset_container_ = false;
bool is_table_ng_part_ = false;
bool is_initial_block_size_indefinite_ = false; bool is_initial_block_size_indefinite_ = false;
bool is_inline_formatting_context_; bool is_inline_formatting_context_;
bool is_first_for_node_ = true; bool is_first_for_node_ = true;
......
...@@ -224,6 +224,7 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment( ...@@ -224,6 +224,7 @@ NGPhysicalBoxFragment::NGPhysicalBoxFragment(
may_have_descendant_above_block_start_ = may_have_descendant_above_block_start_ =
builder->may_have_descendant_above_block_start_; builder->may_have_descendant_above_block_start_;
is_fieldset_container_ = builder->is_fieldset_container_; is_fieldset_container_ = builder->is_fieldset_container_;
is_table_ng_part_ = builder->is_table_ng_part_;
is_legacy_layout_root_ = builder->is_legacy_layout_root_; is_legacy_layout_root_ = builder->is_legacy_layout_root_;
is_painted_atomically_ = is_painted_atomically_ =
builder->space_ && builder->space_->IsPaintedAtomically(); builder->space_ && builder->space_->IsPaintedAtomically();
...@@ -807,6 +808,7 @@ void NGPhysicalBoxFragment::CheckSameForSimplifiedLayout( ...@@ -807,6 +808,7 @@ void NGPhysicalBoxFragment::CheckSameForSimplifiedLayout(
other.depends_on_percentage_block_size_); other.depends_on_percentage_block_size_);
DCHECK_EQ(is_fieldset_container_, other.is_fieldset_container_); DCHECK_EQ(is_fieldset_container_, other.is_fieldset_container_);
DCHECK_EQ(is_table_ng_part_, other.is_table_ng_part_);
DCHECK_EQ(is_legacy_layout_root_, other.is_legacy_layout_root_); DCHECK_EQ(is_legacy_layout_root_, other.is_legacy_layout_root_);
DCHECK_EQ(is_painted_atomically_, other.is_painted_atomically_); DCHECK_EQ(is_painted_atomically_, other.is_painted_atomically_);
DCHECK_EQ(has_collapsed_borders_, other.has_collapsed_borders_); DCHECK_EQ(has_collapsed_borders_, other.has_collapsed_borders_);
......
...@@ -277,6 +277,7 @@ NGPhysicalFragment::NGPhysicalFragment(NGFragmentBuilder* builder, ...@@ -277,6 +277,7 @@ NGPhysicalFragment::NGPhysicalFragment(NGFragmentBuilder* builder,
style_variant_((unsigned)builder->style_variant_), style_variant_((unsigned)builder->style_variant_),
is_hidden_for_paint_(builder->is_hidden_for_paint_), is_hidden_for_paint_(builder->is_hidden_for_paint_),
is_fieldset_container_(false), is_fieldset_container_(false),
is_table_ng_part_(false),
is_legacy_layout_root_(false), is_legacy_layout_root_(false),
is_painted_atomically_(false), is_painted_atomically_(false),
has_collapsed_borders_(builder->has_collapsed_borders_), has_collapsed_borders_(builder->has_collapsed_borders_),
...@@ -300,6 +301,7 @@ NGPhysicalFragment::NGPhysicalFragment(LayoutObject* layout_object, ...@@ -300,6 +301,7 @@ NGPhysicalFragment::NGPhysicalFragment(LayoutObject* layout_object,
style_variant_((unsigned)style_variant), style_variant_((unsigned)style_variant),
is_hidden_for_paint_(false), is_hidden_for_paint_(false),
is_fieldset_container_(false), is_fieldset_container_(false),
is_table_ng_part_(false),
is_legacy_layout_root_(false), is_legacy_layout_root_(false),
is_painted_atomically_(false), is_painted_atomically_(false),
has_collapsed_borders_(false), has_collapsed_borders_(false),
...@@ -343,6 +345,7 @@ NGPhysicalFragment::NGPhysicalFragment(const NGPhysicalFragment& other) ...@@ -343,6 +345,7 @@ NGPhysicalFragment::NGPhysicalFragment(const NGPhysicalFragment& other)
may_have_descendant_above_block_start_( may_have_descendant_above_block_start_(
other.may_have_descendant_above_block_start_), other.may_have_descendant_above_block_start_),
is_fieldset_container_(other.is_fieldset_container_), is_fieldset_container_(other.is_fieldset_container_),
is_table_ng_part_(other.is_table_ng_part_),
is_legacy_layout_root_(other.is_legacy_layout_root_), is_legacy_layout_root_(other.is_legacy_layout_root_),
is_painted_atomically_(other.is_painted_atomically_), is_painted_atomically_(other.is_painted_atomically_),
has_collapsed_borders_(other.has_collapsed_borders_), has_collapsed_borders_(other.has_collapsed_borders_),
......
...@@ -160,14 +160,20 @@ class CORE_EXPORT NGPhysicalFragment ...@@ -160,14 +160,20 @@ class CORE_EXPORT NGPhysicalFragment
layout_object_->IsRubyBase(); layout_object_->IsRubyBase();
} }
bool IsTable() const { return layout_object_->IsTable(); } bool IsTableNGPart() const { return is_table_ng_part_; }
bool IsTableRow() const { return layout_object_->IsTableRow(); } bool IsTable() const { return IsBox() && layout_object_->IsTable(); }
bool IsTableSection() const { return layout_object_->IsTableSection(); } bool IsTableNGRow() const {
return IsTableNGPart() && layout_object_->IsTableRow();
}
bool IsTableNGSection() const {
return IsTableNGPart() && layout_object_->IsTableSection();
}
bool IsTableNGCell() const { bool IsTableNGCell() const {
return layout_object_->IsTableCell() && return IsTableNGPart() && layout_object_->IsTableCell() &&
!layout_object_->IsTableCellLegacy(); !layout_object_->IsTableCellLegacy();
} }
...@@ -509,6 +515,7 @@ class CORE_EXPORT NGPhysicalFragment ...@@ -509,6 +515,7 @@ class CORE_EXPORT NGPhysicalFragment
// The following are only used by NGPhysicalBoxFragment but are initialized // The following are only used by NGPhysicalBoxFragment but are initialized
// for all types to allow methods using them to be inlined. // for all types to allow methods using them to be inlined.
unsigned is_fieldset_container_ : 1; unsigned is_fieldset_container_ : 1;
unsigned is_table_ng_part_ : 1;
unsigned is_legacy_layout_root_ : 1; unsigned is_legacy_layout_root_ : 1;
unsigned is_painted_atomically_ : 1; unsigned is_painted_atomically_ : 1;
unsigned has_collapsed_borders_ : 1; unsigned has_collapsed_borders_ : 1;
......
...@@ -117,14 +117,16 @@ NGSimplifiedLayoutAlgorithm::NGSimplifiedLayoutAlgorithm( ...@@ -117,14 +117,16 @@ NGSimplifiedLayoutAlgorithm::NGSimplifiedLayoutAlgorithm(
// TODO(atotic,ikilpatrick): Copy across table related data for table, // TODO(atotic,ikilpatrick): Copy across table related data for table,
// table-row, table-section. // table-row, table-section.
DCHECK(!physical_fragment.IsTable()); DCHECK(!physical_fragment.IsTable());
DCHECK(!physical_fragment.IsTableRow()); DCHECK(!physical_fragment.IsTableNGRow());
DCHECK(!physical_fragment.IsTableSection()); DCHECK(!physical_fragment.IsTableNGSection());
if (physical_fragment.IsHiddenForPaint()) if (physical_fragment.IsHiddenForPaint())
container_builder_.SetIsHiddenForPaint(true); container_builder_.SetIsHiddenForPaint(true);
if (physical_fragment.Baseline()) if (physical_fragment.Baseline())
container_builder_.SetBaseline(*physical_fragment.Baseline()); container_builder_.SetBaseline(*physical_fragment.Baseline());
if (physical_fragment.IsTableNGPart())
container_builder_.SetIsTableNGPart();
container_builder_.SetIntrinsicBlockSize(result.IntrinsicBlockSize()); container_builder_.SetIntrinsicBlockSize(result.IntrinsicBlockSize());
container_builder_.SetOverflowBlockSize(result.OverflowBlockSize()); container_builder_.SetOverflowBlockSize(result.OverflowBlockSize());
......
...@@ -924,6 +924,7 @@ scoped_refptr<const NGLayoutResult> NGTableLayoutAlgorithm::GenerateFragment( ...@@ -924,6 +924,7 @@ scoped_refptr<const NGLayoutResult> NGTableLayoutAlgorithm::GenerateFragment(
if (table_baseline) if (table_baseline)
container_builder_.SetBaseline(*table_baseline); container_builder_.SetBaseline(*table_baseline);
container_builder_.SetIsTableNGPart();
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run(); NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run();
return container_builder_.ToBoxFragment(); return container_builder_.ToBoxFragment();
} }
......
...@@ -145,6 +145,7 @@ scoped_refptr<const NGLayoutResult> NGTableRowLayoutAlgorithm::Layout() { ...@@ -145,6 +145,7 @@ scoped_refptr<const NGLayoutResult> NGTableRowLayoutAlgorithm::Layout() {
reported_row_baseline.value_or(row.block_size)); reported_row_baseline.value_or(row.block_size));
if (row.is_collapsed) if (row.is_collapsed)
container_builder_.SetIsHiddenForPaint(true); container_builder_.SetIsHiddenForPaint(true);
container_builder_.SetIsTableNGPart();
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run(); NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run();
return container_builder_.ToBoxFragment(); return container_builder_.ToBoxFragment();
} }
......
...@@ -76,6 +76,7 @@ scoped_refptr<const NGLayoutResult> NGTableSectionLayoutAlgorithm::Layout() { ...@@ -76,6 +76,7 @@ scoped_refptr<const NGLayoutResult> NGTableSectionLayoutAlgorithm::Layout() {
container_builder_.SetFragmentBlockSize(offset.block_offset); container_builder_.SetFragmentBlockSize(offset.block_offset);
if (section_baseline) if (section_baseline)
container_builder_.SetBaseline(*section_baseline); container_builder_.SetBaseline(*section_baseline);
container_builder_.SetIsTableNGPart();
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run(); NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run();
return container_builder_.ToBoxFragment(); return container_builder_.ToBoxFragment();
......
...@@ -506,7 +506,8 @@ bool NGBoxFragmentPainter::ShouldRecordHitTestData( ...@@ -506,7 +506,8 @@ bool NGBoxFragmentPainter::ShouldRecordHitTestData(
return false; return false;
// Table rows/sections do not participate in hit testing. // Table rows/sections do not participate in hit testing.
if (PhysicalFragment().IsTableRow() || PhysicalFragment().IsTableSection()) if (PhysicalFragment().IsTableNGRow() ||
PhysicalFragment().IsTableNGSection())
return false; return false;
return true; return true;
...@@ -760,7 +761,8 @@ void NGBoxFragmentPainter::PaintBlockChildren(const PaintInfo& paint_info, ...@@ -760,7 +761,8 @@ void NGBoxFragmentPainter::PaintBlockChildren(const PaintInfo& paint_info,
layout_object->Paint(paint_info_for_descendants); layout_object->Paint(paint_info_for_descendants);
} }
} }
if (paint_info.phase == PaintPhase::kForeground && box_fragment_.IsTable()) { if (paint_info.phase == PaintPhase::kForeground &&
box_fragment_.IsTableNGPart() && box_fragment_.IsTable()) {
NGTablePainter(box_fragment_) NGTablePainter(box_fragment_)
.PaintCollapsedBorders(paint_info, paint_offset, .PaintCollapsedBorders(paint_info, paint_offset,
VisualRect(paint_offset)); VisualRect(paint_offset));
...@@ -986,23 +988,30 @@ void NGBoxFragmentPainter::PaintBoxDecorationBackground( ...@@ -986,23 +988,30 @@ void NGBoxFragmentPainter::PaintBoxDecorationBackground(
if (PhysicalFragment().IsFieldsetContainer()) { if (PhysicalFragment().IsFieldsetContainer()) {
NGFieldsetPainter(box_fragment_) NGFieldsetPainter(box_fragment_)
.PaintBoxDecorationBackground(paint_info, paint_offset); .PaintBoxDecorationBackground(paint_info, paint_offset);
} else if (box_fragment_.IsTable()) { } else if (PhysicalFragment().IsTableNGPart()) {
NGTablePainter(box_fragment_) if (box_fragment_.IsTableNGCell()) {
.PaintBoxDecorationBackground(paint_info, paint_offset, visual_rect); // Just doing this without any collapsed borders makes 2 extra tests
} else if (box_fragment_.IsTableSection()) { // pass, they used to be off by 1 pixel.
NGTableSectionPainter(box_fragment_) // external/wpt/css/css-backgrounds/background-image-table-cells-zoomed.html
.PaintBoxDecorationBackground(paint_info, paint_offset, visual_rect); // tables/mozilla/bugs/bug131020_iframe.html
} else if (box_fragment_.IsTableRow()) { // TODO(atotic+pdr) Flagged for followup code review.
NGTableRowPainter(box_fragment_) NGTableCellPainter(box_fragment_)
.PaintBoxDecorationBackground(paint_info, paint_offset, visual_rect); .PaintBoxDecorationBackground(paint_info, paint_offset,
} else if (box_fragment_.IsTableNGCell()) { visual_rect);
// Just doing this without any collapsed borders makes 2 extra tests pass, } else if (box_fragment_.IsTableNGRow()) {
// they used to be off by 1 pixel. NGTableRowPainter(box_fragment_)
// external/wpt/css/css-backgrounds/background-image-table-cells-zoomed.html .PaintBoxDecorationBackground(paint_info, paint_offset,
// tables/mozilla/bugs/bug131020_iframe.html visual_rect);
// TODO(atotic+pdr) Flagged for followup code review. } else if (box_fragment_.IsTableNGSection()) {
NGTableCellPainter(box_fragment_) NGTableSectionPainter(box_fragment_)
.PaintBoxDecorationBackground(paint_info, paint_offset, visual_rect); .PaintBoxDecorationBackground(paint_info, paint_offset,
visual_rect);
} else {
DCHECK(box_fragment_.IsTable());
NGTablePainter(box_fragment_)
.PaintBoxDecorationBackground(paint_info, paint_offset,
visual_rect);
}
} else if (box_fragment_.Style().HasBoxDecorationBackground()) { } else if (box_fragment_.Style().HasBoxDecorationBackground()) {
PaintBoxDecorationBackgroundWithRect( PaintBoxDecorationBackgroundWithRect(
contents_paint_state ? contents_paint_state->GetPaintInfo() contents_paint_state ? contents_paint_state->GetPaintInfo()
...@@ -1915,7 +1924,8 @@ bool NGBoxFragmentPainter::NodeAtPoint(const HitTestContext& hit_test, ...@@ -1915,7 +1924,8 @@ bool NGBoxFragmentPainter::NodeAtPoint(const HitTestContext& hit_test,
bool hit_test_self = fragment.IsInSelfHitTestingPhase(hit_test.action); bool hit_test_self = fragment.IsInSelfHitTestingPhase(hit_test.action);
if (hit_test_self) { if (hit_test_self) {
// Table row and table section are never a hit target. // Table row and table section are never a hit target.
if (PhysicalFragment().IsTableRow() || PhysicalFragment().IsTableSection()) if (PhysicalFragment().IsTableNGRow() ||
PhysicalFragment().IsTableNGSection())
hit_test_self = false; hit_test_self = false;
} }
......
...@@ -352,7 +352,7 @@ void NGTablePainter::PaintBoxDecorationBackground( ...@@ -352,7 +352,7 @@ void NGTablePainter::PaintBoxDecorationBackground(
border_spacing.height}; border_spacing.height};
PhysicalRect columns_paint_rect(column_offset, column_size); PhysicalRect columns_paint_rect(column_offset, column_size);
for (const NGLink& child : fragment_.Children()) { for (const NGLink& child : fragment_.Children()) {
if (!child.fragment->IsTableSection()) if (!child.fragment->IsTableNGSection())
continue; // child is a caption. continue; // child is a caption.
PhysicalOffset section_offset = PhysicalOffset section_offset =
child.offset - fragment_.TableGridRect().offset; child.offset - fragment_.TableGridRect().offset;
......
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