Let cull rect of collapsed border contain all painted area

Source/core/paint/TableCellPainter.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/paint/TableCellPainter.h"
 
 #include "core/layout/LayoutTableCell.h"
 #include "core/paint/BlockPainter.h"
 #include "core/paint/BoxPainter.h"
@@ skipping 54 matching lines @@
     return style;
 }
 
 void TableCellPainter::paintCollapsedBorders(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     ASSERT(paintInfo.phase == PaintPhaseCollapsedTableBorders);
 
     if (!paintInfo.shouldPaintWithinRoot(&m_layoutTableCell) || m_layoutTableCell.style()->visibility() != VISIBLE)
         return;
 
-    LayoutRect paintRect = paintBounds(paintOffset, AddOffsetFromParent);
-
-    LayoutRect drawingCullRect(paintRect);
-    drawingCullRect.expandEdges(m_layoutTableCell.table()->outerBorderTop(), m_layoutTableCell.table()->outerBorderRight(),
-        m_layoutTableCell.table()->outerBorderBottom(), m_layoutTableCell.table()->outerBorderLeft());
-    if (drawingCullRect.y() >= paintInfo.rect.maxY())

[chrishtr, 2015/06/04 22:21:06]

You're removing a non-SP culling optimization. Was it previously broken?

[Xianzhu, 2015/06/05 00:02:14]

The cullrect was incorrect (which doesn't cover all collapsed border if the used
collapsed border is wider than the table's border). We should use borderRect
instead.

New line 105 is the new culling optimization.

-        return;
-
-    if (drawingCullRect.maxY() <= paintInfo.rect.y())
-        return;
-
     const CollapsedBorderValue* tableCurrentBorderValue = m_layoutTableCell.table()->currentBorderValue();
     if (!tableCurrentBorderValue)
         return;
 
     const ComputedStyle& styleForCellFlow = m_layoutTableCell.styleForCellFlow();
     const CollapsedBorderValue& leftBorderValue = cachedCollapsedLeftBorder(styleForCellFlow);
     const CollapsedBorderValue& rightBorderValue = cachedCollapsedRightBorder(styleForCellFlow);
     const CollapsedBorderValue& topBorderValue = cachedCollapsedTopBorder(styleForCellFlow);
     const CollapsedBorderValue& bottomBorderValue = cachedCollapsedBottomBorder(styleForCellFlow);
 
     bool shouldPaintTop = topBorderValue.shouldPaint(*tableCurrentBorderValue);
     bool shouldPaintBottom = bottomBorderValue.shouldPaint(*tableCurrentBorderValue);
     bool shouldPaintLeft = leftBorderValue.shouldPaint(*tableCurrentBorderValue);
     bool shouldPaintRight = rightBorderValue.shouldPaint(*tableCurrentBorderValue);
 
     if (!shouldPaintTop && !shouldPaintBottom && !shouldPaintLeft && !shouldPaintRight)
         return;
 
-    GraphicsContext* graphicsContext = paintInfo.context;
-    LayoutObjectDrawingRecorder recorder(*graphicsContext, m_layoutTableCell, paintInfo.phase, drawingCullRect);
-    if (recorder.canUseCachedDrawing())
-        return;
-
-    bool antialias = BoxPainter::shouldAntialiasLines(graphicsContext);
-
     // Adjust our x/y/width/height so that we paint the collapsed borders at the correct location.
     int topWidth = topBorderValue.width();
     int bottomWidth = bottomBorderValue.width();
     int leftWidth = leftBorderValue.width();
     int rightWidth = rightBorderValue.width();
 
+    LayoutRect paintRect = paintBounds(paintOffset, AddOffsetFromParent);

[chrishtr, 2015/06/05 00:31:06]

This is not used?

Why not fix paintBounds?

[Xianzhu, 2015/06/05 00:40:58]

Here paintRect is used to calculate borderRect below. The result of
paintBounds() is for other paintings, not including collapsed borders.

[chrishtr, 2015/06/05 04:55:28]

Oh right, sorry. My comment was too hasty.

     IntRect borderRect = pixelSnappedIntRect(paintRect.x() - leftWidth / 2,
         paintRect.y() - topWidth / 2,
         paintRect.width() + leftWidth / 2 + (rightWidth + 1) / 2,
         paintRect.height() + topWidth / 2 + (bottomWidth + 1) / 2);
 
+    if (!borderRect.intersects(paintInfo.rect))
+        return;
+
+    GraphicsContext* graphicsContext = paintInfo.context;
+    LayoutObjectDrawingRecorder recorder(*graphicsContext, m_layoutTableCell, paintInfo.phase, borderRect);
+    if (recorder.canUseCachedDrawing())
+        return;
+
     Color cellColor = m_layoutTableCell.resolveColor(CSSPropertyColor);
+    bool antialias = BoxPainter::shouldAntialiasLines(graphicsContext);
 
     // We never paint diagonals at the joins.  We simply let the border with the highest
     // precedence paint on top of borders with lower precedence.
     if (shouldPaintTop) {
         ObjectPainter::drawLineForBoxSide(graphicsContext, borderRect.x(), borderRect.y(), borderRect.maxX(), borderRect.y() + topWidth, BSTop,
             topBorderValue.color().resolve(cellColor), collapsedBorderStyle(topBorderValue.style()), 0, 0, antialias);
     }
     if (shouldPaintBottom) {
         ObjectPainter::drawLineForBoxSide(graphicsContext, borderRect.x(), borderRect.maxY() - bottomWidth, borderRect.maxX(), borderRect.maxY(), BSBottom,
             bottomBorderValue.color().resolve(cellColor), collapsedBorderStyle(bottomBorderValue.style()), 0, 0, antialias);
@@ skipping 88 matching lines @@
 LayoutRect TableCellPainter::paintBounds(const LayoutPoint& paintOffset, PaintBoundOffsetBehavior paintBoundOffsetBehavior)
 {
     LayoutPoint adjustedPaintOffset = paintOffset;
     if (paintBoundOffsetBehavior == AddOffsetFromParent)
         adjustedPaintOffset.moveBy(m_layoutTableCell.location());
     return LayoutRect(adjustedPaintOffset, LayoutSize(m_layoutTableCell.pixelSnappedSize()));
 }
 
 } // namespace blink