Bug 159138

Summary: [GTK][EFL] Build with threaded compositor enabled is broken
Product: WebKit Reporter: Miguel Gomez <magomez>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, magomez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Miguel Gomez 2016-06-27 02:54:51 PDT
The changes in r202421 and r202439 broke the build.
Comment 1 Miguel Gomez 2016-06-27 03:14:56 PDT
Created attachment 282115 [details]
Patch
Comment 2 WebKit Commit Bot 2016-06-27 03:16:25 PDT
Attachment 282115 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:49:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:50:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:43:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Carlos Garcia Campos 2016-06-27 03:16:56 PDT
Comment on attachment 282115 [details]
Patch

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

> Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:-124
> -    cairoSurfaceSetDeviceScale(m_compositorSurface.get(), m_resolutionScale, m_resolutionScale);

Are you sure we don't need this here? Shouldn't we pass the resolution scale to the data structure instead?
Comment 4 Miguel Gomez 2016-06-27 03:29:34 PDT
(In reply to comment #3)
> Comment on attachment 282115 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282115&action=review
> 
> > Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:-124
> > -    cairoSurfaceSetDeviceScale(m_compositorSurface.get(), m_resolutionScale, m_resolutionScale);
> 
> Are you sure we don't need this here? Shouldn't we pass the resolution scale
> to the data structure instead?

Sure, we don't need it. The compositor buffer is only used to copy the canvas content and take it to the composition stage when the accelerated canvas is enabled. And according to the comments in r202421, the HiDPI canvas feature was removed, so when using a canvas the resolution factor is always 1.

But we can store the resolutionScale in the data structure as well, yes. If you prefer that approach I can update the patch.
Comment 5 Carlos Garcia Campos 2016-06-27 03:35:32 PDT
Comment on attachment 282115 [details]
Patch

Ok, then
Comment 6 WebKit Commit Bot 2016-06-27 03:57:27 PDT
Comment on attachment 282115 [details]
Patch

Clearing flags on attachment: 282115

Committed r202484: <http://trac.webkit.org/changeset/202484>
Comment 7 WebKit Commit Bot 2016-06-27 03:57:30 PDT
All reviewed patches have been landed.  Closing bug.