Bug 147137 – [CoordinatedGraphics] Remove unnecessary two virtual methods from TiledBackingStoreClient

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

tiledBackingStoreContentsRect() and tiledBackingStoreVisibleRect() are ambiguous methods to get contentsRect and visibleRect from CoordinatedGraphicsLayer.
CoordinatedGraphicsLayer can pass them directly if needed.

[Ryuan Choi]

Created attachment 257156 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 257156 [details]
Patch

Any reason you didn't submit to EWS ?

[Ryuan Choi]

(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

[Gyuyoung Kim]

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));
}

[Ryuan Choi]

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.

[Ryuan Choi]

Created attachment 260026 [details]
Patch

[Gyuyoung Kim]

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.

[WebKit Commit Bot]

Comment on attachment 260026 [details]
Patch

Clearing flags on attachment: 260026

Committed r189161: <http://trac.webkit.org/changeset/189161>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.