Bug 118265 - [GTK][ThreadedCompositor] Add support for threaded compositor
Summary: [GTK][ThreadedCompositor] Add support for threaded compositor
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: Gwang Yoon Hwang
URL:
Keywords:
Depends on: 117227 118383
Blocks: 100341
  Show dependency treegraph
 
Reported: 2013-07-01 15:33 PDT by Gwang Yoon Hwang
Modified: 2015-01-07 20:29 PST (History)
10 users (show)

See Also:


Attachments
Patch (44.51 KB, patch)
2013-07-01 16:10 PDT, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (44.41 KB, patch)
2013-07-04 05:05 PDT, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (55.63 KB, patch)
2014-12-15 04:17 PST, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (55.87 KB, patch)
2015-01-06 06:58 PST, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (55.94 KB, patch)
2015-01-06 09:05 PST, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (55.90 KB, patch)
2015-01-06 09:14 PST, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (55.86 KB, patch)
2015-01-06 20:07 PST, Gwang Yoon 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 Gwang Yoon Hwang 2013-07-01 15:33:10 PDT
By implementing ThreadedCoordinatedLayerTreeHost, this patch supports
    threaded compositor for WK2. In this initial implementation, threaded
    compositor only supports fixedLayout.
    
    COORDINATED_GRAPHICS_IPC is introduced to classify IPC specific codes
    from Coordinated Graphics.
    
    Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp :
        Calls XInitThreads to use thread-safe xlib.
    
    No new tests, this is an experimental feature.
Comment 1 Gwang Yoon Hwang 2013-07-01 16:10:03 PDT
Created attachment 205842 [details]
Patch
Comment 2 Gwang Yoon Hwang 2013-07-04 05:05:50 PDT
Created attachment 206079 [details]
Patch
Comment 3 Gwang Yoon Hwang 2014-12-15 04:17:34 PST
Created attachment 243287 [details]
Patch
Comment 4 Gwang Yoon Hwang 2015-01-06 06:58:13 PST
Created attachment 244059 [details]
Patch
Comment 5 Gwang Yoon Hwang 2015-01-06 09:05:08 PST
Created attachment 244065 [details]
Patch
Comment 6 Gwang Yoon Hwang 2015-01-06 09:14:50 PST
Created attachment 244066 [details]
Patch
Comment 7 Gwang Yoon Hwang 2015-01-06 09:17:02 PST
(In reply to comment #6)
> Created attachment 244066 [details]
> Patch

Sorry for noise.
I made a little typo in previous patch.

Changes:

* Change COORDINATED_GRAPHICS_IPC to COORDINATED_GRAPHICS_MULITIPROCESS
* Rebase for current HEAD
Comment 8 Martin Robinson 2015-01-06 11:39:18 PST
Comment on attachment 244066 [details]
Patch

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

Looks good to me, thought the changes to platform independent files will need to be approved by Apple.

> Source/WebKit2/ChangeLog:8
> +        This patch introduces the Threaded Compositor for WebKitGTK+.

Nit: the Threaded Compositor -> a threaded compositor

> Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:527
> +#if USE(COORDINATED_GRAPHICS_MULTIPROCESS)
>      m_page->send(Messages::WebPageProxy::DidChangeContentSize(size));
>  #endif
> +#endif

These blocks do not have to be nested. I think it's clearer to not nest them actually.

> Source/cmake/WebKitFeatures.cmake:152
> +    WEBKIT_OPTION_DEFINE(ENABLE_THREADED_COMPOSITOR "Toggle Toggle Threaded Compositor support" OFF)

Nit: Threaded Compositor -> threaded compositor

> Tools/Scripts/webkitperl/FeatureList.pm:386
> +    { option => "threaded-compositor", desc => "Toggle Threaded Compositor support",

Ditto.
Comment 9 Gwang Yoon Hwang 2015-01-06 20:07:00 PST
Created attachment 244129 [details]
Patch
Comment 10 Gwang Yoon Hwang 2015-01-06 20:14:23 PST
(In reply to comment #8)
> Comment on attachment 244066 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244066&action=review
> 
> Looks good to me, thought the changes to platform independent files will
> need to be approved by Apple.
> 
Okay, I'll ask for approval later.

> > Source/WebKit2/ChangeLog:8
> > +        This patch introduces the Threaded Compositor for WebKitGTK+.
> 
> Nit: the Threaded Compositor -> a threaded compositor
> 
> > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:527
> > +#if USE(COORDINATED_GRAPHICS_MULTIPROCESS)
> >      m_page->send(Messages::WebPageProxy::DidChangeContentSize(size));
> >  #endif
> > +#endif
> 
> These blocks do not have to be nested. I think it's clearer to not nest them
> actually.
> 

Good point. Modified.

> > Source/cmake/WebKitFeatures.cmake:152
> > +    WEBKIT_OPTION_DEFINE(ENABLE_THREADED_COMPOSITOR "Toggle Toggle Threaded Compositor support" OFF)
> 
> Nit: Threaded Compositor -> threaded compositor
> 

Fixed. :)

> > Tools/Scripts/webkitperl/FeatureList.pm:386
> > +    { option => "threaded-compositor", desc => "Toggle Threaded Compositor support",
> 
> Ditto.

Fixed.
Comment 11 Anders Carlsson 2015-01-07 12:39:16 PST
Comment on attachment 244066 [details]
Patch

Cross platform files look fine to me.
Comment 12 WebKit Commit Bot 2015-01-07 20:29:25 PST
Comment on attachment 244129 [details]
Patch

Clearing flags on attachment: 244129

Committed r178095: <http://trac.webkit.org/changeset/178095>
Comment 13 WebKit Commit Bot 2015-01-07 20:29:31 PST
All reviewed patches have been landed.  Closing bug.