Bug 159562

Summary: Build fails with GSTREAMER_GL when both desktop GL and GLES2 are enabled in gst-plugins-bad
Product: WebKit Reporter: Olivier Blin <olivier.blin>
Component: WebKitGTKAssignee: Miguel Gomez <magomez>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, eocanha, magomez, mcatanzaro, pnormand, yoon, zan
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Olivier Blin 2016-07-08 09:21:14 PDT
When both desktop GL and GLES2 are enabled in gst-plugins-bad, and when GSTREAMER_GL is enabled, the build of the GStreamer media backend fails with the errors below.

In file included from /home/blino/WebKit/Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:43:0,
                 from /home/blino/WebKit/Source/WebCore/platform/graphics/GraphicsContext3D.h:29,
                 from /home/blino/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperGL.h:29,
                 from /home/blino/WebKit/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:38,
                 from /home/blino/WebKit/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:25:
/home/blino/WebKit/Source/WebCore/platform/graphics/OpenGLShims.h:257:67: error: expected type-specifier before 'glActiveTexture'
 #define glActiveTexture                        LOOKUP_GL_FUNCTION(glActiveTexture)
                                                                   ^
/home/blino/WebKit/Source/WebCore/platform/graphics/OpenGLShims.h:257:67: error: expected initializer before 'glActiveTexture'

...
many more errors with GL functions
...

/home/blino/WebKit/Source/WebCore/platform/graphics/OpenGLShims.h:312:67: error: 'openGLFunctionTable' function with trailing return type not declared with 'auto' type specifier
In file included from /usr/include/gstreamer-1.0/gst/gl/gstglapi.h:43:0,
                 from /usr/include/gstreamer-1.0/gst/gl/gstgl_fwd.h:26,
                 from /usr/include/gstreamer-1.0/gst/gl/gl.h:29,
                 from /home/blino/WebKit/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:49:
/usr/include/GLES2/gl2ext.h:740:59: error: expected ')' before 'n'
 GL_APICALL void GL_APIENTRY glGenVertexArraysOES (GLsizei n, GLuint *arrays);

...
Comment 1 Olivier Blin 2016-07-08 09:28:30 PDT
For reference, my build command is:
./Tools/Scripts/build-webkit --gtk --cmakeargs="-DUSE_GSTREAMER_GL=ON"
Comment 2 Philippe Normand 2016-07-13 08:37:04 PDT
I think Yoon had a similar issue, which was solved by changing the #include order of some files, IIRC.
Comment 3 Olivier Blin 2016-07-19 10:29:12 PDT
Created attachment 284017 [details]
Patch
Comment 4 Philippe Normand 2016-07-20 01:33:40 PDT
Comment on attachment 284017 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        OpenGLShims.h redefines GL functions.

How is OpenGLShims.h pulled in?

Not sure I like the #include move to the Base.h header, we try to keep our headers free of #includes as much as possible.
Comment 5 Olivier Blin 2016-07-20 07:31:04 PDT
(In reply to comment #4)
> Comment on attachment 284017 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284017&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        OpenGLShims.h redefines GL functions.
> 
> How is OpenGLShims.h pulled in?
> 
> Not sure I like the #include move to the Base.h header, we try to keep our
> headers free of #includes as much as possible.

I don't like it either, it would be easy to forget and break.

Here is the include flow:
  MediaPlayerPrivateGStreamerBase.cpp
  MediaPlayerPrivateGStreamerBase.h
  TextureMapperGL.h (for TextureMapperGL::Flags m_textureMapperRotationFlag)
  GraphicsContext3D.h
  ANGLEWebKitBridge.h
  OpenGLShims.h
Comment 6 Philippe Normand 2016-07-20 07:47:52 PDT
And the enum can't be forward-declared.

Maybe we should refactor the rotation flag code so that a member variable isn't needed?
Comment 7 Michael Catanzaro 2016-07-27 08:51:57 PDT
Comment on attachment 284017 [details]
Patch

(Removing from request queue based on comments above.)
Comment 8 Miguel Gomez 2016-09-20 08:05:12 PDT
Created attachment 289344 [details]
Patch
Comment 9 Philippe Normand 2016-09-20 08:09:00 PDT
Comment on attachment 289344 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:117
> +        return 0;

ASSERT_NOT_REACHED() ?
Comment 10 Miguel Gomez 2016-09-20 08:24:05 PDT
Created attachment 289348 [details]
Patch
Comment 11 Olivier Blin 2016-09-20 09:06:52 PDT
(In reply to comment #10)
> Created attachment 289348 [details]
> Patch

I can confirm this fixes the build, thanks!
Comment 12 WebKit Commit Bot 2016-09-21 00:50:46 PDT
Comment on attachment 289348 [details]
Patch

Clearing flags on attachment: 289348

Committed r206202: <http://trac.webkit.org/changeset/206202>
Comment 13 WebKit Commit Bot 2016-09-21 00:50:51 PDT
All reviewed patches have been landed.  Closing bug.