Bug 107910 - Coordinated Graphics: Refactor code related to debug border and repaint count.
Summary: Coordinated Graphics: Refactor code related to debug border and repaint count.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
Depends on: 105787
Blocks: 107198
  Show dependency treegraph
 
Reported: 2013-01-24 21:30 PST by Dongseong Hwang
Modified: 2013-01-28 23:46 PST (History)
5 users (show)

See Also:


Attachments
Patch (21.59 KB, patch)
2013-01-24 21:39 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (21.59 KB, patch)
2013-01-28 14:07 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (24.75 KB, patch)
2013-01-28 16:21 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (24.96 KB, patch)
2013-01-28 16:22 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (24.74 KB, patch)
2013-01-28 16:44 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch for landing (24.54 KB, patch)
2013-01-28 22:33 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch for landing (24.79 KB, patch)
2013-01-28 22:36 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch for landing (24.54 KB, patch)
2013-01-28 22:40 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2013-01-24 21:30:12 PST
Match drawing debug visuals of Coordinated Graphics to Bug 105787.
Comment 1 Dongseong Hwang 2013-01-24 21:39:09 PST
Created attachment 184657 [details]
Patch
Comment 2 Dongseong Hwang 2013-01-24 21:40:56 PST
I did not change WK2 part.
See Bug 105787#c54 and Bug 105787#c55
Comment 3 EFL EWS Bot 2013-01-25 01:58:44 PST
Comment on attachment 184657 [details]
Patch

Attachment 184657 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16111466
Comment 4 Early Warning System Bot 2013-01-25 03:35:52 PST
Comment on attachment 184657 [details]
Patch

Attachment 184657 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16110642
Comment 5 Dongseong Hwang 2013-01-28 14:07:19 PST
Created attachment 185059 [details]
Patch
Comment 6 Dongseong Hwang 2013-01-28 14:13:49 PST
This patch was already reviewed by noam in Bug 105787.
I separated WK2 part in this bug.
Comment 7 Benjamin Poulain 2013-01-28 14:30:56 PST
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.
Comment 8 Dongseong Hwang 2013-01-28 15:06:21 PST
(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?
Comment 9 Dongseong Hwang 2013-01-28 16:21:26 PST
Created attachment 185093 [details]
Patch
Comment 10 Dongseong Hwang 2013-01-28 16:22:24 PST
Created attachment 185094 [details]
Patch
Comment 11 Dongseong Hwang 2013-01-28 16:44:22 PST
Created attachment 185104 [details]
Patch
Comment 12 Dongseong Hwang 2013-01-28 16:55:48 PST
noam, benjaminp agreed on this patch.
could you review again? :)
Comment 13 Noam Rosenthal 2013-01-28 22:03:42 PST
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
Comment 14 Dongseong Hwang 2013-01-28 22:33:00 PST
Created attachment 185157 [details]
Patch for landing
Comment 15 Dongseong Hwang 2013-01-28 22:36:34 PST
Created attachment 185160 [details]
Patch for landing
Comment 16 Dongseong Hwang 2013-01-28 22:40:40 PST
Created attachment 185161 [details]
Patch for landing
Comment 17 WebKit Review Bot 2013-01-28 23:46:29 PST
Comment on attachment 185161 [details]
Patch for landing

Clearing flags on attachment: 185161

Committed r141065: <http://trac.webkit.org/changeset/141065>
Comment 18 WebKit Review Bot 2013-01-28 23:46:34 PST
All reviewed patches have been landed.  Closing bug.