From efd88ee9995dca701786cd13204d57899335f569 Mon Sep 17 00:00:00 2001 From: win7-7 Date: Wed, 29 Jan 2020 11:14:34 +0200 Subject: Issue #1355 - Speed up the traversal of a table row frame's child cells Speed up getting the first cellframe in a row and the next cellframe after the given one --- layout/tables/nsTableCellFrame.cpp | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index ec9458f76..043509fc4 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -59,20 +59,6 @@ nsTableCellFrame::~nsTableCellFrame() NS_IMPL_FRAMEARENA_HELPERS(nsTableCellFrame) -nsTableCellFrame* -nsTableCellFrame::GetNextCell() const -{ - nsIFrame* childFrame = GetNextSibling(); - while (childFrame) { - nsTableCellFrame *cellFrame = do_QueryFrame(childFrame); - if (cellFrame) { - return cellFrame; - } - childFrame = childFrame->GetNextSibling(); - } - return nullptr; -} - void nsTableCellFrame::Init(nsIContent* aContent, nsContainerFrame* aParent, -- cgit v1.2.3 From fddc35e4384c20b1ad5d0c9818f490be29b1ece5 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 2 Feb 2020 02:16:41 +0100 Subject: Issue #1378 - Align the drawing of table cell backgrounds with the spec. --- layout/tables/nsTableCellFrame.cpp | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index 043509fc4..2862bb201 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -450,19 +450,40 @@ PaintTableCellSelection(nsIFrame* aFrame, DrawTarget* aDrawTarget, aPt); } +bool +nsTableCellFrame::ShouldPaintBordersAndBackgrounds() const +{ + // If we're not visible, we don't paint. + if (!StyleVisibility()->IsVisible()) { + return false; + } + + // Consider 'empty-cells', but only in separated borders mode. + if (!GetContentEmpty()) { + return true; + } + + nsTableFrame* tableFrame = GetTableFrame(); + if (tableFrame->IsBorderCollapse()) { + return true; + } + + return StyleTableBorder()->mEmptyCells == NS_STYLE_TABLE_EMPTY_CELLS_SHOW; +} + +bool +nsTableCellFrame::ShouldPaintBackground(nsDisplayListBuilder* aBuilder) +{ + return ShouldPaintBordersAndBackgrounds() && IsVisibleInSelection(aBuilder); +} + void nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { DO_GLOBAL_REFLOW_COUNT_DSP("nsTableCellFrame"); - nsTableFrame* tableFrame = GetTableFrame(); - int32_t emptyCellStyle = GetContentEmpty() && !tableFrame->IsBorderCollapse() ? - StyleTableBorder()->mEmptyCells - : NS_STYLE_TABLE_EMPTY_CELLS_SHOW; - // take account of 'empty-cells' - if (StyleVisibility()->IsVisible() && - (NS_STYLE_TABLE_EMPTY_CELLS_HIDE != emptyCellStyle)) { + if (ShouldPaintBordersAndBackgrounds()) { // display outset box-shadows if we need to. bool hasBoxShadow = !!StyleEffects()->mBoxShadow; if (hasBoxShadow) { @@ -487,7 +508,7 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, } // display borders if we need to - ProcessBorders(tableFrame, aBuilder, aLists); + ProcessBorders(GetTableFrame(), aBuilder, aLists); // and display the selection border if we need to if (IsSelected()) { -- cgit v1.2.3 From be372691233b6e8d359cb31df337f3ee3f6cb68d Mon Sep 17 00:00:00 2001 From: win7-7 Date: Mon, 3 Feb 2020 20:47:41 +0200 Subject: Issue #1386 - Devirtualize GetRowSpan/GetColSpan It's at ~1.5% on the perf log for the Netflix use case, which seems a bit too much. --- layout/tables/nsTableCellFrame.cpp | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index 2862bb201..d1736995e 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -711,16 +711,18 @@ nsTableCellFrame::GetCellBaseline() const borderPadding; } -int32_t nsTableCellFrame::GetRowSpan() +int32_t +nsTableCellFrame::GetRowSpan() { int32_t rowSpan=1; - nsGenericHTMLElement *hc = nsGenericHTMLElement::FromContent(mContent); // Don't look at the content's rowspan if we're a pseudo cell - if (hc && !StyleContext()->GetPseudo()) { - const nsAttrValue* attr = hc->GetParsedAttr(nsGkAtoms::rowspan); + if (!StyleContext()->GetPseudo()) { + dom::Element* elem = mContent->AsElement(); + const nsAttrValue* attr = elem->GetParsedAttr(nsGkAtoms::rowspan); // Note that we don't need to check the tag name, because only table cells - // and table headers parse the "rowspan" attribute into an integer. + // (including MathML ) and table headers parse the "rowspan" attribute + // into an integer. if (attr && attr->Type() == nsAttrValue::eInteger) { rowSpan = attr->GetIntegerValue(); } @@ -728,16 +730,20 @@ int32_t nsTableCellFrame::GetRowSpan() return rowSpan; } -int32_t nsTableCellFrame::GetColSpan() +int32_t +nsTableCellFrame::GetColSpan() { int32_t colSpan=1; - nsGenericHTMLElement *hc = nsGenericHTMLElement::FromContent(mContent); // Don't look at the content's colspan if we're a pseudo cell - if (hc && !StyleContext()->GetPseudo()) { - const nsAttrValue* attr = hc->GetParsedAttr(nsGkAtoms::colspan); + if (!StyleContext()->GetPseudo()) { + dom::Element* elem = mContent->AsElement(); + const nsAttrValue* attr = elem->GetParsedAttr( + MOZ_UNLIKELY(elem->IsMathMLElement()) ? nsGkAtoms::columnspan_ + : nsGkAtoms::colspan); // Note that we don't need to check the tag name, because only table cells - // and table headers parse the "colspan" attribute into an integer. + // (including MathML ) and table headers parse the "colspan" attribute + // into an integer. if (attr && attr->Type() == nsAttrValue::eInteger) { colSpan = attr->GetIntegerValue(); } -- cgit v1.2.3 From 28971524e4d4832e5c92276cc989535e75f04acc Mon Sep 17 00:00:00 2001 From: win7-7 Date: Wed, 5 Feb 2020 16:57:42 +0200 Subject: fix whitespace fix whitespace. --- layout/tables/nsTableCellFrame.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index d1736995e..cd846efa2 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -740,7 +740,7 @@ nsTableCellFrame::GetColSpan() dom::Element* elem = mContent->AsElement(); const nsAttrValue* attr = elem->GetParsedAttr( MOZ_UNLIKELY(elem->IsMathMLElement()) ? nsGkAtoms::columnspan_ - : nsGkAtoms::colspan); + : nsGkAtoms::colspan); // Note that we don't need to check the tag name, because only table cells // (including MathML ) and table headers parse the "colspan" attribute // into an integer. -- cgit v1.2.3 From c9fd86cb784b81ab30461b773667b7251347fae6 Mon Sep 17 00:00:00 2001 From: win7-7 Date: Sun, 16 Feb 2020 16:06:53 +0200 Subject: Issue #1355 - Make nsTableCellFrame::GetColIndex/GetRowIndex faster We can devirtualize it, remove some branches. --- layout/tables/nsTableCellFrame.cpp | 51 ++++++-------------------------------- 1 file changed, 8 insertions(+), 43 deletions(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index cd846efa2..8b811df1e 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -74,8 +74,7 @@ nsTableCellFrame::Init(nsIContent* aContent, if (aPrevInFlow) { // Set the column index nsTableCellFrame* cellFrame = (nsTableCellFrame*)aPrevInFlow; - int32_t colIndex; - cellFrame->GetColIndex(colIndex); + uint32_t colIndex = cellFrame->ColIndex(); SetColIndex(colIndex); } } @@ -168,34 +167,6 @@ nsTableCellFrame::NeedsToObserve(const ReflowInput& aReflowInput) fType == nsGkAtoms::tableWrapperFrame); } -nsresult -nsTableCellFrame::GetRowIndex(int32_t &aRowIndex) const -{ - nsresult result; - nsTableRowFrame* row = static_cast(GetParent()); - if (row) { - aRowIndex = row->GetRowIndex(); - result = NS_OK; - } - else { - aRowIndex = 0; - result = NS_ERROR_NOT_INITIALIZED; - } - return result; -} - -nsresult -nsTableCellFrame::GetColIndex(int32_t &aColIndex) const -{ - if (GetPrevInFlow()) { - return static_cast(FirstInFlow())->GetColIndex(aColIndex); - } - else { - aColIndex = mColIndex; - return NS_OK; - } -} - nsresult nsTableCellFrame::AttributeChanged(int32_t aNameSpaceID, nsIAtom* aAttribute, @@ -224,13 +195,13 @@ nsTableCellFrame::DidSetStyleContext(nsStyleContext* aOldStyleContext) nsTableFrame* tableFrame = GetTableFrame(); if (tableFrame->IsBorderCollapse() && tableFrame->BCRecalcNeeded(aOldStyleContext, StyleContext())) { - int32_t colIndex, rowIndex; - GetColIndex(colIndex); - GetRowIndex(rowIndex); + uint32_t colIndex = ColIndex(); + uint32_t rowIndex = RowIndex(); // row span needs to be clamped as we do not create rows in the cellmap // which do not have cells originating in them TableArea damageArea(colIndex, rowIndex, GetColSpan(), - std::min(GetRowSpan(), tableFrame->GetRowCount() - rowIndex)); + std::min(static_cast(GetRowSpan()), + tableFrame->GetRowCount() - rowIndex)); tableFrame->AddBCDamageArea(damageArea); } } @@ -820,14 +791,13 @@ CalcUnpaginatedBSize(nsTableCellFrame& aCellFrame, nsTableRowGroupFrame* firstRGInFlow = static_cast(row->GetParent()); - int32_t rowIndex; - firstCellInFlow->GetRowIndex(rowIndex); + uint32_t rowIndex = firstCellInFlow->RowIndex(); int32_t rowSpan = aTableFrame.GetEffectiveRowSpan(*firstCellInFlow); nscoord computedBSize = firstTableInFlow->GetRowSpacing(rowIndex, rowIndex + rowSpan - 1); computedBSize -= aBlockDirBorderPadding; - int32_t rowX; + uint32_t rowX; for (row = firstRGInFlow->GetFirstRow(), rowX = 0; row; row = row->GetNextRow(), rowX++) { if (rowX > rowIndex + rowSpan - 1) { break; @@ -1042,12 +1012,7 @@ nsTableCellFrame::AccessibleType() NS_IMETHODIMP nsTableCellFrame::GetCellIndexes(int32_t &aRowIndex, int32_t &aColIndex) { - nsresult res = GetRowIndex(aRowIndex); - if (NS_FAILED(res)) - { - aColIndex = 0; - return res; - } + aRowIndex = RowIndex(); aColIndex = mColIndex; return NS_OK; } -- cgit v1.2.3 From cbb61ab832508e9c231a256fb161d38d35faeabf Mon Sep 17 00:00:00 2001 From: win7-7 Date: Tue, 25 Feb 2020 00:17:54 +0200 Subject: Issue #1355 - Better way to create display items for column backgrounds Part 1: Remove current table item, as it's never set. Part 2: Get rid of generic table painting code, and handle each class separately. Part 4: Hoist outline skipping into col(group) frame code. Part 5: Skip box-shadow for table column and column groups. Part 6: Store column and column group backgrounds separately, and then append them before the rest of the table contents. Part 7: Pass rects in display list coordinates to AppendBackgroundItemsToTop. Part 8: Create column and column group background display items as part of the cell's BuildDisplayList. Part 9: Used cached values instead of calling nsDisplayListBuilder::ToReferenceFrame when possible, since it can be expensive when the requested frame isn't the builder's current frame. Part 10: Make sure we build display items for table parts where only the normal position is visible, since we may need to create background items for ancestors at that position. Part 11: Create an AutoBuildingDisplayList when we create background items for table columns and column groups, so that we initialize the invalidation state correctly. --- layout/tables/nsTableCellFrame.cpp | 54 ++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index 8b811df1e..9c715d999 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -450,7 +450,6 @@ nsTableCellFrame::ShouldPaintBackground(nsDisplayListBuilder* aBuilder) void nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { DO_GLOBAL_REFLOW_COUNT_DSP("nsTableCellFrame"); @@ -462,14 +461,14 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, this)); } + nsRect bgRect = GetRectRelativeToSelf() + aBuilder->ToReferenceFrame(this); + // display background if we need to. if (aBuilder->IsForEventDelivery() || !StyleBackground()->IsTransparent() || StyleDisplay()->mAppearance) { - nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, - this, - GetRectRelativeToSelf(), - aLists.BorderBackground()); + nsDisplayBackgroundImage::AppendBackgroundItemsToTop( + aBuilder, this, bgRect, aLists.BorderBackground()); } // display inset box-shadows if we need to. @@ -488,16 +487,49 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, "TableCellSelection", nsDisplayItem::TYPE_TABLE_CELL_SELECTION)); } + + // This can be null if display list building initiated in the middle + // of the table, which can happen with background-clip:text and + // -moz-element. + nsDisplayTableBackgroundSet* backgrounds = + aBuilder->GetTableBackgroundSet(); + if (backgrounds) { + // Compute bgRect relative to reference frame, but using the + // normal (without position:relative offsets) positions for the + // cell, row and row group. + bgRect = GetRectRelativeToSelf() + GetNormalPosition(); + + nsTableRowFrame* row = GetTableRowFrame(); + bgRect += row->GetNormalPosition(); + + nsTableRowGroupFrame* rowGroup = row->GetTableRowGroupFrame(); + bgRect += rowGroup->GetNormalPosition(); + + bgRect += backgrounds->TableToReferenceFrame(); + + // Create backgrounds items as needed for the column and column + // group that this cell occupies. + nsTableColFrame* col = backgrounds->GetColForIndex(ColIndex()); + nsTableColGroupFrame* colGroup = col->GetTableColGroupFrame(); + + Maybe buildingForColGroup; + nsDisplayBackgroundImage::AppendBackgroundItemsToTop( + aBuilder, colGroup, bgRect, backgrounds->ColGroupBackgrounds(), false, + nullptr, colGroup->GetRect() + backgrounds->TableToReferenceFrame(), + this, &buildingForColGroup); + + Maybe buildingForCol; + nsDisplayBackgroundImage::AppendBackgroundItemsToTop( + aBuilder, col, bgRect, backgrounds->ColBackgrounds(), false, nullptr, + col->GetRect() + colGroup->GetPosition() + + backgrounds->TableToReferenceFrame(), + this, &buildingForCol); + } } // the 'empty-cells' property has no effect on 'outline' DisplayOutline(aBuilder, aLists); - // Push a null 'current table item' so that descendant tables can't - // accidentally mess with our table - nsAutoPushCurrentTableItem pushTableItem; - pushTableItem.Push(aBuilder, nullptr); - nsIFrame* kid = mFrames.FirstChild(); NS_ASSERTION(kid && !kid->GetNextSibling(), "Table cells should have just one child"); // The child's background will go in our BorderBackground() list. @@ -506,7 +538,7 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, // because that/ would put the child's background in the Content() list // which isn't right (e.g., would end up on top of our child floats for // event handling). - BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists); + BuildDisplayListForChild(aBuilder, kid, aLists); } nsIFrame::LogicalSides -- cgit v1.2.3 From dba09fa5c43276bb455cc4da6bd0ec302f798189 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Wed, 26 Feb 2020 20:51:22 +0100 Subject: Revert "Issue #1355 - Better way to create display items for column backgrounds" This reverts commit 44c47c50388f526c2d134e16d5debebe94a0faf8. --- layout/tables/nsTableCellFrame.cpp | 54 ++++++++------------------------------ 1 file changed, 11 insertions(+), 43 deletions(-) (limited to 'layout/tables/nsTableCellFrame.cpp') diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index 9c715d999..8b811df1e 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -450,6 +450,7 @@ nsTableCellFrame::ShouldPaintBackground(nsDisplayListBuilder* aBuilder) void nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, + const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { DO_GLOBAL_REFLOW_COUNT_DSP("nsTableCellFrame"); @@ -461,14 +462,14 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, this)); } - nsRect bgRect = GetRectRelativeToSelf() + aBuilder->ToReferenceFrame(this); - // display background if we need to. if (aBuilder->IsForEventDelivery() || !StyleBackground()->IsTransparent() || StyleDisplay()->mAppearance) { - nsDisplayBackgroundImage::AppendBackgroundItemsToTop( - aBuilder, this, bgRect, aLists.BorderBackground()); + nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, + this, + GetRectRelativeToSelf(), + aLists.BorderBackground()); } // display inset box-shadows if we need to. @@ -487,49 +488,16 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, "TableCellSelection", nsDisplayItem::TYPE_TABLE_CELL_SELECTION)); } - - // This can be null if display list building initiated in the middle - // of the table, which can happen with background-clip:text and - // -moz-element. - nsDisplayTableBackgroundSet* backgrounds = - aBuilder->GetTableBackgroundSet(); - if (backgrounds) { - // Compute bgRect relative to reference frame, but using the - // normal (without position:relative offsets) positions for the - // cell, row and row group. - bgRect = GetRectRelativeToSelf() + GetNormalPosition(); - - nsTableRowFrame* row = GetTableRowFrame(); - bgRect += row->GetNormalPosition(); - - nsTableRowGroupFrame* rowGroup = row->GetTableRowGroupFrame(); - bgRect += rowGroup->GetNormalPosition(); - - bgRect += backgrounds->TableToReferenceFrame(); - - // Create backgrounds items as needed for the column and column - // group that this cell occupies. - nsTableColFrame* col = backgrounds->GetColForIndex(ColIndex()); - nsTableColGroupFrame* colGroup = col->GetTableColGroupFrame(); - - Maybe buildingForColGroup; - nsDisplayBackgroundImage::AppendBackgroundItemsToTop( - aBuilder, colGroup, bgRect, backgrounds->ColGroupBackgrounds(), false, - nullptr, colGroup->GetRect() + backgrounds->TableToReferenceFrame(), - this, &buildingForColGroup); - - Maybe buildingForCol; - nsDisplayBackgroundImage::AppendBackgroundItemsToTop( - aBuilder, col, bgRect, backgrounds->ColBackgrounds(), false, nullptr, - col->GetRect() + colGroup->GetPosition() + - backgrounds->TableToReferenceFrame(), - this, &buildingForCol); - } } // the 'empty-cells' property has no effect on 'outline' DisplayOutline(aBuilder, aLists); + // Push a null 'current table item' so that descendant tables can't + // accidentally mess with our table + nsAutoPushCurrentTableItem pushTableItem; + pushTableItem.Push(aBuilder, nullptr); + nsIFrame* kid = mFrames.FirstChild(); NS_ASSERTION(kid && !kid->GetNextSibling(), "Table cells should have just one child"); // The child's background will go in our BorderBackground() list. @@ -538,7 +506,7 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, // because that/ would put the child's background in the Content() list // which isn't right (e.g., would end up on top of our child floats for // event handling). - BuildDisplayListForChild(aBuilder, kid, aLists); + BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists); } nsIFrame::LogicalSides -- cgit v1.2.3