Bug 107910 – Coordinated Graphics: Refactor code related to debug border and repaint count.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dongseong Hwang]

Match drawing debug visuals of Coordinated Graphics to Bug 105787.

[Dongseong Hwang]

Created attachment 184657 [details]
Patch

[Dongseong Hwang]

I did not change WK2 part.
See Bug 105787#c54 and Bug 105787#c55

[EFL EWS Bot]

Comment on attachment 184657 [details]
Patch

Attachment 184657 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16111466

[Early Warning System Bot]

Comment on attachment 184657 [details]
Patch

Attachment 184657 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16110642

[Dongseong Hwang]

Created attachment 185059 [details]
Patch

[Dongseong Hwang]

This patch was already reviewed by noam in Bug 105787.
I separated WK2 part in this bug.

[Benjamin Poulain]

Comment on attachment 185059 [details]
Patch

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

I have nothing against this for WebKit2. Please have No'am review it.

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.cpp:165
> +    HashMap<uint32_t, CoordinatedBackingStoreTile>::iterator end = m_tiles.end();
> +    for (HashMap<uint32_t, CoordinatedBackingStoreTile>::iterator it = m_tiles.begin(); it != end; ++it)

Looks like your hashmap is desperate to get a typedef :)

> Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.h:74
> +    WebCore::FloatRect rect() const { return WebCore::FloatRect(WebCore::FloatPoint::zero(), m_size); }

This name seems too generic. I have no idea what is a rect for a backing store.

[Dongseong Hwang]

(In reply to comment #7)
> (From update of attachment 185059 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185059&action=review
> 
> I have nothing against this for WebKit2. Please have No'am review it.

Thank you for review! I ask noam :)

> Looks like your hashmap is desperate to get a typedef :)

I'll use typedef!

> > Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.h:74
> > +    WebCore::FloatRect rect() const { return WebCore::FloatRect(WebCore::FloatPoint::zero(), m_size); }
> 
> This name seems too generic. I have no idea what is a rect for a backing store.

This method matches to TextureMapperTiledBackingStore::rect(). rect is contents rect of a backing store.
noam, is it better to change name? something like contentsRect?

[Dongseong Hwang]

Created attachment 185093 [details]
Patch

[Dongseong Hwang]

Created attachment 185094 [details]
Patch

[Dongseong Hwang]

Created attachment 185104 [details]
Patch

[Dongseong Hwang]

noam, benjaminp agreed on this patch.
could you review again? :)

[Noam Rosenthal]

Comment on attachment 185104 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:380
> +

Line not necessary

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:390
> +

Ditto

[Dongseong Hwang]

Created attachment 185157 [details]
Patch for landing

[Dongseong Hwang]

Created attachment 185160 [details]
Patch for landing

[Dongseong Hwang]

Created attachment 185161 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 185161 [details]
Patch for landing

Clearing flags on attachment: 185161

Committed r141065: <http://trac.webkit.org/changeset/141065>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.