[PATCH] compositor-drm: fix z-order inversion in plane assignment

[Matt Hoosier]

On Thu, Aug 24, 2017 at 9:24 AM, Matt Hoosier <matt.hoosier at gmail.com> wrote:
> As discussed in the following thread:
>
> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034755.html
>
> the existing plane assignment in the DRM backend is vulnerable to
> accidental masking of the intended fullscreen surface. This change
> adds a simple stateful memory to the plane assignment algorithm
> to prevent that.
> ---
>  libweston/compositor-drm.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8e1e788f..81dc8fe6 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1495,6 +1495,7 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data)
>         struct weston_view *ev, *next;
>         pixman_region32_t overlap, surface_overlap;
>         struct weston_plane *primary, *next_plane;
> +       bool picked_scanout;
>
>         /*
>          * Find a surface for each sprite in the output using some heuristics:
> @@ -1516,6 +1517,8 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data)
>         output->cursor_plane.x = INT32_MIN;
>         output->cursor_plane.y = INT32_MIN;
>
> +       picked_scanout = false;
> +
>         wl_list_for_each_safe(ev, next, &output_base->compositor->view_list, link) {
>                 struct weston_surface *es = ev->surface;
>
> @@ -1545,10 +1548,22 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data)
>                         next_plane = primary;
>                 if (next_plane == NULL)
>                         next_plane = drm_output_prepare_cursor_view(output, ev);
> -               if (next_plane == NULL)
> +
> +               /* If a higher-stacked view already got assigned to scanout, it's incorrect to
> +                * assign a subsequent (lower-stacked) view to scanout.
> +                */
> +               if (next_plane == NULL && !picked_scanout) {
>                         next_plane = drm_output_prepare_scanout_view(output, ev);
> -               if (next_plane == NULL)
> +                       if (next_plane)
> +                               picked_scanout = true;
> +               }
> +
> +               /* Similarly, it's incorrect to elevate a view to an overlay if some higher-stacked
> +                * view is already identified as full-screen scanout.
> +                */
> +               if (next_plane == NULL && !picked_scanout)
>                         next_plane = drm_output_prepare_overlay_view(output, ev);
> +
>                 if (next_plane == NULL)
>                         next_plane = primary;
>
> --
> 2.13.3
>

Any opinion on this being a correct fix for the problem discussed
previously in master's plane assignment loop, Daniel?