[PATCH weston 08/11] compositor: Untangle mapedness from outputs

Giulio Camuffo giuliocamuffo at gmail.com
Thu Jun 23 12:41:57 UTC 2016


2016-06-18 19:15 GMT+02:00 Armin Krezović <krezovic.armin at gmail.com>:
> Currently, weston assumes a surface/view is mapped if
> it has an output assigned. In a zero outputs scenario,
> this isn't really desirable.
>
> This patch introduces a new flag to weston_surface and
> weston_view, which has to be set manually to indicate
> that a surface/view is mapped.
>
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
>  src/compositor.c  | 29 ++++++++++-------------------
>  src/compositor.h  |  4 ++++
>  src/data-device.c |  2 ++
>  src/input.c       |  2 ++
>  4 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index d246046..b25c671 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1549,19 +1549,13 @@ weston_view_set_mask_infinite(struct weston_view *view)
>  WL_EXPORT bool
>  weston_view_is_mapped(struct weston_view *view)
>  {
> -       if (view->output)
> -               return true;
> -       else
> -               return false;
> +       return view->is_mapped;
>  }
>
>  WL_EXPORT bool
>  weston_surface_is_mapped(struct weston_surface *surface)
>  {
> -       if (surface->output)
> -               return true;
> -       else
> -               return false;
> +       return surface->is_mapped;
>  }

I don't understand what this function means. It's here from before the
surface/view split happened, but right now i am not sure what the
purpose of it is, and i would argue it shuold go. How can a surface be
mapped if no views of it are mapped? Maybe you have a different
definition of "mapped" than me, in which case i would like to see it
as part of the documentation for these functions.
For the use-case Pekka mentioned, remembering if a surface was mapped
earlier and so retaining information like the position, all that can
be done by having custom logic in the shell_surface, which knows all
it needs to know about the surface and the views. We don't need the
surface mappedness for that.

>
>  static void
> @@ -1738,6 +1732,7 @@ weston_view_unmap(struct weston_view *view)
>         weston_view_damage_below(view);
>         view->output = NULL;
>         view->plane = NULL;
> +       view->is_mapped = false;
>         weston_layer_entry_remove(&view->layer_link);
>         wl_list_remove(&view->link);
>         wl_list_init(&view->link);
> @@ -1769,6 +1764,7 @@ weston_surface_unmap(struct weston_surface *surface)
>
>         wl_list_for_each(view, &surface->views, surface_link)
>                 weston_view_unmap(view);
> +       surface->is_mapped = false;
>         surface->output = NULL;
>  }
>
> @@ -2131,6 +2127,7 @@ view_list_add_subsurface_view(struct weston_compositor *compositor,
>
>         view->parent_view = parent;
>         weston_view_update_transform(view);
> +       view->is_mapped = true;
>
>         if (wl_list_empty(&sub->surface->subsurface_list)) {
>                 wl_list_insert(compositor->view_list.prev, &view->link);
> @@ -2152,6 +2149,7 @@ view_list_add(struct weston_compositor *compositor,
>         struct weston_subsurface *sub;
>
>         weston_view_update_transform(view);
> +       view->is_mapped = true;

If you always set is_mapped to true here, what is the difference to
just checking if the view is in the view_list?

>
>         if (wl_list_empty(&view->surface->subsurface_list)) {
>                 wl_list_insert(compositor->view_list.prev, &view->link);
> @@ -3244,7 +3242,6 @@ subsurface_get_label(struct weston_surface *surface, char *buf, size_t len)
>  static void
>  subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t dy)
>  {
> -       struct weston_compositor *compositor = surface->compositor;
>         struct weston_view *view;
>
>         wl_list_for_each(view, &surface->views, surface_link)
> @@ -3256,8 +3253,9 @@ subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t dy)
>          * mapped, parent is not in a visible layer, so this sub-surface
>          * will not be drawn either.
>          */
> -       if (!weston_surface_is_mapped(surface)) {
> -               struct weston_output *output;
> +
> +       if (!weston_surface_is_mapped(surface))
> +               surface->is_mapped = true;
>
>                 /* Cannot call weston_view_update_transform(),
>                  * because that would call it also for the parent surface,
> @@ -3265,18 +3263,11 @@ subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t dy)
>                  * inconsistent state, where the window could never be
>                  * mapped.
>                  *
> -                * Instead just assign any output, to make
> +                * Instead just force the is_mapped flag on, to make
>                  * weston_surface_is_mapped() return true, so that when the
>                  * parent surface does get mapped, this one will get
>                  * included, too. See view_list_add().
>                  */
> -               assert(!wl_list_empty(&compositor->output_list));
> -               output = container_of(compositor->output_list.next,
> -                                     struct weston_output, link);
> -
> -               surface->output = output;
> -               weston_surface_update_output_mask(surface, 1u << output->id);
> -       }
>  }
>
>  static struct weston_subsurface *
> diff --git a/src/compositor.h b/src/compositor.h
> index 9e5155c..8770982 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -964,6 +964,8 @@ struct weston_view {
>
>         /* Per-surface Presentation feedback flags, controlled by backend. */
>         uint32_t psf_flags;
> +
> +       bool is_mapped;
>  };
>
>  struct weston_surface_state {
> @@ -1082,6 +1084,8 @@ struct weston_surface {
>         const char *role_name;
>
>         struct weston_timeline_object timeline;
> +
> +       bool is_mapped;
>  };
>
>  struct weston_subsurface {
> diff --git a/src/data-device.c b/src/data-device.c
> index f04f030..44a08f9 100644
> --- a/src/data-device.c
> +++ b/src/data-device.c
> @@ -421,6 +421,8 @@ drag_surface_configure(struct weston_drag *drag,
>                 weston_layer_entry_insert(list, &drag->icon->layer_link);
>                 weston_view_update_transform(drag->icon);
>                 pixman_region32_clear(&es->pending.input);
> +               es->is_mapped = true;
> +               drag->icon->is_mapped = true;
>         }
>
>         drag->dx += sx;
> diff --git a/src/input.c b/src/input.c
> index 08378d1..5ccc75f 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -1950,6 +1950,8 @@ pointer_cursor_surface_configure(struct weston_surface *es,
>                 weston_layer_entry_insert(&es->compositor->cursor_layer.view_list,
>                                           &pointer->sprite->layer_link);
>                 weston_view_update_transform(pointer->sprite);
> +               es->is_mapped = true;
> +               pointer->sprite->is_mapped = true;
>         }
>  }
>
> --
> 2.9.0
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list