[webkit-reviews] review denied: [Bug 65491] Implement a faster path for painting tables with overflowing cells : [Attachment 102570] Proposed algorithmic change: Use a memory / performance tradeoff to paint fastly without blowing up the memory.

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 65491: Implement a faster path for painting tables with overflowing cells
https://bugs.webkit.org/show_bug.cgi?id=65491

Attachment 102570: Proposed algorithmic change: Use a memory / performance
tradeoff to paint fastly without blowing up the memory.
https://bugs.webkit.org/attachment.cgi?id=102570&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=102570&action=review


Minusing because of the double painting issue since that would cause a
regression.

> Source/WebCore/rendering/RenderTableSection.cpp:1097
> -	       std::stable_sort(cells.begin(), cells.end(),
compareCellPositions);
> +	       if (!m_overflowingCells.size())
> +		   std::stable_sort(cells.begin(), cells.end(),
compareCellPositions);
> +	       else
> +		   std::stable_sort(cells.begin(), cells.end(),
compareCellPositionsWithOverflowingCells);

Is it really necessary to have a different comparison function here? Does
compareCellPositions not look at rows?

> Source/WebCore/rendering/RenderTableSection.cpp:1212
> -    if (m_hasOverflowingCell) {
> +    if (hasOverflowingCell()) {

Why not patch hit testing as well? You've left it slow?