Bug 175125

Summary: [GTK][WPE] CFLAGS from pkg-config for (E)GL are not passed to WebKit
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WebKitGTKAssignee: Carlos Alberto Lopez Perez <clopez>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, beidson, bugs-noreply, cgarcia, clopez, commit-queue, darin, mcatanzaro, youennf, ysuzuki, zan
Priority: P2    
Version: Other   
Hardware: All   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=145408
https://bugs.webkit.org/show_bug.cgi?id=178081
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Adrian Perez
Reported 2017-08-03 03:56:15 PDT
For example, in “FindEGL.cmake”, the “Cflags” field from “egl.pc” is stored in the “${EGL_DEFINITIONS}” CMake variable, which is used to pass the flags to WebCore, but the value is never passed to WebKit, and building WebKit can fail. One such case is when building in a Wayland-only system (no X11 support), where Mesa's “egl.pc” contains “-DMESA_EGL_NO_X11_HEADERS”, but not passing that when building WebKit causes Mesa's “eglplatform.h” to try to include the X11 headers — and the build fails.
Attachments
Patch (2.42 KB, patch)
2017-08-06 14:51 PDT, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Patch (7.70 KB, patch)
2017-08-07 04:29 PDT, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Patch (6.44 KB, patch)
2017-08-07 04:32 PDT, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Patch (6.44 KB, patch)
2017-08-07 04:34 PDT, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Adrian Perez
Comment 1 2017-08-03 03:57:23 PDT
Carlos López has posted a working patch at http://sprunge.us/aUhc
Carlos Alberto Lopez Perez
Comment 2 2017-08-06 14:51:00 PDT
Created attachment 317384 [details] Patch
Michael Catanzaro
Comment 3 2017-08-06 16:05:28 PDT
Comment on attachment 317384 [details] Patch CMake is so fragile. How could this happen. :(
Michael Catanzaro
Comment 4 2017-08-06 16:05:58 PDT
I mean, the fact that it has been possible to build at all, for years, without the right flags being passed... that is surprising.
WebKit Commit Bot
Comment 5 2017-08-06 16:35:02 PDT
Comment on attachment 317384 [details] Patch Clearing flags on attachment: 317384 Committed r220326: <http://trac.webkit.org/changeset/220326>
WebKit Commit Bot
Comment 6 2017-08-06 16:35:03 PDT
All reviewed patches have been landed. Closing bug.
Carlos Alberto Lopez Perez
Comment 7 2017-08-06 17:21:38 PDT
(In reply to Michael Catanzaro from comment #4) > I mean, the fact that it has been possible to build at all, for years, > without the right flags being passed... that is surprising. Two things explains this: * The default build (with Mesa as GL library and with X11 support built into Mesa's EGL) works fine without passing any cflag. You only run into this failure if your GL library requires some cflag to build. Requiring this is the exception rather than the norm. I have only seen this with the Freescale GL libraries (Vivante) or when you disable the X11 support when building Mesa's EGL libraries (as Adrian comments). * When we landed r184954 <https://trac.webkit.org/r184954>, we didn't used any (E)GL function on the UIProcess, so it was not needed to add this there at that time. Only in WebCore was needed by then.
Carlos Alberto Lopez Perez
Comment 8 2017-08-07 02:54:01 PDT
Committed r220331: <http://trac.webkit.org/changeset/220331>
Zan Dobersek
Comment 9 2017-08-07 03:33:47 PDT
Comment on attachment 317384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317384&action=review > Source/WebKit/CMakeLists.txt:97 > + if (USE_OPENGL) This doesn't account for libepoxy, where OPENGL and EGL packages aren't leveraged. > Source/WebKit/CMakeLists.txt:127 > list(APPEND WebKit2_INCLUDE_DIRECTORIES > "${THIRDPARTY_DIR}/ANGLE" > "${THIRDPARTY_DIR}/ANGLE/include/KHR" We shouldn't be using ANGLE headers in the first place.
Zan Dobersek
Comment 10 2017-08-07 03:56:09 PDT
Comment on attachment 317384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317384&action=review >> Source/WebKit/CMakeLists.txt:127 >> "${THIRDPARTY_DIR}/ANGLE/include/KHR" > > We shouldn't be using ANGLE headers in the first place. ... but it's apparently needed due to indirect GraphicsContext3D.h inclusions. This should be addressed later then, after GC3D usage cleanup.
Carlos Alberto Lopez Perez
Comment 11 2017-08-07 03:58:10 PDT
(In reply to Zan Dobersek from comment #9) > Comment on attachment 317384 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317384&action=review > > > Source/WebKit/CMakeLists.txt:97 > > + if (USE_OPENGL) > > This doesn't account for libepoxy, where OPENGL and EGL packages aren't > leveraged. > Good catch. I didn't realized about libepoxy. Let's re-open. > > Source/WebKit/CMakeLists.txt:127 > > list(APPEND WebKit2_INCLUDE_DIRECTORIES > > "${THIRDPARTY_DIR}/ANGLE" > > "${THIRDPARTY_DIR}/ANGLE/include/KHR" > > We shouldn't be using ANGLE headers in the first place. That was already there before, I didn't introduced this include of the ANGLE headers.
Carlos Alberto Lopez Perez
Comment 12 2017-08-07 04:00:57 PDT
Reopen
Carlos Alberto Lopez Perez
Comment 13 2017-08-07 04:29:50 PDT
Created attachment 317412 [details] Patch
Carlos Alberto Lopez Perez
Comment 14 2017-08-07 04:32:41 PDT
Created attachment 317413 [details] Patch
Carlos Alberto Lopez Perez
Comment 15 2017-08-07 04:34:51 PDT
Created attachment 317414 [details] Patch fix typo on changelog
Michael Catanzaro
Comment 16 2017-08-07 07:05:39 PDT
Comment on attachment 317412 [details] Patch The ChangeLog is messed up and all EWS are red....
Carlos Alberto Lopez Perez
Comment 17 2017-08-07 07:19:27 PDT
(In reply to Michael Catanzaro from comment #16) > Comment on attachment 317412 [details] > Patch > > The ChangeLog is messed up and all EWS are red.... Why do you comment on an obsoleted attachment? please see https://bugs.webkit.org/attachment.cgi?id=317414&action=review
Michael Catanzaro
Comment 18 2017-08-07 08:14:43 PDT
Comment on attachment 317414 [details] Patch I'm bad at Bugzilla! Much better.
WebKit Commit Bot
Comment 19 2017-08-07 08:44:17 PDT
Comment on attachment 317414 [details] Patch Clearing flags on attachment: 317414 Committed r220334: <http://trac.webkit.org/changeset/220334>
WebKit Commit Bot
Comment 20 2017-08-07 08:44:19 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.