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:[email protected]] On Behalf Of Quentin Glidic Sent: Dienstag, 17. Oktober 2017 13:21 To: [email protected] 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 <[email protected]> > --- > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
