Bug 115721

Summary: [GTK] Always use EGL to create the GL context when running on Wayland
Product: WebKit Reporter: Iago Toral <itoral>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, d-r, kalyan.kondapally, laszlo.gombos, mrobinson, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 81456    
Attachments:
Description Flags
Always use EGL when running on Wayland to create GL context
none
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
none
Always use EGL when running on Wayland to create GL context
mrobinson: review+, commit-queue: commit-queue-
patch with changelog none

Description Iago Toral 2013-05-07 03:57:27 PDT
When USE(GLX) is set, GLContext will attempt to create the GL context using GLX before trying EGL. This is problematic at least in the case where a Wayland compositor is running inside X, as in this case both GLX and EGL may be available and GLContext will use GLX.
Comment 1 Iago Toral 2013-05-07 04:20:55 PDT
Created attachment 200890 [details]
Always use EGL when running on Wayland to create GL context
Comment 2 Build Bot 2013-05-07 04:51:59 PDT
Comment on attachment 200890 [details]
Always use EGL when running on Wayland to create GL context

Attachment 200890 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/377466

New failing tests:
css3/filters/custom/filter-fallback-to-software.html
Comment 3 Build Bot 2013-05-07 04:52:00 PDT
Created attachment 200891 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 4 Martin Robinson 2013-05-11 12:45:19 PDT
Comment on attachment 200890 [details]
Always use EGL when running on Wayland to create GL context

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

> Source/WebCore/platform/graphics/cairo/GLContext.cpp:128
> +    GdkDisplay* display =
> +        gdk_display_manager_get_default_display(gdk_display_manager_get());
> +

It's okay for this to be one line. Lines in WebKit extend to 120 characters and beyond (beyond, beyond, beyond...).

> Source/WebCore/platform/graphics/cairo/GLContext.cpp:167
> +    GdkDisplay* display =
> +        gdk_display_manager_get_default_display(gdk_display_manager_get());
> +
> +    if (GDK_IS_WAYLAND_DISPLAY(display)) {
> +        if (OwnPtr<GLContext> eglContext = GLContextEGL::createContext(0, sharingContext))
> +            return eglContext.release();
> +        return nullptr;
> +    }
> +#endif
> +
>  #if USE(GLX)
>      if (OwnPtr<GLContext> glxContext = GLContextGLX::createContext(0, sharingContext))
>          return glxContext.release();

To avoid code duplication, could this method just call GLContext::createContextForWindow(0, sharingContext)?
Comment 5 Iago Toral 2013-05-13 00:26:33 PDT
Created attachment 201536 [details]
Always use EGL when running on Wayland to create GL context
Comment 6 Martin Robinson 2013-05-13 08:25:49 PDT
Comment on attachment 201536 [details]
Always use EGL when running on Wayland to create GL context

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

> Source/WebCore/platform/graphics/cairo/GLContext.cpp:131
> +#if PLATFORM(GTK) && defined(GDK_WINDOWING_WAYLAND) && USE(EGL)
> +    GdkDisplay* display = gdk_display_manager_get_default_display(gdk_display_manager_get());
> +
> +    if (GDK_IS_WAYLAND_DISPLAY(display)) {
> +        if (OwnPtr<GLContext> eglContext = GLContextEGL::createContext(windowHandle, sharingContext))
> +            return eglContext.release();
> +        return nullptr;

It's a pity there is no platform-independent way to detect that wayland is running, but this will suffice for now.
Comment 7 WebKit Commit Bot 2013-05-20 10:01:26 PDT
Comment on attachment 201536 [details]
Always use EGL when running on Wayland to create GL context

Rejecting attachment 201536 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 201536, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/git.webkit.org/WebKit
   2300bd1..8d5c9c0  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 150370 = 2300bd17f208fb06584b8ed4b1baa2d377d8efed
r150371 = 8d5c9c064cbf9c02ecdc28738012d12a858fc30f
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.appspot.com/results/293204
Comment 8 Martin Robinson 2013-05-20 10:02:36 PDT
Whoops. Your patch is missing a ChangeLog. Please review: http://www.webkit.org/coding/contributing.html
Comment 9 Iago Toral 2013-05-21 00:08:30 PDT
Created attachment 202385 [details]
patch with changelog
Comment 10 WebKit Commit Bot 2013-05-21 06:36:52 PDT
Comment on attachment 202385 [details]
patch with changelog

Clearing flags on attachment: 202385

Committed r150440: <http://trac.webkit.org/changeset/150440>
Comment 11 WebKit Commit Bot 2013-05-21 06:36:56 PDT
All reviewed patches have been landed.  Closing bug.