Closed Bug 1425842 Opened 2 years ago Closed 2 years ago

[Wayland] - Implement Wayland CompositorWidget class

Categories

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

All
Linux
enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

X11CompositorWidget needs to be updated or reimplemented for Wayland.
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 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.
Attachment #8937515 - Flags: review?(jhorak) → review-
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 on attachment 8937515 [details]
Bug 1425842 - Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support,

https://reviewboard.mozilla.org/r/208202/#review214836
Attachment #8937515 - Flags: review?(jhorak) → review+
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/646f72ac8ed6
Rename X11CompositorWidget to GtkCompositorWidget and implement WindowSurfaceWayland support, r=jhorak
https://hg.mozilla.org/mozilla-central/rev/646f72ac8ed6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.