[PATCH] Xwayland: fix output changes not getting applied in some, cases

[Roman Gilg]

On Mon, Apr 22, 2019 at 1:30 PM Ferdi265 <theferdi265 at gmail.com> wrote:
>
> Hello,
>
> I investigated a bug I encountered while using the Sway Compositor and
> traced it back to some behaviour in Xwayland:
>
> The xdg_output and wl_output events are handled in such a way that
> output changes (e.g. position) sometimes don't get applied if only
> xdg_output is sent instead of both xdg_output and wl_output. (Sway does
> this)

This issue was something we noted in KWin as well and fixed it
downstream by making sure the events get always sent together.
More info here: https://phabricator.kde.org/D19253

Would a downstream fix work for you as well? Not saying it can't be
fixed upstream in XWayland, but I haven't yet looked at your patch in
detail and I just remember the output code in Xwayland being somewhat
difficult, so maybe a downstream fix would be easier.

> Below I attached a patch that fixes this. I am not very familiar with
> the Xorg codebase, so I hope this patch is fine.
>
> Here (https://github.com/swaywm/sway/issues/4064) is the Sway bug report
> in question.
>
> Kind Regards,
> Ferdinand "Ferdi265" Bachmann
>
> --- PATCH BELOW ---
>
> From 431a176c112d373a07b861fdf95766b3741ba95c Mon Sep 17 00:00:00 2001
> From: Ferdinand Bachmann <theferdi265 at gmail.com>
> Date: Mon, 22 Apr 2019 00:35:41 +0200
> Subject: [PATCH] Xwayland: fix output changes not getting applied in
> some cases
>
> On some wayland compositors wl_output and xdg_output events don't
> neccessarily happen together, so waiting until the wl_output event is
> done might wait indefinitely.
>
> This fixes output changes not getting applied on newer wlroots-based
> compositors, such as sway.
>
> Since these event's don't always happen together, the changes need to
> be applied regardless of whether the other event is finished or not,
> making the wl_output_done and xdg_output_done flags redundant.
> ---
>  hw/xwayland/xwayland-output.c | 17 ++---------------
>  hw/xwayland/xwayland.h        |  2 --
>  2 files changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
> index cc68f0340..497e56db4 100644
> --- a/hw/xwayland/xwayland-output.c
> +++ b/hw/xwayland/xwayland-output.c
> @@ -218,10 +218,6 @@ apply_output_change(struct xwl_output *xwl_output)
>      RRModePtr randr_mode;
>      Bool need_rotate;
>
> -    /* Clear out the "done" received flags */
> -    xwl_output->wl_output_done = FALSE;
> -    xwl_output->xdg_output_done = FALSE;
> -
>      /* xdg-output sends output size in compositor space. so already
> rotated */
>      need_rotate = (xwl_output->xdg_output == NULL);
>
> @@ -268,13 +264,7 @@ static void
>  output_handle_done(void *data, struct wl_output *wl_output)
>  {
>      struct xwl_output *xwl_output = data;
> -
> -    xwl_output->wl_output_done = TRUE;
> -    /* Apply the changes from wl_output only if both "done" events are
> received,
> -     * or if xdg-output is not supported.
> -     */
> -    if (xwl_output->xdg_output_done || !xwl_output->xdg_output)
> -        apply_output_change(xwl_output);
> +    apply_output_change(xwl_output);
>  }
>
>  static void
> @@ -313,10 +303,7 @@ static void
>  xdg_output_handle_done(void *data, struct zxdg_output_v1 *xdg_output)
>  {
>      struct xwl_output *xwl_output = data;
> -
> -    xwl_output->xdg_output_done = TRUE;
> -    if (xwl_output->wl_output_done)
> -        apply_output_change(xwl_output);
> +    apply_output_change(xwl_output);
>  }
>
>  static const struct zxdg_output_v1_listener xdg_output_listener = {
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index 92664e812..1ab2f04b5 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -371,8 +371,6 @@ struct xwl_output {
>      RRCrtcPtr randr_crtc;
>      int32_t x, y, width, height, refresh;
>      Rotation rotation;
> -    Bool wl_output_done;
> -    Bool xdg_output_done;
>  };
>
>  void xwl_sync_events (struct xwl_screen *xwl_screen);
> --
> 2.21.0
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel