Bug 220183 – [GTK][CMake] Add a build switch USE_OPENGL_OR_ES to be able to disable OpenGL and ES

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[monson]

In bug 219551, ENABLE_GRAPHICS_CONTEXT_GL was removed, leaving ENABLE_GLES2 only to control whether using OPENGL or GLES2. But there is a situation where we have neither of them, with the dependency-inferring removed, some webkit options (ENABLE_ASYNC_SCROLLING, USE_GSTREAMER_GL and maybe ENABLE_3D_TRANSFORMS, and of course ENABLE_WEBGL) may be broken because of lack of GL API.

We hope to bring back ENABLE_GRAPHICS_CONTEXT_GL (or in whatever name), or a mechanism of auto disabling GL-related options.

Maybe relate to bug 219916.

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

> Source/cmake/OptionsGTK.cmake:-113
> -WEBKIT_OPTION_DEPEND(ENABLE_3D_TRANSFORMS ENABLE_GRAPHICS_CONTEXT_GL)
> -WEBKIT_OPTION_DEPEND(ENABLE_ASYNC_SCROLLING ENABLE_GRAPHICS_CONTEXT_GL)
> -WEBKIT_OPTION_DEPEND(ENABLE_GLES2 ENABLE_GRAPHICS_CONTEXT_GL)
> -WEBKIT_OPTION_DEPEND(ENABLE_WEBGL ENABLE_GRAPHICS_CONTEXT_GL)
>  WEBKIT_OPTION_DEPEND(USE_ANGLE_WEBGL ENABLE_WEBGL)
> -WEBKIT_OPTION_DEPEND(USE_WPE_RENDERER ENABLE_GRAPHICS_CONTEXT_GL)

[Fujii Hironori]

Created attachment 416851 [details]
Patch

[Fujii Hironori]

Created attachment 416852 [details]
Patch

[Fujii Hironori]

What do you think about this patch? Could you test the patch on your platform?

[Charlene Wendling]

Created attachment 416862 [details]
it's also needed to disable USE_GSTREAMER_GL

Hi,

Thanks for taking care of this!

I've tried your patch against the latest WebKit sources on OpenBSD/amd64,
the build failed because it still defines USE_GSTREAMER_GL, so there are
missing symbols. 

I'm attaching your patch with an addition; i made USE_GSTREAMER_GL depends
on USE_OPENGL_OR_ES.

Then it builds properly on this platform; runtime is fine as long i don't play
videos, in such a case the web process crash with:

"WebKit wasn't able to find a WebVTT encoder. Not continuing without platform support for subtitles."

When using our package, and as such with GL enabled, the webprocess does
not crash, while displaying the same message about WebVTT. This is not a deal
breaker as far as i'm concerned, i can disable USE_VIDEO -- but i'm pretty sure
you won't have the same point of view.

I can't try on OpenBSD/macppc for the moment (that's where that new option
really matters for the OpenBSD port), but it should cause no issue.

[Fujii Hironori]

Comment on attachment 416862 [details]
it's also needed to disable USE_GSTREAMER_GL

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

Thank you very much for the testing.

> Source/cmake/GStreamerDependencies.cmake:1
> +WEBKIT_OPTION_DEPEND(USE_GSTREAMER_GL USE_OPENGL_OR_ES)

Hmm, this file is also used by WPE port which always uses OpenGL and doesn't have USE_OPENGL_OR_ES build switch.

[Fujii Hironori]

Created attachment 416871 [details]
Patch

[Fujii Hironori]

Hi Charlene and Monson, could you debug the video issue in another bug ticket? It seems that GTK port maintainers aren't interested in no OpenGL build configuration.

[Fujii Hironori]

If nobody is willing to maintaining the no OpenGL build configuration, WebKitGTK should remove those compilation switches.

[monson]

Comment on attachment 416871 [details]
Patch

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

> Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp:35
> +#include <X11/Xutil.h>

Looks good to me but I don't speak for this X11 line, because I'm mainly building GTK port on macos, not familiar with X11 target nor WPE port.

[Kenneth Russell]

Comment on attachment 416871 [details]
Patch

In case you still need a review, upon glance this looks fine to me - but if there are other stakeholders who aren't reviewers, please wait for their go-ahead. r+

[Fujii Hironori]

Comment on attachment 416871 [details]
Patch

Clearing flags on attachment: 416871

Committed r271220: <https://trac.webkit.org/changeset/271220>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.