Bug 163482

Summary: eglplatform.h does not support Wayland
Product: WebKit Reporter: ManDay <manday>
Component: ANGLEAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, aperez, buildbot, clopez, dino, graouts, jmcasanova, kondapallykalyan, mcatanzaro, rniwa, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews100 for mac-elcapitan
none
Patch none

Description ManDay 2016-10-15 03:06:07 PDT
ANGLE's include/EGL/eglplatform.h does require X11 headers. I managed to work around this by replacing it by MESA's include/EGL/eglplatform.h (surprisingly, since it requires MESA_EGL_NO_X11_HEADERS defined in order not to require them).
Comment 1 Adrian Perez 2017-07-12 03:25:58 PDT
I am also facing this issue when building WebKitGTK+ 2.16.5 using
Buildroot with Wayland enabled and X.org disabled. Better than
replacing the “eglplatform.h” header with the Mesa version, I can
try to make a patch which includes the X11 headers when ANGLE_USE_X11
is defined, and otherwise provides the same definitions as the Mesa
version when MESA_EGL_NO_X11_HEADERS is defined. Probably it is also
a good idea to add the definitions for Wayland when WL_EGL_PLATFORM
is defined.
Comment 2 Adrian Perez 2017-07-12 03:50:29 PDT
Created attachment 315227 [details]
Patch
Comment 3 Build Bot 2017-07-12 03:51:27 PDT
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 4 Build Bot 2017-07-12 06:22:03 PDT
Comment on attachment 315227 [details]
Patch

Attachment 315227 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4106665

New failing tests:
storage/websql/database-lock-after-reload.html
Comment 5 Build Bot 2017-07-12 06:22:04 PDT
Created attachment 315234 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 6 Michael Catanzaro 2017-07-12 07:25:25 PDT
You'll need to update the ANGLE patch (changes.diff)
Comment 7 Adrian Perez 2017-07-12 08:14:51 PDT
Created attachment 315241 [details]
Patch
Comment 8 Adrian Perez 2017-07-12 08:17:53 PDT
(In reply to Adrian Perez from comment #7)
> Created attachment 315241 [details]
> Patch

This new version of the patch also updates “changes.diff” ­— thanks for the
reminder, Michael!

I am doing a Wayland-only build right now in Buildroot to double-check that
the patch works as intended. Let's wait until it finishes before landing,
and wait for reviewer's comments in the meanwhile.
Comment 9 Adrian Perez 2017-07-12 16:48:15 PDT
(In reply to Adrian Perez from comment #8)

> I am doing a Wayland-only build right now in Buildroot to double-check that
> the patch works as intended. Let's wait until it finishes before landing,
> and wait for reviewer's comments in the meanwhile.

The build finished fine with this patch applied. Additionally I needed to
pass the following to CMake in the Buildroot recipe to avoid the system
Mesa headers including X11 headers:

  -DCMAKE_C_FLAGS='$(TARGET_CFLAGS) -DMESA_EGL_NO_X11_HEADERS'
  -DCMAKE_CXX_FLAGS='$(TARGET_CXXFLAGS) -DMESA_EGL_NO_X11_HEADERS'

The above is needed because this patch affects only the copy included within
ANGLE but not the system (in this case, sysroot-wide) EGL header. I think this
is fine and I'll wait until tomorrow to land this patch, just in case anybody
wants to comment, and also that way I can keep an eye on the buildbots (right
now is very late for me).
Comment 10 Adrian Perez 2017-07-13 02:21:57 PDT
Comment on attachment 315241 [details]
Patch

Clearing flags on attachment: 315241

Committed r219446: <http://trac.webkit.org/changeset/219446>
Comment 11 Adrian Perez 2017-07-13 02:22:02 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Adrian Perez 2017-07-13 05:56:22 PDT
The patch was also attached to a bug report in the upstream bug tracker,
as recommended by the guidelines for “Source/ThirdParty/ANGLE/”:

  https://bugs.chromium.org/p/angleproject/issues/detail?id=2105
Comment 13 Adrian Perez 2017-07-21 07:11:19 PDT
(In reply to Adrian Perez from comment #12)
> The patch was also attached to a bug report in the upstream bug tracker,
> as recommended by the guidelines for “Source/ThirdParty/ANGLE/”:
> 
>   https://bugs.chromium.org/p/angleproject/issues/detail?id=2105

The patch (with small modifications) was merged upstream in ANGLE, yay!
Comment 14 Carlos Alberto Lopez Perez 2018-03-19 13:04:53 PDT
On revision r225340 <https://trac.webkit.org/r225340> ANGLE was updated.

The Wayland EGL definitions are on the new eglplatform.h file, but not the check for MESA_EGL_NO_X11_HEADERS

I guess that is enough, right?
Comment 15 ManDay 2018-03-24 05:18:42 PDT
Is RESOLVED FIXED correct? I don't know which ANGLE revision is in the current webkit-gtk tarballs, but I can say for sure that the eglplatform.h that comes with current webkit is still "broken" and needs to be replaced by the MESA one form me.
Comment 16 Adrian Perez 2018-09-21 11:58:30 PDT
(In reply to ManDay from comment #15)
> Is RESOLVED FIXED correct? I don't know which ANGLE revision is in the
> current webkit-gtk tarballs, but I can say for sure that the eglplatform.h
> that comes with current webkit is still "broken" and needs to be replaced by
> the MESA one form me.

Yes, the change that went in the patch that the ANGLE maintainers merged
upstream is lacking one part that I had made originally to fix this bug.
I have opened bug #189844 to re-add the needed part in our bundled copy
of ANGLE because now I am hitting this issue again.