Bug 175125 – [GTK][WPE] CFLAGS from pkg-config for (E)GL are not passed to WebKit

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

[Adrian Perez]

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.

[Adrian Perez]

Carlos López has posted a working patch at http://sprunge.us/aUhc

[Carlos Alberto Lopez Perez]

Created attachment 317384 [details]
Patch

[Michael Catanzaro]

Comment on attachment 317384 [details]
Patch

CMake is so fragile. How could this happen. :(

[Michael Catanzaro]

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 on attachment 317384 [details]
Patch

Clearing flags on attachment: 317384

Committed r220326: <http://trac.webkit.org/changeset/220326>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Alberto Lopez Perez]

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

Committed r220331: <http://trac.webkit.org/changeset/220331>

[Zan Dobersek]

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 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]

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

Reopen

[Carlos Alberto Lopez Perez]

Created attachment 317412 [details]
Patch

[Carlos Alberto Lopez Perez]

Created attachment 317413 [details]
Patch

[Carlos Alberto Lopez Perez]

Created attachment 317414 [details]
Patch

fix typo on changelog

[Michael Catanzaro]

Comment on attachment 317412 [details]
Patch

The ChangeLog is messed up and all EWS are red....

[Carlos Alberto Lopez Perez]

(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 on attachment 317414 [details]
Patch

I'm bad at Bugzilla! Much better.

[WebKit Commit Bot]

Comment on attachment 317414 [details]
Patch

Clearing flags on attachment: 317414

Committed r220334: <http://trac.webkit.org/changeset/220334>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.