[PATCH weston 8/14] ivi-shell: added libweston-desktop-api implementation
Teyfel, Michael (ADITG/ESB)
mteyfel at de.adit-jv.com
Fri Oct 27 17:53:36 UTC 2017
Hi Quentin,
Thank you very much for your review. Unfortunately I had been and still am out of office, I plan to send an updated version of my patches by next week. Thanks again - the review of my patches is very appreciated.
Best regards
Michael Teyfel
Engineering Software Base (ADITG/ESB)
Tel. +49 5121 49 6932
-----Original Message-----
From: wayland-devel [mailto:wayland-devel-bounces at lists.freedesktop.org] On Behalf Of Quentin Glidic
Sent: Dienstag, 17. Oktober 2017 13:21
To: wayland-devel at lists.freedesktop.org
Cc: Teyfel, Michael (ADITG/ESB)
Subject: Re: [PATCH weston 8/14] ivi-shell: added libweston-desktop-api implementation
Hi,
I will limit my review to the libweston-desktop usage, as it’s the only part I know.
On 10/17/17 11:59 AM, Michael Teyfel wrote:
> Signed-off-by: Michael Teyfel <mteyfel at de.adit-jv.com>
> ---
> ivi-shell/ivi-shell.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 172 insertions(+)
>
> diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
> index 84db2c9..049aa43 100644
> --- a/ivi-shell/ivi-shell.c
> +++ b/ivi-shell/ivi-shell.c
> @@ -490,6 +490,178 @@ shell_add_bindings(struct weston_compositor *compositor,
> }
>
> /*
> + * libweston-desktop
> + */
> +
> +static void
> +desktop_surface_ping_timeout(struct weston_desktop_client *client,
> + void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_ping_timeout is not supported\n");
You can just let these functions to NULL, if you want, as only
surface_added and surface_removed are mandatory.
> +}
> +
> +static void
> +desktop_surface_pong(struct weston_desktop_client *client,
> + void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_pong is not supported\n");
> +}
> +
> +static void
> +desktop_surface_added(struct weston_desktop_surface *surface,
> + void *user_data)
> +{
> + struct ivi_shell *shell = (struct ivi_shell *) user_data;
> + struct ivi_layout_surface *layout_surface;
> + struct ivi_shell_surface *ivisurf;
> + struct weston_surface *weston_surf =
> + weston_desktop_surface_get_surface(surface);
> +
> + layout_surface = ivi_layout_desktop_surface_create(weston_surf,
> + IVI_INVALID_ID);
> + if (!layout_surface) {
> + return;
> + }
> +
> + layout_surface->weston_desktop_surface = surface;
> +
> + ivisurf = zalloc(sizeof *ivisurf);
> + if (!ivisurf) {
> + return;
> + }
> +
> + wl_list_init(&ivisurf->link);
> + wl_list_insert(&shell->ivi_surface_list, &ivisurf->link);
> +
> + ivisurf->shell = shell;
> + ivisurf->id_surface = IVI_INVALID_ID;
> +
> + ivisurf->width = 0;
> + ivisurf->height = 0;
> + ivisurf->layout_surface = layout_surface;
> + ivisurf->surface = weston_surf;
Here, you have a big issue. libweston-desktop handles popups internally,
but to do that, it needs to know when you create a view for a toplevel
surface. This is done via weston_desktop_surface_create_view(), which
you never call (I traced the full call stack to ivi_view_create()).
One solution is to change ivi_view_create() to check for
weston_surface_is_desktop_surface(), and calling
weston_desktop_surface_create_view() instead of weston_view_create() in
this case.
> +}
> +
> +static void
> +desktop_surface_removed(struct weston_desktop_surface *surface,
> + void *user_data)
> +{
> + struct ivi_shell *shell = (struct ivi_shell *) user_data;
> + struct ivi_shell_surface *ivisurf = NULL;
> +
> + wl_list_for_each(ivisurf, &shell->ivi_surface_list, link) {
> + if(ivisurf->layout_surface->weston_desktop_surface == surface)
This loop (and the same in committed) are not needed. Just use
weston_desktop_surface_set_user_data(surface, ivisurf); in surface_added
and use get_user_data() in other callbacks. you can check for NULL since
you have 1 code path that may let it NULL, but otherwise, surface_added
is guaranteed to be call before any other callback.
Thanks,
> + {
> + assert(ivisurf != NULL);
> +
> + if (ivisurf->layout_surface)
> + layout_surface_cleanup(ivisurf);
> +
> + break;
> + }
> + }
> +}
> +
> +static void
> +desktop_surface_committed(struct weston_desktop_surface *surface,
> + int32_t sx, int32_t sy, void *user_data)
> +{
> + struct ivi_shell_surface *ivisurf = NULL;
> + struct ivi_shell *shell = user_data;
> + struct weston_surface *weston_surf =
> + weston_desktop_surface_get_surface(surface);
> + int found = 0;
> +
> + wl_list_for_each(ivisurf, &shell->ivi_surface_list, link) {
> + if (ivisurf->surface == weston_surf) {
> + found = 1;
> + break;
> + }
> + }
> +
> + if(!found)
> + return;
> +
> + if (weston_surf->width == 0 || weston_surf->height == 0)
> + return;
> +
> + if (ivisurf->width != weston_surf->width ||
> + ivisurf->height != weston_surf->height) {
> + ivisurf->width = weston_surf->width;
> + ivisurf->height = weston_surf->height;
> +
> + ivi_layout_desktop_surface_configure(ivisurf->layout_surface,
> + weston_surf->width,
> + weston_surf->height);
> + }
> +}
> +
> +static void
> +desktop_surface_move(struct weston_desktop_surface *surface,
> + struct weston_seat *seat, uint32_t serial, void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_move is not supported\n");
> +}
> +
> +static void
> +desktop_surface_resize(struct weston_desktop_surface *surface,
> + struct weston_seat *seat, uint32_t serial,
> + enum weston_desktop_surface_edge edges, void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_resize is not supported\n");
> +}
> +
> +static void
> +desktop_surface_fullscreen_requested(struct weston_desktop_surface *surface,
> + bool fullscreen,
> + struct weston_output *output,
> + void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_fullscreen_requested is not supported\n");
> +}
> +
> +static void
> +desktop_surface_maximized_requested(struct weston_desktop_surface *surface,
> + bool maximized, void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_maximized_requested is not supported\n");
> +}
> +
> +static void
> +desktop_surface_minimized_requested(struct weston_desktop_surface *surface,
> + void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_minimized_requested is not supported\n");
> +}
> +
> +static void
> +desktop_surface_set_xwayland_position(struct weston_desktop_surface *surface,
> + int32_t x, int32_t y, void *user_data)
> +{
> + weston_log("ivi-shell: desktop_surface_set_xwayland_position is not supported\n");
> +}
> +
> +static const struct weston_desktop_api shell_desktop_api = {
> + .struct_size = sizeof(struct weston_desktop_api),
> + .ping_timeout = desktop_surface_ping_timeout,
> + .pong = desktop_surface_pong,
> + .surface_added = desktop_surface_added,
> + .surface_removed = desktop_surface_removed,
> + .committed = desktop_surface_committed,
> +
> + .move = desktop_surface_move,
> + .resize = desktop_surface_resize,
> + .fullscreen_requested = desktop_surface_fullscreen_requested,
> + .maximized_requested = desktop_surface_maximized_requested,
> + .minimized_requested = desktop_surface_minimized_requested,
> + .set_xwayland_position = desktop_surface_set_xwayland_position,
> +};
> +
> +/*
> + * end of libweston-desktop
> + */
> +
> +/*
> * Initialization of ivi-shell.
> */
> WL_EXPORT int
>
--
Quentin “Sardem FF7” Glidic
_______________________________________________
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