Bug 136829 - [GTK] Add Wayland support to GLContextEGL
Summary: [GTK] Add Wayland support to GLContextEGL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on: 136849
Blocks:
  Show dependency tree / graph
 
Reported: 2014-09-15 12:07 PDT by Zan Dobersek
Modified: 2014-09-17 23:22 PDT (History)
1 user (show)

See Also:

Attachments
Patch (2.24 KB, patch)
2014-09-15 12:21 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch (2.63 KB, patch)
2014-09-16 02:19 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch (2.63 KB, patch)
2014-09-16 13:28 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch (2.62 KB, patch)
2014-09-17 05:39 PDT, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2014-09-15 12:07:31 PDT 
[GTK] Add Wayland support to GLContextEGL
Comment 1 Zan Dobersek 2014-09-15 12:21:05 PDT 
Created attachment 238136 [details]
Patch
Comment 2 Martin Robinson 2014-09-15 18:47:58 PDT 
Comment on attachment 238136 [details]
Patch

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

> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:46
> +#if PLATFORM(GTK)
> +#include <gdk/gdk.h>
> +#if PLATFORM(X11)
> +#include <gdk/gdkx.h>
> +#endif
> +#if PLATFORM(WAYLAND) && !defined(GTK_API_VERSION_2)
> +#include <gdk/gdkwayland.h>
> +#endif
> +#endif

Sad to still see GTK+ 2 stuff here. :/

> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:81
> +#if PLATFORM(GTK) && !defined(GTK_API_VERSION_2)
> +        GdkDisplay* display = gdk_display_manager_get_default_display(gdk_display_manager_get());
>  #if PLATFORM(X11)
> +        if (GDK_IS_X11_DISPLAY(display))
> +            gSharedEGLDisplay = eglGetDisplay(GLContext::sharedX11Display());
> +#endif
> +#if PLATFORM(WAYLAND)
> +        if (GDK_IS_WAYLAND_DISPLAY(display))
> +            gSharedEGLDisplay = eglGetDisplay(GLContext::sharedWaylandDisplay());
> +#endif
> +#elif PLATFORM(X11)

It looks like this can just be:

#if PLATFORM(GTK) && !defined(GTK_API_VERSION_2) && PLATFORM(WAYLAND)
GdkDisplay* display = gdk_display_manager_get_default_display(gdk_display_manager_get());
if (GDK_IS_WAYLAND_DISPLAY(display))
    gSharedEGLDisplay = eglGetDisplay(GLContext::sharedWaylandDisplay());
else
#endif

to avoid duplicating the X11 version.
Comment 3 Zan Dobersek 2014-09-16 01:22:39 PDT 
Comment on attachment 238136 [details]
Patch

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

>> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:81
>> +#elif PLATFORM(X11)
> 
> It looks like this can just be:
> 
> #if PLATFORM(GTK) && !defined(GTK_API_VERSION_2) && PLATFORM(WAYLAND)
> GdkDisplay* display = gdk_display_manager_get_default_display(gdk_display_manager_get());
> if (GDK_IS_WAYLAND_DISPLAY(display))
>     gSharedEGLDisplay = eglGetDisplay(GLContext::sharedWaylandDisplay());
> else
> #endif
> 
> to avoid duplicating the X11 version.

OK, though I'm not a big fan of branches reaching through the #if/#endif borders.
Comment 4 Zan Dobersek 2014-09-16 02:19:13 PDT 
Created attachment 238162 [details]
Patch
Comment 5 Zan Dobersek 2014-09-16 13:28:50 PDT 
Created attachment 238197 [details]
Patch
Comment 6 Zan Dobersek 2014-09-17 05:39:29 PDT 
Created attachment 238248 [details]
Patch
Comment 7 Martin Robinson 2014-09-17 16:21:31 PDT 
Comment on attachment 238248 [details]
Patch

Let's do this.
Comment 8 Zan Dobersek 2014-09-17 23:22:17 PDT 
Comment on attachment 238248 [details]
Patch

Clearing flags on attachment: 238248

Committed r173716: <http://trac.webkit.org/changeset/173716>
Comment 9 Zan Dobersek 2014-09-17 23:22:25 PDT 
All reviewed patches have been landed.  Closing bug.