Index: Source/core/rendering/RenderTableSection.cpp |
diff --git a/Source/core/rendering/RenderTableSection.cpp b/Source/core/rendering/RenderTableSection.cpp |
index b0264c6f7fd92c939df76fadccf26cf87534875b..fe988609cb0e84c1c72dcb9d74afd1754b6d0267 100644 |
--- a/Source/core/rendering/RenderTableSection.cpp |
+++ b/Source/core/rendering/RenderTableSection.cpp |
@@ -485,57 +485,61 @@ static bool compareRowSpanCellsInHeightDistributionOrder(const RenderTableCell* |
return false; |
} |
-bool RenderTableSection::isHeightNeededForRowHavingOnlySpanningCells(unsigned row) |
+unsigned RenderTableSection::calcRowHeightHavingOnlySpanningCells(unsigned row, unsigned& extraHeightToPropagate, unsigned r1, int& accumulatedPositionIncrease, unsigned r2, Vector<int>& rowsCountWithOnlySpanningCells) |
Julien - ping for review
2014/10/08 00:16:08
r1 and r2 are not good variable names :(
Also thi
a.suchit
2014/10/08 11:30:08
changed and name of r1 variable
I did not observe
|
{ |
+ ASSERT(rowHasOnlySpanningCells(row)); |
+ |
unsigned totalCols = m_grid[row].row.size(); |
if (!totalCols) |
- return false; |
+ return 0; |
+ |
+ unsigned rowHeight = 0; |
for (unsigned col = 0; col < totalCols; col++) { |
const CellStruct& rowSpanCell = cellAt(row, col); |
if (rowSpanCell.cells.size()) { |
RenderTableCell* cell = rowSpanCell.cells[0]; |
- const unsigned rowIndex = cell->rowIndex(); |
- const unsigned rowSpan = cell->rowSpan(); |
- int totalRowSpanCellHeight = 0; |
- for (unsigned row = 0; row < rowSpan; row++) { |
- unsigned actualRow = row + rowIndex; |
- totalRowSpanCellHeight += m_rowPos[actualRow + 1] - m_rowPos[actualRow]; |
- } |
- totalRowSpanCellHeight -= borderSpacingForRow(rowIndex + rowSpan - 1); |
+ if (cell->rowSpan() > 1) { |
+ const unsigned rowIndex = cell->rowIndex(); |
+ const unsigned rowSpan = cell->rowSpan(); |
- if (totalRowSpanCellHeight < cell->logicalHeightForRowSizing()) |
- return true; |
- } |
- } |
+ unsigned endRow = rowIndex + rowSpan; |
+ unsigned spanningCellsRowsCountHavingZeroHeight = rowsCountWithOnlySpanningCells[endRow -1] - rowsCountWithOnlySpanningCells[max(rowIndex, row)]; |
Julien - ping for review
2014/10/08 00:16:08
max(rowIndex, row) is repeated 4 times, it seems l
a.suchit
2014/10/08 11:30:08
Done.
|
- return false; |
-} |
+ if (max(rowIndex, row)) { |
+ spanningCellsRowsCountHavingZeroHeight += rowsCountWithOnlySpanningCells[max(rowIndex, row)] - rowsCountWithOnlySpanningCells[max(rowIndex, row) - 1]; |
Julien - ping for review
2014/10/08 00:16:08
I am probably confused by this line but it seems d
a.suchit
2014/10/08 11:30:08
Acknowledged.
|
+ } else { |
+ spanningCellsRowsCountHavingZeroHeight += rowsCountWithOnlySpanningCells[0]; |
Julien - ping for review
2014/10/08 00:16:08
This else is suspicious, I don't see why some cell
a.suchit
2014/10/08 11:30:08
Acknowledged.
|
+ } |
-unsigned RenderTableSection::calcRowHeightHavingOnlySpanningCells(unsigned row) |
-{ |
- ASSERT(rowHasOnlySpanningCells(row)); |
+ int totalRowspanCellHeight = m_rowPos[rowIndex + rowSpan] - m_rowPos[rowIndex]; |
- unsigned totalCols = m_grid[row].row.size(); |
+ totalRowspanCellHeight -= borderSpacingForRow(rowIndex + rowSpan - 1); |
+ if (r1 > rowIndex && r1 < endRow) |
+ totalRowspanCellHeight += extraHeightToPropagate; |
+ if (r2 > rowIndex && r2 < endRow) |
+ totalRowspanCellHeight += accumulatedPositionIncrease; |
- if (!totalCols) |
- return 0; |
+ if (totalRowspanCellHeight < cell->logicalHeightForRowSizing()) { |
+ unsigned extraHeightRequired = rowSpanCell.cells[0]->logicalHeightForRowSizing() - totalRowspanCellHeight; |
+ int remainder = extraHeightRequired % spanningCellsRowsCountHavingZeroHeight; |
- unsigned rowHeight = 0; |
+ if (remainder) |
+ extraHeightRequired += spanningCellsRowsCountHavingZeroHeight - remainder; |
- for (unsigned col = 0; col < totalCols; col++) { |
- const CellStruct& rowSpanCell = cellAt(row, col); |
- if (rowSpanCell.cells.size() && rowSpanCell.cells[0]->rowSpan() > 1) |
- rowHeight = std::max(rowHeight, rowSpanCell.cells[0]->logicalHeightForRowSizing() / rowSpanCell.cells[0]->rowSpan()); |
+ rowHeight = std::max(rowHeight, extraHeightRequired / spanningCellsRowsCountHavingZeroHeight); |
+ } |
+ } |
+ } |
} |
return rowHeight; |
} |
-void RenderTableSection::updateRowsHeightHavingOnlySpanningCells(RenderTableCell* cell, struct SpanningRowsHeight& spanningRowsHeight) |
+void RenderTableSection::updateRowsHeightHavingOnlySpanningCells(RenderTableCell* cell, struct SpanningRowsHeight& spanningRowsHeight, unsigned& extraHeightToPropagate, Vector<int>& rowsCountWithOnlySpanningCells) |
{ |
ASSERT(spanningRowsHeight.rowHeight.size()); |
@@ -547,8 +551,8 @@ void RenderTableSection::updateRowsHeightHavingOnlySpanningCells(RenderTableCell |
for (unsigned row = 0; row < spanningRowsHeight.rowHeight.size(); row++) { |
unsigned actualRow = row + rowIndex; |
- if (!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow) && isHeightNeededForRowHavingOnlySpanningCells(actualRow)) { |
- spanningRowsHeight.rowHeight[row] = calcRowHeightHavingOnlySpanningCells(actualRow); |
+ if (!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow)) { |
+ spanningRowsHeight.rowHeight[row] = calcRowHeightHavingOnlySpanningCells(actualRow, extraHeightToPropagate, rowIndex + rowSpan, accumulatedPositionIncrease, actualRow, rowsCountWithOnlySpanningCells); |
accumulatedPositionIncrease += spanningRowsHeight.rowHeight[row]; |
} |
m_rowPos[actualRow + 1] += accumulatedPositionIncrease; |
@@ -571,6 +575,16 @@ void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells& |
unsigned lastRowIndex = 0; |
unsigned lastRowSpan = 0; |
+ Vector<int> rowsCountWithOnlySpanningCells; |
+ |
+ // At this stage, Height of the rows are zero those contains only spanning cells. |
Julien - ping for review
2014/10/08 00:16:08
Let's do correct English sentence:
// At this sta
a.suchit
2014/10/08 11:30:08
Done.
|
+ int count = 0; |
+ for (unsigned row = 0; row < m_grid.size(); row++) { |
+ if (rowHasOnlySpanningCells(row)) |
+ count++; |
+ rowsCountWithOnlySpanningCells.append(count); |
+ } |
Julien - ping for review
2014/10/08 00:16:08
You've added an unrelated O(N) loop here inside a
a.suchit
2014/10/08 11:30:08
If it is not done here then repeatedly, need to ch
|
+ |
for (unsigned i = 0; i < rowSpanCells.size(); i++) { |
RenderTableCell* cell = rowSpanCells[i]; |
@@ -608,7 +622,7 @@ void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells& |
// Here we are handling only row(s) who have only rowspanning cells and do not have any empty cell. |
if (spanningRowsHeight.isAnyRowWithOnlySpanningCells) |
- updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight); |
+ updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight, extraHeightToPropagate, rowsCountWithOnlySpanningCells); |
// This code handle row(s) that have rowspanning cell(s) and at least one empty cell. |
// Such rows are not handled below and end up having a height of 0. That would mean |