Bug 117227

Summary: [WK1][GTK] Add support to use Threaded Coordinated Graphics.
Product: WebKit Reporter: Gwang Yoon Hwang <yoon>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: calvaris, cmarcelo, commit-queue, gtk-ews, jaepark, luiz, mrobinson, noam, xan.lopez, zan, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 109661, 118383    
Bug Blocks: 100341, 118265    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from APPLE-EWS-7 for win-future
none
Addendum to fix GtkLauncher linkage none

Description Gwang Yoon Hwang 2013-06-04 23:44:06 PDT
This patch adds supports for the tiled backing store and the threaded
    coordinated graphics, because coordianted graphics is
    tightly coupled with tiled backing store.
    
    By implementing ThreadedCoordinatedCompositor, accelerated compositing
    using Coordinated Graphics in WebKit1 Gtk+ is enabled.
    ThreadedCoordinatedCompositor sends commands to CoordinatedCompositingThread
    which owns actual run-loop for compositing.
    
    This patch also adds ThreadedCompositingContext in WebKit/gtk/WebCoreSupport,
    to bind with webkit1 gtk port.
    
    Threaded compositing is disabled as a default. To enable this feature,
    --enable-threaded-compositor option should be passed to build script.
    
    No new test, because it is in experimental stage.
Comment 1 Gwang Yoon Hwang 2013-06-05 02:53:48 PDT
Created attachment 203776 [details]
Patch
Comment 2 Gwang Yoon Hwang 2013-07-01 15:05:19 PDT
Created attachment 205839 [details]
Patch
Comment 3 kov's GTK+ EWS bot 2013-07-01 17:48:48 PDT
Comment on attachment 205839 [details]
Patch

Attachment 205839 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1009555
Comment 4 Gwang Yoon Hwang 2013-07-04 05:04:31 PDT
Created attachment 206078 [details]
Patch
Comment 5 Build Bot 2013-07-05 14:56:38 PDT
Comment on attachment 206078 [details]
Patch

Attachment 206078 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/946529

New failing tests:
fast/forms/select/popup-closes-on-blur.html
Comment 6 Build Bot 2013-07-05 14:56:41 PDT
Created attachment 206170 [details]
Archive of layout-test-results from APPLE-EWS-7 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-7  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 7 Xabier Rodríguez Calvar 2013-07-10 05:38:10 PDT
I cannot build the patch with --enable-threaded-compositor=yes --enable-accelerated-compositing=yes . Am I missing anything?
Comment 8 Gwang Yoon Hwang 2013-07-10 18:23:11 PDT
(In reply to comment #7)
> I cannot build the patch with --enable-threaded-compositor=yes --enable-accelerated-compositing=yes . Am I missing anything?

Well this contaions only WK1 GTK part. You should apply below bug
https://bugs.webkit.org/show_bug.cgi?id=118383
which contains TC implementation.

To help review process, I'd separated this patch to GTk specific and TC implementation.
Comment 9 Xabier Rodríguez Calvar 2013-07-11 03:06:15 PDT
Created attachment 206442 [details]
Addendum to fix GtkLauncher linkage

(In reply to comment #8)
> (In reply to comment #7)
> > I cannot build the patch with --enable-threaded-compositor=yes --enable-accelerated-compositing=yes . Am I missing anything?
> 
> Well this contaions only WK1 GTK part. You should apply below bug
> https://bugs.webkit.org/show_bug.cgi?id=118383
> which contains TC implementation.

Yes, I also saw that according to bug dependencies it also needs patch for bug 118265 to compile properly.

What I also saw was that GtkLauncher was not linking for me so I hacked this small patch that should be included as part of the main patch, probably after some more work depending on how we want to manage dependencies.

More issues are that I am trying video playback with a 1080p video in my thinkpad x220 and:
1) I have  the feeling that performance-wise, it is slower (might be just an impression)
2) If you try the video inside a MediaDocument (just opening the video URL without being included in any HTML enclosure), the controls are scaled up so there is something weird there. I'll try to include a snapshop to clarify this.
3) if I run GtkLauncher without --enable-accelerated-compositing=true, I can't see the video. I can't see anything.
Comment 10 Xabier Rodríguez Calvar 2013-07-11 03:21:34 PDT
Comment on attachment 206078 [details]
Patch

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

I hope you don't mind this unofficial review as I am not a reviewer, but I spotted these issues in this very shallow check.

> Source/WebKit/gtk/ChangeLog:18
> +        * WebCoreSupport/AcceleratedCompositingContext.h:

I think ChangeLogs need a deeper explanation and specially this one.

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextGL.cpp:397
> +#endif // USE(ACCELERATED_COMPOSITING) && USE(TEXTURE_MAPPER_GL) !USE(COORDINATED_GRAPHICS_THREADED)

Nitpick: adding && to the comment as in the other case?

> Tools/GtkLauncher/main.c:502
> +#ifdef WTF_USE_COORDINATED_GRAPHICS_THREADED

USE(COORDINATED_GRAPHICS_THREADED) ?
Comment 11 Martin Robinson 2014-04-01 07:11:17 PDT
We are going to remove WebKit1.
Comment 12 Anders Carlsson 2014-04-01 07:11:37 PDT
Comment on attachment 206078 [details]
Patch

The GTK+ port of WK1 has been removed.