Bug 159138 - [GTK][EFL] Build with threaded compositor enabled is broken
Summary: [GTK][EFL] Build with threaded compositor enabled is broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-27 02:54 PDT by Miguel Gomez
Modified: 2016-06-27 03:57 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.61 KB, patch)
2016-06-27 03:14 PDT, Miguel Gomez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.