[weston,1/2] compositor: add weston_surface_set_description()

Submitted by Pekka Paalanen on Nov. 27, 2014, 7:07 a.m.

Details

Message ID 1417072040-15171-1-git-send-email-ppaalanen@gmail.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Pekka Paalanen Nov. 27, 2014, 7:07 a.m.
From: Pekka Paalanen <pq@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@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(+)

Patch hide | download patch | download mbox

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,
+			       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;

Comments

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@gmail.com>:
> From: Pekka Paalanen <pq@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
On Thu, 27 Nov 2014 10:17:21 +0200
Giulio Camuffo <giuliocamuffo@gmail.com> wrote:

> 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@gmail.com>:
> > From: Pekka Paalanen <pq@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@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/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().

Yes, weston_surface_set_get_description_func() it would be indeed, but
it was getting a bit long, especially when the argument is usually
a fairly long word too.

Descriptor smells a bit like an fd thing, though.

weston_surface_set_... eh... umm

What if I did s/description/label/g?

So we'd have e.g.:
weston_surface_set_label_func(surface, shell_surface_get_label)
and weston_surface::get_label field.

How's that?


Thanks,
pq


> 
> > +                              int (*desc)(struct weston_surface *,
> > +                                          char *, size_t))
> > +{
> > +       surface->get_description = desc;
> > +}
> > +
2014-11-27 12:42 GMT+02:00 Pekka Paalanen <ppaalanen@gmail.com>:
> On Thu, 27 Nov 2014 10:17:21 +0200
> Giulio Camuffo <giuliocamuffo@gmail.com> wrote:
>
>> 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@gmail.com>:
>> > From: Pekka Paalanen <pq@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@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/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().
>
> Yes, weston_surface_set_get_description_func() it would be indeed, but
> it was getting a bit long, especially when the argument is usually
> a fairly long word too.
>
> Descriptor smells a bit like an fd thing, though.
>
> weston_surface_set_... eh... umm
>
> What if I did s/description/label/g?
>
> So we'd have e.g.:
> weston_surface_set_label_func(surface, shell_surface_get_label)
> and weston_surface::get_label field.
>
> How's that?

I think that's ok.

>
>
> Thanks,
> pq
>
>
>>
>> > +                              int (*desc)(struct weston_surface *,
>> > +                                          char *, size_t))
>> > +{
>> > +       surface->get_description = desc;
>> > +}
>> > +