Bug 1409114 - Part 6: Store column and column group backgrounds separately, and then append then before the rest of the table contents. r=dbaron
authorMatt Woodrow <mwoodrow@mozilla.com>
Mon, 20 May 2019 23:15:39 +0000
changeset 474632 e88eae3b48a71d611ae395582d170548caf9bd28
parent 474631 34651ebcbaf4c94329b367e1752ba15dc0d25604
child 474633 df9a4342bf9750ad45219104bfd25674260f6080
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1409114
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1409114 - Part 6: Store column and column group backgrounds separately, and then append then before the rest of the table contents. r=dbaron This also changes behaviour a bit, previously we interleaved column and column group backgrounds. where we now put all the column group backgrounds behind all columns. I believe this is the correct ordering as per CSS2.2 Appendix E. Column backgrounds can overlap when using 'span', and we now render this in a different order, but this matches what other browsers do. Differential Revision: https://phabricator.services.mozilla.com/D29278
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
layout/reftests/table-background/reftest.list
layout/reftests/table-background/table-col-overlapping-ref.html
layout/reftests/table-background/table-col-overlapping.html
layout/tables/nsTableColFrame.cpp
layout/tables/nsTableColGroupFrame.cpp
layout/tables/nsTableFrame.cpp
layout/tables/nsTableFrame.h
layout/tables/nsTableRowGroupFrame.cpp
layout/tables/nsTableRowGroupFrame.h
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -1146,16 +1146,17 @@ nsDisplayListBuilder::nsDisplayListBuild
       mCurrentAGR(mRootAGR),
       mUsedAGRBudget(0),
       mDirtyRect(-1, -1, -1, -1),
       mGlassDisplayItem(nullptr),
       mScrollInfoItemsForHoisting(nullptr),
       mFirstClipChainToDestroy(nullptr),
       mActiveScrolledRootForRootScrollframe(nullptr),
       mMode(aMode),
+      mTableBackgroundSet(nullptr),
       mCurrentScrollParentId(ScrollableLayerGuid::NULL_SCROLL_ID),
       mCurrentScrollbarTarget(ScrollableLayerGuid::NULL_SCROLL_ID),
       mSVGEffectsBuildingDepth(0),
       mFilterASR(nullptr),
       mContainsBlendMode(false),
       mIsBuildingScrollbar(false),
       mCurrentScrollbarWillHaveLayer(false),
       mBuildCaret(aBuildCaret),
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -58,16 +58,17 @@
 class gfxContext;
 class nsIContent;
 class nsDisplayList;
 class nsDisplayTableItem;
 class nsIScrollableFrame;
 class nsSubDocumentFrame;
 class nsDisplayCompositorHitTestInfo;
 class nsDisplayScrollInfoLayer;
+class nsDisplayTableBackgroundSet;
 class nsCaret;
 enum class nsDisplayOwnLayerFlags;
 struct WrFiltersHolder;
 
 namespace mozilla {
 class FrameLayerBuilder;
 class PresShell;
 struct MotionPathData;
@@ -869,16 +870,26 @@ class nsDisplayListBuilder {
    * Indicates whether we should synchronously decode images. If true, we decode
    * and draw whatever image data has been loaded. If false, we just draw
    * whatever has already been decoded.
    */
   void SetSyncDecodeImages(bool aSyncDecodeImages) {
     mSyncDecodeImages = aSyncDecodeImages;
   }
 
+  nsDisplayTableBackgroundSet* SetTableBackgroundSet(
+      nsDisplayTableBackgroundSet* aTableSet) {
+    nsDisplayTableBackgroundSet* old = mTableBackgroundSet;
+    mTableBackgroundSet = aTableSet;
+    return old;
+  }
+  nsDisplayTableBackgroundSet* GetTableBackgroundSet() const {
+    return mTableBackgroundSet;
+  }
+
   void FreeClipChains();
 
   /*
    * Frees the temporary display items created during merging.
    */
   void FreeTemporaryItems();
 
   /**
@@ -1919,16 +1930,17 @@ class nsDisplayListBuilder {
   nsTArray<RefPtr<ActiveScrolledRoot>> mActiveScrolledRoots;
   std::unordered_set<const DisplayItemClipChain*, DisplayItemClipChainHasher,
                      DisplayItemClipChainEqualer>
       mClipDeduplicator;
   DisplayItemClipChain* mFirstClipChainToDestroy;
   nsTArray<nsDisplayItem*> mTemporaryItems;
   const ActiveScrolledRoot* mActiveScrolledRootForRootScrollframe;
   nsDisplayListBuilderMode mMode;
+  nsDisplayTableBackgroundSet* mTableBackgroundSet;
   ViewID mCurrentScrollParentId;
   ViewID mCurrentScrollbarTarget;
   MaybeScrollDirection mCurrentScrollbarDirection;
   Preserves3DContext mPreserves3DCtx;
   int32_t mSVGEffectsBuildingDepth;
   // When we are inside a filter, the current ASR at the time we entered the
   // filter. Otherwise nullptr.
   const ActiveScrolledRoot* mFilterASR;
--- a/layout/reftests/table-background/reftest.list
+++ b/layout/reftests/table-background/reftest.list
@@ -59,8 +59,10 @@ fuzzy-if(d2d||skiaContent,0-1,0-95000) =
 == empty-cells-default-1.html empty-cells-default-1-ref.html
 == empty-cells-default-2.html empty-cells-default-2-ref.html
 fuzzy-if(OSX,0-1,0-113) fuzzy-if(winWidget,0-1,0-12) fuzzy-if(winWidget&&!layersGPUAccelerated,0-82,0-116) fuzzy-if(skiaContent,0-84,0-5500) fuzzy-if(Android,0-2,0-5957) == table-row-opacity-dynamic-1.html table-row-opacity-dynamic-1-ref.html
 == table-row-opacity-dynamic-2.html table-row-opacity-dynamic-2-ref.html
 
 == hidden-cells-1.html about:blank
 == hidden-cells-2.html about:blank
 == hidden-cells-3.html hidden-cells-3-ref.html
+
+== table-col-overlapping.html table-col-overlapping-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/table-background/table-col-overlapping-ref.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+  td {
+    width: 20px;
+    height: 20px;
+    background-color: green;
+  }
+  table {
+    border-collapse:separate;
+    border-spacing: 0px;
+  }
+</style>
+</head>
+<body>
+<table>
+  <tr>
+    <td></td>
+    <td style="background-color: blue"></td>
+  <tr>
+    <td></td>
+    <td></td>
+  </tr>
+</table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/table-background/table-col-overlapping.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+  td {
+    width: 20px;
+    height: 20px;
+  }
+  table {
+    border-collapse:separate;
+    border-spacing: 0px;
+  }
+</style>
+</head>
+<body>
+<table>
+  <col style="background: green"></col>
+  <col style="background: blue"></col>
+  <tr>
+    <td></td>
+    <td rowspan=2></td>
+  <tr>
+    <td colspan=2></td>
+  </tr>
+</table>
+</body>
+</html>
--- a/layout/tables/nsTableColFrame.cpp
+++ b/layout/tables/nsTableColFrame.cpp
@@ -129,18 +129,19 @@ void nsTableColFrame::BuildDisplayList(n
   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, aLists, colIdx,
-                                             offset);
+    rowGroup->PaintCellBackgroundsForColumns(
+        this, aBuilder, aBuilder->GetTableBackgroundSet()->ColBackgrounds(),
+        colIdx, offset);
   }
 
   for (nsIFrame* kid : PrincipalChildList()) {
     BuildDisplayListForChild(aBuilder, kid, aLists);
   }
 }
 
 int32_t nsTableColFrame::GetSpan() { return StyleTable()->mSpan; }
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@@ -372,18 +372,20 @@ void nsTableColGroupFrame::BuildDisplayL
     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, aLists, colIdx,
-                                               offset);
+      rowGroup->PaintCellBackgroundsForColumns(
+          this, aBuilder,
+          aBuilder->GetTableBackgroundSet()->ColGroupBackgrounds(), colIdx,
+          offset);
     }
   }
 
   for (nsIFrame* kid : PrincipalChildList()) {
     BuildDisplayListForChild(aBuilder, kid, aLists);
   }
 }
 
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1287,32 +1287,38 @@ void nsTableFrame::CalcHasBCBorders() {
 // table paint code is concerned primarily with borders and bg color
 // SEC: TODO: adjust the rect for captions
 void nsTableFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                     const nsDisplayListSet& aLists) {
   DO_GLOBAL_REFLOW_COUNT_DSP_COLOR("nsTableFrame", NS_RGB(255, 128, 255));
 
   DisplayBorderBackgroundOutline(aBuilder, aLists);
 
+  nsDisplayTableBackgroundSet tableBGs(aBuilder);
+  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* kid : GetChildList(kColGroupList)) {
-    BuildDisplayListForChild(aBuilder, kid, aLists);
+    BuildDisplayListForChild(aBuilder, kid, lists);
   }
 
   for (nsIFrame* kid : PrincipalChildList()) {
-    BuildDisplayListForChild(aBuilder, kid, aLists);
-  }
+    BuildDisplayListForChild(aBuilder, kid, lists);
+  }
+
+  tableBGs.MoveTo(aLists);
+  lists.MoveTo(aLists);
 
   if (IsVisibleForPainting()) {
     // In the collapsed border model, overlay all collapsed borders.
     if (IsBorderCollapse()) {
       if (HasBCBorders()) {
         aLists.BorderBackground()->AppendNewToTop<nsDisplayTableBorderCollapse>(
             aBuilder, this);
       }
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@@ -65,16 +65,55 @@ class nsDisplayTableItem : public nsPain
 
   void UpdateForFrameBackground(nsIFrame* aFrame);
 
  private:
   bool mPartHasFixedBackground;
   bool mDrawsBackground;
 };
 
+class nsDisplayTableBackgroundSet {
+ public:
+  nsDisplayList* ColGroupBackgrounds() { return &mColGroupBackgrounds; }
+
+  nsDisplayList* ColBackgrounds() { return &mColBackgrounds; }
+
+  explicit nsDisplayTableBackgroundSet(nsDisplayListBuilder* aBuilder)
+      : mBuilder(aBuilder) {
+    mPrevTableBackgroundSet = mBuilder->SetTableBackgroundSet(this);
+  }
+
+  ~nsDisplayTableBackgroundSet() {
+    mozilla::DebugOnly<nsDisplayTableBackgroundSet*> result =
+        mBuilder->SetTableBackgroundSet(mPrevTableBackgroundSet);
+    MOZ_ASSERT(result == this);
+  }
+
+  /**
+   * Move all display items in our lists to top of the corresponding lists in
+   * the destination.
+   */
+  void MoveTo(const nsDisplayListSet& aDestination) {
+    aDestination.BorderBackground()->AppendToTop(ColGroupBackgrounds());
+    aDestination.BorderBackground()->AppendToTop(ColBackgrounds());
+  }
+
+ 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!
+  void* operator new(size_t sz) CPP_THROW_NEW;
+
+ protected:
+  nsDisplayListBuilder* mBuilder;
+  nsDisplayTableBackgroundSet* mPrevTableBackgroundSet;
+
+  nsDisplayList mColGroupBackgrounds;
+  nsDisplayList mColBackgrounds;
+};
+
 /* ========================================================================== */
 
 enum nsTableColType {
   eColContent = 0,            // there is real col content associated
   eColAnonymousCol = 1,       // the result of a span on a col
   eColAnonymousColGroup = 2,  // the result of a span on a col group
   eColAnonymousCell = 3       // the result of a cell alone
 };
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -232,19 +232,18 @@ static void DisplayRows(nsDisplayListBui
     kid = kid->GetNextSibling();
   }
   if (cursor) {
     cursor->FinishBuildingCursor();
   }
 }
 
 void nsTableRowGroupFrame::PaintCellBackgroundsForColumns(
-    nsIFrame* aFrame, nsDisplayListBuilder* aBuilder,
-    const nsDisplayListSet& aLists, const nsTArray<uint32_t>& aColIdx,
-    const nsPoint& aOffset) {
+    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;
     }
@@ -264,17 +263,17 @@ void nsTableRowGroupFrame::PaintCellBack
       }
 
       auto cellPos = cell->GetNormalPosition() + rowPos;
       auto cellRect = nsRect(cellPos, cell->GetSize());
       if (!aBuilder->GetDirtyRect().Intersects(cellRect)) {
         continue;
       }
       nsDisplayBackgroundImage::AppendBackgroundItemsToTop(
-          aBuilder, aFrame, cellRect, aLists.BorderBackground(), false, nullptr,
+          aBuilder, aFrame, cellRect, aList, false, nullptr,
           aFrame->GetRectRelativeToSelf(), cell);
     }
   }
 }
 
 void nsTableRowGroupFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                             const nsDisplayListSet& aLists) {
   DisplayOutsetBoxShadow(aBuilder, aLists.BorderBackground());
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@@ -73,17 +73,17 @@ class nsTableRowGroupFrame final : publi
   virtual nsMargin GetUsedBorder() const override;
   virtual nsMargin GetUsedPadding() const override;
 
   virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                 const nsDisplayListSet& aLists) override;
 
   void PaintCellBackgroundsForColumns(nsIFrame* aFrame,
                                       nsDisplayListBuilder* aBuilder,
-                                      const nsDisplayListSet& aLists,
+                                      nsDisplayList* aList,
                                       const nsTArray<uint32_t>& aColIdx,
                                       const nsPoint& aOffset);
 
   /**
    * Calls Reflow for all of its child rows.
    *
    * Rows are all set to the same isize and stacked in the block direction.
    *