Fold setCellLogicalWidths logic into RenderTableSection layout
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Oct 2012 16:42:02 +0000 (16:42 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Oct 2012 16:42:02 +0000 (16:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=99382

Reviewed by Eric Seidel.

setCellLogicalWidths was implemented as a pre-phase to laying out
the table's sections. This split was artificial as any change in
the columns' logical width should trigger a sections' relayout, which
could propagate and mark the cells / rows as needed.

Merging setCellLogicalWidths into RenderTableSection::layout removes
an unneeded cells walking and some clunkiness from our implementation.

Refactoring covered by the existing tests.

* rendering/RenderTable.cpp:
(WebCore::RenderTable::RenderTable): Initialize our new boolean.
(WebCore::RenderTable::layout):
If m_columnLogicalWidthChanged, we force a relayout on our sections so that the cells and rows
are marked for layout if there is the logical width change.

* rendering/RenderTable.h:
(WebCore::RenderTable):
Added a new boolean to track if a column logical width changed (m_columnLogicalWidthChanged).

(WebCore::RenderTable::setColumnPosition):
If a column position changed, register that our column logical widths changed. This is not
totally true, so added a comment about when it will be wrong.

* rendering/RenderTableCell.h:
* rendering/RenderTableCell.cpp:
(WebCore::RenderTableCell::setCellLogicalWidth):
Updated the function to mark the cell and the row for layout. Also changed the argument to
be an 'int' as this was what was passed in.

* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::layout):
* rendering/RenderTableSection.h:
Removed setCellLogicalWidths and merged the logic into RenderTableSection::layout. We propagate
the table layout's logical widths first so that rows are marked as needing layout as appropriate.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@131465 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTable.h
Source/WebCore/rendering/RenderTableCell.cpp
Source/WebCore/rendering/RenderTableCell.h
Source/WebCore/rendering/RenderTableSection.cpp
Source/WebCore/rendering/RenderTableSection.h

index 757f964..5a7b61d 100644 (file)
@@ -1,3 +1,46 @@
+2012-10-16  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Fold setCellLogicalWidths logic into RenderTableSection layout
+        https://bugs.webkit.org/show_bug.cgi?id=99382
+
+        Reviewed by Eric Seidel.
+
+        setCellLogicalWidths was implemented as a pre-phase to laying out
+        the table's sections. This split was artificial as any change in
+        the columns' logical width should trigger a sections' relayout, which
+        could propagate and mark the cells / rows as needed.
+
+        Merging setCellLogicalWidths into RenderTableSection::layout removes
+        an unneeded cells walking and some clunkiness from our implementation.
+
+        Refactoring covered by the existing tests.
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::RenderTable): Initialize our new boolean.
+        (WebCore::RenderTable::layout):
+        If m_columnLogicalWidthChanged, we force a relayout on our sections so that the cells and rows
+        are marked for layout if there is the logical width change.
+
+        * rendering/RenderTable.h:
+        (WebCore::RenderTable):
+        Added a new boolean to track if a column logical width changed (m_columnLogicalWidthChanged).
+
+        (WebCore::RenderTable::setColumnPosition):
+        If a column position changed, register that our column logical widths changed. This is not
+        totally true, so added a comment about when it will be wrong.
+
+        * rendering/RenderTableCell.h:
+        * rendering/RenderTableCell.cpp:
+        (WebCore::RenderTableCell::setCellLogicalWidth):
+        Updated the function to mark the cell and the row for layout. Also changed the argument to
+        be an 'int' as this was what was passed in.
+
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::layout):
+        * rendering/RenderTableSection.h:
+        Removed setCellLogicalWidths and merged the logic into RenderTableSection::layout. We propagate
+        the table layout's logical widths first so that rows are marked as needing layout as appropriate.
+
 2012-10-16  Takashi Sakamoto  <tasak@google.com>
 
         [Meta] [Shadow] contenteditable attribute for distributed nodes.
index 2698f78..ee7c8b8 100644 (file)
@@ -58,6 +58,7 @@ RenderTable::RenderTable(Node* node)
     , m_collapsedBordersValid(false)
     , m_hasColElements(false)
     , m_needsSectionRecalc(false)
+    , m_columnLogicalWidthChanged(false)
     , m_hSpacing(0)
     , m_vSpacing(0)
     , m_borderStart(0)
@@ -369,8 +370,6 @@ void RenderTable::layout()
 //     if ( oldWidth != width() || columns.size() + 1 != columnPos.size() )
     m_tableLayout->layout();
 
-    setCellLogicalWidths();
-
     LayoutUnit totalSectionLogicalHeight = 0;
     LayoutUnit oldTableLogicalTop = 0;
     for (unsigned i = 0; i < m_captions.size(); i++)
@@ -380,8 +379,10 @@ void RenderTable::layout()
 
     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
         if (child->isTableSection()) {
-            child->layoutIfNeeded();
             RenderTableSection* section = toRenderTableSection(child);
+            if (m_columnLogicalWidthChanged)
+                section->setChildNeedsLayout(true, MarkOnlyThis);
+            section->layoutIfNeeded();
             totalSectionLogicalHeight += section->calcRowLogicalHeight();
             if (collapsing)
                 section->recalcOuterBorder();
@@ -496,6 +497,7 @@ void RenderTable::layout()
             repaintRectangle(LayoutRect(movedSectionLogicalTop, visualOverflowRect().y(), visualOverflowRect().maxX() - movedSectionLogicalTop, visualOverflowRect().height()));
     }
 
+    m_columnLogicalWidthChanged = false;
     setNeedsLayout(false);
 }
 
@@ -550,12 +552,6 @@ void RenderTable::addOverflowFromChildren()
         addOverflowFromChild(section);
 }
 
-void RenderTable::setCellLogicalWidths()
-{
-    for (RenderTableSection* section = topSection(); section; section = sectionBelow(section))
-        section->setCellLogicalWidths();
-}
-
 void RenderTable::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     LayoutPoint adjustedPaintOffset = paintOffset + location();
index 6a77646..de4b5a6 100644 (file)
@@ -147,6 +147,9 @@ public:
     const Vector<int>& columnPositions() const { return m_columnPos; }
     void setColumnPosition(unsigned index, int position)
     {
+        // Note that if our horizontal border-spacing changed, our position will change but not
+        // our column's width. In practice, horizontal border-spacing won't change often.
+        m_columnLogicalWidthChanged |= m_columnPos[index] != position;
         m_columnPos[index] = position;
     }
 
@@ -283,8 +286,6 @@ private:
     virtual RenderBlock* firstLineBlock() const;
     virtual void updateFirstLetter();
     
-    virtual void setCellLogicalWidths();
-
     virtual void updateLogicalWidth() OVERRIDE;
 
     LayoutUnit convertStyleLogicalWidthToComputedWidth(const Length& styleLogicalWidth, LayoutUnit availableWidth);
@@ -317,7 +318,8 @@ private:
     
     mutable bool m_hasColElements : 1;
     mutable bool m_needsSectionRecalc : 1;
-    
+    bool m_columnLogicalWidthChanged : 1;
+
     short m_hSpacing;
     short m_vSpacing;
     int m_borderStart;
index f53fb07..55ac553 100644 (file)
@@ -224,12 +224,18 @@ void RenderTableCell::updateLogicalWidth()
 {
 }
 
-void RenderTableCell::setCellLogicalWidth(LayoutUnit w)
+void RenderTableCell::setCellLogicalWidth(int tableLayoutLogicalWidth)
 {
-    if (w == logicalWidth())
+    if (tableLayoutLogicalWidth == logicalWidth())
         return;
 
-    setLogicalWidth(w);
+    setNeedsLayout(true, MarkOnlyThis);
+    row()->setChildNeedsLayout(true, MarkOnlyThis);
+
+    if (!table()->selfNeedsLayout() && checkForRepaintDuringLayout())
+        repaint();
+
+    setLogicalWidth(tableLayoutLogicalWidth);
     setCellWidthChanged(true);
 }
 
index a873a51..f2903a3 100644 (file)
@@ -104,7 +104,7 @@ public:
 
     virtual void computePreferredLogicalWidths();
 
-    void setCellLogicalWidth(LayoutUnit);
+    void setCellLogicalWidth(int constrainedLogicalWidth);
 
     virtual int borderLeft() const;
     virtual int borderRight() const;
index 72e826d..7dce602 100644 (file)
@@ -261,47 +261,6 @@ void RenderTableSection::addCell(RenderTableCell* cell, RenderTableRow* row)
     cell->setCol(table()->effColToCol(col));
 }
 
-void RenderTableSection::setCellLogicalWidths()
-{
-    const Vector<int>& columnPos = table()->columnPositions();
-
-    LayoutStateMaintainer statePusher(view());
-
-    for (unsigned i = 0; i < m_grid.size(); i++) {
-        Row& row = m_grid[i].row;
-        unsigned cols = row.size();
-        for (unsigned j = 0; j < cols; j++) {
-            CellStruct& current = row[j];
-            RenderTableCell* cell = current.primaryCell();
-            if (!cell || current.inColSpan)
-              continue;
-            unsigned endCol = j;
-            unsigned cspan = cell->colSpan();
-            while (cspan && endCol < cols) {
-                ASSERT(endCol < table()->columns().size());
-                cspan -= table()->columns()[endCol].span;
-                endCol++;
-            }
-            int w = columnPos[endCol] - columnPos[j] - table()->hBorderSpacing();
-            int oldLogicalWidth = cell->logicalWidth();
-            if (w != oldLogicalWidth) {
-                cell->setNeedsLayout(true);
-                if (!table()->selfNeedsLayout() && cell->checkForRepaintDuringLayout()) {
-                    if (!statePusher.didPush()) {
-                        // Technically, we should also push state for the row, but since
-                        // rows don't push a coordinate transform, that's not necessary.
-                        statePusher.push(this, locationOffset());
-                    }
-                    cell->repaint();
-                }
-                cell->setCellLogicalWidth(w);
-            }
-        }
-    }
-    
-    statePusher.pop(); // only pops if we pushed
-}
-
 int RenderTableSection::calcRowLogicalHeight()
 {
 #ifndef NDEBUG
@@ -404,12 +363,35 @@ void RenderTableSection::layout()
     m_grid.shrinkToFit();
 
     LayoutStateMaintainer statePusher(view(), this, locationOffset(), style()->isFlippedBlocksWritingMode());
-    for (RenderObject* child = children()->firstChild(); child; child = child->nextSibling()) {
-        if (child->isTableRow()) {
-            child->layoutIfNeeded();
-            ASSERT(!child->needsLayout());
+
+    const Vector<int>& columnPos = table()->columnPositions();
+
+    for (unsigned r = 0; r < m_grid.size(); ++r) {
+        Row& row = m_grid[r].row;
+        unsigned cols = row.size();
+        // First, propagate our table layout's information to the cells. This will mark the row as needing layout
+        // if there was a column logical width change.
+        for (unsigned startColumn = 0; startColumn < cols; ++startColumn) {
+            CellStruct& current = row[startColumn];
+            RenderTableCell* cell = current.primaryCell();
+            if (!cell || current.inColSpan)
+                continue;
+
+            unsigned endCol = startColumn;
+            unsigned cspan = cell->colSpan();
+            while (cspan && endCol < cols) {
+                ASSERT(endCol < table()->columns().size());
+                cspan -= table()->columns()[endCol].span;
+                endCol++;
+            }
+            int tableLayoutLogicalWidth = columnPos[endCol] - columnPos[startColumn] - table()->hBorderSpacing();
+            cell->setCellLogicalWidth(tableLayoutLogicalWidth);
         }
+
+        if (RenderTableRow* rowRenderer = m_grid[r].rowRenderer)
+            rowRenderer->layoutIfNeeded();
     }
+
     statePusher.pop();
     setNeedsLayout(false);
 }
index 02a5299..2bb9ce3 100644 (file)
@@ -74,7 +74,6 @@ public:
 
     void addCell(RenderTableCell*, RenderTableRow* row);
 
-    void setCellLogicalWidths();
     int calcRowLogicalHeight();
     void layoutRows();