- Revision
- 131465
- Author
- jchaffr...@webkit.org
- Date
- 2012-10-16 09:42:02 -0700 (Tue, 16 Oct 2012)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (131464 => 131465)
--- trunk/Source/WebCore/ChangeLog 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/ChangeLog 2012-10-16 16:42:02 UTC (rev 131465)
@@ -1,3 +1,46 @@
+2012-10-16 Julien Chaffraix <jchaffr...@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 <ta...@google.com>
[Meta] [Shadow] contenteditable attribute for distributed nodes.
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (131464 => 131465)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2012-10-16 16:42:02 UTC (rev 131465)
@@ -58,6 +58,7 @@
, 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 @@
// 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 @@
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 @@
repaintRectangle(LayoutRect(movedSectionLogicalTop, visualOverflowRect().y(), visualOverflowRect().maxX() - movedSectionLogicalTop, visualOverflowRect().height()));
}
+ m_columnLogicalWidthChanged = false;
setNeedsLayout(false);
}
@@ -550,12 +552,6 @@
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();
Modified: trunk/Source/WebCore/rendering/RenderTable.h (131464 => 131465)
--- trunk/Source/WebCore/rendering/RenderTable.h 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/rendering/RenderTable.h 2012-10-16 16:42:02 UTC (rev 131465)
@@ -147,6 +147,9 @@
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 @@
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 @@
mutable bool m_hasColElements : 1;
mutable bool m_needsSectionRecalc : 1;
-
+ bool m_columnLogicalWidthChanged : 1;
+
short m_hSpacing;
short m_vSpacing;
int m_borderStart;
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (131464 => 131465)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-10-16 16:42:02 UTC (rev 131465)
@@ -224,12 +224,18 @@
{
}
-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);
}
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (131464 => 131465)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2012-10-16 16:42:02 UTC (rev 131465)
@@ -104,7 +104,7 @@
virtual void computePreferredLogicalWidths();
- void setCellLogicalWidth(LayoutUnit);
+ void setCellLogicalWidth(int constrainedLogicalWidth);
virtual int borderLeft() const;
virtual int borderRight() const;
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (131464 => 131465)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2012-10-16 16:42:02 UTC (rev 131465)
@@ -261,47 +261,6 @@
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 @@
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);
}
Modified: trunk/Source/WebCore/rendering/RenderTableSection.h (131464 => 131465)
--- trunk/Source/WebCore/rendering/RenderTableSection.h 2012-10-16 16:23:43 UTC (rev 131464)
+++ trunk/Source/WebCore/rendering/RenderTableSection.h 2012-10-16 16:42:02 UTC (rev 131465)
@@ -74,7 +74,6 @@
void addCell(RenderTableCell*, RenderTableRow* row);
- void setCellLogicalWidths();
int calcRowLogicalHeight();
void layoutRows();
