[PATCH weston 1/2] compositor: add weston_surface_set_description()
Giulio Camuffo
giuliocamuffo at gmail.com
Thu Nov 27 00:17:21 PST 2014
Just a quick comment below, I haven't looked at it carefully yet.
--
Giulio
2014-11-27 9:07 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> From: Pekka Paalanen <pq at iki.fi>
>
> When printing out logs from Weston's actions, mainly for debugging, it
> can be very difficult to identify the different surfaces. Inspecting
> the configure function pointer is not useful, as the configure functions
> may live in modules.
>
> Add vfunc get_description to weston_surface, which will produce a short,
> human-readable description of the surface, which allows identifying it
> better, rather than just looking at the surface size, for instance.
>
> Set the description from most parts of Weston, to identify cursors and
> drag icons, and panels, backgrounds, screensavers and lock surfaces, and
> the desktop shell's application surfaces.
>
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
> desktop-shell/input-panel.c | 9 ++++
> desktop-shell/shell.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
> src/compositor.c | 17 ++++++++
> src/compositor.h | 7 +++
> src/data-device.c | 19 +++++++++
> src/input.c | 10 +++++
> 6 files changed, 163 insertions(+)
>
> diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
> index 0b42c2f..d699c47 100644
> --- a/desktop-shell/input-panel.c
> +++ b/desktop-shell/input-panel.c
> @@ -150,6 +150,13 @@ update_input_panels(struct wl_listener *listener, void *data)
> memcpy(&shell->text_input.cursor_rectangle, data, sizeof(pixman_box32_t));
> }
>
> +static int
> +input_panel_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "input panel");
> +}
> +
> static void
> input_panel_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
> {
> @@ -187,6 +194,7 @@ destroy_input_panel_surface(struct input_panel_surface *input_panel_surface)
> wl_list_remove(&input_panel_surface->link);
>
> input_panel_surface->surface->configure = NULL;
> + weston_surface_set_description(input_panel_surface->surface, NULL);
> weston_view_destroy(input_panel_surface->view);
>
> free(input_panel_surface);
> @@ -228,6 +236,7 @@ create_input_panel_surface(struct desktop_shell *shell,
>
> surface->configure = input_panel_configure;
> surface->configure_private = input_panel_surface;
> + weston_surface_set_description(surface, input_panel_get_description);
>
> input_panel_surface->shell = shell;
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 9e8d45a..22e1b25 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -274,6 +274,32 @@ static void
> shell_surface_set_parent(struct shell_surface *shsurf,
> struct weston_surface *parent);
>
> +static int
> +shell_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + struct shell_surface *shsurf;
> + const char *typestr[] = {
> + [SHELL_SURFACE_NONE] = "unidentified",
> + [SHELL_SURFACE_TOPLEVEL] = "top-level",
> + [SHELL_SURFACE_POPUP] = "popup",
> + [SHELL_SURFACE_XWAYLAND] = "Xwayland",
> + };
> + const char *t, *c;
> +
> + shsurf = get_shell_surface(surface);
> + if (!shsurf)
> + return snprintf(buf, len, "unidentified window");
> +
> + t = shsurf->title;
> + c = shsurf->class;
> +
> + return snprintf(buf, len, "%s window%s%s%s%s%s",
> + typestr[shsurf->type],
> + t ? " '" : "", t ?: "", t ? "'" : "",
> + c ? " of " : "", c ?: "");
> +}
> +
> static bool
> shell_surface_is_top_fullscreen(struct shell_surface *shsurf)
> {
> @@ -634,6 +660,13 @@ get_default_output(struct weston_compositor *compositor)
> struct weston_output, link);
> }
>
> +static int
> +focus_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "focus highlight effect for output %s",
> + surface->output->name);
> +}
>
> /* no-op func for checking focus surface */
> static void
> @@ -683,6 +716,7 @@ create_focus_surface(struct weston_compositor *ec,
> surface->configure = focus_surface_configure;
> surface->output = output;
> surface->configure_private = fsurf;
> + weston_surface_set_description(surface, focus_surface_get_description);
>
> fsurf->view = weston_view_create(surface);
> if (fsurf->view == NULL) {
> @@ -2741,6 +2775,34 @@ shell_surface_get_shell(struct shell_surface *shsurf)
> return shsurf->shell;
> }
>
> +static int
> +black_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + struct weston_surface *fs_surface = surface->configure_private;
> + int n;
> + int rem;
> + int ret;
> +
> + n = snprintf(buf, len, "black background surface for ");
> + if (n < 0)
> + return n;
> +
> + rem = (int)len - n;
> + if (rem < 0)
> + rem = 0;
> +
> + if (fs_surface->get_description)
> + ret = fs_surface->get_description(fs_surface, buf + n, rem);
> + else
> + ret = snprintf(buf + n, rem, "<unknown>");
> +
> + if (ret < 0)
> + return n;
> +
> + return n + ret;
> +}
> +
> static void
> black_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy);
>
> @@ -2766,6 +2828,7 @@ create_black_surface(struct weston_compositor *ec,
>
> surface->configure = black_surface_configure;
> surface->configure_private = fs_surface;
> + weston_surface_set_description(surface, black_surface_get_description);
> weston_surface_set_color(surface, 0.0, 0.0, 0.0, 1);
> pixman_region32_fini(&surface->opaque);
> pixman_region32_init_rect(&surface->opaque, 0, 0, w, h);
> @@ -3373,6 +3436,7 @@ destroy_shell_surface(struct shell_surface *shsurf)
> */
> wl_list_remove(&shsurf->surface_destroy_listener.link);
> shsurf->surface->configure = NULL;
> + weston_surface_set_description(shsurf->surface, NULL);
> free(shsurf->title);
>
> weston_view_destroy(shsurf->view);
> @@ -3476,6 +3540,7 @@ create_common_surface(struct shell_client *owner, void *shell,
>
> surface->configure = shell_surface_configure;
> surface->configure_private = shsurf;
> + weston_surface_set_description(surface, shell_surface_get_description);
>
> shsurf->resource_destroy_listener.notify = handle_resource_destroy;
> wl_resource_add_destroy_listener(surface->resource,
> @@ -4143,6 +4208,7 @@ configure_static_view(struct weston_view *ev, struct weston_layer *layer)
> if (v->output == ev->output && v != ev) {
> weston_view_unmap(v);
> v->surface->configure = NULL;
> + weston_surface_set_description(v->surface, NULL);
> }
> }
>
> @@ -4154,6 +4220,14 @@ configure_static_view(struct weston_view *ev, struct weston_layer *layer)
> }
> }
>
> +static int
> +background_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "background for output %s",
> + surface->output->name);
> +}
> +
> static void
> background_configure(struct weston_surface *es, int32_t sx, int32_t sy)
> {
> @@ -4189,6 +4263,7 @@ desktop_shell_set_background(struct wl_client *client,
>
> surface->configure = background_configure;
> surface->configure_private = shell;
> + weston_surface_set_description(surface, background_get_description);
> surface->output = wl_resource_get_user_data(output_resource);
> view->output = surface->output;
> desktop_shell_send_configure(resource, 0,
> @@ -4197,6 +4272,14 @@ desktop_shell_set_background(struct wl_client *client,
> surface->output->height);
> }
>
> +static int
> +panel_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "panel for output %s",
> + surface->output->name);
> +}
> +
> static void
> panel_configure(struct weston_surface *es, int32_t sx, int32_t sy)
> {
> @@ -4232,6 +4315,7 @@ desktop_shell_set_panel(struct wl_client *client,
>
> surface->configure = panel_configure;
> surface->configure_private = shell;
> + weston_surface_set_description(surface, panel_get_description);
> surface->output = wl_resource_get_user_data(output_resource);
> view->output = surface->output;
> desktop_shell_send_configure(resource, 0,
> @@ -4240,6 +4324,13 @@ desktop_shell_set_panel(struct wl_client *client,
> surface->output->height);
> }
>
> +static int
> +lock_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "lock window");
> +}
> +
> static void
> lock_surface_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
> {
> @@ -4294,6 +4385,7 @@ desktop_shell_set_lock_surface(struct wl_client *client,
> weston_view_create(surface);
> surface->configure = lock_surface_configure;
> surface->configure_private = shell;
> + weston_surface_set_description(surface, lock_surface_get_description);
> }
>
> static void
> @@ -5657,6 +5749,14 @@ bind_desktop_shell(struct wl_client *client,
> "permission to bind desktop_shell denied");
> }
>
> +static int
> +screensaver_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "screensaver for output %s",
> + surface->output->name);
> +}
> +
> static void
> screensaver_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
> {
> @@ -5704,6 +5804,7 @@ screensaver_set_surface(struct wl_client *client,
>
> surface->configure = screensaver_configure;
> surface->configure_private = shell;
> + weston_surface_set_description(surface, screensaver_get_description);
> surface->output = output;
> }
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 17e930c..ea5cc14 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2736,6 +2736,13 @@ weston_subsurface_parent_commit(struct weston_subsurface *sub,
> weston_subsurface_synchronized_commit(sub);
> }
>
> +static int
> +subsurface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "sub-surface");
> +}
> +
> static void
> subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t dy)
> {
> @@ -2819,6 +2826,14 @@ weston_surface_set_role(struct weston_surface *surface,
> return -1;
> }
>
> +WL_EXPORT void
> +weston_surface_set_description(struct weston_surface *surface,
I find the naming a bit confusing, I'd expect this to set the
description of the surface, that is a char*. The get_description vfunc
on the other hand doesn't return the function passed here, but the
char*. I'd call it something like
weston_surface_set_description_func(), or maybe
weston_surface_set_descriptor().
> + int (*desc)(struct weston_surface *,
> + char *, size_t))
> +{
> + surface->get_description = desc;
> +}
> +
> static void
> subsurface_set_position(struct wl_client *client,
> struct wl_resource *resource, int32_t x, int32_t y)
> @@ -3051,6 +3066,7 @@ weston_subsurface_destroy(struct weston_subsurface *sub)
>
> sub->surface->configure = NULL;
> sub->surface->configure_private = NULL;
> + weston_surface_set_description(sub->surface, NULL);
> } else {
> /* the dummy weston_subsurface for the parent itself */
> assert(sub->parent_destroy_listener.notify == NULL);
> @@ -3182,6 +3198,7 @@ subcompositor_get_subsurface(struct wl_client *client,
>
> surface->configure = subsurface_configure;
> surface->configure_private = sub;
> + weston_surface_set_description(surface, subsurface_get_description);
> }
>
> static void
> diff --git a/src/compositor.h b/src/compositor.h
> index 2bec183..e49e597 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -898,6 +898,8 @@ struct weston_surface {
> */
> void (*configure)(struct weston_surface *es, int32_t sx, int32_t sy);
> void *configure_private;
> + int (*get_description)(struct weston_surface *surface,
> + char *buf, size_t len);
>
> /* Parent's list of its sub-surfaces, weston_subsurface:parent_link.
> * Contains also the parent itself as a dummy weston_subsurface,
> @@ -1243,6 +1245,11 @@ weston_surface_set_role(struct weston_surface *surface,
> struct wl_resource *error_resource,
> uint32_t error_code);
>
> +void
> +weston_surface_set_description(struct weston_surface *surface,
> + int (*desc)(struct weston_surface *,
> + char *, size_t));
> +
> struct weston_buffer *
> weston_buffer_from_resource(struct wl_resource *resource);
>
> diff --git a/src/data-device.c b/src/data-device.c
> index d28325d..8ef10a5 100644
> --- a/src/data-device.c
> +++ b/src/data-device.c
> @@ -214,6 +214,13 @@ drag_surface_configure(struct weston_drag *drag,
> weston_view_set_position(drag->icon, fx, fy);
> }
>
> +static int
> +pointer_drag_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "pointer drag icon");
> +}
> +
> static void
> pointer_drag_surface_configure(struct weston_surface *es,
> int32_t sx, int32_t sy)
> @@ -226,6 +233,13 @@ pointer_drag_surface_configure(struct weston_surface *es,
> drag_surface_configure(&drag->base, pointer, NULL, es, sx, sy);
> }
>
> +static int
> +touch_drag_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "touch drag icon");
> +}
> +
> static void
> touch_drag_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy)
> {
> @@ -351,6 +365,7 @@ data_device_end_drag_grab(struct weston_drag *drag,
> weston_view_unmap(drag->icon);
>
> drag->icon->surface->configure = NULL;
> + weston_surface_set_description(drag->icon->surface, NULL);
> pixman_region32_clear(&drag->icon->surface->pending.input);
> wl_list_remove(&drag->icon_destroy_listener.link);
> weston_view_destroy(drag->icon);
> @@ -560,6 +575,8 @@ weston_pointer_start_drag(struct weston_pointer *pointer,
>
> icon->configure = pointer_drag_surface_configure;
> icon->configure_private = drag;
> + weston_surface_set_description(icon,
> + pointer_drag_surface_get_description);
> } else {
> drag->base.icon = NULL;
> }
> @@ -615,6 +632,8 @@ weston_touch_start_drag(struct weston_touch *touch,
>
> icon->configure = touch_drag_surface_configure;
> icon->configure_private = drag;
> + weston_surface_set_description(icon,
> + touch_drag_surface_get_description);
> } else {
> drag->base.icon = NULL;
> }
> diff --git a/src/input.c b/src/input.c
> index 15ff6ed..b2f0b0e 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -431,6 +431,7 @@ pointer_unmap_sprite(struct weston_pointer *pointer)
> wl_list_remove(&pointer->sprite_destroy_listener.link);
> surface->configure = NULL;
> surface->configure_private = NULL;
> + weston_surface_set_description(surface, NULL);
> weston_view_destroy(pointer->sprite);
> pointer->sprite = NULL;
> }
> @@ -1583,6 +1584,13 @@ notify_touch_frame(struct weston_seat *seat)
> grab->interface->frame(grab);
> }
>
> +static int
> +pointer_cursor_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "cursor");
> +}
> +
> static void
> pointer_cursor_surface_configure(struct weston_surface *es,
> int32_t dx, int32_t dy)
> @@ -1654,6 +1662,8 @@ pointer_set_cursor(struct wl_client *client, struct wl_resource *resource,
>
> surface->configure = pointer_cursor_surface_configure;
> surface->configure_private = pointer;
> + weston_surface_set_description(surface,
> + pointer_cursor_surface_get_description);
> pointer->sprite = weston_view_create(surface);
> pointer->hotspot_x = x;
> pointer->hotspot_y = y;
> --
> 2.0.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list