Bug 110067 – [WebGL][EFL] Support for creating surface with alpha disabled.

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Kalyan]

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.

[Kalyan]

Created attachment 189343 [details]
patch

[Kalyan]

Created attachment 189348 [details]
Patch

[WebKit Review Bot]

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 on attachment 189348 [details]
Patch

Removing request for review, looking into the style issue.

[Kalyan]

Created attachment 189391 [details]
Patch

[WebKit Review Bot]

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 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]

Created attachment 189617 [details]
Patch

[Kalyan]

Comment on attachment 189617 [details]
Patch

Another try with style fix

[Kenneth Rohde Christiansen]

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]

Created attachment 189692 [details]
Patch

[Kalyan]

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

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

(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 on attachment 189692 [details]
Patch

Clearing flags on attachment: 189692

Committed r143704: <http://trac.webkit.org/changeset/143704>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.