Bug 183008 - including both gl3.h and gl2.h when USE_OPENGL_ES is enabled
Summary: including both gl3.h and gl2.h when USE_OPENGL_ES is enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Miguel Gomez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-21 10:56 PST by Hyun Woo Kwon
Modified: 2018-02-22 07:33 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.22 KB, patch)
2018-02-22 01:56 PST, 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 Hyun Woo Kwon 2018-02-21 10:56:22 PST
Both gl2.h and gl3.h are included when USE_OPENGL_ES is enabled in [1]. This breaks the build for systems that only support OpenGLES2, but not OpenGLES3. USE_OPENGL_ES should specify the version of the API and include the corresponding header only, to avoid any confusion.

[1] https://github.com/WebKit/webkit/blob/master/Source/WebCore/platform/graphics/GLContext.cpp#L31

Thanks,
-hyun
Comment 1 Michael Catanzaro 2018-02-21 12:01:32 PST
USE_OPENGL_ES is supposed to require only OpenGL ES 2, not OpenGL ES 3.

Looks like this has been broken since r209233, bug #165283, over a year ago.

I thought we had a requirement to support WebKit on devices that have only OpenGL ES 2. However, if nobody has noticed that this has been broken for over a year, maybe that's not so important anymore.
Comment 2 Miguel Gomez 2018-02-22 01:22:16 PST
> Both gl2.h and gl3.h are included when USE_OPENGL_ES is enabled in [1]. This
> breaks the build for systems that only support OpenGLES2, but not OpenGLES3.
> USE_OPENGL_ES should specify the version of the API and include the
> corresponding header only, to avoid any confusion.

It was that way actually. USE_OPENGL_ES was USE_OPENGL_ES_2 and (for linux ports) meant we were using the GLES2 API, but it was renamed to USE_OPENGL_ES in r228590 as in apple ports it can mean GLES2 or GLES3 APIs.

In any case, the <GLES3/gl3.h> include should not be necessary. They seem to include it just for the definition of GL_MAJOR_VERSION, which which seems to be only used in Extensions3DOpenGLCommon::initializeAvailableExtensions() in a platform guarded code.

I'll send a temptative patch removing that include to see what exactly (if anything) fails in the wincairo bot.
Comment 3 Miguel Gomez 2018-02-22 01:56:12 PST
Created attachment 334443 [details]
Patch
Comment 4 WebKit Commit Bot 2018-02-22 07:33:08 PST
Comment on attachment 334443 [details]
Patch

Clearing flags on attachment: 334443

Committed r228917: <https://trac.webkit.org/changeset/228917>
Comment 5 WebKit Commit Bot 2018-02-22 07:33:09 PST
All reviewed patches have been landed.  Closing bug.