RelevantRepaintedObjects heuristic should ensure there is some coverage in the bottom half of the relevant view rect

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

The RelevantRepaintedObjects heuristic should ensure there is some coverage in the bottom half of the relevant view rect. Making sure we have coverage in both halfs helps to prevent cases where we have a fully loaded menu bar or masthead with no content beneath that.

<rdar://problem/12257164>

[Beth Dakin]

Created attachment 190828 [details]
Patch

[Tim Horton]

Comment on attachment 190828 [details]
Patch

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

> Source/WebCore/page/Page.cpp:1338
> +    // both halfs helps to prevent cases where we have a fully loaded menu bar or masthead with

halves.

[Simon Fraser (smfr)]

Comment on attachment 190828 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        We need two Regions now -- one for the top and another for the bottom. Make sure 
> +        we have at least half of our desired coverage in both.

This seems a bit western-centric. What about languages that might load a strip down one side first?

> Source/WebCore/page/Page.cpp:1348
> +    if (topRelevantRect.intersects(snappedPaintRect))
> +        m_topRelevantPaintedRegion.unite(snappedPaintRect);
> +    else
> +        m_bottomRelevantPaintedRegion.unite(snappedPaintRect);

Do you want to ignore coverage over the bottom rect if the paint rect happens to just touch the top rect?

[Beth Dakin]

(In reply to comment #3)
> (From update of attachment 190828 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190828&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        We need two Regions now -- one for the top and another for the bottom. Make sure 
> > +        we have at least half of our desired coverage in both.
> 
> This seems a bit western-centric. What about languages that might load a strip down one side first?
> 

Hmm, good point. We should drum up some foreign language test cases and see if they would be improved by looking at left/right.

> > Source/WebCore/page/Page.cpp:1348
> > +    if (topRelevantRect.intersects(snappedPaintRect))
> > +        m_topRelevantPaintedRegion.unite(snappedPaintRect);
> > +    else
> > +        m_bottomRelevantPaintedRegion.unite(snappedPaintRect);
> 
> Do you want to ignore coverage over the bottom rect if the paint rect happens to just touch the top rect?

Hmm, good point. I changed this code to be more precise: 

    // If the rect straddles both Regions, split it appropriately.
    if (topRelevantRect.intersects(snappedPaintRect) && bottomRelevantRect.intersects(snappedPaintRect)) {
        IntRect topIntersection = snappedPaintRect;
        topIntersection.intersect(pixelSnappedIntRect(topRelevantRect));
        m_topRelevantPaintedRegion.unite(topIntersection);

        IntRect bottomIntersection = snappedPaintRect;
        bottomIntersection.intersect(pixelSnappedIntRect(bottomRelevantRect));
        m_bottomRelevantPaintedRegion.unite(bottomIntersection);
    } else if (topRelevantRect.intersects(snappedPaintRect))
        m_topRelevantPaintedRegion.unite(snappedPaintRect);
    else
        m_bottomRelevantPaintedRegion.unite(snappedPaintRect);

[Beth Dakin]

http://trac.webkit.org/changeset/144395