Bug 147137 - [CoordinatedGraphics] Remove unnecessary two virtual methods from TiledBackingStoreClient
Summary: [CoordinatedGraphics] Remove unnecessary two virtual methods from TiledBackin...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-20 19:30 PDT by Ryuan Choi
Modified: 2015-08-30 16:30 PDT (History)
8 users (show)

See Also:


Attachments
Patch (13.56 KB, patch)
2015-07-20 19:39 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (13.82 KB, patch)
2015-08-26 20:38 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2015-07-20 19:30:22 PDT
tiledBackingStoreContentsRect() and tiledBackingStoreVisibleRect() are ambiguous methods to get contentsRect and visibleRect from CoordinatedGraphicsLayer.
CoordinatedGraphicsLayer can pass them directly if needed.
Comment 1 Ryuan Choi 2015-07-20 19:39:16 PDT
Created attachment 257156 [details]
Patch
Comment 2 Gyuyoung Kim 2015-08-17 01:05:02 PDT
Comment on attachment 257156 [details]
Patch

Any reason you didn't submit to EWS ?
Comment 3 Ryuan Choi 2015-08-26 06:53:38 PDT
(In reply to comment #2)
> Comment on attachment 257156 [details]
> Patch
> 
> Any reason you didn't submit to EWS ?

Sorry for the late answer.

I didn't know it.

I submit to EWS
Comment 4 Gyuyoung Kim 2015-08-26 18:07:59 PDT
Comment on attachment 257156 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:-132
> -    IntRect dirtyRect = mapFromContents(contentsRect);

When we don't adjust mapFromContents(), is there any problem ?

mapFromContents() applies a content scale factor.

IntRect TiledBackingStore::mapFromContents(const IntRect& rect) const
{
    return enclosingIntRect(FloatRect(rect.x() * m_contentsScale,
        rect.y() * m_contentsScale,
        rect.width() * m_contentsScale,
        rect.height() * m_contentsScale));
}
Comment 5 Ryuan Choi 2015-08-26 18:32:43 PDT
Comment on attachment 257156 [details]
Patch

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

>> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:-132
>> -    IntRect dirtyRect = mapFromContents(contentsRect);
> 
> When we don't adjust mapFromContents(), is there any problem ?
> 
> mapFromContents() applies a content scale factor.
> 
> IntRect TiledBackingStore::mapFromContents(const IntRect& rect) const
> {
>     return enclosingIntRect(FloatRect(rect.x() * m_contentsScale,
>         rect.y() * m_contentsScale,
>         rect.width() * m_contentsScale,
>         rect.height() * m_contentsScale));
> }

With this patch, coverageRatio will get dirtyRect which is already scaled, while previous coverageRatio get contentsRect.

> Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:151
> +void TiledBackingStore::createTiles(const IntRect& visibleRect, const IntRect& contentsRect)

I will rename these as scaledXXX not to confuse.
Comment 6 Ryuan Choi 2015-08-26 20:38:25 PDT
Created attachment 260026 [details]
Patch
Comment 7 Gyuyoung Kim 2015-08-27 03:45:23 PDT
Comment on attachment 260026 [details]
Patch

I can't find behavior change here. So rs=me that there is no behavior change on the assumption. However someone(yoon?) might want to take a final look before landing.
Comment 8 WebKit Commit Bot 2015-08-30 16:30:11 PDT
Comment on attachment 260026 [details]
Patch

Clearing flags on attachment: 260026

Committed r189161: <http://trac.webkit.org/changeset/189161>
Comment 9 WebKit Commit Bot 2015-08-30 16:30:22 PDT
All reviewed patches have been landed.  Closing bug.