[weston,v5,09/14] xwm: convert WM_DEBUG into a weston-debug scope

Submitted by Daniel Stone on July 20, 2018, 7:03 p.m.

Details

Message ID 20180720190335.23880-10-daniels@collabora.com
State Accepted
Commit 9b72eb7930ef60aa44ab49c53b2aec9e7242cf1c
Headers show
Series "weston-debug API and tool" ( rev: 2 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Daniel Stone July 20, 2018, 7:03 p.m.
From: Pekka Paalanen <pq@iki.fi>

Instead of a compile time choice, offer the XWM debugging messages
through the weston-debug protocol and tool on demand. Users will not
need to recompile weston to get XWM debugging, and it won't flood the
weston log.

The debug scope needs to be initialized in launcher.c for it be
available from start, before the first X11 client tries to connect and
initializes XWM.

Signed-off-by: Pekka Paalanen <pq@iki.fi>

pass the wm_debug scope to weston_debug_scope_printf API to append
the scopename to the timestr

Signed-off-by: Maniraj Devadoss <Maniraj.Devadoss@in.bosch.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
---
 xwayland/launcher.c       |   7 ++
 xwayland/window-manager.c | 166 ++++++++++++++++++++------------------
 xwayland/xwayland.h       |   3 +
 3 files changed, 99 insertions(+), 77 deletions(-)

Patch hide | download patch | download mbox

diff --git a/xwayland/launcher.c b/xwayland/launcher.c
index 0ecdb205e..c5b993851 100644
--- a/xwayland/launcher.c
+++ b/xwayland/launcher.c
@@ -229,6 +229,8 @@  weston_xserver_destroy(struct wl_listener *l, void *data)
 	if (wxs->loop)
 		weston_xserver_shutdown(wxs);
 
+	weston_debug_scope_destroy(wxs->wm_debug);
+
 	free(wxs);
 }
 
@@ -391,5 +393,10 @@  weston_module_init(struct weston_compositor *compositor)
 	wxs->destroy_listener.notify = weston_xserver_destroy;
 	wl_signal_add(&compositor->destroy_signal, &wxs->destroy_listener);
 
+	wxs->wm_debug = weston_compositor_add_debug_scope(wxs->compositor,
+			"xwm-wm-x11",
+			"XWM's window management X11 events\n",
+			NULL, NULL);
+
 	return 0;
 }
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 4a26f6e7f..ccdae57fd 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -193,23 +193,27 @@  static void
 xserver_map_shell_surface(struct weston_wm_window *window,
 			  struct weston_surface *surface);
 
-static int __attribute__ ((format (printf, 1, 2)))
-wm_log(const char *fmt, ...)
+static bool
+wm_debug_is_enabled(struct weston_wm *wm)
 {
-#ifdef WM_DEBUG
-	int l;
-	va_list argp;
+	return weston_debug_scope_is_enabled(wm->server->wm_debug);
+}
 
-	va_start(argp, fmt);
-	l = weston_vlog(fmt, argp);
-	va_end(argp);
+static void __attribute__ ((format (printf, 2, 3)))
+wm_printf(struct weston_wm *wm, const char *fmt, ...)
+{
+	va_list ap;
+	char timestr[128];
 
-	return l;
-#else
-	return 0;
-#endif
-}
+	if (wm_debug_is_enabled(wm))
+		weston_debug_scope_printf(wm->server->wm_debug, "%s ",
+				weston_debug_scope_timestamp(wm->server->wm_debug,
+				timestr, sizeof timestr));
 
+	va_start(ap, fmt);
+	weston_debug_scope_vprintf(wm->server->wm_debug, fmt, ap);
+	va_end(ap);
+}
 static void
 weston_output_weak_ref_init(struct weston_output_weak_ref *ref)
 {
@@ -717,10 +721,10 @@  weston_wm_handle_configure_request(struct weston_wm *wm, xcb_generic_event_t *ev
 	uint32_t mask, values[16];
 	int x, y, width, height, i = 0;
 
-	wm_log("XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n",
-	       configure_request->window,
-	       configure_request->x, configure_request->y,
-	       configure_request->width, configure_request->height);
+	wm_printf(wm, "XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n",
+		  configure_request->window,
+		  configure_request->x, configure_request->y,
+		  configure_request->width, configure_request->height);
 
 	if (!wm_lookup_window(wm, configure_request->window, &window))
 		return;
@@ -786,11 +790,11 @@  weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t *eve
 		wm->server->compositor->xwayland_interface;
 	struct weston_wm_window *window;
 
-	wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
-	       configure_notify->window,
-	       configure_notify->x, configure_notify->y,
-	       configure_notify->width, configure_notify->height,
-	       configure_notify->override_redirect ? ", override" : "");
+	wm_printf(wm, "XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
+		  configure_notify->window,
+		  configure_notify->x, configure_notify->y,
+		  configure_notify->width, configure_notify->height,
+		  configure_notify->override_redirect ? ", override" : "");
 
 	if (!wm_lookup_window(wm, configure_notify->window, &window))
 		return;
@@ -839,7 +843,7 @@  weston_wm_create_surface(struct wl_listener *listener, void *data)
 	if (wl_resource_get_client(surface->resource) != wm->server->client)
 		return;
 
-	wm_log("XWM: create weston_surface %p\n", surface);
+	wm_printf(wm, "XWM: create weston_surface %p\n", surface);
 
 	wl_list_for_each(window, &wm->unpaired_window_list, link)
 		if (window->surface_id ==
@@ -1096,8 +1100,8 @@  weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event)
 	struct weston_output *output;
 
 	if (our_resource(wm, map_request->window)) {
-		wm_log("XCB_MAP_REQUEST (window %d, ours)\n",
-		       map_request->window);
+		wm_printf(wm, "XCB_MAP_REQUEST (window %d, ours)\n",
+			  map_request->window);
 		return;
 	}
 
@@ -1126,10 +1130,10 @@  weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event)
 		weston_wm_window_create_frame(window); /* sets frame_id */
 	assert(window->frame_id != XCB_WINDOW_NONE);
 
-	wm_log("XCB_MAP_REQUEST (window %d, %p, frame %d, %dx%d @ %d,%d)\n",
-	       window->id, window, window->frame_id,
-	       window->width, window->height,
-	       window->map_request_x, window->map_request_y);
+	wm_printf(wm, "XCB_MAP_REQUEST (window %d, %p, frame %d, %dx%d @ %d,%d)\n",
+		  window->id, window, window->frame_id,
+		  window->width, window->height,
+		  window->map_request_x, window->map_request_y);
 
 	weston_wm_window_set_allow_commits(window, false);
 	weston_wm_window_set_wm_state(window, ICCCM_NORMAL_STATE);
@@ -1157,13 +1161,13 @@  weston_wm_handle_map_notify(struct weston_wm *wm, xcb_generic_event_t *event)
 	xcb_map_notify_event_t *map_notify = (xcb_map_notify_event_t *) event;
 
 	if (our_resource(wm, map_notify->window)) {
-		wm_log("XCB_MAP_NOTIFY (window %d, ours)\n",
-		       map_notify->window);
+		wm_printf(wm, "XCB_MAP_NOTIFY (window %d, ours)\n",
+			  map_notify->window);
 			return;
 	}
 
-	wm_log("XCB_MAP_NOTIFY (window %d%s)\n", map_notify->window,
-	       map_notify->override_redirect ? ", override" : "");
+	wm_printf(wm, "XCB_MAP_NOTIFY (window %d%s)\n", map_notify->window,
+		  map_notify->override_redirect ? ", override" : "");
 }
 
 static void
@@ -1173,10 +1177,10 @@  weston_wm_handle_unmap_notify(struct weston_wm *wm, xcb_generic_event_t *event)
 		(xcb_unmap_notify_event_t *) event;
 	struct weston_wm_window *window;
 
-	wm_log("XCB_UNMAP_NOTIFY (window %d, event %d%s)\n",
-	       unmap_notify->window,
-	       unmap_notify->event,
-	       our_resource(wm, unmap_notify->window) ? ", ours" : "");
+	wm_printf(wm, "XCB_UNMAP_NOTIFY (window %d, event %d%s)\n",
+		  unmap_notify->window,
+		  unmap_notify->event,
+		  our_resource(wm, unmap_notify->window) ? ", ours" : "");
 
 	if (our_resource(wm, unmap_notify->window))
 		return;
@@ -1216,7 +1220,7 @@  weston_wm_window_draw_decoration(struct weston_wm_window *window)
 	cairo_t *cr;
 	int width, height;
 
-	wm_log("XWM: draw decoration, win %d\n", window->id);
+	wm_printf(window->wm, "XWM: draw decoration, win %d\n", window->id);
 
 	weston_wm_window_get_frame_size(window, &width, &height);
 
@@ -1279,8 +1283,8 @@  weston_wm_window_set_pending_state(struct weston_wm_window *window)
 		input_h = height;
 	}
 
-	wm_log("XWM: win %d geometry: %d,%d %dx%d\n",
-	       window->id, input_x, input_y, input_w, input_h);
+	wm_printf(window->wm, "XWM: win %d geometry: %d,%d %dx%d\n",
+		  window->id, input_x, input_y, input_w, input_h);
 
 	pixman_region32_fini(&window->surface->pending.input);
 	pixman_region32_init_rect(&window->surface->pending.input,
@@ -1347,7 +1351,7 @@  weston_wm_window_schedule_repaint(struct weston_wm_window *window)
 	if (window->repaint_source)
 		return;
 
-	wm_log("XWM: schedule repaint, win %d\n", window->id);
+	wm_printf(wm, "XWM: schedule repaint, win %d\n", window->id);
 
 	window->repaint_source =
 		wl_event_loop_add_idle(wm->server->loop,
@@ -1360,18 +1364,24 @@  weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t *even
 	xcb_property_notify_event_t *property_notify =
 		(xcb_property_notify_event_t *) event;
 	struct weston_wm_window *window;
-	FILE *fp;
+	FILE *fp = NULL;
 	char *logstr;
 	size_t logsize;
+	char timestr[128];
 
 	if (!wm_lookup_window(wm, property_notify->window, &window))
 		return;
 
 	window->properties_dirty = 1;
 
-	fp = open_memstream(&logstr, &logsize);
+	if (wm_debug_is_enabled(wm))
+		fp = open_memstream(&logstr, &logsize);
+
 	if (fp) {
-		fprintf(fp, "XCB_PROPERTY_NOTIFY: window %d, ", property_notify->window);
+		fprintf(fp, "%s XCB_PROPERTY_NOTIFY: window %d, ",
+			weston_debug_scope_timestamp(wm->server->wm_debug,
+			timestr, sizeof timestr),
+			property_notify->window);
 		if (property_notify->state == XCB_PROPERTY_DELETE)
 			fprintf(fp, "deleted %s\n",
 					get_atom_name(wm->conn, property_notify->atom));
@@ -1380,7 +1390,8 @@  weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t *even
 					       property_notify->atom);
 
 		if (fclose(fp) == 0)
-			wm_log("%s", logstr);
+			weston_debug_scope_write(wm->server->wm_debug,
+						 logstr, logsize);
 		free(logstr);
 	} else {
 		/* read_and_dump_property() is a X11 roundtrip.
@@ -1406,7 +1417,7 @@  weston_wm_window_create(struct weston_wm *wm,
 
 	window = zalloc(sizeof *window);
 	if (window == NULL) {
-		wm_log("failed to allocate window\n");
+		wm_printf(wm, "failed to allocate window\n");
 		return;
 	}
 
@@ -1479,12 +1490,12 @@  weston_wm_handle_create_notify(struct weston_wm *wm, xcb_generic_event_t *event)
 	xcb_create_notify_event_t *create_notify =
 		(xcb_create_notify_event_t *) event;
 
-	wm_log("XCB_CREATE_NOTIFY (window %d, at (%d, %d), width %d, height %d%s%s)\n",
-	       create_notify->window,
-	       create_notify->x, create_notify->y,
-	       create_notify->width, create_notify->height,
-	       create_notify->override_redirect ? ", override" : "",
-	       our_resource(wm, create_notify->window) ? ", ours" : "");
+	wm_printf(wm, "XCB_CREATE_NOTIFY (window %d, at (%d, %d), width %d, height %d%s%s)\n",
+		  create_notify->window,
+		  create_notify->x, create_notify->y,
+		  create_notify->width, create_notify->height,
+		  create_notify->override_redirect ? ", override" : "",
+		  our_resource(wm, create_notify->window) ? ", ours" : "");
 
 	if (our_resource(wm, create_notify->window))
 		return;
@@ -1502,10 +1513,10 @@  weston_wm_handle_destroy_notify(struct weston_wm *wm, xcb_generic_event_t *event
 		(xcb_destroy_notify_event_t *) event;
 	struct weston_wm_window *window;
 
-	wm_log("XCB_DESTROY_NOTIFY, win %d, event %d%s\n",
-	       destroy_notify->window,
-	       destroy_notify->event,
-	       our_resource(wm, destroy_notify->window) ? ", ours" : "");
+	wm_printf(wm, "XCB_DESTROY_NOTIFY, win %d, event %d%s\n",
+		  destroy_notify->window,
+		  destroy_notify->event,
+		  our_resource(wm, destroy_notify->window) ? ", ours" : "");
 
 	if (our_resource(wm, destroy_notify->window))
 		return;
@@ -1523,11 +1534,11 @@  weston_wm_handle_reparent_notify(struct weston_wm *wm, xcb_generic_event_t *even
 		(xcb_reparent_notify_event_t *) event;
 	struct weston_wm_window *window;
 
-	wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d%s)\n",
-	       reparent_notify->window,
-	       reparent_notify->parent,
-	       reparent_notify->event,
-	       reparent_notify->override_redirect ? ", override" : "");
+	wm_printf(wm, "XCB_REPARENT_NOTIFY (window %d, parent %d, event %d%s)\n",
+		  reparent_notify->window,
+		  reparent_notify->parent,
+		  reparent_notify->event,
+		  reparent_notify->override_redirect ? ", override" : "");
 
 	if (reparent_notify->parent == wm->screen->root) {
 		weston_wm_window_create(wm, reparent_notify->window, 10, 10,
@@ -1734,7 +1745,7 @@  surface_destroy(struct wl_listener *listener, void *data)
 		container_of(listener,
 			     struct weston_wm_window, surface_destroy_listener);
 
-	wm_log("surface for xid %d destroyed\n", window->id);
+	wm_printf(window->wm, "surface for xid %d destroyed\n", window->id);
 
 	/* This should have been freed by the shell.
 	 * Don't try to use it later. */
@@ -1750,7 +1761,8 @@  weston_wm_window_handle_surface_id(struct weston_wm_window *window,
 	struct wl_resource *resource;
 
 	if (window->surface_id != 0) {
-		wm_log("already have surface id for window %d\n", window->id);
+		wm_printf(wm, "already have surface id for window %d\n",
+			  window->id);
 		return;
 	}
 
@@ -1782,14 +1794,14 @@  weston_wm_handle_client_message(struct weston_wm *wm,
 		(xcb_client_message_event_t *) event;
 	struct weston_wm_window *window;
 
-	wm_log("XCB_CLIENT_MESSAGE (%s %d %d %d %d %d win %d)\n",
-	       get_atom_name(wm->conn, client_message->type),
-	       client_message->data.data32[0],
-	       client_message->data.data32[1],
-	       client_message->data.data32[2],
-	       client_message->data.data32[3],
-	       client_message->data.data32[4],
-	       client_message->window);
+	wm_printf(wm, "XCB_CLIENT_MESSAGE (%s %d %d %d %d %d win %d)\n",
+		  get_atom_name(wm->conn, client_message->type),
+		  client_message->data.data32[0],
+		  client_message->data.data32[1],
+		  client_message->data.data32[2],
+		  client_message->data.data32[3],
+		  client_message->data.data32[4],
+		  client_message->window);
 
 	/* The window may get created and destroyed before we actually
 	 * handle the message.  If it doesn't exist, bail.
@@ -2007,9 +2019,9 @@  weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event)
 	uint32_t button_id;
 	uint32_t double_click = 0;
 
-	wm_log("XCB_BUTTON_%s (detail %d)\n",
-	       button->response_type == XCB_BUTTON_PRESS ?
-	       "PRESS" : "RELEASE", button->detail);
+	wm_printf(wm, "XCB_BUTTON_%s (detail %d)\n",
+		  button->response_type == XCB_BUTTON_PRESS ?
+		  "PRESS" : "RELEASE", button->detail);
 
 	if (!wm_lookup_window(wm, button->event, &window) ||
 	    !window->decorate)
@@ -2221,7 +2233,7 @@  weston_wm_handle_event(int fd, uint32_t mask, void *data)
 			weston_wm_handle_destroy_notify(wm, event);
 			break;
 		case XCB_MAPPING_NOTIFY:
-			wm_log("XCB_MAPPING_NOTIFY\n");
+			wm_printf(wm, "XCB_MAPPING_NOTIFY\n");
 			break;
 		case XCB_PROPERTY_NOTIFY:
 			weston_wm_handle_property_notify(wm, event);
@@ -2837,8 +2849,8 @@  xserver_map_shell_surface(struct weston_wm_window *window,
 						   window->surface,
 						   &shell_client);
 
-	wm_log("XWM: map shell surface, win %d, weston_surface %p, xwayland surface %p\n",
-	       window->id, window->surface, window->shsurf);
+	wm_printf(wm, "XWM: map shell surface, win %d, weston_surface %p, xwayland surface %p\n",
+		  window->id, window->surface, window->shsurf);
 
 	if (window->name)
 		xwayland_interface->set_title(window->shsurf, window->name);
diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
index 52da67869..507d534d3 100644
--- a/xwayland/xwayland.h
+++ b/xwayland/xwayland.h
@@ -33,6 +33,7 @@ 
 #include "compositor.h"
 #include "compositor/weston.h"
 #include "xwayland-api.h"
+#include "weston-debug.h"
 
 #define SEND_EVENT_MASK (0x80)
 #define EVENT_TYPE(event) ((event)->response_type & ~SEND_EVENT_MASK)
@@ -52,6 +53,8 @@  struct weston_xserver {
 	struct wl_listener destroy_listener;
 	weston_xwayland_spawn_xserver_func_t spawn_func;
 	void *user_data;
+
+	struct weston_debug_scope *wm_debug;
 };
 
 struct weston_wm {