1425842 - [Wayland] - Implement Wayland CompositorWidget class

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement, P2)

Description

[Martin Stránský [:stransky] (ni? me)]

X11CompositorWidget needs to be updated or reimplemented for Wayland.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

I tend to rename X11CompositorWidget to GtkCompositorWidget and handle both X11/Wayland there as it's implemented at https://github.com/stransky/gecko-dev

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

- Rename X11CompositorWidget to GtkCompositorWidget to handle both X11 and Wayland backends.
- Rename X11CompositorWidgetInitData to GtkCompositorWidgetInitData
- Implement Wayland support at WindowSurfaceProvider, WindowSurfaceProvider::CreateWindowSurface()
  is able to create WindowSurfaceWayland.
- Initialize Wayland surface provider at nsWindow::Create()

Review commit: https://reviewboard.mozilla.org/r/208202/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208202/

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8937515 [details]
Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208202/diff/1-2/

Comment 4

[Jan Horak [:jhorak]]

Comment on attachment 8937515 [details]
Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

https://reviewboard.mozilla.org/r/208202/#review214792

::: widget/gtk/WindowSurfaceProvider.cpp:60
(Diff revision 2)
> +
> +#ifdef MOZ_WAYLAND
> +void WindowSurfaceProvider::Initialize(nsWindow *aWidget)
> +{
> +  mWidget = aWidget;
> +  mIsX11Display = aWidget->IsX11Display();

mIsX11Display should be false all the time, maybe add assertion there.

::: widget/gtk/WindowSurfaceProvider.cpp:76
(Diff revision 2)
>  {
> +#ifdef MOZ_WAYLAND
> +  if (!mIsX11Display) {
> +    LOGDRAW(("Drawing to nsWindow %p using wl_surface\n", (void*)this));
> +    return MakeUnique<WindowSurfaceWayland>(mWidget);
> +  } else

no need to use 'else' when returned line before.

::: widget/gtk/WindowSurfaceProvider.cpp:121
(Diff revision 2)
>        return nullptr;
>    }
>  
>    *aBufferMode = BufferMode::BUFFER_NONE;
>    RefPtr<DrawTarget> dt = nullptr;
> -  if (!(dt = mWindowSurface->Lock(aInvalidRegion)) &&
> +  if (!(dt = mWindowSurface->Lock(aInvalidRegion)) && mIsX11Display &&

Please add comment why this only applies for X11 display and what can be expected when on wayland backend

::: widget/gtk/GtkCompositorWidget.cpp:32
(Diff revision 2)
>    } else {
>      mXDisplay = XOpenDisplay(aInitData.XDisplayString().get());
> -  }
> +#ifdef MOZ_WAYLAND
> +    if (!mXDisplay) {
> +      // TODO - not implemented
> +      MOZ_CRASH();

Please describe why you're crashing here.

::: widget/gtk/nsWindow.cpp:4065
(Diff revision 2)
>        mXVisual = gdk_x11_visual_get_xvisual(gdkVisual);
>        mXDepth = gdk_visual_get_depth(gdkVisual);
>  
>        mSurfaceProvider.Initialize(mXDisplay, mXWindow, mXVisual, mXDepth);
>      }
> +#ifdef MOZ_WAYLAND

Consider opening a new bug for unifying Initialize(nsWindow) for X11 and wayland.

::: widget/gtk/nsWindow.cpp:7052
(Diff revision 2)
>  }
>  
>  void nsWindow::GetCompositorWidgetInitData(mozilla::widget::CompositorWidgetInitData* aInitData)
>  {
> -  #ifdef MOZ_X11
> +#ifdef MOZ_X11
> -  *aInitData = mozilla::widget::X11CompositorWidgetInitData(
> +#ifdef MOZ_WAYLAND

As above, it would be cool of we have the same ...InitData for X and wayland.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8937515 [details]
Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

https://reviewboard.mozilla.org/r/208202/#review214792

> As above, it would be cool of we have the same ...InitData for X and wayland.

I'll file follow up bug for that.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8937515 [details]
Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208202/diff/2-3/

Comment 7

[Jan Horak [:jhorak]]

Comment on attachment 8937515 [details]
Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

https://reviewboard.mozilla.org/r/208202/#review214836

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/646f72ac8ed6
Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support, r=jhorak

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/646f72ac8ed6