Bug 1409114 - Part 8: Create column and column group background display items as part of the cell's BuildDisplayList. r=dbaron

This is the main performance improvement, and means that we no longer have to iterate all the cells for each column. It has a couple of behaviour changes: The first is that we no longer apply stacking context effects (like opacity) to column and column group backgrounds. I believe this is correct as per both CSS2.1 Appendix E, and css-tables-3 (quoted in nsTableColFrame::BuildDisplayList). This matches the behaviour of blink and WebKit. We also previously created items in column,row ordering, whereas now they will be in row,column. In cases where two cells overlap (using rowspan and colspan to extend multiple neighbours in to the same place) this can render backgrounds in a different order, but the new behaviour matches blink and WebKit. Differential Revision: https://phabricator.services.mozilla.com/D29280 --HG-- extra : moz-landing-system : lando
2024-10-19 00:05:36 +00:00 · 2019-05-20 23:16:09 +00:00 · 2019-05-20 23:16:09 +00:00 · 9f39c7b608
commit 9f39c7b608
parent 1f9adb1ed4
14 changed files with 93 additions and 119 deletions
--- a/layout/reftests/table-background/border-collapse-opacity-table-column-group-ref.html
+++ b/layout/reftests/table-background/border-collapse-opacity-table-column-group-ref.html
@ -10,8 +10,6 @@
 	html, body { margin: 0; padding: 0; border: 0; font-size: 16px; }
 	body { padding: 15px; }

-	.o { opacity: 0.5; }
-
 	/*
 	table { 
 		margin: 0px 3px 2px 4px;
@ -55,8 +53,8 @@
 </head>
 <body>

-<div class="color o"></div>
-<div class="imagetl o"></div>
+<div class="color"></div>
+<div class="imagetl"></div>
 <div class="imagebr"></div>

 </body>
--- a/layout/reftests/table-background/border-collapse-opacity-table-column-ref.html
+++ b/layout/reftests/table-background/border-collapse-opacity-table-column-ref.html
@ -10,8 +10,6 @@
 	html, body { margin: 0; padding: 0; border: 0; font-size: 16px; }
 	body { padding: 15px; }

-	.o { opacity: 0.5; }
-
 	/*
 	table { 
 		margin: 0px 3px 2px 4px;
@ -55,8 +53,8 @@
 </head>
 <body>

-<div class="color o"></div>
-<div class="imagetl o"></div>
+<div class="color"></div>
+<div class="imagetl"></div>
 <div class="imagebr"></div>

 </body>
--- a/layout/reftests/table-background/border-separate-opacity-table-column-group-ref.html
+++ b/layout/reftests/table-background/border-separate-opacity-table-column-group-ref.html
@ -10,8 +10,6 @@
 	html, body { margin: 0; padding: 0; border: 0; font-size: 16px; }
 	body { padding: 15px; }

-	.o { opacity: 0.5; }
-
 	/*
 	table { 
 		margin: 0px 3px 2px 4px;
@ -65,8 +63,8 @@
 <body>

 <div class="color"></div>
-<div class="imagetl o"></div>
-<div class="imagebr o"></div>
+<div class="imagetl"></div>
+<div class="imagebr"></div>

 <div class="hstripe" style="top: 46px"></div>
 <div class="hstripe" style="top: 70px"></div>
--- a/layout/reftests/table-background/border-separate-opacity-table-column-ref.html
+++ b/layout/reftests/table-background/border-separate-opacity-table-column-ref.html
@ -10,8 +10,6 @@
 	html, body { margin: 0; padding: 0; border: 0; font-size: 16px; }
 	body { padding: 15px; }

-	.o { opacity: 0.5; }
-
 	/*
 	table { 
 		margin: 0px 3px 2px 4px;
@ -65,8 +63,8 @@
 <body>

 <div class="color"></div>
-<div class="imagetl o"></div>
-<div class="imagebr o"></div>
+<div class="imagetl"></div>
+<div class="imagebr"></div>

 <div class="hstripe" style="top: 46px"></div>
 <div class="hstripe" style="top: 70px"></div>
--- a/layout/reftests/table-background/reftest.list
+++ b/layout/reftests/table-background/reftest.list
@ -66,3 +66,4 @@ fuzzy-if(OSX,0-1,0-113) fuzzy-if(winWidget,0-1,0-12) fuzzy-if(winWidget&&!layers
 == hidden-cells-3.html hidden-cells-3-ref.html

 == table-col-overlapping.html table-col-overlapping-ref.html
+== table-col-span-1.html table-col-span-1-ref.html
--- a/layout/reftests/table-background/table-col-span-1-ref.html
+++ b/layout/reftests/table-background/table-col-span-1-ref.html
@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <table border>
+    <col style="background:purple"></col>
+    <col style="background:purple"></col>
+    <col style="background:blue"></col>
+    <tr>
+      <td>x</td>
+      <td>y</td>
+      <td>z</td>
+    </tr>
+  </table>
+</body>
+</html>
--- a/layout/reftests/table-background/table-col-span-1.html
+++ b/layout/reftests/table-background/table-col-span-1.html
@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <table border>
+    <col span=2 style="background:purple"></col>
+    <col style="background:blue"></col>
+    <tr>
+      <td>x</td>
+      <td>y</td>
+      <td>z</td>
+    </tr>
+  </table>
+</body>
+</html>
--- a/layout/tables/nsTableCellFrame.cpp
+++ b/layout/tables/nsTableCellFrame.cpp
@ -474,6 +474,41 @@ void nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
      aLists.BorderBackground()->AppendNewToTop<nsDisplayTableCellSelection>(
          aBuilder, this);
    }
+
+    // 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());
+      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+          aBuilder, col, bgRect, backgrounds->ColBackgrounds(), false, nullptr,
+          col->GetRectRelativeToSelf() + aBuilder->ToReferenceFrame(col), this);
+
+      nsIFrame* colGroup = col->GetParent();
+      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
+          aBuilder, colGroup, bgRect, backgrounds->ColGroupBackgrounds(), false,
+          nullptr,
+          colGroup->GetRectRelativeToSelf() +
+              aBuilder->ToReferenceFrame(colGroup),
+          this);
+    }
  }

  // the 'empty-cells' property has no effect on 'outline'
--- a/layout/tables/nsTableColFrame.cpp
+++ b/layout/tables/nsTableColFrame.cpp
@ -119,29 +119,7 @@ void nsTableColFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
  // ignored, except when explicitly specified by this specification."
  // CSS outlines and box-shadows fall into this category, so we skip them
  // on these boxes.
-
-  // Compute background rect by iterating all cell frame.
-  AutoTArray<uint32_t, 1> colIdx;
-  colIdx.AppendElement(GetColIndex());
-
-  nsTableFrame* table = GetTableFrame();
-  nsTableFrame::RowGroupArray rowGroups;
-  table->OrderRowGroups(rowGroups);
-  for (nsTableRowGroupFrame* rowGroup : rowGroups) {
-    auto offset = rowGroup->GetNormalPosition() - GetNormalPosition() -
-                  GetTableColGroupFrame()->GetNormalPosition();
-    if (!aBuilder->GetDirtyRect().Intersects(
-            nsRect(offset, rowGroup->GetSize()))) {
-      continue;
-    }
-    rowGroup->PaintCellBackgroundsForColumns(
-        this, aBuilder, aBuilder->GetTableBackgroundSet()->ColBackgrounds(),
-        colIdx, offset);
-  }
-
-  for (nsIFrame* kid : PrincipalChildList()) {
-    BuildDisplayListForChild(aBuilder, kid, aLists);
-  }
+  MOZ_ASSERT_UNREACHABLE("Cols don't paint themselves");
 }

 int32_t nsTableColFrame::GetSpan() { return StyleTable()->mSpan; }
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@ -357,36 +357,7 @@ void nsTableColGroupFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
  // ignored, except when explicitly specified by this specification."
  // CSS outlines and box-shadows fall into this category, so we skip them
  // on these boxes.
-
-  // Collecting column index.
-  AutoTArray<uint32_t, 1> colIdx;
-  for (nsTableColFrame* col = GetFirstColumn(); col; col = col->GetNextCol()) {
-    MOZ_ASSERT(colIdx.IsEmpty() || static_cast<uint32_t>(col->GetColIndex()) >
-                                       colIdx.LastElement());
-    colIdx.AppendElement(col->GetColIndex());
-  }
-
-  if (!colIdx.IsEmpty()) {
-    // We have some actual cells that live inside this rowgroup.
-    nsTableFrame* table = GetTableFrame();
-    nsTableFrame::RowGroupArray rowGroups;
-    table->OrderRowGroups(rowGroups);
-    for (nsTableRowGroupFrame* rowGroup : rowGroups) {
-      auto offset = rowGroup->GetNormalPosition() - GetNormalPosition();
-      if (!aBuilder->GetDirtyRect().Intersects(
-              nsRect(offset, rowGroup->GetSize()))) {
-        continue;
-      }
-      rowGroup->PaintCellBackgroundsForColumns(
-          this, aBuilder,
-          aBuilder->GetTableBackgroundSet()->ColGroupBackgrounds(), colIdx,
-          offset);
-    }
-  }
-
-  for (nsIFrame* kid : PrincipalChildList()) {
-    BuildDisplayListForChild(aBuilder, kid, aLists);
-  }
+  MOZ_ASSERT_UNREACHABLE("Colgroups don't paint themselves");
 }

 nsTableColFrame* nsTableColGroupFrame::GetFirstColumn() {
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@ -1292,7 +1292,7 @@ void nsTableFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,

  DisplayBorderBackgroundOutline(aBuilder, aLists);

-  nsDisplayTableBackgroundSet tableBGs(aBuilder);
+  nsDisplayTableBackgroundSet tableBGs(aBuilder, this);
  nsDisplayListCollection lists(aBuilder);

  // This is similar to what
@ -1304,8 +1304,10 @@ void nsTableFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
  // 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 : GetChildList(kColGroupList)) {
-    BuildDisplayListForChild(aBuilder, kid, lists);
+  for (nsIFrame* colGroup : FirstContinuation()->GetChildList(kColGroupList)) {
+    for (nsIFrame* col : colGroup->PrincipalChildList()) {
+      tableBGs.AddColumn((nsTableColFrame*)col);
+    }
  }

  for (nsIFrame* kid : PrincipalChildList()) {
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@ -76,9 +76,12 @@ class nsDisplayTableBackgroundSet {

  nsDisplayList* ColBackgrounds() { return &mColBackgrounds; }

-  explicit nsDisplayTableBackgroundSet(nsDisplayListBuilder* aBuilder)
+  nsDisplayTableBackgroundSet(nsDisplayListBuilder* aBuilder, nsIFrame* aTable)
      : mBuilder(aBuilder) {
    mPrevTableBackgroundSet = mBuilder->SetTableBackgroundSet(this);
+    mozilla::DebugOnly<const nsIFrame*> reference =
+        mBuilder->FindReferenceFrameFor(aTable, &mToReferenceFrame);
+    MOZ_ASSERT(nsLayoutUtils::IsAncestorFrameCrossDoc(reference, aTable));
  }

  ~nsDisplayTableBackgroundSet() {
@ -96,6 +99,12 @@ class nsDisplayTableBackgroundSet {
    aDestination.BorderBackground()->AppendToTop(ColBackgrounds());
  }

+  void AddColumn(nsTableColFrame* aFrame) { mColumns.AppendElement(aFrame); }
+
+  nsTableColFrame* GetColForIndex(int32_t aIndex) { return mColumns[aIndex]; }
+
+  const nsPoint& TableToReferenceFrame() { return mToReferenceFrame; }
+
 private:
  // This class is only used on stack, so we don't have to worry about leaking
  // it.  Don't let us be heap-allocated!
@ -107,6 +116,9 @@ class nsDisplayTableBackgroundSet {

  nsDisplayList mColGroupBackgrounds;
  nsDisplayList mColBackgrounds;
+
+  nsTArray<nsTableColFrame*> mColumns;
+  nsPoint mToReferenceFrame;
 };

 /* ========================================================================== */
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@ -236,46 +236,6 @@ static void DisplayRows(nsDisplayListBuilder* aBuilder, nsFrame* aFrame,
  }
 }

-void nsTableRowGroupFrame::PaintCellBackgroundsForColumns(
-    nsIFrame* aFrame, nsDisplayListBuilder* aBuilder, nsDisplayList* aList,
-    const nsTArray<uint32_t>& aColIdx, const nsPoint& aOffset) {
-  MOZ_DIAGNOSTIC_ASSERT(!aColIdx.IsEmpty(),
-                        "Must be painting backgrounds for something");
-
-  for (nsTableRowFrame* row = GetFirstRow(); row; row = row->GetNextRow()) {
-    auto rowPos = row->GetNormalPosition() + aOffset;
-    if (!aBuilder->GetDirtyRect().Intersects(nsRect(rowPos, row->GetSize()))) {
-      continue;
-    }
-    for (nsTableCellFrame* cell = row->GetFirstCell(); cell;
-         cell = cell->GetNextCell()) {
-      uint32_t curColIdx = cell->ColIndex();
-      if (!aColIdx.ContainsSorted(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 +
-                     aBuilder->ToReferenceFrame(aFrame);
-      auto cellRect = nsRect(cellPos, cell->GetSize());
-      if (!aBuilder->GetDirtyRect().Intersects(cellRect)) {
-        continue;
-      }
-      nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
-          aBuilder, aFrame, cellRect, aList, false, nullptr,
-          aFrame->GetRectRelativeToSelf() + aBuilder->ToReferenceFrame(aFrame),
-          cell);
-    }
-  }
-}
-
 void nsTableRowGroupFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                            const nsDisplayListSet& aLists) {
  DisplayOutsetBoxShadow(aBuilder, aLists.BorderBackground());
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@ -76,12 +76,6 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
  virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                const nsDisplayListSet& aLists) override;

-  void PaintCellBackgroundsForColumns(nsIFrame* aFrame,
-                                      nsDisplayListBuilder* aBuilder,
-                                      nsDisplayList* aList,
-                                      const nsTArray<uint32_t>& aColIdx,
-                                      const nsPoint& aOffset);
-
  /**
   * Calls Reflow for all of its child rows.
   *