WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
120321
[GTK][WK2] Only set up a RedirectedXCompositeWindow if running under an X11 display
https://bugs.webkit.org/show_bug.cgi?id=120321
Summary
[GTK][WK2] Only set up a RedirectedXCompositeWindow if running under an X11 d...
Zan Dobersek
Reported
2013-08-26 11:27:43 PDT
[GTK][WK2] Only set up a RedirectedXCompositeWindow if running under an X11 display
Attachments
Patch
(3.10 KB, patch)
2013-08-26 11:34 PDT
,
Zan Dobersek
gustavo
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Zan Dobersek
Comment 1
2013-08-26 11:34:50 PDT
Created
attachment 209667
[details]
Patch
WebKit Commit Bot
Comment 2
2013-08-26 11:37:09 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Gustavo Noronha (kov)
Comment 3
2013-08-26 12:41:10 PDT
Comment on
attachment 209667
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=209667&action=review
> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:60 > +#ifdef GDK_WINDOWING_X11
I've been pondering that we should probably have a macro that tests that both PLATFORM(X11) and GDK_WINDOWING_X11 are present for these, what do you think?
> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:387 > + GdkDisplay* display = gdk_display_manager_get_default_display(gdk_display_manager_get()); > + if (GDK_IS_X11_DISPLAY(display)) { > + priv->redirectedWindow = RedirectedXCompositeWindow::create(IntSize(1, 1), RedirectedXCompositeWindow::DoNotCreateGLContext); > + if (priv->redirectedWindow) > + priv->redirectedWindow->setDamageNotifyCallback(redirectedWindowDamagedCallback, object); > + }
Need the preprocessor check here as well.
Zan Dobersek
Comment 4
2013-08-27 01:38:47 PDT
Comment on
attachment 209667
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=209667&action=review
>> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:60 >> +#ifdef GDK_WINDOWING_X11 > > I've been pondering that we should probably have a macro that tests that both PLATFORM(X11) and GDK_WINDOWING_X11 are present for these, what do you think?
I think we could test for PLATFORM(X11) instead of GDK_WINDOWING_X11 if we explicitly test for gdk-x11-3.0.pc when building for the X11 target. I believe that finding the gdk-x11 package indirectly assures that GDK_WINDOWING_X11 is defined, though we can add additional checks for this in the configuration step. I'd rather be using PLATFORM(X11) than GDK_WINDOWING_X11 simply because it's more WebKit-like. I know that PLATFORM(X11) is also applicable to other ports, but the `#ifdef GDK_WINDOWING_X11` checks are still GTK-specific, so using PLATFORM(X11) in their place shouldn't break anything. Similar would apply for GDK_WINDOWING_WAYLAND and PLATFORM(WAYLAND).
Zan Dobersek
Comment 5
2013-08-27 02:17:04 PDT
This patch enables running UIProcesses under Wayland even when building with accelerated compositing enabled - before, the process would assert due to an invalid cast from GdkWaylandDisplay to GdkX11Display. The accelerated compositing is enabled by default in WK2 though, so page rendering under Wayland goes wrong whenever it switches to the accelerated compositing mode. To avoid that, I'm at the moment disabling accelerated compositing under Wayland displays in webkitWebViewGroupAttachSettingsToPageGroup. Is that feasible/sane? It works, and it's needed, but it's kind of a dirty hack.
Bug #120347
has the patch.
Zan Dobersek
Comment 6
2013-08-28 02:44:08 PDT
Committed
r154729
: <
http://trac.webkit.org/changeset/154729
>
Gustavo Noronha (kov)
Comment 7
2013-09-02 05:18:10 PDT
(In reply to
comment #4
)
> I think we could test for PLATFORM(X11) instead of GDK_WINDOWING_X11 if we explicitly test for gdk-x11-3.0.pc when building for the X11 target. I believe that finding the gdk-x11 package indirectly assures that GDK_WINDOWING_X11 is defined, though we can add additional checks for this in the configuration step. >
I think that would be better, I'm thinking about a flying-cars future where X is disabled and only wayland is enabled, though. I guess we can cross that bridge when we get to it, anyway.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug