Bug 142498 - [GTK] Add a configure option to build with OpenGL ES 2
Summary: [GTK] Add a configure option to build with OpenGL ES 2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2015-03-09 11:08 PDT by Carlos Garcia Campos
Modified: 2015-03-10 06:20 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.44 KB, patch)
2015-03-09 11:14 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (6.45 KB, patch)
2015-03-09 11:28 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (6.57 KB, patch)
2015-03-09 11:32 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (7.09 KB, patch)
2015-03-09 11:47 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2015-03-09 11:08:06 PDT
It's not currently possible, I guess it's another regression of the CMake switch, or that we never supported it.
Comment 1 Martin Robinson 2015-03-09 11:13:53 PDT
OpenGLES2 should probably force EGL as well. GLX + OpenGLES doesn't really make sense, as far as I know.
Comment 2 Carlos Garcia Campos 2015-03-09 11:14:02 PDT
Created attachment 248261 [details]
Patch
Comment 3 WebKit Commit Bot 2015-03-09 11:16:58 PDT
Attachment 248261 [details] did not pass style-queue:


ERROR: Source/cmake/OptionsGTK.cmake:72:  One space between command "endif" and its parentheses, should be "endif ("  [whitespace/parentheses] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Martin Robinson 2015-03-09 11:18:12 PDT
Comment on attachment 248261 [details]
Patch

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

Looks okay to me, but we should probably not allow combining GLX + OpenGLES.

> Source/cmake/OptionsGTK.cmake:69
> +    find_package(GLES REQUIRED)

Unrelated to this patch, but FindGLES is poorly named. There exists a historical OpenGLES which is different than OpenGLES2.
Comment 5 Carlos Garcia Campos 2015-03-09 11:19:44 PDT
(In reply to comment #4)
> Comment on attachment 248261 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248261&action=review
> 
> Looks okay to me, but we should probably not allow combining GLX + OpenGLES.

It's not possible, I think GLX is searched in find_package(OpenGL), that is only called when OpenGLES is didabled.

> > Source/cmake/OptionsGTK.cmake:69
> > +    find_package(GLES REQUIRED)
> 
> Unrelated to this patch, but FindGLES is poorly named. There exists a
> historical OpenGLES which is different than OpenGLES2.

Did't even know it :-)
Comment 6 Martin Robinson 2015-03-09 11:22:42 PDT
(In reply to comment #5)

> It's not possible, I think GLX is searched in find_package(OpenGL), that is
> only called when OpenGLES is didabled.

It looks like EGL is always detected and GLX is detected when building for X11. The checks shouldn't succeed if EGL isn't found, but OpenGLES2 is enabled.
Comment 7 Carlos Garcia Campos 2015-03-09 11:28:28 PDT
Created attachment 248262 [details]
Updated patch

Use the FindOpenGLES2.cmake one, which is indeed much better because it tries with pkg-config first
Comment 8 Carlos Garcia Campos 2015-03-09 11:29:38 PDT
(In reply to comment #6)
> (In reply to comment #5)
> 
> > It's not possible, I think GLX is searched in find_package(OpenGL), that is
> > only called when OpenGLES is didabled.
> 
> It looks like EGL is always detected and GLX is detected when building for
> X11. The checks shouldn't succeed if EGL isn't found, but OpenGLES2 is
> enabled.

Oh, good point, we need EGL when using OpenGLES2
Comment 9 Carlos Garcia Campos 2015-03-09 11:32:45 PDT
Created attachment 248263 [details]
Updated patch

Fail if EGL not found when using GLES2
Comment 10 Martin Robinson 2015-03-09 11:35:19 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=248262&action=review

> Source/cmake/OptionsGTK.cmake:94
> +        # FIXME: Should we search for cairo-glesv2 instead of cairo-gl in this case?

Yeah, this FIXME looks to have the correct suggestion. We probably need to pass an argument to FindCairoGL.

> Source/cmake/OptionsGTK.cmake:329
> +if ((OPENGL_FOUND OR OPENGLES2_FOUND) AND (GLX_FOUND OR EGL_FOUND))

I think the OPENGES2 + GLX combination is still bogus. :) One easy way to fix it is to error out early if EGL isn't found.
Comment 11 Carlos Garcia Campos 2015-03-09 11:37:57 PDT
(In reply to comment #10)
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248262&action=review
> 
> > Source/cmake/OptionsGTK.cmake:94
> > +        # FIXME: Should we search for cairo-glesv2 instead of cairo-gl in this case?
> 
> Yeah, this FIXME looks to have the correct suggestion. We probably need to
> pass an argument to FindCairoGL.
> 
> > Source/cmake/OptionsGTK.cmake:329
> > +if ((OPENGL_FOUND OR OPENGLES2_FOUND) AND (GLX_FOUND OR EGL_FOUND))
> 
> I think the OPENGES2 + GLX combination is still bogus. :)

Yes, it is, but it simply can't happen if I understood correctly how cmake works. GLX_FOUND is set by find_package(OpenGL), that is only called when OPENGLES2 option is not present, so it's not possible to have both.

> One easy way to
> fix it is to error out early if EGL isn't found.

That's what the patch does.
Comment 12 Carlos Garcia Campos 2015-03-09 11:42:54 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=248262&action=review
> > 
> > > Source/cmake/OptionsGTK.cmake:94
> > > +        # FIXME: Should we search for cairo-glesv2 instead of cairo-gl in this case?
> > 
> > Yeah, this FIXME looks to have the correct suggestion. We probably need to
> > pass an argument to FindCairoGL.
> > 
> > > Source/cmake/OptionsGTK.cmake:329
> > > +if ((OPENGL_FOUND OR OPENGLES2_FOUND) AND (GLX_FOUND OR EGL_FOUND))
> > 
> > I think the OPENGES2 + GLX combination is still bogus. :)
> 
> Yes, it is, but it simply can't happen if I understood correctly how cmake
> works. GLX_FOUND is set by find_package(OpenGL), that is only called when
> OPENGLES2 option is not present, so it's not possible to have both.
>

check_include_files is the thing searching for GLX? and setting GLX_FOUND? is that the case, I'm indeed wrong.
Comment 13 Martin Robinson 2015-03-09 11:46:20 PDT
(In reply to comment #12)

> check_include_files is the thing searching for GLX? and setting GLX_FOUND?
> is that the case, I'm indeed wrong.


That was my understanding, but I may be mistaken.
Comment 14 Carlos Garcia Campos 2015-03-09 11:47:56 PDT
Created attachment 248265 [details]
Updated patch

I was wrong
Comment 15 Martin Robinson 2015-03-09 11:55:16 PDT
Comment on attachment 248265 [details]
Updated patch

The day that we don't have to worry about GLX and Desktop GL cannot come soon enough.
Comment 16 Carlos Garcia Campos 2015-03-10 06:20:11 PDT
Committed r181321: <http://trac.webkit.org/changeset/181321>