Bug 126460 – [CSS Regions] Improve RenderFlowThread::repaintRectangleInRegions() to avoid loop over all the regions

• Status: RESOLVED
• Resolution: WONTFIX
• Component: CSS

[Manuel Rego Casasnovas]

In each RenderSelectionInfo::repaint() it's used the RenderObject::containerForRepaint(). Which in most of the situations returns the RenderNamedFlowThread for all the elements under RenderNamedFlowThread in the render tree.

This causes that for every element under RenderNamedFlowThread the method RenderFlowThread::repaintRectangleInRegions() is called. Taking a look to this method it has a loop over all the regions forcing a repaint. This means that if you have 1000 regions, even if you're just selecting in one of them, a repaint in the rest of regions is executed:
void RenderFlowThread::repaintRectangleInRegions(const LayoutRect& repaintRect, bool immediate) const
{
    if (!shouldRepaint(repaintRect) || !hasValidRegionInfo())
        return;

    LayoutStateDisabler layoutStateDisabler(&view()); // We can't use layout state to repaint, since the regions are somewhere else.

    // We can't use currentFlowThread as it is possible to have interleaved flow threads and the wrong one could be used.
    // Let each region figure out the proper enclosing flow thread.
    CurrentRenderFlowThreadDisabler disabler(&view());

    for (auto iter = m_regionList.begin(), end = m_regionList.end(); iter != end; ++iter) {
        RenderRegion* region = *iter;

        region->repaintFlowThreadContent(repaintRect, immediate);
    }
}

This could be avoided but just issuing a repaint for the regions affected by the repaintRect.

[Manuel Rego Casasnovas]

Created attachment 220340 [details]
Patch

In my machine Layout/RegionsSelection.html goes down from 1040ms to 900ms.

[Build Bot]

Comment on attachment 220340 [details]
Patch

Attachment 220340 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5429366674685952

New failing tests:
fast/regions/repaint/repaint-regions-overflow.html

[Build Bot]

Created attachment 220349 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 220340 [details]
Patch

Attachment 220340 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4578599420035072

New failing tests:
fast/regions/repaint/repaint-regions-overflow.html

[Build Bot]

Created attachment 220351 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 220340 [details]
Patch

Attachment 220340 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5164571471904768

New failing tests:
fast/regions/repaint/repaint-regions-overflow.html

[Build Bot]

Created attachment 220353 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Manuel Rego Casasnovas]

Created attachment 220554 [details]
Patch

[EFL EWS Bot]

Comment on attachment 220554 [details]
Patch

Attachment 220554 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/6258986852548608

[Manuel Rego Casasnovas]

Created attachment 220556 [details]
Patch

[Manuel Rego Casasnovas]

Created attachment 221299 [details]
Patch

Add improvement percentages for new perftests.

[Brent Fulgham]

This patch looks good to me, but I think hyatt or dino should give it a review before we land it.

[Andrei Bucur]

Comment on attachment 221299 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=221299&action=review

> Source/WebCore/rendering/RenderFlowThread.cpp:441
> +        // If no regions intersect the repaint rect, it is in the flow thread overflow.

Unfortunately things are a bit more complex now that content can overflow any region. Imagine the case of two regions and the first region has a really tall video inside it, overflowing the first region. The controls will be at the bottom of the video. If you want to repaint them, the rectangle will "geometrically" fit inside the second region. However, because the video is unsplittable it is rendered inside the first region. So basically you will ask the second region to do the repaint when actually the first region should do it.

Such a change should also take into account the originating box or the region range of the box. This is also really tricky to do because it means we should trigger a repaint when the range of a box changes. I don't think there's an easy way to implement this optimization :(.

[Brent Fulgham]

CSS Regions were removed in Bug 174978.