Bug 195094 - [CoordinatedGraphics] Unify all LayerTreeHost classes
Summary: [CoordinatedGraphics] Unify all LayerTreeHost classes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2019-02-27 03:22 PST by Carlos Garcia Campos
Modified: 2019-02-28 08:46 PST (History)
5 users (show)

See Also:


Attachments
Patch (54.04 KB, patch)
2019-02-27 03:24 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (59.90 KB, patch)
2019-02-27 04:12 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (53.83 KB, patch)
2019-02-27 04:49 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (74.53 KB, patch)
2019-02-27 19:05 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch for landing (62.57 KB, patch)
2019-02-28 03:50 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch for landing (61.13 KB, patch)
2019-02-28 04:26 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch for landing (61.92 KB, patch)
2019-02-28 04:36 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2019-02-27 03:22:19 PST
There's no reason to have 3 classes, since currently LayerTreeHost is only used by coordinated graphics based ports.
Comment 1 Carlos Garcia Campos 2019-02-27 03:24:04 PST
Created attachment 363083 [details]
Patch
Comment 2 Carlos Garcia Campos 2019-02-27 04:12:43 PST
Created attachment 363085 [details]
Patch
Comment 3 EWS Watchlist 2019-02-27 04:15:07 PST
Attachment 363085 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp:122:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp:299:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp:311:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:142:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:151:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:161:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:167:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:191:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:305:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebPage/WebPage.cpp:6377:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:150:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:155:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:345:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:559:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 14 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Carlos Garcia Campos 2019-02-27 04:45:34 PST
Comment on attachment 363085 [details]
Patch

Oops, wrong patch
Comment 5 Carlos Garcia Campos 2019-02-27 04:49:08 PST
Created attachment 363086 [details]
Patch
Comment 6 Carlos Garcia Campos 2019-02-27 06:01:51 PST
I don't understand the windows build failure
Comment 7 Don Olmstead 2019-02-27 11:01:51 PST
(In reply to Carlos Garcia Campos from comment #6)
> I don't understand the windows build failure

I'll take a look at it now. Just a heads up we're working on enabling coordinated graphics in https://bugs.webkit.org/show_bug.cgi?id=186364 and there's also the matter of https://bugs.webkit.org/show_bug.cgi?id=186444
Comment 8 Don Olmstead 2019-02-27 11:58:36 PST
LayerTreeHost has a #if USE(COORDINATED_GRAPHICS_THREADED) around it. That's not enabled on WinCairo currently. As mentioned we're working through that on a different bug.
Comment 9 Don Olmstead 2019-02-27 17:07:41 PST
Carlos and Zan one thing about this patch I have a feeling about is that GTK is going to break without OpenGL support.

https://github.com/WebKit/webkit/blob/master/Source/cmake/OptionsGTK.cmake#L293-L327

So you might also want to check that out before it lands.
Comment 10 Fujii Hironori 2019-02-27 17:29:51 PST
Comment on attachment 363086 [details]
Patch

Thank you for giving us heads-up about WinCairo issue. I'm going to look into it today.
Comment 11 Fujii Hironori 2019-02-27 19:05:41 PST
Created attachment 363180 [details]
Patch
Comment 12 Fujii Hironori 2019-02-27 19:11:02 PST
Comment on attachment 363180 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        [CoordinatedGraphics] Unify all LayerTreeHost classes

Why don't you unify AcceleratedDrawingArea and DrawingAreaImpl?
DrawingAreaImpl should be renamed to DrawingAreaTextureMapper, then DrawingAreaCoordinatedGraphics after WinCairo will switch to CoordinatedGraphics.
Comment 13 Carlos Garcia Campos 2019-02-28 02:02:01 PST
(In reply to Fujii Hironori from comment #11)
> Created attachment 363180 [details]
> Patch

Thanks!
Comment 14 Carlos Garcia Campos 2019-02-28 03:50:44 PST
Created attachment 363216 [details]
Patch for landing

This should also fix the GTK build with OpenGL disabled.
Comment 15 Carlos Garcia Campos 2019-02-28 04:26:22 PST
Created attachment 363217 [details]
Patch for landing
Comment 16 Carlos Garcia Campos 2019-02-28 04:36:51 PST
Created attachment 363218 [details]
Patch for landing
Comment 17 Carlos Garcia Campos 2019-02-28 05:17:52 PST
Committed r242199: <https://trac.webkit.org/changeset/242199>
Comment 18 Carlos Garcia Campos 2019-02-28 08:46:45 PST
(In reply to Fujii Hironori from comment #12)
> Comment on attachment 363180 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363180&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        [CoordinatedGraphics] Unify all LayerTreeHost classes
> 
> Why don't you unify AcceleratedDrawingArea and DrawingAreaImpl?
> DrawingAreaImpl should be renamed to DrawingAreaTextureMapper, then
> DrawingAreaCoordinatedGraphics after WinCairo will switch to
> CoordinatedGraphics.

Sure, see https://bugs.webkit.org/show_bug.cgi?id=195167 But I used DrawingAreaCoordinatedGraphics directly.