TiledBackingStore: Don't intersect invalidated rects with the keep rect

• Component: New Bugs
• Status: RESOLVED FIXED

[Jocelyn Turcotte]

TiledBackingStore: Don't intersect invalidated rects with the keep rect

[Jocelyn Turcotte]

Created attachment 139996 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 139996 [details]
Patch

Add a comment?

[zalan]

just stumbled upon this and here is my educated review: 
user scroll -> user scrolls in the changelog. looks good otherwise. :)

[Jocelyn Turcotte]

(In reply to comment #2)
> (From update of attachment 139996 [details])
> Add a comment?

Will do.

It can be reproduced with the current multiplier on qt.nokia.com:
- Click on "Qt Creator 2.4.1"
This will change page and move the viewport down to the anchor just enough so that the first row of tiles intersects the keep rect.
- Press back
This will invalidate all tiles and bring the viewport back to the top.

[Jocelyn Turcotte]

Created attachment 141410 [details]
Patch

This patch would cause costly iterations if a layer is scaled down considerably.
Updated to iterate tiles on the intesected rect like before, but mark them dirty using the full dirtyRect.

[Kenneth Rohde Christiansen]

Comment on attachment 141410 [details]
Patch

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

> Source/WebCore/platform/graphics/TiledBackingStore.cpp:92
> -    IntRect dirtyRect(intersection(mapFromContents(contentsDirtyRect), m_keepRect));
> +    IntRect dirtyRect(mapFromContents(contentsDirtyRect));
>  
> -    Tile::Coordinate topLeft = tileCoordinateForPoint(dirtyRect.location());
> -    Tile::Coordinate bottomRight = tileCoordinateForPoint(innerBottomRight(dirtyRect));
> +    // Only iterate on the part of the rect that we know we might have tiles.
> +    IntRect coveredDirtyRect = intersection(dirtyRect, m_keepRect);
> +    Tile::Coordinate topLeft = tileCoordinateForPoint(coveredDirtyRect.location());
> +    Tile::Coordinate bottomRight = tileCoordinateForPoint(innerBottomRight(coveredDirtyRect));

I dont see the difference here:

coveredDirtyRect seems to be the exact same as the old dirtyRect.

[Jocelyn Turcotte]

(In reply to comment #6)
> I dont see the difference here:
> 
> coveredDirtyRect seems to be the exact same as the old dirtyRect.

coveredDirtyRect is intersected with m_keepRect and dirtyRect isn't.
It is exactly as the old dirtyRect, it's dirtyRect that changed.

[Kenneth Rohde Christiansen]

Comment on attachment 141410 [details]
Patch

Ah got it now... I wonder if that could be made more clear, but I guess not.

[WebKit Review Bot]

Comment on attachment 141410 [details]
Patch

Clearing flags on attachment: 141410

Committed r116938: <http://trac.webkit.org/changeset/116938>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.