Bug 120123 - [BlackBerry] Incorrect origin of indexOfTile in LayerTiler can cause unnecessary texture jobs and waste memory
Summary: [BlackBerry] Incorrect origin of indexOfTile in LayerTiler can cause unnecess...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jacky Jiang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-21 10:58 PDT by Jacky Jiang
Modified: 2013-08-21 14:39 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.83 KB, patch)
2013-08-21 11:09 PDT, Jacky Jiang
yong.li.webkit: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>