[GTK][WPE] Use libepoxy
Created attachment 310114 [details] WIP
Created attachment 311580 [details] WIP Works for WPE, GTK+ next.
Created attachment 312662 [details] WIP patch Also tested on the GTK+ port.
Created attachment 313394 [details] WIP Cleaned up WPE CMake files.
Attachment 313394 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:45: "EpoxyShims.h" already included at Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:30 [build/include] [4] ERROR: Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:38: "epoxy/egl.h" already included at Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:28 [build/include] [4] ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/platform/graphics/EpoxyShims.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:106: "epoxy/gl.h" already included at Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:66 [build/include] [4] Total errors found: 7 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Change of plans, I'd only enable libepoxy for the WPE port, for starters. It's simpler there since only the EGL + OpenGL ES 2.0 combo is supported. We'd approach the GTK+ port afterwards, once libepoxy is shown to work well.
Created attachment 314638 [details] WIP patch
Attachment 314638 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:44: "EpoxyShims.h" already included at Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:30 [build/include] [4] ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:103: "epoxy/gl.h" already included at Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:64 [build/include] [4] Total errors found: 5 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Looks like the style checker errors are valid.
Created attachment 314826 [details] WIP patch
Created attachment 314967 [details] Patch
Attachment 314967 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Nice!
Comment on attachment 314967 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314967&action=review Nice! r=me, conditional on getting approval from Miguel as well. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:69 > +#if USE(LIBEPOXY) > +// Include the <epoxy/gl.h> header before <gst/gl/gl.h>. > +// Define GL_EXT_blend_func_extended since libepoxy headers don't, despite > +// providing all the GL entrypoints and constants relevant to that extension. > +#include <epoxy/gl.h> > +#define GL_EXT_blend_func_extended 1 > +#endif Did you report a bug at https://github.com/anholt/libepoxy? Our friends at Endless are maintaining libepoxy nowadays, so it should be easy to get changes upstreamed. Let me know if you need me to ping Emmanuele. > Source/cmake/OptionsWPE.cmake:78 > +find_package(LibEpoxy REQUIRED) Do you know if a particular minimum version is required? That would be good to research. If 1.3.1 is new enough, then I think we could even make it a mandatory dependency for WebKitGTK+. If you need 1.4, then it will have to remain optional for a long time, because we need to support Ubuntu 16.04.
Comment on attachment 314967 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314967&action=review >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:69 >> +#endif > > Did you report a bug at https://github.com/anholt/libepoxy? Our friends at Endless are maintaining libepoxy nowadays, so it should be easy to get changes upstreamed. Let me know if you need me to ping Emmanuele. This was actually a problem only with the 1.8 version of gst-plugins-bad, where these headers originate from. This isn't an issue anymore with the 1.10 version of GStreamer packages to which we upgraded recently. The main problem here was that GSTREAMER_GL was getting enabled for WPE even with the 1.8 version, which should not happen. The GTK+ port only enables the feature if version 1.10 or newer is present. >> Source/cmake/OptionsWPE.cmake:78 >> +find_package(LibEpoxy REQUIRED) > > Do you know if a particular minimum version is required? That would be good to research. > > If 1.3.1 is new enough, then I think we could even make it a mandatory dependency for WebKitGTK+. If you need 1.4, then it will have to remain optional for a long time, because we need to support Ubuntu 16.04. 1.3.1 works fine, and it might be enough for all the functionality we want from the library.
Created attachment 315233 [details] Patch for landing
Attachment 315233 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Tools/WebKitTestRunner/wpe/HeadlessViewBackend.h:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 315233 [details] Patch for landing Clearing flags on attachment: 315233 Committed r219391: <http://trac.webkit.org/changeset/219391>
All reviewed patches have been landed. Closing bug.
Comment on attachment 315233 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=315233&action=review > Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:39 > +#elif USE(OPENGL_ES2) This is a typo that broke the non USE_LIBEPOXY build. See bug 174442.