RESOLVED FIXED 110067
[WebGL][EFL] Support for creating surface with alpha disabled. 
https://bugs.webkit.org/show_bug.cgi?id=110067
Summary [WebGL][EFL] Support for creating surface with alpha disabled.
Kalyan
Reported 2013-02-17 18:36:07 PST
compositing/webgl/webgl-no-alpha.html Currently we hardcode surface to always create a drawable with Alpha support. We should rather create it only if graphicscontext3d has alpha = true.
Attachments
patch (31.86 KB, text/plain)
2013-02-20 11:10 PST, Kalyan 		no flags
Details
Patch (33.19 KB, patch)
2013-02-20 11:44 PST, Kalyan 		no flags
DetailsFormatted DiffDiff
Patch (33.24 KB, patch)
2013-02-20 15:17 PST, Kalyan 		no flags
DetailsFormatted DiffDiff
Patch (32.75 KB, patch)
2013-02-21 15:27 PST, Kalyan 		no flags
DetailsFormatted DiffDiff
Patch (33.52 KB, patch)
2013-02-21 23:18 PST, Kalyan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Kalyan
Comment 1 2013-02-20 11:10:40 PST
Created attachment 189343 [details] patch
Kalyan
Comment 2 2013-02-20 11:44:38 PST
Created attachment 189348 [details] Patch
WebKit Review Bot
Comment 3 2013-02-20 11:48:06 PST
Attachment 189348 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/efl/compositing/webgl/webgl-no-alpha-expected.png', u'LayoutTests/platform/efl/compositing/webgl/webgl-no-alpha-expected.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp', u'Source/WebCore/platform/graphics/opengl/GLPlatformSurface.cpp', u'Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h', u'Source/WebCore/platform/graphics/surfaces/egl/EGLConfigSelector.cpp', u'Source/WebCore/platform/graphics/surfaces/egl/EGLConfigSelector.h', u'Source/WebCore/platform/graphics/surfaces/egl/EGLSurface.cpp', u'Source/WebCore/platform/graphics/surfaces/egl/EGLSurface.h', u'Source/WebCore/platform/graphics/surfaces/glx/GLXConfigSelector.h', u'Source/WebCore/platform/graphics/surfaces/glx/GLXContext.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.h', u'Source/WebCore/platform/graphics/surfaces/glx/GraphicsSurfaceGLX.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/X11Helper.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/X11Helper.h']" exit_code: 1 LayoutTests/platform/efl/compositing/webgl/webgl-no-alpha-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kalyan
Comment 4 2013-02-20 11:56:04 PST
Comment on attachment 189348 [details] Patch Removing request for review, looking into the style issue.
Kalyan
Comment 5 2013-02-20 15:17:13 PST
Created attachment 189391 [details] Patch
WebKit Review Bot
Comment 6 2013-02-20 15:19:29 PST
Attachment 189391 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/efl/compositing/webgl/webgl-no-alpha-expected.png', u'LayoutTests/platform/efl/compositing/webgl/webgl-no-alpha-expected.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp', u'Source/WebCore/platform/graphics/opengl/GLPlatformSurface.cpp', u'Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h', u'Source/WebCore/platform/graphics/surfaces/egl/EGLConfigSelector.cpp', u'Source/WebCore/platform/graphics/surfaces/egl/EGLConfigSelector.h', u'Source/WebCore/platform/graphics/surfaces/egl/EGLSurface.cpp', u'Source/WebCore/platform/graphics/surfaces/egl/EGLSurface.h', u'Source/WebCore/platform/graphics/surfaces/glx/GLXConfigSelector.h', u'Source/WebCore/platform/graphics/surfaces/glx/GLXContext.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/GLXSurface.h', u'Source/WebCore/platform/graphics/surfaces/glx/GraphicsSurfaceGLX.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/X11Helper.cpp', u'Source/WebCore/platform/graphics/surfaces/glx/X11Helper.h']" exit_code: 1 LayoutTests/platform/efl/compositing/webgl/webgl-no-alpha-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kenneth Rohde Christiansen
Comment 7 2013-02-21 02:08:34 PST
Comment on attachment 189391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189391&action=review > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:47 > + DoubleBuffer = 0x04 DoubleBuffer*ed* ? > Source/WebCore/platform/graphics/surfaces/glx/GLXConfigSelector.h:63 > + GLXConfigSelector(GLPlatformSurface::PlatformSurfaceAttributes attr = GLPlatformSurface::DefaultAttributes) Why not just called Attributes when it is in the class namespace? This is really long
Kalyan
Comment 8 2013-02-21 15:27:33 PST
Created attachment 189617 [details] Patch
Kalyan
Comment 9 2013-02-21 15:28:17 PST
Comment on attachment 189617 [details] Patch Another try with style fix
Kenneth Rohde Christiansen
Comment 10 2013-02-21 15:32:15 PST
Comment on attachment 189617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189617&action=review > Source/WebCore/platform/graphics/opengl/GLPlatformSurface.h:50 > + enum SurfaceAttributes { > + Default = 0x00, // No Alpha channel. Only R,G,B values set. > + SupportAlpha = 0x01, > + DoubleBuffered = 0x04 > + }; > + > + typedef int Attributes; Why are you doing this typedef trick and not using the enum directly? You would just use SurfaceAttributes instead of Attributes SurfaceAttributes attributes, even looks a bit nicer
Kalyan
Comment 11 2013-02-21 23:18:42 PST
Created attachment 189692 [details] Patch
Kalyan
Comment 12 2013-02-21 23:19:42 PST
(In reply to comment #7) > (From update of attachment 189391 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189391&action=review > DoubleBuffer*ed* ? done > > Source/WebCore/platform/graphics/surfaces/glx/GLXConfigSelector.h:63 > > + GLXConfigSelector(GLPlatformSurface::PlatformSurfaceAttributes attr = GLPlatformSurface::DefaultAttributes) > > Why not just called Attributes when it is in the class namespace? This is really long changed.
Kalyan
Comment 13 2013-02-21 23:21:23 PST
(In reply to comment #10) > (From update of attachment 189617 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189617&action=review > Why are you doing this typedef trick and not using the enum directly? > > You would just use SurfaceAttributes instead of Attributes > > SurfaceAttributes attributes, even looks a bit nicer This would give us a possibility to use it as a flag. As in future we might need to expand on this list and enable or disable features.
Kalyan
Comment 14 2013-02-21 23:22:53 PST
(In reply to comment #10) > (From update of attachment 189617 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189617&action=review > > > SurfaceAttributes attributes, even looks a bit nicer changed the api to reflect the same.
WebKit Review Bot
Comment 15 2013-02-22 01:34:58 PST
Comment on attachment 189692 [details] Patch Clearing flags on attachment: 189692 Committed r143704: <http://trac.webkit.org/changeset/143704>
WebKit Review Bot
Comment 16 2013-02-22 01:35:03 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.