Bug 117227 – [WK1][GTK] Add support to use Threaded Coordinated Graphics.

• Component: Layout and Rendering
• Status: RESOLVED WONTFIX

[Gwang Yoon Hwang]

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.

[Gwang Yoon Hwang]

Created attachment 203776 [details]
Patch

[Gwang Yoon Hwang]

Created attachment 205839 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 205839 [details]
Patch

Attachment 205839 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1009555

[Gwang Yoon Hwang]

Created attachment 206078 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Xabier Rodríguez Calvar]

I cannot build the patch with --enable-threaded-compositor=yes --enable-accelerated-compositing=yes . Am I missing anything?

[Gwang Yoon Hwang]

(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.

[Xabier Rodríguez Calvar]

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.

[Xabier Rodríguez Calvar]

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) ?

[Martin Robinson]

We are going to remove WebKit1.

[Anders Carlsson]

Comment on attachment 206078 [details]
Patch

The GTK+ port of WK1 has been removed.