[GTK][WK2] Only set up a RedirectedXCompositeWindow if running under an X11 display

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

[GTK][WK2] Only set up a RedirectedXCompositeWindow if running under an X11 display

[Zan Dobersek]

Created attachment 209667 [details]
Patch

[WebKit Commit Bot]

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 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 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]

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]

Committed r154729: <http://trac.webkit.org/changeset/154729>

[Gustavo Noronha (kov)]

(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.