Bug 195094

Summary: [CoordinatedGraphics] Unify all LayerTreeHost classes
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, don.olmstead, ews-watchlist, Hironori.Fujii, zan
Priority: P2 Keywords: Gtk
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing none

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.