Bug 85488 - TiledBackingStore: Don't intersect invalidated rects with the keep rect
Summary: TiledBackingStore: Don't intersect invalidated rects with the keep rect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jocelyn Turcotte
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-03 06:05 PDT by Jocelyn Turcotte
Modified: 2012-05-14 06:06 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.87 KB, patch)
2012-05-03 06:08 PDT, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
Patch (2.68 KB, patch)
2012-05-11 07:01 PDT, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jocelyn Turcotte 2012-05-03 06:05:37 PDT
TiledBackingStore: Don't intersect invalidated rects with the keep rect
Comment 1 Jocelyn Turcotte 2012-05-03 06:08:56 PDT
Created attachment 139996 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-05-03 06:19:58 PDT
Comment on attachment 139996 [details]
Patch

Add a comment?
Comment 3 zalan 2012-05-03 06:20:32 PDT
just stumbled upon this and here is my educated review: 
user scroll -> user scrolls in the changelog. looks good otherwise. :)
Comment 4 Jocelyn Turcotte 2012-05-03 06:35:54 PDT
(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.
Comment 5 Jocelyn Turcotte 2012-05-11 07:01:39 PDT
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.
Comment 6 Kenneth Rohde Christiansen 2012-05-11 09:51:19 PDT
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.
Comment 7 Jocelyn Turcotte 2012-05-11 10:14:19 PDT
(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.
Comment 8 Kenneth Rohde Christiansen 2012-05-11 12:33:39 PDT
Comment on attachment 141410 [details]
Patch

Ah got it now... I wonder if that could be made more clear, but I guess not.
Comment 9 WebKit Review Bot 2012-05-14 06:06:18 PDT
Comment on attachment 141410 [details]
Patch

Clearing flags on attachment: 141410

Committed r116938: <http://trac.webkit.org/changeset/116938>
Comment 10 WebKit Review Bot 2012-05-14 06:06:23 PDT
All reviewed patches have been landed.  Closing bug.