On Thu, 6 Mar 2014 18:56:58 +0900 Nobuhiko Tanibata <nobuhiko_tanib...@xddp.denso.co.jp> wrote:
> In-Vehicle Infotainment system traditionally manages surfaces with global > identification. A protocol, ivi_application, supports such a feature by > implementing a request, ivi_application::surface_creation defined in > ivi_application.xml. Additionally, it initialize a library, weston-layout, > to manage properties of surfaces and group surfaces in layer. In detail, > refer library, weston_layout. > > Signed-off-by: Nobuhiko Tanibata <nobuhiko_tanib...@xddp.denso.co.jp> > --- > ivi-shell/ivi-shell.c | 333 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 333 insertions(+) > create mode 100644 ivi-shell/ivi-shell.c > > diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c > new file mode 100644 > index 0000000..f4c4d25 > --- /dev/null > +++ b/ivi-shell/ivi-shell.c > @@ -0,0 +1,333 @@ > +/* > + * Copyright (C) 2013 DENSO CORPORATION > + * > + * Permission to use, copy, modify, distribute, and sell this software and > + * its documentation for any purpose is hereby granted without fee, provided > + * that the above copyright notice appear in all copies and that both that > + * copyright notice and this permission notice appear in supporting > + * documentation, and that the name of the copyright holders not be used in > + * advertising or publicity pertaining to distribution of the software > + * without specific, written prior permission. The copyright holders make > + * no representations about the suitability of this software for any > + * purpose. It is provided "as is" without express or implied warranty. > + * > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER > + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF > + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + */ > + > + > +/** > + * ivi-shell supports a type of shell for In-Vehicle Infotainment system. > + * In-Vehicle Infotainment system traditionally manages surfaces with global > + * identification. A protocol, ivi_application, supports such a feature > + * by implementing a request, ivi_application::surface_creation defined in > + * ivi_application.xml. > + * > + * Additionally, it initialize a library, weston-layout, to manage > properties of > + * surfaces and group surfaces in layer. In detail, refer weston_layout. > + */ > + > +#include <sys/wait.h> > +#include <unistd.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <string.h> > +#include <linux/input.h> > + > +#include "compositor.h" > +#include "ivi-application-server-protocol.h" > +#include "weston-layout.h" weston-layout.h is introduced in the following patch, so this patch won't build, breaking bisectability if this series was applied. > + > +struct ivi_shell; > + > +struct ivi_shell_surface > +{ > + struct ivi_shell *shell; > + struct weston_layout_surface *layout_surface; > + > + struct weston_surface *surface; > + uint32_t id_surface; > + > + int32_t width; > + int32_t height; > + > + struct wl_list link; > +}; > + > +struct ivi_shell > +{ > + struct wl_resource *resource; > + struct wl_listener destroy_listener; > + > + struct weston_compositor *compositor; > + struct weston_seat *seat; 'compositor' and 'seat' seem pretty unused? > + > + struct wl_list list_surface; We prefer the convention <something>_list instead. Personally, I've also added a comment about what is in this list, like /* ivi_shell_surface::link */ because that is an invariant. > +}; > + > +/* ------------------------------------------------------------------------- > */ > +/* common functions > */ > +/* ------------------------------------------------------------------------- > */ > + > +static void > +configure(struct weston_view *view, float x, float y) > +{ > + if (view != NULL) { > + weston_view_set_position(view, x, y); > + weston_view_update_transform(view); Something in this feels odd... the part about calling weston_view_update_transform() manually... is that ok, or should it be called only when mapping the surface? > + } > +} > + > +/** > + * Implementation of ivi_surface > + */ > + > +static void > +ivi_shell_surface_configure(struct weston_surface *, int32_t, int32_t); > + > +static struct ivi_shell_surface * > +get_ivi_shell_surface(struct weston_surface *surface) > +{ > + if (surface->configure == ivi_shell_surface_configure) { > + return surface->configure_private; > + } else { > + return NULL; > + } > +} > + > +static void > +ivi_shell_surface_configure(struct weston_surface *es, > + int32_t sx, int32_t sy) > +{ > + struct ivi_shell_surface *ivisurf = get_ivi_shell_surface(es); > + struct weston_view *view = NULL; > + float from_x = 0.0f; > + float from_y = 0.0f; > + float to_x = 0.0f; > + float to_y = 0.0f; > + > + if ((es->width == 0) || (es->height == 0) || (ivisurf == NULL)) { > + return; > + } > + > + view = weston_layout_get_weston_view(ivisurf->layout_surface); > + if (view == NULL) { > + return; > + } > + > + if (ivisurf->width != es->width || ivisurf->height != es->height) { > + > + ivisurf->width = es->width; > + ivisurf->height = es->height; > + > + weston_view_to_global_float(view, 0, 0, &from_x, &from_y); > + weston_view_to_global_float(view, sx, sy, &to_x, &to_y); > + configure(view, > + view->geometry.x + to_x - from_x, > + view->geometry.y + to_y - from_y); > + > + weston_layout_surfaceConfigure(ivisurf->layout_surface, es->width, > es->height); > + } > +} > + > +static void > +surface_destroy(struct wl_client *client, > + struct wl_resource *resource) > +{ > + struct ivi_shell_surface *ivisurf = wl_resource_get_user_data(resource); > + struct ivi_shell *shell = ivisurf->shell; > + > + weston_layout_surfaceRemove(ivisurf->layout_surface); > + > + ivisurf->surface->configure = NULL; > + ivisurf->surface->configure_private = NULL; > + > + if (!wl_list_empty(&ivisurf->link)) { > + wl_list_remove(&ivisurf->link); > + } > + wl_list_init(&shell->list_surface); > + > + free(ivisurf); > + ivisurf = NULL; Local variable. I just realized that your protocol spec does not define what should happen on destruction. > +} > + > +static const struct ivi_surface_interface surface_implementation = { > + surface_destroy, > +}; > + > +static struct ivi_shell_surface * > +is_surf_in_surfaces(struct wl_list *list_surf, uint32_t id_surface) > +{ > + struct ivi_shell_surface *ivisurf; > + > + wl_list_for_each(ivisurf, list_surf, link) { > + if (ivisurf->id_surface == id_surface) { > + return ivisurf; > + } > + } > + > + return NULL; > +} > + > +/** > + * Implementation of ivi_application::surface_create. > + * Creating new ivi_shell_surface with identification to identify the surface > + * in the system. > + */ > +static void > +application_surface_create(struct wl_client *client, > + struct wl_resource *resource, > + uint32_t id_surface, > + struct wl_resource *surface_resource, > + uint32_t id) > +{ > + struct ivi_shell *shell = wl_resource_get_user_data(resource); > + struct weston_layout_surface *layout_surface = NULL; > + struct ivi_shell_surface *ivisurf = NULL; > + struct weston_surface *es = NULL; > + > + es = wl_resource_get_user_data(surface_resource); > + if (es == NULL) { > + weston_log("application_surface_create: invalid surface\n"); > + return; > + } > + > + ivisurf = is_surf_in_surfaces(&shell->list_surface, id_surface); > + > + layout_surface = weston_layout_surfaceCreate(es, id_surface); > + if (layout_surface == NULL) { > +//FIXME Forgot something? Seems like double-checking the same thing, slightly confusing. Avoid calling weston_layout_surfaceCreate() to begin with, if (ivisurf)? > + weston_log("id_surface is already created\n"); > + > + ivisurf->surface = es; > + ivisurf->width = 0; > + ivisurf->height = 0; > + > + es->configure = ivi_shell_surface_configure; > + es->configure_private = ivisurf; > + > + resource = wl_resource_create(client, &ivi_surface_interface, 1, id); > + if (resource == NULL) { > + weston_log("couldn't get surface object"); > + return; > + } > + > + wl_resource_set_implementation(resource, > + &surface_implementation, > + ivisurf, NULL); Does this mean, that if a client calls ivi_application.surface_create with an already used ID, it will get a handle to the existing ivi_shell_surface? Is that supposed to work? If yes, then ivi_shell_surface needs a list of all wl_resources pointing to it. IDs are global, aren't they? So it could get a handle to some *other* client's ivi_shell_surface. That doesn't sound good. > + return; > + } > + > + ivisurf = calloc(1, sizeof *ivisurf); > + if (ivisurf == NULL) { > + weston_log("fails to allocate memory\n"); > + return; > + } > + > + wl_list_init(&ivisurf->link); > + ivisurf->shell = shell; > + ivisurf->layout_surface = layout_surface; > + ivisurf->surface = es; > + ivisurf->id_surface = id_surface; > + ivisurf->width = 0; > + ivisurf->height = 0; > + > + es->configure = ivi_shell_surface_configure; > + es->configure_private = ivisurf; > + > + wl_list_insert(&shell->list_surface, &ivisurf->link); > + > + resource = wl_resource_create(client, &ivi_surface_interface, 1, id); > + if (resource == NULL) { > + weston_log("couldn't get surface object"); > + return; > + } > + > + wl_resource_set_implementation(resource, > + &surface_implementation, > + ivisurf, NULL); Are you sure you don't need a destructor here? If the client disconnects without explicitly destroying all its protocol objects, how does ivisurf get freed? > +} > + > +static const struct ivi_application_interface application_implementation = { > + application_surface_create > +}; > + > +static void > +bind_ivi_application(struct wl_client *client, > + void *data, uint32_t version, uint32_t id) > +{ > + struct ivi_shell *shell = data; > + struct wl_resource *resource = NULL; > + > + resource = wl_resource_create(client, &ivi_application_interface, 1, id); > + > + wl_resource_set_implementation(resource, > + &application_implementation, > + shell, NULL); > +} > + > +/** > + * Initialization/destruction method of ivi-shell > + */ > +static void > +shell_destroy(struct wl_listener *listener, void *data) > +{ > + struct ivi_shell *shell = NULL; > + > + shell = container_of(listener, struct ivi_shell, destroy_listener); > + > + free(shell); > + shell = NULL; Nulling a local variable again. I think you should do some sanity/leak checks here, like ensuring that the surface list is empty, no? > +} > + > +static void > +init_ivi_shell(struct weston_compositor *ec, struct ivi_shell *shell) > +{ > + shell->compositor = ec; > + > + wl_list_init(&ec->layer_list); > + wl_list_init(&shell->list_surface); > +} > + > +/** > + * Initialization of ivi-shell. A library, weston_layout, is also initialized > + * here by calling weston_layout_initWithCompositor. > + * > + */ > + > +WL_EXPORT int > +module_init(struct weston_compositor *ec, > + int *argc, char *argv[]) > +{ > + struct ivi_shell *shell = NULL; > + struct weston_seat *seat = NULL; > + > + shell = calloc(1, sizeof *shell); > + if (shell == NULL) { > + return -1; > + } > + > + init_ivi_shell(ec, shell); > + weston_layout_initWithCompositor(ec); It's kind of surprising... this module's init requires another module to have been loaded before, otherwise this module will not load, right? I wonder if you get a sensible error message when that happens. > + > + shell->destroy_listener.notify = shell_destroy; > + wl_signal_add(&ec->destroy_signal, &shell->destroy_listener); > + > + if (wl_global_create(ec->wl_display, &ivi_application_interface, 1, > + shell, bind_ivi_application) == NULL) { > + return -1; > + } > + > + wl_list_for_each(seat, &ec->seat_list, link) { > + if (seat) { > + shell->seat = seat; 'seat' cannot be NULL here. It seems you are assuming that there can be only one seat? Why? > + } > + } > + > + return 0; > +} Thanks, pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel