Bug 120123

Summary: [BlackBerry] Incorrect origin of indexOfTile in LayerTiler can cause unnecessary texture jobs and waste memory
Product: WebKit Reporter: Jacky Jiang <jkjiang>
Component: WebKit BlackBerryAssignee: Jacky Jiang <jkjiang>
Status: RESOLVED FIXED    
Severity: Normal CC: anilsson, rwlbuis, staikos, tonikitoo, yong.li.webkit
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch yong.li.webkit: review+

Description Jacky Jiang 2013-08-21 10:58:21 PDT
Jira: 481356.
Comment 1 Jacky Jiang 2013-08-21 11:09:56 PDT
Created attachment 209287 [details]
Patch
Comment 2 Yong Li 2013-08-21 11:58:24 PDT
Comment on attachment 209287 [details]
Patch

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

r+ based on Arvid's review. but please update the comments before committing. BTW, "maxXMaxYCorner" is a little bit confusing, as it gives me a feeling that the returned value is included in the rect.

> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:257
> +        last = indexOfTile(ceiledIntPoint(visibleRect.maxXMaxYCorner()) + IntPoint(-1, -1)); // The origin should be the top left of the bottom right pixel.

shouldn't "top left" be "upper left"?

> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:415
> +        TileIndex last = indexOfTile(job.m_dirtyRect.maxXMaxYCorner() + IntPoint(-1, -1)); // The origin should be the top left of the bottom right pixel.

shouldn't "top left" be "upper left"?
Comment 3 Jacky Jiang 2013-08-21 12:17:17 PDT
Comment on attachment 209287 [details]
Patch

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

>> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:257
>> +        last = indexOfTile(ceiledIntPoint(visibleRect.maxXMaxYCorner()) + IntPoint(-1, -1)); // The origin should be the top left of the bottom right pixel.
> 
> shouldn't "top left" be "upper left"?

As I know, we usually use "top left" instead of "upper left" both in graphics and css, right?  But I can use "upper left" if you prefer this.

>> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:415
>> +        TileIndex last = indexOfTile(job.m_dirtyRect.maxXMaxYCorner() + IntPoint(-1, -1)); // The origin should be the top left of the bottom right pixel.
> 
> shouldn't "top left" be "upper left"?

ditto.
Comment 4 Yong Li 2013-08-21 13:44:48 PDT
Comment on attachment 209287 [details]
Patch

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

Overall comments

>>> Source/WebCore/platform/graphics/blackberry/LayerTiler.cpp:415
>>> +        TileIndex last = indexOfTile(job.m_dirtyRect.maxXMaxYCorner() + IntPoint(-1, -1)); // The origin should be the top left of the bottom right pixel.
>> 
>> shouldn't "top left" be "upper left"?
> 
> ditto.

OK
Comment 5 Jacky Jiang 2013-08-21 14:39:32 PDT
Committed r154418: <http://trac.webkit.org/changeset/154418>