[GTK] Add a configure option to build with OpenGL ES 2

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

It's not currently possible, I guess it's another regression of the CMake switch, or that we never supported it.

[Martin Robinson]

OpenGLES2 should probably force EGL as well. GLX + OpenGLES doesn't really make sense, as far as I know.

[Carlos Garcia Campos]

Created attachment 248261 [details]
Patch

[WebKit Commit Bot]

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.

[Martin Robinson]

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.

[Carlos Garcia Campos]

(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 :-)

[Martin Robinson]

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

[Carlos Garcia Campos]

Created attachment 248262 [details]
Updated patch

Use the FindOpenGLES2.cmake one, which is indeed much better because it tries with pkg-config first

[Carlos Garcia Campos]

(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

[Carlos Garcia Campos]

Created attachment 248263 [details]
Updated patch

Fail if EGL not found when using GLES2

[Martin Robinson]

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.

[Carlos Garcia Campos]

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

[Carlos Garcia Campos]

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

[Martin Robinson]

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

[Carlos Garcia Campos]

Created attachment 248265 [details]
Updated patch

I was wrong

[Martin Robinson]

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.

[Carlos Garcia Campos]

Committed r181321: <http://trac.webkit.org/changeset/181321>