Bug 147137

Summary: [CoordinatedGraphics] Remove unnecessary two virtual methods from TiledBackingStoreClient
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, gyuyoung.kim, kondapallykalyan, lucas.de.marchi, luiz, noam, yoon
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

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.