Bug 144998

Summary: [GTK] Fix Wayland-related header inclusions in GLContext, GLContextEGL, PlatformDisplay
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: NEW    
Severity: Normal CC: bugs-noreply
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mcatanzaro: review+

Zan Dobersek
Reported 2015-05-14 04:26:27 PDT
[GTK] Fix Wayland-related header inclusions in GLContext, GLContextEGL, PlatformDisplay
Attachments
Patch (2.93 KB, patch)
2015-05-14 04:41 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (3.03 KB, patch)
2015-05-14 09:51 PDT, Zan Dobersek 		mcatanzaro: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2015-05-14 04:41:53 PDT
Created attachment 253118 [details] Patch
Carlos Garcia Campos
Comment 2 2015-05-14 05:12:45 PDT
Comment on attachment 253118 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253118&action=review Would it be possible to use forward declarations in the headers and do the includes in the cpp files? > Source/WebCore/platform/graphics/GLContext.h:30 > +#include <wayland-egl.h> This is not specific to GTK, no? > Source/WebCore/platform/graphics/PlatformDisplay.cpp:51 > #if PLATFORM(GTK) > +#if PLATFORM(X11) > #include <gdk/gdkx.h> > #endif > +#if PLATFORM(WAYLAND) > +#include <gdk/gdkwayland.h> > +#endif > +#endif I think the nested ifs makes this more difficult to read, what about something like? #if PLATFORM(GTK) && PLATFORM(X11) #include <gdk/gdkx.h> #endif #if PLATFORM(GTK) && PLATFORM(WAYLAND) #include <gdk/gdkwayland.h> #endif
Zan Dobersek
Comment 3 2015-05-14 09:39:50 PDT
Comment on attachment 253118 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253118&action=review >> Source/WebCore/platform/graphics/GLContext.h:30 >> +#include <wayland-egl.h> > > This is not specific to GTK, no? Right, this shouldn't be. <EGL/eglplatform.h> should. This also breaks EFL build. Oops. >> Source/WebCore/platform/graphics/PlatformDisplay.cpp:51 >> +#endif > > I think the nested ifs makes this more difficult to read, what about something like? > > #if PLATFORM(GTK) && PLATFORM(X11) > #include <gdk/gdkx.h> > #endif > > #if PLATFORM(GTK) && PLATFORM(WAYLAND) > #include <gdk/gdkwayland.h> > #endif Ugh ... It does look nicer, but I'll ping you as soon as someone uploads a patch that folds the guarded #include statements back under one #if PLATFORM(GTK) guard.
Zan Dobersek
Comment 4 2015-05-14 09:51:26 PDT
Created attachment 253124 [details] Patch
Note You need to log in before you can comment on or make changes to this bug.