Bug 195094 – [CoordinatedGraphics] Unify all LayerTreeHost classes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

There's no reason to have 3 classes, since currently LayerTreeHost is only used by coordinated graphics based ports.

[Carlos Garcia Campos]

Created attachment 363083 [details]
Patch

[Carlos Garcia Campos]

Created attachment 363085 [details]
Patch

[EWS Watchlist]

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.

[Carlos Garcia Campos]

Comment on attachment 363085 [details]
Patch

Oops, wrong patch

[Carlos Garcia Campos]

Created attachment 363086 [details]
Patch

[Carlos Garcia Campos]

I don't understand the windows build failure

[Don Olmstead]

(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

[Don Olmstead]

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.

[Don Olmstead]

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.

[Fujii Hironori]

Comment on attachment 363086 [details]
Patch

Thank you for giving us heads-up about WinCairo issue. I'm going to look into it today.

[Fujii Hironori]

Created attachment 363180 [details]
Patch

[Fujii Hironori]

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.

[Carlos Garcia Campos]

(In reply to Fujii Hironori from comment #11)
> Created attachment 363180 [details]
> Patch

Thanks!

[Carlos Garcia Campos]

Created attachment 363216 [details]
Patch for landing

This should also fix the GTK build with OpenGL disabled.

[Carlos Garcia Campos]

Created attachment 363217 [details]
Patch for landing

[Carlos Garcia Campos]

Created attachment 363218 [details]
Patch for landing

[Carlos Garcia Campos]

Committed r242199: <https://trac.webkit.org/changeset/242199>

[Carlos Garcia Campos]

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