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/nsTableFrame.cpp | 115 +++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 45 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 890d050fd..891d7e7cd 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1196,6 +1196,10 @@ PaintRowBackground(nsTableRowFrame* aRow, { // Compute background rect by iterating over all cell frames. for (nsTableCellFrame* cell = aRow->GetFirstCell(); cell; cell = cell->GetNextCell()) { + if (!cell->ShouldPaintBackground(aBuilder)) { + continue; + } + auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset; nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), @@ -1226,6 +1230,10 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { + if (!cell->ShouldPaintBackground(aBuilder)) { + continue; + } + int32_t curColIdx; cell->GetColIndex(curColIdx); if (aColIdx.Contains(curColIdx)) { @@ -1247,66 +1255,83 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists, DisplayGenericTablePartTraversal aTraversal) { - if (aFrame->IsVisibleForPainting(aBuilder)) { + bool isVisible = aFrame->IsVisibleForPainting(aBuilder); + bool isTable = (aFrame->GetType() == nsGkAtoms::tableFrame); + + if (isVisible || !isTable) { nsDisplayTableItem* currentItem = aBuilder->GetCurrentTableItem(); // currentItem may be null, when none of the table parts have a // background or border if (currentItem) { currentItem->UpdateForFrameBackground(aFrame); } + } + + if (isVisible) { + // XXX: should box-shadow for rows/rowgroups/columns/colgroups get painted + // just because we're visible? Or should it depend on the cell visibility + // when we're not the whole table? // Paint the outset box-shadows for the table frames - bool hasBoxShadow = aFrame->StyleEffects()->mBoxShadow != nullptr; - if (hasBoxShadow) { + if (aFrame->StyleEffects()->mBoxShadow) { aLists.BorderBackground()->AppendNewToTop( new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, aFrame)); } + } - if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { - nsTableRowGroupFrame* rowGroup = static_cast(aFrame); - PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists); - } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { - nsTableRowFrame* row = static_cast(aFrame); - PaintRowBackground(row, aFrame, aBuilder, aLists); - } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { - // Compute background rect by iterating all cell frame. - nsTableColGroupFrame* colGroup = static_cast(aFrame); - // Collecting column index. - AutoTArray colIdx; - for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { - colIdx.AppendElement(col->GetColIndex()); - } - - nsTableFrame* table = colGroup->GetTableFrame(); - RowGroupArray rowGroups; - table->OrderRowGroups(rowGroups); - for (nsTableRowGroupFrame* rowGroup : rowGroups) { - auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); - } - } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { - // Compute background rect by iterating all cell frame. - nsTableColFrame* col = static_cast(aFrame); - AutoTArray colIdx; + // Background visibility for rows, rowgroups, columns, colgroups depends on + // the visibility of the _cell_, not of the row/col(group). + // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds + if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { + nsTableRowGroupFrame* rowGroup = static_cast(aFrame); + PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists); + } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { + nsTableRowFrame* row = static_cast(aFrame); + PaintRowBackground(row, aFrame, aBuilder, aLists); + } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { + // Compute background rect by iterating all cell frame. + nsTableColGroupFrame* colGroup = static_cast(aFrame); + // Collecting column index. + AutoTArray colIdx; + for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { colIdx.AppendElement(col->GetColIndex()); + } - nsTableFrame* table = col->GetTableFrame(); - RowGroupArray rowGroups; - table->OrderRowGroups(rowGroups); - for (nsTableRowGroupFrame* rowGroup : rowGroups) { - auto offset = rowGroup->GetNormalPosition() - - col->GetNormalPosition() - - col->GetTableColGroupFrame()->GetNormalPosition(); - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); - } - } else { - nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, - aFrame->GetRectRelativeToSelf(), - aLists.BorderBackground()); + nsTableFrame* table = colGroup->GetTableFrame(); + RowGroupArray rowGroups; + table->OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); + } + } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { + // Compute background rect by iterating all cell frame. + nsTableColFrame* col = static_cast(aFrame); + AutoTArray colIdx; + colIdx.AppendElement(col->GetColIndex()); + + nsTableFrame* table = col->GetTableFrame(); + RowGroupArray rowGroups; + table->OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + auto offset = rowGroup->GetNormalPosition() - + col->GetNormalPosition() - + col->GetTableColGroupFrame()->GetNormalPosition(); + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); } + } else { + nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, + aFrame->GetRectRelativeToSelf(), + aLists.BorderBackground()); + } + + if (isVisible) { + // XXX: should box-shadow for rows/rowgroups/columns/colgroups get painted + // just because we're visible? Or should it depend on the cell visibility + // when we're not the whole table? // Paint the inset box-shadows for the table frames - if (hasBoxShadow) { + if (aFrame->StyleEffects()->mBoxShadow) { aLists.BorderBackground()->AppendNewToTop( new (aBuilder) nsDisplayBoxShadowInner(aBuilder, aFrame)); } @@ -1314,8 +1339,8 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, aTraversal(aBuilder, aFrame, aDirtyRect, aLists); - if (aFrame->IsVisibleForPainting(aBuilder)) { - if (aFrame->GetType() == nsGkAtoms::tableFrame) { + if (isVisible) { + if (isTable) { nsTableFrame* table = static_cast(aFrame); // In the collapsed border model, overlay all collapsed borders. if (table->IsBorderCollapse()) { -- cgit v1.2.3 From bcd6c96aecfd49bca6f4f66a9dc40b4e1683aa85 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 2 Feb 2020 11:31:07 +0100 Subject: Issue #1378 - Follow-up: make sure background items remain table-aligned. --- layout/tables/nsTableFrame.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 891d7e7cd..db6e31e2b 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1319,7 +1319,7 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, col->GetTableColGroupFrame()->GetNormalPosition(); PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); } - } else { + } else if (isVisible) { nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, aFrame->GetRectRelativeToSelf(), aLists.BorderBackground()); -- cgit v1.2.3 From 05c68a17a08fcfc309f7bfdc1cdf272cc298371f Mon Sep 17 00:00:00 2001 From: win7-7 Date: Sun, 2 Feb 2020 13:50:51 +0200 Subject: Issue #1355 - Hit testing in large tables has become extremely slow Add dirty rect intersection checks so that we don't build unnecessary table part display items. --- layout/tables/nsTableFrame.cpp | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index db6e31e2b..e903d6121 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1192,6 +1192,7 @@ PaintRowBackground(nsTableRowFrame* aRow, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists, + const nsRect& aDirtyRect, const nsPoint& aOffset = nsPoint()) { // Compute background rect by iterating over all cell frames. @@ -1201,6 +1202,9 @@ PaintRowBackground(nsTableRowFrame* aRow, } auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset; + if (!aDirtyRect.Intersects(cellRect)) { + continue; + } nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr, @@ -1213,10 +1217,14 @@ static void PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, - const nsDisplayListSet& aLists) + const nsDisplayListSet& aLists, + const nsRect& aDirtyRect) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - PaintRowBackground(row, aFrame, aBuilder, aLists, row->GetNormalPosition()); + if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { + continue; + } + PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition()); } } @@ -1229,6 +1237,10 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, const nsPoint& aOffset) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { + auto rowPos = row->GetNormalPosition() + aOffset; + if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) { + continue; + } for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { if (!cell->ShouldPaintBackground(aBuilder)) { continue; @@ -1237,7 +1249,11 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, int32_t curColIdx; cell->GetColIndex(curColIdx); if (aColIdx.Contains(curColIdx)) { - auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + row->GetNormalPosition() + aOffset; + auto cellPos = cell->GetNormalPosition() + rowPos; + auto cellRect = nsRect(cellPos, cell->GetSize()); + if (!aDirtyRect.Intersects(cellRect)) { + continue; + } nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr, @@ -1284,10 +1300,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { nsTableRowGroupFrame* rowGroup = static_cast(aFrame); - PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists); + PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists, aDirtyRect); } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { nsTableRowFrame* row = static_cast(aFrame); - PaintRowBackground(row, aFrame, aBuilder, aLists); + PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect); } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { // Compute background rect by iterating all cell frame. nsTableColGroupFrame* colGroup = static_cast(aFrame); @@ -1302,7 +1318,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, table->OrderRowGroups(rowGroups); for (nsTableRowGroupFrame* rowGroup : rowGroups) { auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; + } + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); } } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { // Compute background rect by iterating all cell frame. @@ -1317,7 +1336,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, auto offset = rowGroup->GetNormalPosition() - col->GetNormalPosition() - col->GetTableColGroupFrame()->GetNormalPosition(); - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; + } + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); } } else if (isVisible) { nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, -- cgit v1.2.3 From 58f10adc97365a082f9e1b885b6291b4053f400d Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 2 Feb 2020 13:37:13 +0100 Subject: Revert "Issue #1355 - Hit testing in large tables has become extremely slow" This reverts commit f7b2f0a66536e8e74a0b2dc071a098b7693acecb. --- layout/tables/nsTableFrame.cpp | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index e903d6121..db6e31e2b 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1192,7 +1192,6 @@ PaintRowBackground(nsTableRowFrame* aRow, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists, - const nsRect& aDirtyRect, const nsPoint& aOffset = nsPoint()) { // Compute background rect by iterating over all cell frames. @@ -1202,9 +1201,6 @@ PaintRowBackground(nsTableRowFrame* aRow, } auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset; - if (!aDirtyRect.Intersects(cellRect)) { - continue; - } nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr, @@ -1217,14 +1213,10 @@ static void PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, - const nsDisplayListSet& aLists, - const nsRect& aDirtyRect) + const nsDisplayListSet& aLists) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { - continue; - } - PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition()); + PaintRowBackground(row, aFrame, aBuilder, aLists, row->GetNormalPosition()); } } @@ -1237,10 +1229,6 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, const nsPoint& aOffset) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - auto rowPos = row->GetNormalPosition() + aOffset; - if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) { - continue; - } for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { if (!cell->ShouldPaintBackground(aBuilder)) { continue; @@ -1249,11 +1237,7 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, int32_t curColIdx; cell->GetColIndex(curColIdx); if (aColIdx.Contains(curColIdx)) { - auto cellPos = cell->GetNormalPosition() + rowPos; - auto cellRect = nsRect(cellPos, cell->GetSize()); - if (!aDirtyRect.Intersects(cellRect)) { - continue; - } + auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + row->GetNormalPosition() + aOffset; nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr, @@ -1300,10 +1284,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { nsTableRowGroupFrame* rowGroup = static_cast(aFrame); - PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists, aDirtyRect); + PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists); } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { nsTableRowFrame* row = static_cast(aFrame); - PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect); + PaintRowBackground(row, aFrame, aBuilder, aLists); } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { // Compute background rect by iterating all cell frame. nsTableColGroupFrame* colGroup = static_cast(aFrame); @@ -1318,10 +1302,7 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, table->OrderRowGroups(rowGroups); for (nsTableRowGroupFrame* rowGroup : rowGroups) { auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); - if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { - continue; - } - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); } } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { // Compute background rect by iterating all cell frame. @@ -1336,10 +1317,7 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, auto offset = rowGroup->GetNormalPosition() - col->GetNormalPosition() - col->GetTableColGroupFrame()->GetNormalPosition(); - if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { - continue; - } - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); } } else if (isVisible) { nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, -- cgit v1.2.3 From 2109f36dd4a1e04d80ee151b8d5b9c215dfca99a Mon Sep 17 00:00:00 2001 From: win7-7 Date: Sun, 2 Feb 2020 15:02:47 +0200 Subject: Issue #1355 - Hit testing in large tables has become extremely slow Add dirty rect intersection checks so that we don't build unnecessary table part display items. --- layout/tables/nsTableFrame.cpp | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index db6e31e2b..0e54fd093 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1192,6 +1192,7 @@ PaintRowBackground(nsTableRowFrame* aRow, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists, + const nsRect& aDirtyRect, const nsPoint& aOffset = nsPoint()) { // Compute background rect by iterating over all cell frames. @@ -1201,6 +1202,9 @@ PaintRowBackground(nsTableRowFrame* aRow, } auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset; + if (!aDirtyRect.Intersects(cellRect)) { + continue; + } nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr, @@ -1213,10 +1217,14 @@ static void PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, - const nsDisplayListSet& aLists) + const nsDisplayListSet& aLists, + const nsRect& aDirtyRect) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - PaintRowBackground(row, aFrame, aBuilder, aLists, row->GetNormalPosition()); + if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { + continue; + } + PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition()); } } @@ -1225,10 +1233,15 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists, + const nsRect& aDirtyRect, const nsTArray& aColIdx, const nsPoint& aOffset) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { + auto rowPos = row->GetNormalPosition() + aOffset; + if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) { + continue; + } for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { if (!cell->ShouldPaintBackground(aBuilder)) { continue; @@ -1237,7 +1250,11 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, int32_t curColIdx; cell->GetColIndex(curColIdx); if (aColIdx.Contains(curColIdx)) { - auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + row->GetNormalPosition() + aOffset; + auto cellPos = cell->GetNormalPosition() + rowPos; + auto cellRect = nsRect(cellPos, cell->GetSize()); + if (!aDirtyRect.Intersects(cellRect)) { + continue; + } nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, aLists.BorderBackground(), true, nullptr, @@ -1284,10 +1301,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { nsTableRowGroupFrame* rowGroup = static_cast(aFrame); - PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists); + PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists, aDirtyRect); } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { nsTableRowFrame* row = static_cast(aFrame); - PaintRowBackground(row, aFrame, aBuilder, aLists); + PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect); } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { // Compute background rect by iterating all cell frame. nsTableColGroupFrame* colGroup = static_cast(aFrame); @@ -1302,7 +1319,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, table->OrderRowGroups(rowGroups); for (nsTableRowGroupFrame* rowGroup : rowGroups) { auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; + } + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); } } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { // Compute background rect by iterating all cell frame. @@ -1317,7 +1337,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, auto offset = rowGroup->GetNormalPosition() - col->GetNormalPosition() - col->GetTableColGroupFrame()->GetNormalPosition(); - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, colIdx, offset); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; + } + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); } } else if (isVisible) { nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, -- cgit v1.2.3 From da74d1f3f364280e3dc380b99eb2d1e65166d37d Mon Sep 17 00:00:00 2001 From: win7-7 Date: Thu, 6 Feb 2020 17:13:55 +0200 Subject: Issue #1355 - Do less work for columns not in the desired set in PaintRowGroupBackgroundByColIdx Do less work in PaintRowGroupBackgroundByColIdx for cells that are not in our desired set of columns. crash fix: Guard against empty column groups when building a display list for a table. --- layout/tables/nsTableFrame.cpp | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 0e54fd093..868f83bda 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1237,19 +1237,29 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, const nsTArray& aColIdx, const nsPoint& aOffset) { + MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(), + "Must be painting backgrounds for something"); for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { auto rowPos = row->GetNormalPosition() + aOffset; if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) { continue; } for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { + + int32_t curColIdx; + cell->GetColIndex(curColIdx); + if (!aColIdx.Contains(curColIdx)) { + if (curColIdx > aColIdx.LastElement()) { + // We can just stop looking at this row. + break; + } + continue; + } + if (!cell->ShouldPaintBackground(aBuilder)) { continue; } - int32_t curColIdx; - cell->GetColIndex(curColIdx); - if (aColIdx.Contains(curColIdx)) { auto cellPos = cell->GetNormalPosition() + rowPos; auto cellRect = nsRect(cellPos, cell->GetSize()); if (!aDirtyRect.Intersects(cellRect)) { @@ -1260,7 +1270,6 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, true, nullptr, aFrame->GetRectRelativeToSelf(), cell); - } } } } @@ -1311,18 +1320,23 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, // Collecting column index. AutoTArray colIdx; for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { + MOZ_ASSERT(colIdx.IsEmpty() || + col->GetColIndex() > colIdx.LastElement()); colIdx.AppendElement(col->GetColIndex()); } - nsTableFrame* table = colGroup->GetTableFrame(); - RowGroupArray rowGroups; - table->OrderRowGroups(rowGroups); - for (nsTableRowGroupFrame* rowGroup : rowGroups) { - auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); - if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { - continue; - } + if (!colIdx.IsEmpty()) { + // We have some actual cells that live inside this rowgroup. + nsTableFrame* table = colGroup->GetTableFrame(); + RowGroupArray rowGroups; + table->OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; + } PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); + } } } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { // Compute background rect by iterating all cell frame. -- 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/nsTableFrame.cpp | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 868f83bda..4257c9c57 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -368,9 +368,8 @@ nsTableFrame::AttributeChangedFor(nsIFrame* aFrame, nsTableCellMap* cellMap = GetCellMap(); if (cellMap) { // for now just remove the cell from the map and reinsert it - int32_t rowIndex, colIndex; - cellFrame->GetRowIndex(rowIndex); - cellFrame->GetColIndex(colIndex); + uint32_t rowIndex = cellFrame->RowIndex(); + uint32_t colIndex = cellFrame->ColIndex(); RemoveCell(cellFrame, rowIndex); AutoTArray cells; cells.AppendElement(cellFrame); @@ -447,9 +446,7 @@ nsTableFrame::GetEffectiveRowSpan(int32_t aRowIndex, nsTableCellMap* cellMap = GetCellMap(); NS_PRECONDITION (nullptr != cellMap, "bad call, cellMap not yet allocated."); - int32_t colIndex; - aCell.GetColIndex(colIndex); - return cellMap->GetEffectiveRowSpan(aRowIndex, colIndex); + return cellMap->GetEffectiveRowSpan(aRowIndex, aCell.ColIndex()); } int32_t @@ -458,9 +455,8 @@ nsTableFrame::GetEffectiveRowSpan(const nsTableCellFrame& aCell, { nsTableCellMap* tableCellMap = GetCellMap(); if (!tableCellMap) ABORT1(1); - int32_t colIndex, rowIndex; - aCell.GetColIndex(colIndex); - aCell.GetRowIndex(rowIndex); + uint32_t colIndex = aCell.ColIndex(); + uint32_t rowIndex = aCell.RowIndex(); if (aCellMap) return aCellMap->GetRowSpan(rowIndex, colIndex, true); @@ -474,9 +470,8 @@ nsTableFrame::GetEffectiveColSpan(const nsTableCellFrame& aCell, { nsTableCellMap* tableCellMap = GetCellMap(); if (!tableCellMap) ABORT1(1); - int32_t colIndex, rowIndex; - aCell.GetColIndex(colIndex); - aCell.GetRowIndex(rowIndex); + uint32_t colIndex = aCell.ColIndex(); + uint32_t rowIndex = aCell.RowIndex(); if (aCellMap) return aCellMap->GetEffectiveColSpan(*tableCellMap, rowIndex, colIndex); @@ -1221,9 +1216,9 @@ PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup, const nsRect& aDirtyRect) { for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { - continue; - } + if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { + continue; + } PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition()); } } @@ -1234,7 +1229,7 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists, const nsRect& aDirtyRect, - const nsTArray& aColIdx, + const nsTArray& aColIdx, const nsPoint& aOffset) { MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(), @@ -1246,8 +1241,7 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, } for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { - int32_t curColIdx; - cell->GetColIndex(curColIdx); + uint32_t curColIdx = cell->ColIndex(); if (!aColIdx.Contains(curColIdx)) { if (curColIdx > aColIdx.LastElement()) { // We can just stop looking at this row. @@ -1318,10 +1312,10 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, // Compute background rect by iterating all cell frame. nsTableColGroupFrame* colGroup = static_cast(aFrame); // Collecting column index. - AutoTArray colIdx; + AutoTArray colIdx; for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { MOZ_ASSERT(colIdx.IsEmpty() || - col->GetColIndex() > colIdx.LastElement()); + static_cast(col->GetColIndex()) > colIdx.LastElement()); colIdx.AppendElement(col->GetColIndex()); } @@ -1341,7 +1335,7 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { // Compute background rect by iterating all cell frame. nsTableColFrame* col = static_cast(aFrame); - AutoTArray colIdx; + AutoTArray colIdx; colIdx.AppendElement(col->GetColIndex()); nsTableFrame* table = col->GetTableFrame(); @@ -3973,9 +3967,8 @@ nsTableFrame::DumpRowGroup(nsIFrame* aKidFrame) for (nsIFrame* childFrame : cFrame->PrincipalChildList()) { nsTableCellFrame *cellFrame = do_QueryFrame(childFrame); if (cellFrame) { - int32_t colIndex; - cellFrame->GetColIndex(colIndex); - printf("cell(%d)=%p ", colIndex, static_cast(childFrame)); + uint32_t colIndex = cellFrame->ColIndex(); + printf("cell(%u)=%p ", colIndex, static_cast(childFrame)); } } printf("\n"); -- 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/nsTableFrame.cpp | 293 +++++++++++++---------------------------- 1 file changed, 90 insertions(+), 203 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 4257c9c57..32fe38b05 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1161,242 +1161,127 @@ nsDisplayTableBorderCollapse::Paint(nsDisplayListBuilder* aBuilder, static_cast(mFrame)->PaintBCBorders(*drawTarget, mVisibleRect - pt); } -/* static */ void -nsTableFrame::GenericTraversal(nsDisplayListBuilder* aBuilder, nsFrame* aFrame, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) -{ - // This is similar to what nsContainerFrame::BuildDisplayListForNonBlockChildren - // does, except that we allow the children's background and borders to go - // in our BorderBackground list. This doesn't really affect background - // painting --- the children won't actually draw their own backgrounds - // because the nsTableFrame already drew them, unless a child has its own - // stacking context, in which case the child won't use its passed-in - // BorderBackground list anyway. It does affect cell borders though; this - // lets us get cell borders into the nsTableFrame's BorderBackground list. - for (nsIFrame* kid : aFrame->GetChildList(kColGroupList)) { - aFrame->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists); +static inline bool FrameHasBorder(nsIFrame* f) { + if (!f->StyleVisibility()->IsVisible()) { + return false; } - for (nsIFrame* kid : aFrame->PrincipalChildList()) { - aFrame->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists); + if (f->StyleBorder()->HasBorder()) { + return true; } -} - -static void -PaintRowBackground(nsTableRowFrame* aRow, - nsIFrame* aFrame, - nsDisplayListBuilder* aBuilder, - const nsDisplayListSet& aLists, - const nsRect& aDirtyRect, - const nsPoint& aOffset = nsPoint()) -{ - // Compute background rect by iterating over all cell frames. - for (nsTableCellFrame* cell = aRow->GetFirstCell(); cell; cell = cell->GetNextCell()) { - if (!cell->ShouldPaintBackground(aBuilder)) { - continue; - } - auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset; - if (!aDirtyRect.Intersects(cellRect)) { - continue; - } - nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, - aLists.BorderBackground(), - true, nullptr, - aFrame->GetRectRelativeToSelf(), - cell); - } + return false; } -static void -PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup, - nsIFrame* aFrame, - nsDisplayListBuilder* aBuilder, - const nsDisplayListSet& aLists, - const nsRect& aDirtyRect) -{ - for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { - continue; - } - PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition()); +void nsTableFrame::CalcHasBCBorders() { + if (!IsBorderCollapse()) { + SetHasBCBorders(false); + return; } -} -static void -PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, - nsIFrame* aFrame, - nsDisplayListBuilder* aBuilder, - const nsDisplayListSet& aLists, - const nsRect& aDirtyRect, - const nsTArray& aColIdx, - const nsPoint& aOffset) -{ - MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(), - "Must be painting backgrounds for something"); - for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { - auto rowPos = row->GetNormalPosition() + aOffset; - if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) { - continue; - } - for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { - - uint32_t curColIdx = cell->ColIndex(); - if (!aColIdx.Contains(curColIdx)) { - if (curColIdx > aColIdx.LastElement()) { - // We can just stop looking at this row. - break; - } - continue; - } - - if (!cell->ShouldPaintBackground(aBuilder)) { - continue; - } - - auto cellPos = cell->GetNormalPosition() + rowPos; - auto cellRect = nsRect(cellPos, cell->GetSize()); - if (!aDirtyRect.Intersects(cellRect)) { - continue; - } - nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, - aLists.BorderBackground(), - true, nullptr, - aFrame->GetRectRelativeToSelf(), - cell); - } + if (FrameHasBorder(this)) { + SetHasBCBorders(true); + return; } -} -/* static */ void -nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, - nsFrame* aFrame, - const nsRect& aDirtyRect, - const nsDisplayListSet& aLists, - DisplayGenericTablePartTraversal aTraversal) -{ - bool isVisible = aFrame->IsVisibleForPainting(aBuilder); - bool isTable = (aFrame->GetType() == nsGkAtoms::tableFrame); - - if (isVisible || !isTable) { - nsDisplayTableItem* currentItem = aBuilder->GetCurrentTableItem(); - // currentItem may be null, when none of the table parts have a - // background or border - if (currentItem) { - currentItem->UpdateForFrameBackground(aFrame); - } - } - - if (isVisible) { - // XXX: should box-shadow for rows/rowgroups/columns/colgroups get painted - // just because we're visible? Or should it depend on the cell visibility - // when we're not the whole table? - - // Paint the outset box-shadows for the table frames - if (aFrame->StyleEffects()->mBoxShadow) { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, aFrame)); - } - } - - // Background visibility for rows, rowgroups, columns, colgroups depends on - // the visibility of the _cell_, not of the row/col(group). - // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds - if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { - nsTableRowGroupFrame* rowGroup = static_cast(aFrame); - PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists, aDirtyRect); - } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { - nsTableRowFrame* row = static_cast(aFrame); - PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect); - } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { - // Compute background rect by iterating all cell frame. - nsTableColGroupFrame* colGroup = static_cast(aFrame); - // Collecting column index. - AutoTArray colIdx; - for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { - MOZ_ASSERT(colIdx.IsEmpty() || - static_cast(col->GetColIndex()) > colIdx.LastElement()); - colIdx.AppendElement(col->GetColIndex()); - } - - if (!colIdx.IsEmpty()) { - // We have some actual cells that live inside this rowgroup. - nsTableFrame* table = colGroup->GetTableFrame(); - RowGroupArray rowGroups; - table->OrderRowGroups(rowGroups); - for (nsTableRowGroupFrame* rowGroup : rowGroups) { - auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); - if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { - continue; - } - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); - } + // Check col and col group has borders. + for (nsIFrame* f : this->GetChildList(kColGroupList)) { + if (FrameHasBorder(f)) { + SetHasBCBorders(true); + return; } - } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { - // Compute background rect by iterating all cell frame. - nsTableColFrame* col = static_cast(aFrame); - AutoTArray colIdx; - colIdx.AppendElement(col->GetColIndex()); - nsTableFrame* table = col->GetTableFrame(); - RowGroupArray rowGroups; - table->OrderRowGroups(rowGroups); - for (nsTableRowGroupFrame* rowGroup : rowGroups) { - auto offset = rowGroup->GetNormalPosition() - - col->GetNormalPosition() - - col->GetTableColGroupFrame()->GetNormalPosition(); - if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { - continue; + nsTableColGroupFrame* colGroup = static_cast(f); + for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; + col = col->GetNextCol()) { + if (FrameHasBorder(col)) { + SetHasBCBorders(true); + return; } - PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); } - } else if (isVisible) { - nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, - aFrame->GetRectRelativeToSelf(), - aLists.BorderBackground()); } - if (isVisible) { - // XXX: should box-shadow for rows/rowgroups/columns/colgroups get painted - // just because we're visible? Or should it depend on the cell visibility - // when we're not the whole table? - - // Paint the inset box-shadows for the table frames - if (aFrame->StyleEffects()->mBoxShadow) { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayBoxShadowInner(aBuilder, aFrame)); + // check row group, row and cell has borders. + RowGroupArray rowGroups; + OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + if (FrameHasBorder(rowGroup)) { + SetHasBCBorders(true); + return; } - } - aTraversal(aBuilder, aFrame, aDirtyRect, aLists); + for (nsTableRowFrame* row = rowGroup->GetFirstRow(); row; + row = row->GetNextRow()) { + if (FrameHasBorder(row)) { + SetHasBCBorders(true); + return; + } - if (isVisible) { - if (isTable) { - nsTableFrame* table = static_cast(aFrame); - // In the collapsed border model, overlay all collapsed borders. - if (table->IsBorderCollapse()) { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayTableBorderCollapse(aBuilder, table)); - } else { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayBorder(aBuilder, table)); + for (nsTableCellFrame* cell = row->GetFirstCell(); cell; + cell = cell->GetNextCell()) { + if (FrameHasBorder(cell)) { + SetHasBCBorders(true); + return; + } } } } - aFrame->DisplayOutline(aBuilder, aLists); + SetHasBCBorders(false); } // table paint code is concerned primarily with borders and bg color // SEC: TODO: adjust the rect for captions void nsTableFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { DO_GLOBAL_REFLOW_COUNT_DSP_COLOR("nsTableFrame", NS_RGB(255,128,255)); - DisplayGenericTablePart(aBuilder, this, aDirtyRect, aLists); + DisplayBorderBackgroundOutline(aBuilder, aLists); + + nsDisplayTableBackgroundSet tableBGs(aBuilder, this); + nsDisplayListCollection lists(aBuilder); + +// This is similar to what + // nsContainerFrame::BuildDisplayListForNonBlockChildren does, except that we + // allow the children's background and borders to go in our BorderBackground + // list. This doesn't really affect background painting --- the children won't + // actually draw their own backgrounds because the nsTableFrame already drew + // them, unless a child has its own stacking context, in which case the child + // won't use its passed-in BorderBackground list anyway. It does affect cell + // borders though; this lets us get cell borders into the nsTableFrame's + // BorderBackground list. + for (nsIFrame* colGroup : FirstContinuation()->GetChildList(kColGroupList)) { + for (nsIFrame* col : colGroup->PrincipalChildList()) { + tableBGs.AddColumn((nsTableColFrame*)col); + } + } + + for (nsIFrame* kid : PrincipalChildList()) { + BuildDisplayListForChild(aBuilder, kid, lists); + } + + tableBGs.MoveTo(aLists); + lists.MoveTo(aLists); + + if (IsVisibleForPainting(aBuilder)) { + // In the collapsed border model, overlay all collapsed borders. + if (IsBorderCollapse()) { + if (HasBCBorders()) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayTableBorderCollapse + (aBuilder, this)); + } + } else { + const nsStyleBorder* borderStyle = StyleBorder(); + if (borderStyle->HasBorder()) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayBorder + (aBuilder, this)); + } + } + } } nsMargin @@ -4101,6 +3986,7 @@ nsTableFrame::AddBCDamageArea(const TableArea& aValue) #endif SetNeedToCalcBCBorders(true); + SetNeedToCalcHasBCBorders(true); // Get the property BCPropertyData* value = GetOrCreateBCProperty(); if (value) { @@ -4141,6 +4027,7 @@ nsTableFrame::SetFullBCDamageArea() NS_ASSERTION(IsBorderCollapse(), "invalid SetFullBCDamageArea call"); SetNeedToCalcBCBorders(true); + SetNeedToCalcHasBCBorders(true); BCPropertyData* value = GetOrCreateBCProperty(); if (value) { -- 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/nsTableFrame.cpp | 293 ++++++++++++++++++++++++++++------------- 1 file changed, 203 insertions(+), 90 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 32fe38b05..4257c9c57 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1161,127 +1161,242 @@ nsDisplayTableBorderCollapse::Paint(nsDisplayListBuilder* aBuilder, static_cast(mFrame)->PaintBCBorders(*drawTarget, mVisibleRect - pt); } -static inline bool FrameHasBorder(nsIFrame* f) { - if (!f->StyleVisibility()->IsVisible()) { - return false; +/* static */ void +nsTableFrame::GenericTraversal(nsDisplayListBuilder* aBuilder, nsFrame* aFrame, + const nsRect& aDirtyRect, const nsDisplayListSet& aLists) +{ + // This is similar to what nsContainerFrame::BuildDisplayListForNonBlockChildren + // does, except that we allow the children's background and borders to go + // in our BorderBackground list. This doesn't really affect background + // painting --- the children won't actually draw their own backgrounds + // because the nsTableFrame already drew them, unless a child has its own + // stacking context, in which case the child won't use its passed-in + // BorderBackground list anyway. It does affect cell borders though; this + // lets us get cell borders into the nsTableFrame's BorderBackground list. + for (nsIFrame* kid : aFrame->GetChildList(kColGroupList)) { + aFrame->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists); } - if (f->StyleBorder()->HasBorder()) { - return true; + for (nsIFrame* kid : aFrame->PrincipalChildList()) { + aFrame->BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists); } +} - return false; +static void +PaintRowBackground(nsTableRowFrame* aRow, + nsIFrame* aFrame, + nsDisplayListBuilder* aBuilder, + const nsDisplayListSet& aLists, + const nsRect& aDirtyRect, + const nsPoint& aOffset = nsPoint()) +{ + // Compute background rect by iterating over all cell frames. + for (nsTableCellFrame* cell = aRow->GetFirstCell(); cell; cell = cell->GetNextCell()) { + if (!cell->ShouldPaintBackground(aBuilder)) { + continue; + } + + auto cellRect = cell->GetRectRelativeToSelf() + cell->GetNormalPosition() + aOffset; + if (!aDirtyRect.Intersects(cellRect)) { + continue; + } + nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, + aLists.BorderBackground(), + true, nullptr, + aFrame->GetRectRelativeToSelf(), + cell); + } } -void nsTableFrame::CalcHasBCBorders() { - if (!IsBorderCollapse()) { - SetHasBCBorders(false); - return; +static void +PaintRowGroupBackground(nsTableRowGroupFrame* aRowGroup, + nsIFrame* aFrame, + nsDisplayListBuilder* aBuilder, + const nsDisplayListSet& aLists, + const nsRect& aDirtyRect) +{ + for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { + if (!aDirtyRect.Intersects(nsRect(row->GetNormalPosition(), row->GetSize()))) { + continue; + } + PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect, row->GetNormalPosition()); } +} - if (FrameHasBorder(this)) { - SetHasBCBorders(true); - return; +static void +PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, + nsIFrame* aFrame, + nsDisplayListBuilder* aBuilder, + const nsDisplayListSet& aLists, + const nsRect& aDirtyRect, + const nsTArray& aColIdx, + const nsPoint& aOffset) +{ + MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(), + "Must be painting backgrounds for something"); + for (nsTableRowFrame* row = aRowGroup->GetFirstRow(); row; row = row->GetNextRow()) { + auto rowPos = row->GetNormalPosition() + aOffset; + if (!aDirtyRect.Intersects(nsRect(rowPos, row->GetSize()))) { + continue; + } + for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { + + uint32_t curColIdx = cell->ColIndex(); + if (!aColIdx.Contains(curColIdx)) { + if (curColIdx > aColIdx.LastElement()) { + // We can just stop looking at this row. + break; + } + continue; + } + + if (!cell->ShouldPaintBackground(aBuilder)) { + continue; + } + + auto cellPos = cell->GetNormalPosition() + rowPos; + auto cellRect = nsRect(cellPos, cell->GetSize()); + if (!aDirtyRect.Intersects(cellRect)) { + continue; + } + nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, cellRect, + aLists.BorderBackground(), + true, nullptr, + aFrame->GetRectRelativeToSelf(), + cell); + } } +} - // Check col and col group has borders. - for (nsIFrame* f : this->GetChildList(kColGroupList)) { - if (FrameHasBorder(f)) { - SetHasBCBorders(true); - return; +/* static */ void +nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, + nsFrame* aFrame, + const nsRect& aDirtyRect, + const nsDisplayListSet& aLists, + DisplayGenericTablePartTraversal aTraversal) +{ + bool isVisible = aFrame->IsVisibleForPainting(aBuilder); + bool isTable = (aFrame->GetType() == nsGkAtoms::tableFrame); + + if (isVisible || !isTable) { + nsDisplayTableItem* currentItem = aBuilder->GetCurrentTableItem(); + // currentItem may be null, when none of the table parts have a + // background or border + if (currentItem) { + currentItem->UpdateForFrameBackground(aFrame); + } + } + + if (isVisible) { + // XXX: should box-shadow for rows/rowgroups/columns/colgroups get painted + // just because we're visible? Or should it depend on the cell visibility + // when we're not the whole table? + + // Paint the outset box-shadows for the table frames + if (aFrame->StyleEffects()->mBoxShadow) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayBoxShadowOuter(aBuilder, aFrame)); + } + } + + // Background visibility for rows, rowgroups, columns, colgroups depends on + // the visibility of the _cell_, not of the row/col(group). + // See spec at https://drafts.csswg.org/css-tables-3/#drawing-cell-backgrounds + if (aFrame->GetType() == nsGkAtoms::tableRowGroupFrame) { + nsTableRowGroupFrame* rowGroup = static_cast(aFrame); + PaintRowGroupBackground(rowGroup, aFrame, aBuilder, aLists, aDirtyRect); + } else if (aFrame->GetType() == nsGkAtoms::tableRowFrame) { + nsTableRowFrame* row = static_cast(aFrame); + PaintRowBackground(row, aFrame, aBuilder, aLists, aDirtyRect); + } else if (aFrame->GetType() == nsGkAtoms::tableColGroupFrame) { + // Compute background rect by iterating all cell frame. + nsTableColGroupFrame* colGroup = static_cast(aFrame); + // Collecting column index. + AutoTArray colIdx; + for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { + MOZ_ASSERT(colIdx.IsEmpty() || + static_cast(col->GetColIndex()) > colIdx.LastElement()); + colIdx.AppendElement(col->GetColIndex()); + } + + if (!colIdx.IsEmpty()) { + // We have some actual cells that live inside this rowgroup. + nsTableFrame* table = colGroup->GetTableFrame(); + RowGroupArray rowGroups; + table->OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + auto offset = rowGroup->GetNormalPosition() - colGroup->GetNormalPosition(); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; + } + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); + } } + } else if (aFrame->GetType() == nsGkAtoms::tableColFrame) { + // Compute background rect by iterating all cell frame. + nsTableColFrame* col = static_cast(aFrame); + AutoTArray colIdx; + colIdx.AppendElement(col->GetColIndex()); - nsTableColGroupFrame* colGroup = static_cast(f); - for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; - col = col->GetNextCol()) { - if (FrameHasBorder(col)) { - SetHasBCBorders(true); - return; + nsTableFrame* table = col->GetTableFrame(); + RowGroupArray rowGroups; + table->OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + auto offset = rowGroup->GetNormalPosition() - + col->GetNormalPosition() - + col->GetTableColGroupFrame()->GetNormalPosition(); + if (!aDirtyRect.Intersects(nsRect(offset, rowGroup->GetSize()))) { + continue; } + PaintRowGroupBackgroundByColIdx(rowGroup, aFrame, aBuilder, aLists, aDirtyRect, colIdx, offset); } + } else if (isVisible) { + nsDisplayBackgroundImage::AppendBackgroundItemsToTop(aBuilder, aFrame, + aFrame->GetRectRelativeToSelf(), + aLists.BorderBackground()); } - // check row group, row and cell has borders. - RowGroupArray rowGroups; - OrderRowGroups(rowGroups); - for (nsTableRowGroupFrame* rowGroup : rowGroups) { - if (FrameHasBorder(rowGroup)) { - SetHasBCBorders(true); - return; + if (isVisible) { + // XXX: should box-shadow for rows/rowgroups/columns/colgroups get painted + // just because we're visible? Or should it depend on the cell visibility + // when we're not the whole table? + + // Paint the inset box-shadows for the table frames + if (aFrame->StyleEffects()->mBoxShadow) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayBoxShadowInner(aBuilder, aFrame)); } + } - for (nsTableRowFrame* row = rowGroup->GetFirstRow(); row; - row = row->GetNextRow()) { - if (FrameHasBorder(row)) { - SetHasBCBorders(true); - return; - } + aTraversal(aBuilder, aFrame, aDirtyRect, aLists); - for (nsTableCellFrame* cell = row->GetFirstCell(); cell; - cell = cell->GetNextCell()) { - if (FrameHasBorder(cell)) { - SetHasBCBorders(true); - return; - } + if (isVisible) { + if (isTable) { + nsTableFrame* table = static_cast(aFrame); + // In the collapsed border model, overlay all collapsed borders. + if (table->IsBorderCollapse()) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayTableBorderCollapse(aBuilder, table)); + } else { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayBorder(aBuilder, table)); } } } - SetHasBCBorders(false); + aFrame->DisplayOutline(aBuilder, aLists); } // table paint code is concerned primarily with borders and bg color // SEC: TODO: adjust the rect for captions void nsTableFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, + const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { DO_GLOBAL_REFLOW_COUNT_DSP_COLOR("nsTableFrame", NS_RGB(255,128,255)); - DisplayBorderBackgroundOutline(aBuilder, aLists); - - nsDisplayTableBackgroundSet tableBGs(aBuilder, this); - nsDisplayListCollection lists(aBuilder); - -// This is similar to what - // nsContainerFrame::BuildDisplayListForNonBlockChildren does, except that we - // allow the children's background and borders to go in our BorderBackground - // list. This doesn't really affect background painting --- the children won't - // actually draw their own backgrounds because the nsTableFrame already drew - // them, unless a child has its own stacking context, in which case the child - // won't use its passed-in BorderBackground list anyway. It does affect cell - // borders though; this lets us get cell borders into the nsTableFrame's - // BorderBackground list. - for (nsIFrame* colGroup : FirstContinuation()->GetChildList(kColGroupList)) { - for (nsIFrame* col : colGroup->PrincipalChildList()) { - tableBGs.AddColumn((nsTableColFrame*)col); - } - } - - for (nsIFrame* kid : PrincipalChildList()) { - BuildDisplayListForChild(aBuilder, kid, lists); - } - - tableBGs.MoveTo(aLists); - lists.MoveTo(aLists); - - if (IsVisibleForPainting(aBuilder)) { - // In the collapsed border model, overlay all collapsed borders. - if (IsBorderCollapse()) { - if (HasBCBorders()) { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayTableBorderCollapse - (aBuilder, this)); - } - } else { - const nsStyleBorder* borderStyle = StyleBorder(); - if (borderStyle->HasBorder()) { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayBorder - (aBuilder, this)); - } - } - } + DisplayGenericTablePart(aBuilder, this, aDirtyRect, aLists); } nsMargin @@ -3986,7 +4101,6 @@ nsTableFrame::AddBCDamageArea(const TableArea& aValue) #endif SetNeedToCalcBCBorders(true); - SetNeedToCalcHasBCBorders(true); // Get the property BCPropertyData* value = GetOrCreateBCProperty(); if (value) { @@ -4027,7 +4141,6 @@ nsTableFrame::SetFullBCDamageArea() NS_ASSERTION(IsBorderCollapse(), "invalid SetFullBCDamageArea call"); SetNeedToCalcBCBorders(true); - SetNeedToCalcHasBCBorders(true); BCPropertyData* value = GetOrCreateBCProperty(); if (value) { -- cgit v1.2.3 From 22d0b88e375ea3378ec397dc744d36c10112861c Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 16 Mar 2020 09:30:31 +0100 Subject: Issue #1355 - Make addition of cell border display items depend on whether they should be drawn. This reduces the size of display lists for tables by only adding display list items that are actually going to be visibly drawn, which will help overall performance of table drawing. --- layout/tables/nsTableFrame.cpp | 83 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 4 deletions(-) (limited to 'layout/tables/nsTableFrame.cpp') diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 4257c9c57..e5a48139a 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -1268,6 +1268,74 @@ PaintRowGroupBackgroundByColIdx(nsTableRowGroupFrame* aRowGroup, } } +static inline bool FrameHasBorder(nsIFrame* f) +{ + if (!f->StyleVisibility()->IsVisible()) { + return false; + } + + if (f->StyleBorder()->HasBorder()) { + return true; + } + + return false; +} + +void nsTableFrame::CalcHasBCBorders() +{ + if (!IsBorderCollapse()) { + SetHasBCBorders(false); + return; + } + + if (FrameHasBorder(this)) { + SetHasBCBorders(true); + return; + } + + // Check col and col group has borders. + for (nsIFrame* f : this->GetChildList(kColGroupList)) { + if (FrameHasBorder(f)) { + SetHasBCBorders(true); + return; + } + + nsTableColGroupFrame *colGroup = static_cast(f); + for (nsTableColFrame* col = colGroup->GetFirstColumn(); col; col = col->GetNextCol()) { + if (FrameHasBorder(col)) { + SetHasBCBorders(true); + return; + } + } + } + + // check row group, row and cell has borders. + RowGroupArray rowGroups; + OrderRowGroups(rowGroups); + for (nsTableRowGroupFrame* rowGroup : rowGroups) { + if (FrameHasBorder(rowGroup)) { + SetHasBCBorders(true); + return; + } + + for (nsTableRowFrame* row = rowGroup->GetFirstRow(); row; row = row->GetNextRow()) { + if (FrameHasBorder(row)) { + SetHasBCBorders(true); + return; + } + + for (nsTableCellFrame* cell = row->GetFirstCell(); cell; cell = cell->GetNextCell()) { + if (FrameHasBorder(cell)) { + SetHasBCBorders(true); + return; + } + } + } + } + + SetHasBCBorders(false); +} + /* static */ void nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, nsFrame* aFrame, @@ -1375,11 +1443,16 @@ nsTableFrame::DisplayGenericTablePart(nsDisplayListBuilder* aBuilder, nsTableFrame* table = static_cast(aFrame); // In the collapsed border model, overlay all collapsed borders. if (table->IsBorderCollapse()) { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayTableBorderCollapse(aBuilder, table)); + if (table->HasBCBorders()) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayTableBorderCollapse(aBuilder, table)); + } } else { - aLists.BorderBackground()->AppendNewToTop( - new (aBuilder) nsDisplayBorder(aBuilder, table)); + const nsStyleBorder* borderStyle = aFrame->StyleBorder(); + if (borderStyle->HasBorder()) { + aLists.BorderBackground()->AppendNewToTop( + new (aBuilder) nsDisplayBorder(aBuilder, table)); + } } } } @@ -4101,6 +4174,7 @@ nsTableFrame::AddBCDamageArea(const TableArea& aValue) #endif SetNeedToCalcBCBorders(true); + SetNeedToCalcHasBCBorders(true); // Get the property BCPropertyData* value = GetOrCreateBCProperty(); if (value) { @@ -4141,6 +4215,7 @@ nsTableFrame::SetFullBCDamageArea() NS_ASSERTION(IsBorderCollapse(), "invalid SetFullBCDamageArea call"); SetNeedToCalcBCBorders(true); + SetNeedToCalcHasBCBorders(true); BCPropertyData* value = GetOrCreateBCProperty(); if (value) { -- cgit v1.2.3