Bug 117227 - [WK1][GTK] Add support to use Threaded Coordinated Graphics.
Summary: [WK1][GTK] Add support to use Threaded Coordinated Graphics.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 109661 118383
Blocks: 100341 118265
  Show dependency treegraph
 
Reported: 2013-06-04 23:44 PDT by Gwang Yoon Hwang
Modified: 2014-04-01 07:11 PDT (History)
11 users (show)

See Also:


Attachments
Patch (40.82 KB, patch)
2013-06-05 02:53 PDT, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (86.12 KB, patch)
2013-07-01 15:05 PDT, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (38.89 KB, patch)
2013-07-04 05:04 PDT, Gwang Yoon Hwang
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from APPLE-EWS-7 for win-future (851.08 KB, application/zip)
2013-07-05 14:56 PDT, Build Bot
no flags Details
Addendum to fix GtkLauncher linkage (1.67 KB, patch)
2013-07-11 03:06 PDT, Xabier Rodríguez Calvar
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-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.