Table rows are not totally invalidated after r170349

The change removed
RenderTableRow::clippedOverflowRectForPaintInvalidation
but the row's visual overflow doesn't include any cells
spanning several rows, which is required to be able to
invalidate rows correctly.

The fix is to include the spanning cells into the
row's visual overflow. Because they both share the
section's coordinate system, we need to translate the
cell's coordinate into the row's to avoid some wrong
visual overflow (aka over-invalidations).

BUG=417060

Review URL: https://codereview.chromium.org/673533002

git-svn-id: svn://svn.chromium.org/blink/trunk@184296 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 5715d4b5
{
"bounds": [800, 600],
"children": [
{
"bounds": [800, 600],
"contentsOpaque": true,
"drawsContent": true,
"repaintRects": [
[8, 218, 70, 26]
]
}
]
}
<!DOCTYPE html>
<style>
body {
font: 10px/1 Ahem;
}
tr:hover {
background-color: green;
}
table {
position:relative;
top: 180px;
}
</style>
<script src="resources/text-based-repaint.js" type="text/javascript"></script>
<script>
function repaintTest()
{
var secondRowSpan = document.getElementById("secondRowSpan");
var secondRowSpanBox = secondRowSpan.getBoundingClientRect();
var secondRowSpanCenterX = (secondRowSpanBox.left + secondRowSpanBox.right) / 2;
var secondRowSpanCenterY = (secondRowSpanBox.top + secondRowSpanBox.bottom) / 2;
if (window.eventSender) {
eventSender.mouseMoveTo(secondRowSpanCenterX, secondRowSpanCenterY);
eventSender.mouseDown();
eventSender.mouseUp();
}
}
window.addEventListener("load", runRepaintTest);
</script>
<table>
<tr>
<td rowspan="2">1,1</td>
<td>1,4</td>
</tr>
<tr>
<td>2,3</td>
</tr>
<tr class="bla">
<td rowspan="2" id="secondRowSpan">3,1</td>
<td>3,4</td>
</tr>
<tr>
<td>4,4</td>
</tr>
</table>
...@@ -181,6 +181,8 @@ void RenderTableRow::layout() ...@@ -181,6 +181,8 @@ void RenderTableRow::layout()
m_overflow.clear(); m_overflow.clear();
addVisualEffectOverflow(); addVisualEffectOverflow();
// We do not call addOverflowFromCell here. The cell are laid out to be
// measured above and will be sized correctly in a follow-up phase.
// We only ever need to issue paint invalidations if our cells didn't, which means that they didn't need // We only ever need to issue paint invalidations if our cells didn't, which means that they didn't need
// layout, so we know that our bounds didn't change. This code is just making up for // layout, so we know that our bounds didn't change. This code is just making up for
...@@ -244,4 +246,22 @@ RenderTableRow* RenderTableRow::createAnonymousWithParentRenderer(const RenderOb ...@@ -244,4 +246,22 @@ RenderTableRow* RenderTableRow::createAnonymousWithParentRenderer(const RenderOb
return newRow; return newRow;
} }
void RenderTableRow::addOverflowFromCell(const RenderTableCell* cell)
{
// Non-row-spanning-cells don't create overflow (they are fully contained within this row).
if (cell->rowSpan() == 1)
return;
// Cells only generates visual overflow.
LayoutRect cellVisualOverflowRect = cell->visualOverflowRectForPropagation(style());
// The cell and the row share the section's coordinate system. However
// the visual overflow should be determined in the coordinate system of
// the row, that's why we shift it below.
LayoutUnit cellOffsetLogicalTopDifference = cell->location().y() - location().y();
cellVisualOverflowRect.move(0, cellOffsetLogicalTopDifference);
addVisualOverflow(cellVisualOverflowRect);
}
} // namespace blink } // namespace blink
...@@ -93,6 +93,8 @@ public: ...@@ -93,6 +93,8 @@ public:
virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override; virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override;
void addOverflowFromCell(const RenderTableCell*);
private: private:
virtual RenderObjectChildList* virtualChildren() override { return children(); } virtual RenderObjectChildList* virtualChildren() override { return children(); }
virtual const RenderObjectChildList* virtualChildren() const override { return children(); } virtual const RenderObjectChildList* virtualChildren() const override { return children(); }
......
...@@ -955,7 +955,8 @@ void RenderTableSection::layoutRows() ...@@ -955,7 +955,8 @@ void RenderTableSection::layoutRows()
for (unsigned r = 0; r < totalRows; r++) { for (unsigned r = 0; r < totalRows; r++) {
// Set the row's x/y position and width/height. // Set the row's x/y position and width/height.
if (RenderTableRow* rowRenderer = m_grid[r].rowRenderer) { RenderTableRow* rowRenderer = m_grid[r].rowRenderer;
if (rowRenderer) {
rowRenderer->setLocation(LayoutPoint(0, m_rowPos[r])); rowRenderer->setLocation(LayoutPoint(0, m_rowPos[r]));
rowRenderer->setLogicalWidth(logicalWidth()); rowRenderer->setLogicalWidth(logicalWidth());
rowRenderer->setLogicalHeight(m_rowPos[r + 1] - m_rowPos[r] - vspacing); rowRenderer->setLogicalHeight(m_rowPos[r + 1] - m_rowPos[r] - vspacing);
...@@ -1053,6 +1054,9 @@ void RenderTableSection::layoutRows() ...@@ -1053,6 +1054,9 @@ void RenderTableSection::layoutRows()
cell->computeOverflow(oldLogicalHeight, false); cell->computeOverflow(oldLogicalHeight, false);
} }
if (rowRenderer)
rowRenderer->addOverflowFromCell(cell);
LayoutSize childOffset(cell->location() - oldCellRect.location()); LayoutSize childOffset(cell->location() - oldCellRect.location());
if (childOffset.width() || childOffset.height()) { if (childOffset.width() || childOffset.height()) {
// If the child moved, we have to issue paint invalidations to it as well as any floating/positioned // If the child moved, we have to issue paint invalidations to it as well as any floating/positioned
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment